| Message ID | 1450278993-12664-23-git-send-email-tn@semihalf.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hi Tomasz > -----Original Message----- > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel- > owner@vger.kernel.org] On Behalf Of Tomasz Nowicki > Sent: 16 December 2015 15:17 > To: bhelgaas@google.com; arnd@arndb.de; will.deacon@arm.com; > catalin.marinas@arm.com; rjw@rjwysocki.net; hanjun.guo@linaro.org; > Lorenzo.Pieralisi@arm.com; okaya@codeaurora.org; > jiang.liu@linux.intel.com; Stefano.Stabellini@eu.citrix.com > Cc: robert.richter@caviumnetworks.com; mw@semihalf.com; > Liviu.Dudau@arm.com; ddaney@caviumnetworks.com; tglx@linutronix.de; > Wangyijing; Suravee.Suthikulpanit@amd.com; msalter@redhat.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; jchandra@broadcom.com; jcm@redhat.com; Tomasz > Nowicki > Subject: [PATCH V2 22/23] pci, acpi: Match PCI config space accessors > against platfrom specific quirks. > > 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 before PCI buses enumeration. Algorithm that overwrite > accessors matches against platform ID (DMI), domain and bus number, > hopefully enough for all cases. All quirks can be defined using: > DECLARE_ACPI_MCFG_FIXUP() and keep self contained. I've got a couple of comments/questions about this patch.. 1) So according to this mechanism quirks would be supported only by vendors whose BIOS are SMBIOS compliant. Now personally I am ok with this but I don't know if this is OK in general as it would narrow down the number of platforms that would be able to define the quirks... Lorenzo, Arnd what is your opinion here? 2) In the quirk mechanism you proposed, I see that the callback function allows to do some preparation work for the host bridge. For example in Hisilicon hip05 case we would need to read some values from the ACPI table (see acpi_pci_root_hisi_add() function in https://lkml.org/lkml/2015/12/3/426). I am quite new to ACPI and I wonder if it is OK to add such "Packages" to the PCI host bridge ACPI device...or maybe we need to declare a new one...? Many Thanks Gab > > example: > > static const struct dmi_system_id yyy[] = { > { > .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 ecam_pci_ops = { > .map_bus = pci_mcfg_dev_base, > .read = xxxx_ecam_config_read, > .write = xxxx_ecam_config_write, > }; > DECLARE_ACPI_MCFG_FIXUP(yyy, &ecam_pci_ops, <domain_nr>, <bus_nr>); > > Note, that more custom actions can be done via DMI callback hook. > > Signed-off-by: Tomasz Nowicki <tn@semihalf.com> > --- > drivers/acpi/mcfg.c | 35 > +++++++++++++++++++++++++++++++++-- > include/asm-generic/vmlinux.lds.h | 7 +++++++ > include/linux/ecam.h | 17 +++++++++++++++++ > 3 files changed, 57 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c index > a9b2231..6d0194d 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,31 @@ 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(int domain, int > +bus_number) { > + struct pci_mcfg_fixup *fixup; > + > + fixup = __start_acpi_mcfg_fixups; > + while (fixup < __end_acpi_mcfg_fixups) { > + if (dmi_check_system(fixup->system) && > + (fixup->domain == domain || > + fixup->domain == PCI_MCFG_DOMAIN_ANY) && > + (fixup->bus_number == bus_number || > + fixup->bus_number == PCI_MCFG_BUS_ANY)) { > + pr_info(PREFIX "Fixup applied: Platform [%s] domain > [%d] bus number [%d]\n", > + fixup->system->ident, fixup->domain, > + fixup->bus_number); > + return fixup->ops; > + } > + ++fixup; > + } > + > + return NULL; > +} > + > void __iomem * > pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) > { @@ -56,10 +82,15 @@ static struct pci_ops default_pci_mcfg_ops = { > > struct pci_ops *pci_mcfg_get_ops(int domain, int bus) { > + 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(domain, bus); > + if (pci_mcfg_ops_quirk) > + return pci_mcfg_ops_quirk; > > return &default_pci_mcfg_ops; > } > 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..1319fa8 100644 > --- a/include/linux/ecam.h > +++ b/include/linux/ecam.h > @@ -20,6 +20,23 @@ struct pci_mmcfg_region { > bool hot_added; > }; > > +struct pci_mcfg_fixup { > + const struct dmi_system_id *system; > + struct pci_ops *ops; > + int domain; > + int bus_number; > +}; > + > +#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, ops, dom, bus) > \ > + static const struct pci_mcfg_fixup > __mcfg_fixup_##system##dom##bus\ > + __used __attribute__((__section__(".acpi_fixup_mcfg"), > \ > + aligned((sizeof(void *))))) = \ > + { system, 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); > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" > in the body of a message to majordomo@vger.kernel.org More majordomo > info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- 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 Monday 21 December 2015, Gabriele Paoloni wrote: > > -----Original Message----- > > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel- > > owner@vger.kernel.org] On Behalf Of Tomasz Nowicki > > 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 before PCI buses enumeration. Algorithm that overwrite > > accessors matches against platform ID (DMI), domain and bus number, > > hopefully enough for all cases. All quirks can be defined using: > > DECLARE_ACPI_MCFG_FIXUP() and keep self contained. > > I've got a couple of comments/questions about this patch.. > > 1) So according to this mechanism quirks would be supported only by > vendors whose BIOS are SMBIOS compliant. Now personally I am ok > with this but I don't know if this is OK in general as it would > narrow down the number of platforms that would be able to define > the quirks... > Lorenzo, Arnd what is your opinion here? I'd rather not see the quirks in mainline at all, and only support SBSA compliant machines, or require the BIOS to work around the hardware quirks differently (e.g. by trapping config space access through secure firmware, or going through an AML method to be defined). I'm certainly ok with making it depend on SMBIOS if we are going to use something like this. Arnd -- 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 12/21/2015 06:10 AM, Arnd Bergmann wrote: > On Monday 21 December 2015, Gabriele Paoloni wrote: >>> -----Original Message----- >>> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel- >>> owner@vger.kernel.org] On Behalf Of Tomasz Nowicki > >>> 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 before PCI buses enumeration. Algorithm that overwrite >>> accessors matches against platform ID (DMI), domain and bus number, >>> hopefully enough for all cases. All quirks can be defined using: >>> DECLARE_ACPI_MCFG_FIXUP() and keep self contained. >> >> I've got a couple of comments/questions about this patch.. >> >> 1) So according to this mechanism quirks would be supported only by >> vendors whose BIOS are SMBIOS compliant. Now personally I am ok >> with this but I don't know if this is OK in general as it would >> narrow down the number of platforms that would be able to define >> the quirks... >> Lorenzo, Arnd what is your opinion here? > > I'd rather not see the quirks in mainline at all, and only support > SBSA compliant machines, There seems to exist a class of systems that were intended to be SBSA compliant, but after they were manufactured turn out to not be fully complaint. It would be nice to have Linux kernel support for some of these systems There also seems to be historical precedent for quirk frameworks in various kernel subsystems to handle systems and devices that are not fully "Spec. Compliant". > or require the BIOS to work around the hardware > quirks differently (e.g. by trapping config space access through secure > firmware, or going through an AML method to be defined). Some systems don't seem to have this capability. For example, in ARMv8 (A.K.A. arm64), I haven't been able to figure out how to trap these accesses to EL3 for emulation. The specification is 5700 pages long though, so perhaps I missed that bit. > I'm certainly > ok with making it depend on SMBIOS if we are going to use something like this. > > Arnd > -- 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 Monday 21 December 2015, David Daney wrote: > On 12/21/2015 06:10 AM, Arnd Bergmann wrote: > > On Monday 21 December 2015, Gabriele Paoloni wrote: > > > or require the BIOS to work around the hardware > > quirks differently (e.g. by trapping config space access through secure > > firmware, or going through an AML method to be defined). > > Some systems don't seem to have this capability. For example, in ARMv8 > (A.K.A. arm64), I haven't been able to figure out how to trap these > accesses to EL3 for emulation. The specification is 5700 pages long > though, so perhaps I missed that bit. How about using AML then? This would be similar to what CHRP used with RTAS calls to do PCI config space access. Arnd -- 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
Sorry for top-posting. A quick note that SMBIOS3 is required by SBBR so it can be presumed that compliant platforms will provide quirks via DMI.
On 12/21/2015 05:42 PM, Arnd Bergmann wrote: > On Monday 21 December 2015, David Daney wrote: >> On 12/21/2015 06:10 AM, Arnd Bergmann wrote: >>> On Monday 21 December 2015, Gabriele Paoloni wrote: >> >>> or require the BIOS to work around the hardware >>> quirks differently (e.g. by trapping config space access through secure >>> firmware, or going through an AML method to be defined). >> >> Some systems don't seem to have this capability. For example, in ARMv8 >> (A.K.A. arm64), I haven't been able to figure out how to trap these >> accesses to EL3 for emulation. The specification is 5700 pages long >> though, so perhaps I missed that bit. There isn't a way to directly trap to EL3 for emulation (caveat - there might be some nasty hack with an SMMU that wouldn't be supportable). I requested the implementation of a generic mechanism for LPC type emulation (complete with "SMI" traps to EL3 for fixups) about 4 years ago. That wouldn't have helped with this situation, but this was to be on the radar afterward. However, on ARM, it is still early days with respect to transparently trapping and emulating hardware workarounds. > How about using AML then? This would be similar to what CHRP used with > RTAS calls to do PCI config space access. The best way to do it for now (IMO) is via a DMI quirk match and a special method for the early SoCs that aren't implementing MMCONFIG correctly. An effort is underway to correct that in third party IP, and similar directly with the partners for future generations. So this should not get "much" more out of hand than it sadly is so far. Once we have a good upstream solution (which is vital) then it will be an error and a pre-tapeout bug to not be able to boot an upstream kernel with stock ACPI hostbridge working sans nasty quirks. Therefore, the sooner this is upstream, the better it will be for everyone involved. Jon. -- 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 22.12.2015 00:10, Jon Masters wrote: > Sorry for top-posting. A quick note that SMBIOS3 is required by SBBR so it can be presumed that compliant platforms will provide quirks via DMI. > Thanks Jon for confirmation. 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
Hi Jon, thanks for replying > -----Original Message----- > From: Jon Masters [mailto:jcm@redhat.com] > Sent: 21 December 2015 23:11 > To: Arnd Bergmann > Cc: Gabriele Paoloni; Tomasz Nowicki; bhelgaas@google.com; > will.deacon@arm.com; catalin.marinas@arm.com; rjw@rjwysocki.net; > hanjun.guo@linaro.org; Lorenzo.Pieralisi@arm.com; okaya@codeaurora.org; > jiang.liu@linux.intel.com; Stefano.Stabellini@eu.citrix.com; > robert.richter@caviumnetworks.com; mw@semihalf.com; Liviu.Dudau@arm.com; > ddaney@caviumnetworks.com; tglx@linutronix.de; Wangyijing; > Suravee.Suthikulpanit@amd.com; msalter@redhat.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; jchandra@broadcom.com > Subject: Re: [PATCH V2 22/23] pci, acpi: Match PCI config space > accessors against platfrom specific quirks. > > Sorry for top-posting. A quick note that SMBIOS3 is required by SBBR so > it can be presumed that compliant platforms will provide quirks via DMI. Ok so you completely clarified my question 1). Many Thanks for this Gab > > -- > Computer Architect | Sent from my 64-bit #ARM Powered phone > > > On Dec 21, 2015, at 09:11, Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Monday 21 December 2015, Gabriele Paoloni wrote: > >>> -----Original Message----- > >>> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel- > >>> owner@vger.kernel.org] On Behalf Of Tomasz Nowicki > > > >>> 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 before PCI buses enumeration. Algorithm that > overwrite > >>> accessors matches against platform ID (DMI), domain and bus number, > >>> hopefully enough for all cases. All quirks can be defined using: > >>> DECLARE_ACPI_MCFG_FIXUP() and keep self contained. > >> > >> I've got a couple of comments/questions about this patch.. > >> > >> 1) So according to this mechanism quirks would be supported only by > >> vendors whose BIOS are SMBIOS compliant. Now personally I am ok > >> with this but I don't know if this is OK in general as it would > >> narrow down the number of platforms that would be able to define > >> the quirks... > >> Lorenzo, Arnd what is your opinion here? > > > > I'd rather not see the quirks in mainline at all, and only support > > SBSA compliant machines, or require the BIOS to work around the > > hardware quirks differently (e.g. by trapping config space access > > through secure firmware, or going through an AML method to be > > defined). I'm certainly ok with making it depend on SMBIOS if we are > going to use something like this. > > > > Arnd -- 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 21.12.2015 12:47, Gabriele Paoloni wrote: > 2) In the quirk mechanism you proposed, I see that the callback function > allows to do some preparation work for the host bridge. For example in > Hisilicon hip05 case we would need to read some values from the ACPI > table (see acpi_pci_root_hisi_add() function in > https://lkml.org/lkml/2015/12/3/426). > I am quite new to ACPI and I wonder if it is OK to add such "Packages" > to the PCI host bridge ACPI device...or maybe we need to declare a new > one...? I may miss sth so please correct me in that case. https://lkml.org/lkml/2015/12/3/426 shows that you need special handling for root->secondary.start bus number only, right? So how about creating special MCFG region <rc-base:rc-base+rc-size> only for <segment,bus>. Like that: [0008] Base Address : <rc-base> [0002] Segment Group Number : <segment> [0001] Start Bus Number : <root->secondary.start> [0001] End Bus Number : <root->secondary.start> [0004] Reserved : 00000000 static const struct dmi_system_id hisi_quirk[] = { { .ident = "HiSi...", .matches = { DMI_MATCH(<whatever you need to match your platform>), }, }, { } }; static struct pci_ops hisi_ecam_pci_ops = { .map_bus = pci_mcfg_dev_base, .read = hisi_pcie_cfg_read, .write = hisi_pcie_cfg_write, }; DECLARE_ACPI_MCFG_FIXUP(hisi_quirk, &hisi_ecam_pci_ops, <segment>, <bus>); With above code you can use your custom PCI config accessor only for that region. Let me know if that is not enough for you. 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
Hi Tomasz > -----Original Message----- > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel- > owner@vger.kernel.org] On Behalf Of Tomasz Nowicki > Sent: 22 December 2015 10:20 > To: Gabriele Paoloni; bhelgaas@google.com; arnd@arndb.de; > will.deacon@arm.com; catalin.marinas@arm.com; rjw@rjwysocki.net; > hanjun.guo@linaro.org; Lorenzo.Pieralisi@arm.com; okaya@codeaurora.org; > jiang.liu@linux.intel.com; Stefano.Stabellini@eu.citrix.com > Cc: robert.richter@caviumnetworks.com; mw@semihalf.com; > Liviu.Dudau@arm.com; ddaney@caviumnetworks.com; tglx@linutronix.de; > Wangyijing; Suravee.Suthikulpanit@amd.com; msalter@redhat.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; jchandra@broadcom.com; jcm@redhat.com > Subject: Re: [PATCH V2 22/23] pci, acpi: Match PCI config space > accessors against platfrom specific quirks. > > On 21.12.2015 12:47, Gabriele Paoloni wrote: > > 2) In the quirk mechanism you proposed, I see that the callback > function > > allows to do some preparation work for the host bridge. For > example in > > Hisilicon hip05 case we would need to read some values from the > ACPI > > table (see acpi_pci_root_hisi_add() function in > > https://lkml.org/lkml/2015/12/3/426). > > I am quite new to ACPI and I wonder if it is OK to add such > "Packages" > > to the PCI host bridge ACPI device...or maybe we need to declare > a new > > one...? > > I may miss sth so please correct me in that case. > > https://lkml.org/lkml/2015/12/3/426 shows that you need special > handling for root->secondary.start bus number only, right? So how about > creating special MCFG region <rc-base:rc-base+rc-size> only for > <segment,bus>. > Like that: > > [0008] Base Address : <rc-base> > [0002] Segment Group Number : <segment> > [0001] Start Bus Number : <root->secondary.start> > [0001] End Bus Number : <root->secondary.start> > [0004] Reserved : 00000000 > > > static const struct dmi_system_id hisi_quirk[] = { > { > .ident = "HiSi...", > .matches = { > DMI_MATCH(<whatever you need to match your platform>), > }, > }, > { } > }; > > static struct pci_ops hisi_ecam_pci_ops = { > .map_bus = pci_mcfg_dev_base, > .read = hisi_pcie_cfg_read, > .write = hisi_pcie_cfg_write, > }; > > DECLARE_ACPI_MCFG_FIXUP(hisi_quirk, &hisi_ecam_pci_ops, > <segment>, <bus>); > > With above code you can use your custom PCI config accessor only for > that region. > > Let me know if that is not enough for you. In principle I think it can work... Liudongdong, Guo Hanjun what is your opinion about? Thanks Gab > > Tomasz > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" > in the body of a message to majordomo@vger.kernel.org More majordomo > info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- 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 12/22/2015 04:29 AM, Gabriele Paoloni wrote: > Hi Jon, thanks for replying > >> -----Original Message----- >> From: Jon Masters [mailto:jcm@redhat.com] >> Sent: 21 December 2015 23:11 >> To: Arnd Bergmann >> Cc: Gabriele Paoloni; Tomasz Nowicki; bhelgaas@google.com; >> will.deacon@arm.com; catalin.marinas@arm.com; rjw@rjwysocki.net; >> hanjun.guo@linaro.org; Lorenzo.Pieralisi@arm.com; okaya@codeaurora.org; >> jiang.liu@linux.intel.com; Stefano.Stabellini@eu.citrix.com; >> robert.richter@caviumnetworks.com; mw@semihalf.com; Liviu.Dudau@arm.com; >> ddaney@caviumnetworks.com; tglx@linutronix.de; Wangyijing; >> Suravee.Suthikulpanit@amd.com; msalter@redhat.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; jchandra@broadcom.com >> Subject: Re: [PATCH V2 22/23] pci, acpi: Match PCI config space >> accessors against platfrom specific quirks. >> >> Sorry for top-posting. A quick note that SMBIOS3 is required by SBBR so >> it can be presumed that compliant platforms will provide quirks via DMI. > > Ok so you completely clarified my question 1). Many Thanks for this No problem. One of the (many) reasons I pushed for requiring SMBIOS/DMI in SBBR (I was lead author of one of the early drafts of that document) was to make the experience ultimately equivalent across architectures. We already know how to do quirks and handle platform deviations, and the major Operating System vendors did not want to reinvent the wheel. Jon. -- 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 12/22/2015 11:36 AM, Jon Masters wrote: > On 12/22/2015 04:29 AM, Gabriele Paoloni wrote: >> Hi Jon, thanks for replying >> >>> -----Original Message----- >>> From: Jon Masters [mailto:jcm@redhat.com] >>> Sent: 21 December 2015 23:11 >>> To: Arnd Bergmann >>> Cc: Gabriele Paoloni; Tomasz Nowicki; bhelgaas@google.com; >>> will.deacon@arm.com; catalin.marinas@arm.com; rjw@rjwysocki.net; >>> hanjun.guo@linaro.org; Lorenzo.Pieralisi@arm.com; okaya@codeaurora.org; >>> jiang.liu@linux.intel.com; Stefano.Stabellini@eu.citrix.com; >>> robert.richter@caviumnetworks.com; mw@semihalf.com; Liviu.Dudau@arm.com; >>> ddaney@caviumnetworks.com; tglx@linutronix.de; Wangyijing; >>> Suravee.Suthikulpanit@amd.com; msalter@redhat.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; jchandra@broadcom.com >>> Subject: Re: [PATCH V2 22/23] pci, acpi: Match PCI config space >>> accessors against platfrom specific quirks. >>> >>> Sorry for top-posting. A quick note that SMBIOS3 is required by SBBR so >>> it can be presumed that compliant platforms will provide quirks via DMI. >> >> Ok so you completely clarified my question 1). Many Thanks for this > > No problem. One of the (many) reasons I pushed for requiring SMBIOS/DMI > in SBBR (I was lead author of one of the early drafts of that document) > was to make the experience ultimately equivalent across architectures. > We already know how to do quirks and handle platform deviations, and the > major Operating System vendors did not want to reinvent the wheel. Additional: it is clear that more prescription is required to get the vendors onto the bandwagon that we have with other architectures (e.g. that other one). So there will be a Red Hat "ARM server whitepaper" coming in the early new year that will include the kind of "server 101" material we want to make sure people know. Things like making sure you implement and test PCIe correctly, handle backward compatibility (you will build hardware in the future that runs my existing OS release), design the hardware to allow for workarounds later, etc. I expect some other Operating System vendors to be involved in reviewing that. Ultimately my objective is to make this whole thing dull and boring. You will get RHEL(SA)/upstream kernels and it will either boot or it will not. If it does not boot, vendor X screwed up their hardware. We know this story, it's been this way for over a decade already, and that is exactly how it is going to be with ARM servers shortly. Jon. -- 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
> -----Original Message----- > From: Jon Masters [mailto:jcm@redhat.com] > Sent: 22 December 2015 16:45 > To: Gabriele Paoloni; Arnd Bergmann > Cc: Tomasz Nowicki; bhelgaas@google.com; will.deacon@arm.com; > catalin.marinas@arm.com; rjw@rjwysocki.net; hanjun.guo@linaro.org; > Lorenzo.Pieralisi@arm.com; okaya@codeaurora.org; > jiang.liu@linux.intel.com; Stefano.Stabellini@eu.citrix.com; > robert.richter@caviumnetworks.com; mw@semihalf.com; Liviu.Dudau@arm.com; > ddaney@caviumnetworks.com; tglx@linutronix.de; Wangyijing; > Suravee.Suthikulpanit@amd.com; msalter@redhat.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; jchandra@broadcom.com > Subject: Re: [PATCH V2 22/23] pci, acpi: Match PCI config space > accessors against platfrom specific quirks. > > On 12/22/2015 11:36 AM, Jon Masters wrote: > > On 12/22/2015 04:29 AM, Gabriele Paoloni wrote: > >> Hi Jon, thanks for replying > >> > >>> -----Original Message----- > >>> From: Jon Masters [mailto:jcm@redhat.com] > >>> Sent: 21 December 2015 23:11 > >>> To: Arnd Bergmann > >>> Cc: Gabriele Paoloni; Tomasz Nowicki; bhelgaas@google.com; > >>> will.deacon@arm.com; catalin.marinas@arm.com; rjw@rjwysocki.net; > >>> hanjun.guo@linaro.org; Lorenzo.Pieralisi@arm.com; > >>> okaya@codeaurora.org; jiang.liu@linux.intel.com; > >>> Stefano.Stabellini@eu.citrix.com; robert.richter@caviumnetworks.com; > >>> mw@semihalf.com; Liviu.Dudau@arm.com; ddaney@caviumnetworks.com; > >>> tglx@linutronix.de; Wangyijing; Suravee.Suthikulpanit@amd.com; > >>> msalter@redhat.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; > >>> jchandra@broadcom.com > >>> Subject: Re: [PATCH V2 22/23] pci, acpi: Match PCI config space > >>> accessors against platfrom specific quirks. > >>> > >>> Sorry for top-posting. A quick note that SMBIOS3 is required by > SBBR > >>> so it can be presumed that compliant platforms will provide quirks > via DMI. > >> > >> Ok so you completely clarified my question 1). Many Thanks for this > > > > No problem. One of the (many) reasons I pushed for requiring > > SMBIOS/DMI in SBBR (I was lead author of one of the early drafts of > > that document) was to make the experience ultimately equivalent > across architectures. > > We already know how to do quirks and handle platform deviations, and > > the major Operating System vendors did not want to reinvent the wheel. > > Additional: it is clear that more prescription is required to get the > vendors onto the bandwagon that we have with other architectures (e.g. > that other one). So there will be a Red Hat "ARM server whitepaper" > coming in the early new year that will include the kind of "server 101" > material we want to make sure people know. Things like making sure you > implement and test PCIe correctly, handle backward compatibility (you > will build hardware in the future that runs my existing OS release), > design the hardware to allow for workarounds later, etc. I expect some > other Operating System vendors to be involved in reviewing that. > > Ultimately my objective is to make this whole thing dull and boring. > You will get RHEL(SA)/upstream kernels and it will either boot or it > will not. If it does not boot, vendor X screwed up their hardware. We > know this story, it's been this way for over a decade already, and that > is exactly how it is going to be with ARM servers shortly. Ok got it. Many Thanks Gab > > Jon. -- 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 12/22/2015 10:48 PM, Gabriele Paoloni wrote: > Hi Tomasz > >> -----Original Message----- >> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel- >> owner@vger.kernel.org] On Behalf Of Tomasz Nowicki >> Sent: 22 December 2015 10:20 >> To: Gabriele Paoloni; bhelgaas@google.com; arnd@arndb.de; >> will.deacon@arm.com; catalin.marinas@arm.com; rjw@rjwysocki.net; >> hanjun.guo@linaro.org; Lorenzo.Pieralisi@arm.com; okaya@codeaurora.org; >> jiang.liu@linux.intel.com; Stefano.Stabellini@eu.citrix.com >> Cc: robert.richter@caviumnetworks.com; mw@semihalf.com; >> Liviu.Dudau@arm.com; ddaney@caviumnetworks.com; tglx@linutronix.de; >> Wangyijing; Suravee.Suthikulpanit@amd.com; msalter@redhat.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; jchandra@broadcom.com; jcm@redhat.com >> Subject: Re: [PATCH V2 22/23] pci, acpi: Match PCI config space >> accessors against platfrom specific quirks. >> >> On 21.12.2015 12:47, Gabriele Paoloni wrote: >>> 2) In the quirk mechanism you proposed, I see that the callback >> function >>> allows to do some preparation work for the host bridge. For >> example in >>> Hisilicon hip05 case we would need to read some values from the >> ACPI >>> table (see acpi_pci_root_hisi_add() function in >>> https://lkml.org/lkml/2015/12/3/426). >>> I am quite new to ACPI and I wonder if it is OK to add such >> "Packages" >>> to the PCI host bridge ACPI device...or maybe we need to declare >> a new >>> one...? >> >> I may miss sth so please correct me in that case. >> >> https://lkml.org/lkml/2015/12/3/426 shows that you need special >> handling for root->secondary.start bus number only, right? So how about >> creating special MCFG region <rc-base:rc-base+rc-size> only for >> <segment,bus>. >> Like that: >> >> [0008] Base Address : <rc-base> >> [0002] Segment Group Number : <segment> >> [0001] Start Bus Number : <root->secondary.start> >> [0001] End Bus Number : <root->secondary.start> >> [0004] Reserved : 00000000 >> >> >> static const struct dmi_system_id hisi_quirk[] = { >> { >> .ident = "HiSi...", >> .matches = { >> DMI_MATCH(<whatever you need to match your platform>), >> }, >> }, >> { } >> }; >> >> static struct pci_ops hisi_ecam_pci_ops = { >> .map_bus = pci_mcfg_dev_base, >> .read = hisi_pcie_cfg_read, >> .write = hisi_pcie_cfg_write, >> }; >> >> DECLARE_ACPI_MCFG_FIXUP(hisi_quirk, &hisi_ecam_pci_ops, >> <segment>, <bus>); >> >> With above code you can use your custom PCI config accessor only for >> that region. >> >> Let me know if that is not enough for you. > > In principle I think it can work... > > Liudongdong, Guo Hanjun what is your opinion about? Let me and Dongdong prepare a patch for Hip05 and then will back to this discussion to see if we met some problems. Thanks Hanjun -- 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 Wed, 2015-12-16 at 16:16 +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 before PCI buses enumeration. Algorithm that overwrite accessors > matches against platform ID (DMI), domain and bus number, hopefully > enough for all cases. All quirks can be defined using: > DECLARE_ACPI_MCFG_FIXUP() and keep self contained. > > example: > > static const struct dmi_system_id yyy[] = { > { > .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"), > }, > }, > { } > }; > This seems awkward to me in the case where the quirk is SoC-based and there may be multiple platforms affected. Needing a DECLARE_ACPI_MCFG_FIXUP for each platform using such a SoC (i.e. Mustang and Moonshot) doesn't seem right. In that case, I think it'd be better to check CPUID and possibly some SoC register to cover all platforms affected. Also, there doesn't seem to be a way to connect a given quirk check to the MCFG/device requesting the ops. So if there is a platform with multiple PCIE roots and not all of them have quirks, how does one no whether to override the default ecam ops? -- 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 08.01.2016 15:16, Mark Salter wrote: > On Wed, 2015-12-16 at 16:16 +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 before PCI buses enumeration. Algorithm that overwrite accessors >> matches against platform ID (DMI), domain and bus number, hopefully >> enough for all cases. All quirks can be defined using: >> DECLARE_ACPI_MCFG_FIXUP() and keep self contained. >> >> example: >> >> static const struct dmi_system_id yyy[] = { >> { >> .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"), >> }, >> }, >> { } >> }; >> > > This seems awkward to me in the case where the quirk is SoC-based and there > may be multiple platforms affected. Needing a DECLARE_ACPI_MCFG_FIXUP for > each platform using such a SoC (i.e. Mustang and Moonshot) doesn't seem > right. In that case, I think it'd be better to check CPUID and possibly > some SoC register to cover all platforms affected. Right, my next version already has alternative to DMI match handler, so there will be two ways to match: 1. DMI, like in this patch set 2. int (*match)(struct pci_mcfg_fixup *) where you can read CPUID, and whatever is necessary. > > Also, there doesn't seem to be a way to connect a given quirk check to the > MCFG/device requesting the ops. So if there is a platform with multiple PCIE > roots and not all of them have quirks, how does one no whether to override the > default ecam ops? > Then we can identify them using <domain:bus>. I was wondering to pass acpi device handler to match handler for the case where we need e.g. extra properties from related DSDT device descriptor. Does it make sense to you? 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 01/08/2016 08:16 AM, Mark Salter wrote: > On Wed, 2015-12-16 at 16:16 +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 before PCI buses enumeration. Algorithm that overwrite accessors >> matches against platform ID (DMI), domain and bus number, hopefully >> enough for all cases. All quirks can be defined using: >> DECLARE_ACPI_MCFG_FIXUP() and keep self contained. ... > This seems awkward to me in the case where the quirk is SoC-based and there > may be multiple platforms affected. Needing a DECLARE_ACPI_MCFG_FIXUP for > each platform using such a SoC (i.e. Mustang and Moonshot) doesn't seem > right. In that case, I think it'd be better to check CPUID and possibly > some SoC register to cover all platforms affected. > > Also, there doesn't seem to be a way to connect a given quirk check to the > MCFG/device requesting the ops. So if there is a platform with multiple PCIE > roots and not all of them have quirks, how does one no whether to override the > default ecam ops? That was the thinking that lead me to quirk the xgene based m400 using the root bridge VID/DID and DECLARE_PCI_FIXUP_EARLY(). A solution which works because the default config space accessors work sufficiently to read the VID/DID. I think that solution might work for at least one other vendor as well, and IMHO is better than using DMI for the reasons you list. I will rebase/post those patches RSN... (please ignore the message below, i'm working getting that taken care of as well) IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -- 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 Fri, 2016-01-08 at 15:36 +0100, Tomasz Nowicki wrote: > On 08.01.2016 15:16, Mark Salter wrote: > > On Wed, 2015-12-16 at 16:16 +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 before PCI buses enumeration. Algorithm that overwrite accessors > > > matches against platform ID (DMI), domain and bus number, hopefully > > > enough for all cases. All quirks can be defined using: > > > DECLARE_ACPI_MCFG_FIXUP() and keep self contained. > > > > > > example: > > > > > > static const struct dmi_system_id yyy[] = { > > > { > > > .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"), > > > }, > > > }, > > > { } > > > }; > > > > > > > This seems awkward to me in the case where the quirk is SoC-based and there > > may be multiple platforms affected. Needing a DECLARE_ACPI_MCFG_FIXUP for > > each platform using such a SoC (i.e. Mustang and Moonshot) doesn't seem > > right. In that case, I think it'd be better to check CPUID and possibly > > some SoC register to cover all platforms affected. > > Right, my next version already has alternative to DMI match handler, so > there will be two ways to match: > 1. DMI, like in this patch set > 2. int (*match)(struct pci_mcfg_fixup *) where you can read CPUID, and > whatever is necessary. > Great. Thanks. > > > > Also, there doesn't seem to be a way to connect a given quirk check to the > > MCFG/device requesting the ops. So if there is a platform with multiple PCIE > > roots and not all of them have quirks, how does one no whether to override the > > default ecam ops? > > > > Then we can identify them using <domain:bus>. I was wondering to pass > acpi device handler to match handler for the case where we need e.g. > extra properties from related DSDT device descriptor. Does it make sense > to you? > The thing with domain:bus is that it is really a firmware setting. So on one platform domain 0 may be associated with hwdev X but on another platform it may be associated with hwdev Y. But yes, an acpi handle would make it much easier for the match handler to find out which hw dev it is being asked to match. -- 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 Fri, Jan 08, 2016 at 09:16:21AM -0500, Mark Salter wrote: > On Wed, 2015-12-16 at 16:16 +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 before PCI buses enumeration. Algorithm that overwrite accessors > > matches against platform ID (DMI), domain and bus number, hopefully > > enough for all cases. All quirks can be defined using: > > DECLARE_ACPI_MCFG_FIXUP() and keep self contained. > > > > example: > > > > static const struct dmi_system_id yyy[] = { > > { > > .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"), > > }, > > }, > > { } > > }; > > > > This seems awkward to me in the case where the quirk is SoC-based and there > may be multiple platforms affected. Needing a DECLARE_ACPI_MCFG_FIXUP for > each platform using such a SoC (i.e. Mustang and Moonshot) doesn't seem > right. In that case, I think it'd be better to check CPUID and possibly > some SoC register to cover all platforms affected. CPUs get reused across SoCs, so as you've implicitly noted, the CPUID alone is insufficient. Given that IP blocks get moved around between SoC variants, I don't think you can check "some SoC register" based on the CPU ID -- you can end up bringing the board down at that point. If the CPU ID alone is insufficient to tell you about a component, it cannot give you enough information about a component you can use to query more information from. If your platform requires a quirk, it's always going to be painful (and to some extent, rightfulyl so). We should aim for correctness here with explicit matching. Thanks, Mark. -- 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 Fri, Jan 08, 2016 at 03:01:37PM +0000, Mark Rutland wrote: > On Fri, Jan 08, 2016 at 09:16:21AM -0500, Mark Salter wrote: > > On Wed, 2015-12-16 at 16:16 +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 before PCI buses enumeration. Algorithm that overwrite accessors > > > matches against platform ID (DMI), domain and bus number, hopefully > > > enough for all cases. All quirks can be defined using: > > > DECLARE_ACPI_MCFG_FIXUP() and keep self contained. > > > > > > example: > > > > > > static const struct dmi_system_id yyy[] = { > > > { > > > .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"), > > > }, > > > }, > > > { } > > > }; > > > > > > > This seems awkward to me in the case where the quirk is SoC-based and there > > may be multiple platforms affected. Needing a DECLARE_ACPI_MCFG_FIXUP for > > each platform using such a SoC (i.e. Mustang and Moonshot) doesn't seem > > right. In that case, I think it'd be better to check CPUID and possibly > > some SoC register to cover all platforms affected. > > CPUs get reused across SoCs, so as you've implicitly noted, the CPUID > alone is insufficient. > > Given that IP blocks get moved around between SoC variants, I don't > think you can check "some SoC register" based on the CPU ID -- you can > end up bringing the board down at that point. > > If the CPU ID alone is insufficient to tell you about a component, it > cannot give you enough information about a component you can use to > query more information from. > > If your platform requires a quirk, it's always going to be painful (and > to some extent, rightfulyl so). We should aim for correctness here with > explicit matching. Further, if there is going to be an ever-expanding set of platforms requring quirks, then we need a standard mechanism in ACPI to enable the platform to tell us explicitly either which specific PCI implementation is used, or which common quirk is necessary. Thanks, Mark. -- 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 Fri, 2016-01-08 at 15:12 +0000, Mark Rutland wrote: > On Fri, Jan 08, 2016 at 03:01:37PM +0000, Mark Rutland wrote: > > On Fri, Jan 08, 2016 at 09:16:21AM -0500, Mark Salter wrote: > > > On Wed, 2015-12-16 at 16:16 +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 before PCI buses enumeration. Algorithm that overwrite accessors > > > > matches against platform ID (DMI), domain and bus number, hopefully > > > > enough for all cases. All quirks can be defined using: > > > > DECLARE_ACPI_MCFG_FIXUP() and keep self contained. > > > > > > > > example: > > > > > > > > static const struct dmi_system_id yyy[] = { > > > > { > > > > .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"), > > > > }, > > > > }, > > > > { } > > > > }; > > > > > > > > > > This seems awkward to me in the case where the quirk is SoC-based and there > > > may be multiple platforms affected. Needing a DECLARE_ACPI_MCFG_FIXUP for > > > each platform using such a SoC (i.e. Mustang and Moonshot) doesn't seem > > > right. In that case, I think it'd be better to check CPUID and possibly > > > some SoC register to cover all platforms affected. > > > > CPUs get reused across SoCs, so as you've implicitly noted, the CPUID > > alone is insufficient. > > > > Given that IP blocks get moved around between SoC variants, I don't > > think you can check "some SoC register" based on the CPU ID -- you can > > end up bringing the board down at that point. > > > > If the CPU ID alone is insufficient to tell you about a component, it > > cannot give you enough information about a component you can use to > > query more information from. > > > > If your platform requires a quirk, it's always going to be painful (and > > to some extent, rightfulyl so). We should aim for correctness here with > > explicit matching. > > Further, if there is going to be an ever-expanding set of platforms > requring quirks, then we need a standard mechanism in ACPI to enable the > platform to tell us explicitly either which specific PCI implementation > is used, or which common quirk is necessary. > No, an ever-expanding set is exactly what we don't want. I think you've convinced me that I'm taking a wrong view of the problem. Putting something in the ACPI standard would be going too far and I think a hard sell to the standards folk. There really is no foolproof way to match a plug and play ACPI PCIe root to specific hardware without considering the exact platform and/or BIOS info. So yeah, it should be painful in order to give incentive to the silicon vendors to get it right. -- 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
diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c index a9b2231..6d0194d 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,31 @@ 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(int domain, int bus_number) +{ + struct pci_mcfg_fixup *fixup; + + fixup = __start_acpi_mcfg_fixups; + while (fixup < __end_acpi_mcfg_fixups) { + if (dmi_check_system(fixup->system) && + (fixup->domain == domain || + fixup->domain == PCI_MCFG_DOMAIN_ANY) && + (fixup->bus_number == bus_number || + fixup->bus_number == PCI_MCFG_BUS_ANY)) { + pr_info(PREFIX "Fixup applied: Platform [%s] domain [%d] bus number [%d]\n", + fixup->system->ident, fixup->domain, + fixup->bus_number); + return fixup->ops; + } + ++fixup; + } + + return NULL; +} + void __iomem * pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) { @@ -56,10 +82,15 @@ static struct pci_ops default_pci_mcfg_ops = { struct pci_ops *pci_mcfg_get_ops(int domain, int bus) { + 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(domain, bus); + if (pci_mcfg_ops_quirk) + return pci_mcfg_ops_quirk; return &default_pci_mcfg_ops; } 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..1319fa8 100644 --- a/include/linux/ecam.h +++ b/include/linux/ecam.h @@ -20,6 +20,23 @@ struct pci_mmcfg_region { bool hot_added; }; +struct pci_mcfg_fixup { + const struct dmi_system_id *system; + struct pci_ops *ops; + int domain; + int bus_number; +}; + +#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, ops, dom, bus) \ + static const struct pci_mcfg_fixup __mcfg_fixup_##system##dom##bus\ + __used __attribute__((__section__(".acpi_fixup_mcfg"), \ + aligned((sizeof(void *))))) = \ + { system, 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 before PCI buses enumeration. Algorithm that overwrite accessors matches against platform ID (DMI), domain and bus number, hopefully enough for all cases. All quirks can be defined using: DECLARE_ACPI_MCFG_FIXUP() and keep self contained. example: static const struct dmi_system_id yyy[] = { { .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 ecam_pci_ops = { .map_bus = pci_mcfg_dev_base, .read = xxxx_ecam_config_read, .write = xxxx_ecam_config_write, }; DECLARE_ACPI_MCFG_FIXUP(yyy, &ecam_pci_ops, <domain_nr>, <bus_nr>); Note, that more custom actions can be done via DMI callback hook. Signed-off-by: Tomasz Nowicki <tn@semihalf.com> --- drivers/acpi/mcfg.c | 35 +++++++++++++++++++++++++++++++++-- include/asm-generic/vmlinux.lds.h | 7 +++++++ include/linux/ecam.h | 17 +++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-)