Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757537AbcCRPuE (ORCPT ); Fri, 18 Mar 2016 11:50:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46698 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757514AbcCRPt6 (ORCPT ); Fri, 18 Mar 2016 11:49:58 -0400 Message-ID: <1458316194.23434.104.camel@redhat.com> Subject: Re: [PATCH V5 13/15] pci, acpi: Match PCI config space accessors against platfrom specific quirks. From: Mark Salter To: Tomasz Nowicki , helgaas@kernel.org, arnd@arndb.de, will.deacon@arm.com, catalin.marinas@arm.com, rafael@kernel.org, hanjun.guo@linaro.org, Lorenzo.Pieralisi@arm.com, okaya@codeaurora.org, jiang.liu@linux.intel.com, jchandra@broadcom.com, Stefano.Stabellini@eu.citrix.com Cc: robert.richter@caviumnetworks.com, mw@semihalf.com, Liviu.Dudau@arm.com, ddaney@caviumnetworks.com, wangyijing@huawei.com, Suravee.Suthikulpanit@amd.com, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, linaro-acpi@lists.linaro.org, jcm@redhat.com Date: Fri, 18 Mar 2016 11:49:54 -0400 In-Reply-To: <1455630825-27253-14-git-send-email-tn@semihalf.com> References: <1455630825-27253-1-git-send-email-tn@semihalf.com> <1455630825-27253-14-git-send-email-tn@semihalf.com> Organization: Red Hat, Inc Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7200 Lines: 185 On Tue, 2016-02-16 at 14:53 +0100, Tomasz Nowicki wrote: > Some platforms may not be fully compliant with generic set of PCI config > accessors. For these cases we implement the way to overwrite accessors > set prior to PCI buses enumeration. Algorithm traverses available quirk > list, matches against tuple and > returns corresponding accessors. All quirks can be defined using: > DECLARE_ACPI_MCFG_FIXUP() and kept self contained. Example, > > static const struct dmi_system_id foo_dmi[] = { >         { >                 .ident = "", >                 .callback = , >                 .matches = { >                         DMI_MATCH(DMI_SYS_VENDOR, ""), >                         DMI_MATCH(DMI_PRODUCT_NAME, ""), >                         DMI_MATCH(DMI_PRODUCT_VERSION, "product version"), >                 }, >         }, >         { } > }; > > static struct pci_ops foo_ecam_pci_ops = { >         .map_bus = pci_mcfg_dev_base, >         .read = foo_ecam_config_read, >         .write = foo_ecam_config_write, > }; > DECLARE_ACPI_MCFG_FIXUP(foo_dmi, NULL, &foo_ecam_pci_ops, , ); > > More custom (non-DMI) matching can be done via an extra call. > Note that there is possibility to assign quirk related private data to > root->sysdata which will be available along read/wriate accessor, example: > > static int boo_match(struct pci_mcfg_fixup *fixup, struct acpi_pci_root *root) > { >         return [condition] ? 1 : 0; > } > > int boo_ecam_config_read(struct pci_bus *bus, unsigned int devfn, >                           int where, int size, u32 *val) > { >         struct acpi_pci_root *root = bus->sysdata; >         struct boo_priv_data *boo_data = root->sysdata; > >         [..] > } > > static struct pci_ops boo_ecam_pci_ops = { > .map_bus = pci_mcfg_dev_base, > .read = boo_ecam_config_read, > .write = boo_ecam_config_write, > }; > DECLARE_ACPI_MCFG_FIXUP(NULL, boo_match, &boo_ecam_pci_ops, , ); > > Signed-off-by: Tomasz Nowicki > Tested-by: Duc Dang > Tested-by: Dongdong Liu > Tested-by: Hanjun Guo > Tested-by: Graeme Gregory > Tested-by: Sinan Kaya > --- >  drivers/acpi/pci_mcfg.c           | 32 ++++++++++++++++++++++++++++++-- >  include/acpi/acpi_bus.h           |  1 + >  include/asm-generic/vmlinux.lds.h |  7 +++++++ >  include/linux/pci-acpi.h          | 18 ++++++++++++++++++ >  4 files changed, 56 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c > index 0062257..b343547 100644 > --- a/drivers/acpi/pci_mcfg.c > +++ b/drivers/acpi/pci_mcfg.c > @@ -41,6 +41,29 @@ int __weak raw_pci_write(unsigned int domain, unsigned int bus, >   return PCIBIOS_DEVICE_NOT_FOUND; >  } >   > +extern struct pci_mcfg_fixup __start_acpi_mcfg_fixups[]; > +extern struct pci_mcfg_fixup __end_acpi_mcfg_fixups[]; > + > +static struct pci_ops *pci_mcfg_check_quirks(struct acpi_pci_root *root) > +{ > + struct pci_mcfg_fixup *f; > + int bus_num = root->secondary.start; > + int domain = root->segment; > + > + /* > +  * First match against PCI topology then use DMI or > +  * custom match handler. > +  */ > + for (f = __start_acpi_mcfg_fixups; f < __end_acpi_mcfg_fixups; f++) { > + if ((f->domain == domain || f->domain == PCI_MCFG_DOMAIN_ANY) && > +     (f->bus_num == bus_num || f->bus_num == PCI_MCFG_BUS_ANY) && > +     (f->system ? dmi_check_system(f->system) : 1 && > +      f->match ? f->match(f, root) : 1)) The parens are not quite right here ^^^^ If dmi_check_system() returns true, f->match won't get called. This should be:     (f->system ? dmi_check_system(f->system) : 1) &&      f->match ? f->match(f, root) : 1) > + return f->ops; > + } > + return NULL; > +} > + >  void __iomem * >  pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) >  { > @@ -63,10 +86,15 @@ static struct pci_ops default_pci_mcfg_ops = { >   >  struct pci_ops *pci_mcfg_get_ops(struct acpi_pci_root *root) >  { > + struct pci_ops *pci_mcfg_ops_quirk; > + >   /* > -  * TODO: Match against platform specific quirks and return > -  * corresponding PCI config space accessor set. > +  * Match against platform specific quirks and return corresponding > +  * PCI config space accessor set. >    */ > + pci_mcfg_ops_quirk = pci_mcfg_check_quirks(root); > + if (pci_mcfg_ops_quirk) > + return pci_mcfg_ops_quirk; >   >   return &default_pci_mcfg_ops; >  } > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index 14362a8..0fc6f13 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -556,6 +556,7 @@ struct acpi_pci_root { >   struct pci_bus *bus; >   u16 segment; >   struct resource secondary; /* downstream bus range */ > + void *sysdata; >   >   u32 osc_support_set; /* _OSC state of support bits */ >   u32 osc_control_set; /* _OSC state of control bits */ > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > index c4bd0e2..c93fc97 100644 > --- a/include/asm-generic/vmlinux.lds.h > +++ b/include/asm-generic/vmlinux.lds.h > @@ -298,6 +298,13 @@ >   VMLINUX_SYMBOL(__end_pci_fixups_suspend_late) = .; \ >   } \ >   \ > + /* ACPI MCFG quirks */ \ > + .acpi_fixup        : AT(ADDR(.acpi_fixup) - LOAD_OFFSET) { \ > + VMLINUX_SYMBOL(__start_acpi_mcfg_fixups) = .; \ > + *(.acpi_fixup_mcfg) \ > + VMLINUX_SYMBOL(__end_acpi_mcfg_fixups) = .; \ > + } \ > + \ >   /* Built-in firmware blobs */ \ >   .builtin_fw        : AT(ADDR(.builtin_fw) - LOAD_OFFSET) { \ >   VMLINUX_SYMBOL(__start_builtin_fw) = .; \ > diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h > index 93feb04..9e1bedd 100644 > --- a/include/linux/pci-acpi.h > +++ b/include/linux/pci-acpi.h > @@ -123,6 +123,24 @@ struct pci_mmcfg_region { >   bool hot_added; >  }; >   > +struct pci_mcfg_fixup { > + const struct dmi_system_id *system; > + int (*match)(struct pci_mcfg_fixup *, struct acpi_pci_root *); > + struct pci_ops *ops; > + int domain; > + int bus_num; > +}; > + > +#define PCI_MCFG_DOMAIN_ANY -1 > +#define PCI_MCFG_BUS_ANY -1 > + > +/* Designate a routine to fix up buggy MCFG */ > +#define DECLARE_ACPI_MCFG_FIXUP(system, match, ops, dom, bus) \ > + static const struct pci_mcfg_fixup __mcfg_fixup_##system##dom##bus\ > +  __used __attribute__((__section__(".acpi_fixup_mcfg"), \ > + aligned((sizeof(void *))))) = \ > + { system, match, ops, dom, bus }; > + >  extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus); >  extern struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start, >   int end, u64 addr);