| Message ID | 1452691267-32240-21-git-send-email-tn@semihalf.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2016-01-13 at 14:21 +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 <platform ID (DMI), domain, bus number> tuple and > returns corresponding accessors. All quirks can be defined using: > DECLARE_ACPI_MCFG_FIXUP() and keep self contained. Example, > > static const struct dmi_system_id foo_dmi[] = { > { > .ident = "<Platform ident string>", > .callback = <handler>, > .matches = { > DMI_MATCH(DMI_SYS_VENDOR, "<system vendor>"), > DMI_MATCH(DMI_PRODUCT_NAME, "<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, <domain_nr>, <bus_nr>); > > More custom (non-DMI) matching can be done via an alternative 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, <domain_nr>, <bus_nr>); > > Signed-off-by: Tomasz Nowicki <tn@semihalf.com> > --- > drivers/acpi/mcfg.c | 33 +++++++++++++++++++++++++++++++-- > include/acpi/acpi_bus.h | 1 + > include/asm-generic/vmlinux.lds.h | 7 +++++++ > include/linux/ecam.h | 18 ++++++++++++++++++ > 4 files changed, 57 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c > index dfc2d14..ec5fe7b 100644 > --- a/drivers/acpi/mcfg.c > +++ b/drivers/acpi/mcfg.c > @@ -8,6 +8,7 @@ > */ > > #include <linux/acpi.h> > +#include <linux/dmi.h> > #include <linux/ecam.h> > #include <linux/pci.h> > #include <linux/pci-acpi.h> > @@ -34,6 +35,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 <domain:bus> 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) : 0 || > + f->match ? f->match(f, root) : 0)) > + return f->ops; I think this would be better as: (f->system ? dmi_check_system(f->system) : 1 && f->match ? f->match(f, root) : 1)) return f->ops; Otherwise, one has to call dmi_check_system() from f->match() if access to root is needed. > + } > + return NULL; > +} > + > void __iomem * > pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) > { > @@ -56,10 +80,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 ad0a5ff..ea4d2b5 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -554,6 +554,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/ecam.h b/include/linux/ecam.h > index e0f322e..21215be 100644 > --- a/include/linux/ecam.h > +++ b/include/linux/ecam.h > @@ -20,6 +20,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 }; > + > struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus); > struct pci_mmcfg_region *pci_mmconfig_alloc(int segment, int start, > int end, u64 addr);
On 14.01.2016 16:36, Mark Salter wrote: >> +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 <domain:bus> 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) : 0 || >> >+ f->match ? f->match(f, root) : 0)) >> >+ return f->ops; > I think this would be better as: > > (f->system ? dmi_check_system(f->system) : 1 && > f->match ? f->match(f, root) : 1)) > return f->ops; > > Otherwise, one has to call dmi_check_system() from f->match() if > access to root is needed. Makes a lot of sense to me, I will modify as you suggested. Tomasz
Hi Tomasz, Mark ? 2016/1/18 20:41, Tomasz Nowicki ??: > On 14.01.2016 16:36, Mark Salter wrote: >>> +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 <domain:bus> 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) : 0 || >>> >+ f->match ? f->match(f, root) : 0)) >>> >+ return f->ops; >> I think this would be better as: >> >> (f->system ? dmi_check_system(f->system) : 1 && >> f->match ? f->match(f, root) : 1)) >> return f->ops; >> >> Otherwise, one has to call dmi_check_system() from f->match() if >> access to root is needed. > Non-DMI, we need not to call dmi_check_system() from f->match(), we can use _HID to decide to hook the pci_ops or not. Device (PCI1) { Name (_HID, "HISI0080") // PCI Express Root Bridge Name (_CID, "PNP0A03") // Compatible PCI Root Bridge ... } static const struct acpi_device_id hisi_pcie_ids[] = { {"HISI0080", 0}, {"", 0}, }; static int hisi_pcie_match(struct pci_mcfg_fixup *fixup, struct acpi_pci_root *root) { int ret; struct acpi_device *device; device = root->device; ret = acpi_match_device_ids(device, hisi_pcie_ids); if (ret) return 0; ...... return 1; } static struct pci_ops hisi_ecam_pci_ops = { .map_bus = pci_mcfg_dev_base, .read = hisi_pci_read, .write = hisi_pci_write, }; DECLARE_ACPI_MCFG_FIXUP(NULL, hisi_pcie_match, &hisi_ecam_pci_ops, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY); Thanks Dongdong > Makes a lot of sense to me, I will modify as you suggested. > > Tomasz > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > . >
On 19.01.2016 02:49, liudongdong (C) wrote: > Hi Tomasz, Mark > > ? 2016/1/18 20:41, Tomasz Nowicki ??: >> On 14.01.2016 16:36, Mark Salter wrote: >>>> +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 <domain:bus> 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) : 0 || >>>> >+ f->match ? f->match(f, root) : 0)) >>>> >+ return f->ops; >>> I think this would be better as: >>> >>> (f->system ? dmi_check_system(f->system) : 1 && >>> f->match ? f->match(f, root) : 1)) >>> return f->ops; >>> >>> Otherwise, one has to call dmi_check_system() from f->match() if >>> access to root is needed. >> > > Non-DMI, we need not to call dmi_check_system() from f->match(), > we can use _HID to decide to hook the pci_ops or not. Sorry, but I dont understand your point. Can you elaborate? With Mark modification, you can use the following cases to identify platform: 1. DMI only 2. f->match() only (_HID can be used there) 3. DMI and f->match() DMI used to be very convenient way to recognise platform, sometimes it is not enough, hence f->match() alternative. Tomasz
? 2016/1/19 15:55, Tomasz Nowicki ??: > On 19.01.2016 02:49, liudongdong (C) wrote: >> Hi Tomasz, Mark >> >> ? 2016/1/18 20:41, Tomasz Nowicki ??: >>> On 14.01.2016 16:36, Mark Salter wrote: >>>>> +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 <domain:bus> 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) : 0 || >>>>> >+ f->match ? f->match(f, root) : 0)) >>>>> >+ return f->ops; >>>> I think this would be better as: >>>> >>>> (f->system ? dmi_check_system(f->system) : 1 && >>>> f->match ? f->match(f, root) : 1)) >>>> return f->ops; >>>> >>>> Otherwise, one has to call dmi_check_system() from f->match() if >>>> access to root is needed. >>> >> >> Non-DMI, we need not to call dmi_check_system() from f->match(), >> we can use _HID to decide to hook the pci_ops or not. > > Sorry, but I dont understand your point. Can you elaborate? > > With Mark modification, you can use the following cases to identify platform: > 1. DMI only > 2. f->match() only (_HID can be used there) > 3. DMI and f->match() > > DMI used to be very convenient way to recognise platform, sometimes it is not enough, hence f->match() alternative. > Yes, you are right, I was wrong. In my case, I can use the second point. 2. f->match() only (_HID can be used there) Thanks Dongdong > Tomasz > > > > . >
diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c index dfc2d14..ec5fe7b 100644 --- a/drivers/acpi/mcfg.c +++ b/drivers/acpi/mcfg.c @@ -8,6 +8,7 @@ */ #include <linux/acpi.h> +#include <linux/dmi.h> #include <linux/ecam.h> #include <linux/pci.h> #include <linux/pci-acpi.h> @@ -34,6 +35,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 <domain:bus> 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) : 0 || + f->match ? f->match(f, root) : 0)) + return f->ops; + } + return NULL; +} + void __iomem * pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) { @@ -56,10 +80,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 ad0a5ff..ea4d2b5 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -554,6 +554,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/ecam.h b/include/linux/ecam.h index e0f322e..21215be 100644 --- a/include/linux/ecam.h +++ b/include/linux/ecam.h @@ -20,6 +20,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 }; + struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus); struct pci_mmcfg_region *pci_mmconfig_alloc(int segment, int start, int end, u64 addr);
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 <platform ID (DMI), domain, bus number> tuple and returns corresponding accessors. All quirks can be defined using: DECLARE_ACPI_MCFG_FIXUP() and keep self contained. Example, static const struct dmi_system_id foo_dmi[] = { { .ident = "<Platform ident string>", .callback = <handler>, .matches = { DMI_MATCH(DMI_SYS_VENDOR, "<system vendor>"), DMI_MATCH(DMI_PRODUCT_NAME, "<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, <domain_nr>, <bus_nr>); More custom (non-DMI) matching can be done via an alternative 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, <domain_nr>, <bus_nr>); Signed-off-by: Tomasz Nowicki <tn@semihalf.com> --- drivers/acpi/mcfg.c | 33 +++++++++++++++++++++++++++++++-- include/acpi/acpi_bus.h | 1 + include/asm-generic/vmlinux.lds.h | 7 +++++++ include/linux/ecam.h | 18 ++++++++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-)