Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and revision (the ones from standard header of MCFG table).
Static array is used to keep quirk entries. Each entry consist of metioned MCFG IDs along with custom pci_ops structure and initialization call.
As an example, last patch presents quirk handling mechanism usage for ThunderX PEM driver.
Tomasz Nowicki (5): PCI: Embed pci_ecam_ops in pci_config_window structure PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver PCI: Check platform specific ECAM quirks ARM64/PCI: Start using quirks handling for ACPI based PCI host controller PCI: thunder: Add ThunderX PEM MCFG quirk to the list
arch/arm64/kernel/pci.c | 42 +---------------- drivers/acpi/pci_mcfg.c | 40 ++++++++++++++++ drivers/pci/ecam.c | 6 +-- drivers/pci/host/Makefile | 1 + drivers/pci/host/mcfg-quirks.c | 95 +++++++++++++++++++++++++++++++++++++ drivers/pci/host/mcfg-quirks.h | 24 ++++++++++ drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------ include/linux/pci-acpi.h | 5 ++ include/linux/pci-ecam.h | 2 +- 9 files changed, 254 insertions(+), 57 deletions(-) create mode 100644 drivers/pci/host/mcfg-quirks.c create mode 100644 drivers/pci/host/mcfg-quirks.h
pci_config_window keeps pointer to pci_ecam_ops and every time we want to deallocate pci_config_window (pci_ecam_free()) we need to make sure to free pci_ecam_ops in case it was dynamically allocated prior to pci_ecam_create() call.
To avoid that extra effort, embed pci_ecam_ops in pci_config_window, instead of just a pointer.
Signed-off-by: Tomasz Nowicki tn@semihalf.com To: Jayachandran C jchandra@broadcom.com --- arch/arm64/kernel/pci.c | 2 +- drivers/pci/ecam.c | 6 +++--- include/linux/pci-ecam.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index 94cd43c..cb4dee7 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -196,7 +196,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) return NULL; }
- acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops; + acpi_pci_root_ops.pci_ops = &ri->cfg->ops.pci_ops; bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common, ri->cfg); if (!bus) diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c index 43ed08d..24242f0 100644 --- a/drivers/pci/ecam.c +++ b/drivers/pci/ecam.c @@ -52,7 +52,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev, return ERR_PTR(-ENOMEM);
cfg->parent = dev; - cfg->ops = ops; + cfg->ops = *ops; cfg->busr.start = busr->start; cfg->busr.end = busr->end; cfg->busr.flags = IORESOURCE_BUS; @@ -138,7 +138,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn, int where) { struct pci_config_window *cfg = bus->sysdata; - unsigned int devfn_shift = cfg->ops->bus_shift - 8; + unsigned int devfn_shift = cfg->ops.bus_shift - 8; unsigned int busn = bus->number; void __iomem *base;
@@ -149,7 +149,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn, if (per_bus_mapping) base = cfg->winp[busn]; else - base = cfg->win + (busn << cfg->ops->bus_shift); + base = cfg->win + (busn << cfg->ops.bus_shift); return base + (devfn << devfn_shift) + where; }
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h index 7adad20..0ce2920 100644 --- a/include/linux/pci-ecam.h +++ b/include/linux/pci-ecam.h @@ -39,7 +39,7 @@ struct pci_config_window { struct resource res; struct resource busr; void *priv; - struct pci_ecam_ops *ops; + struct pci_ecam_ops ops; union { void __iomem *win; /* 64-bit single mapping */ void __iomem **winp; /* 32-bit per-bus mapping */
pci_acpi_setup_ecam_mapping() is not really ARM64 specific so move it out of arch/arm64/ directory. In preparation for adding MCFG quirk handling extend pci_acpi_setup_ecam_mapping() functionality to accept custom PCI config accessors (function's argument).
For ARM64 ACPI based PCI host controller we still use pci_generic_ecam_ops.
Signed-off-by: Tomasz Nowicki tn@semihalf.com --- arch/arm64/kernel/pci.c | 41 ++--------------------------------------- drivers/acpi/pci_mcfg.c | 40 ++++++++++++++++++++++++++++++++++++++++ include/linux/pci-acpi.h | 3 +++ 3 files changed, 45 insertions(+), 39 deletions(-)
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index cb4dee7..addf00c 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -127,44 +127,6 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) return 0; }
-/* - * Lookup the bus range for the domain in MCFG, and set up config space - * mapping. - */ -static struct pci_config_window * -pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root) -{ - struct resource *bus_res = &root->secondary; - u16 seg = root->segment; - struct pci_config_window *cfg; - struct resource cfgres; - unsigned int bsz; - - /* Use address from _CBA if present, otherwise lookup MCFG */ - if (!root->mcfg_addr) - root->mcfg_addr = pci_mcfg_lookup(seg, bus_res); - - if (!root->mcfg_addr) { - dev_err(&root->device->dev, "%04x:%pR ECAM region not found\n", - seg, bus_res); - return NULL; - } - - bsz = 1 << pci_generic_ecam_ops.bus_shift; - cfgres.start = root->mcfg_addr + bus_res->start * bsz; - cfgres.end = cfgres.start + resource_size(bus_res) * bsz - 1; - cfgres.flags = IORESOURCE_MEM; - cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res, - &pci_generic_ecam_ops); - if (IS_ERR(cfg)) { - dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n", - seg, bus_res, PTR_ERR(cfg)); - return NULL; - } - - return cfg; -} - /* release_info: free resources allocated by init_info */ static void pci_acpi_generic_release_info(struct acpi_pci_root_info *ci) { @@ -190,7 +152,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) if (!ri) return NULL;
- ri->cfg = pci_acpi_setup_ecam_mapping(root); + ri->cfg = pci_acpi_setup_ecam_mapping(root, + &pci_generic_ecam_ops.pci_ops); if (!ri->cfg) { kfree(ri); return NULL; diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c index b5b376e..331b560 100644 --- a/drivers/acpi/pci_mcfg.c +++ b/drivers/acpi/pci_mcfg.c @@ -22,6 +22,7 @@ #include <linux/kernel.h> #include <linux/pci.h> #include <linux/pci-acpi.h> +#include <linux/pci-ecam.h>
/* Structure to hold entries from the MCFG table */ struct mcfg_entry { @@ -52,6 +53,45 @@ phys_addr_t pci_mcfg_lookup(u16 seg, struct resource *bus_res) return 0; }
+/* + * Lookup the bus range for the domain in MCFG, and set up config space + * mapping. + */ +struct pci_config_window * +pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops) +{ + struct resource *bus_res = &root->secondary; + u16 seg = root->segment; + struct pci_config_window *cfg; + struct resource cfgres; + struct pci_ecam_ops ecam_ops = { + .bus_shift = 20, + .pci_ops = *ops, + }; + + /* Use address from _CBA if present, otherwise lookup MCFG */ + if (!root->mcfg_addr) + root->mcfg_addr = pci_mcfg_lookup(seg, bus_res); + + if (!root->mcfg_addr) { + dev_err(&root->device->dev, "%04x:%pR ECAM region not found\n", + seg, bus_res); + return NULL; + } + + cfgres.start = root->mcfg_addr + (bus_res->start << 20); + cfgres.end = cfgres.start + (resource_size(bus_res) << 20) - 1; + cfgres.flags = IORESOURCE_MEM; + cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res, &ecam_ops); + if (IS_ERR(cfg)) { + dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n", + seg, bus_res, PTR_ERR(cfg)); + return NULL; + } + + return cfg; +} + static __init int pci_mcfg_parse(struct acpi_table_header *header) { struct acpi_table_mcfg *mcfg; diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h index 7d63a66..e9bfe00 100644 --- a/include/linux/pci-acpi.h +++ b/include/linux/pci-acpi.h @@ -26,6 +26,9 @@ extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);
extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res);
+extern struct pci_config_window * +pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops); + static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev) { struct pci_bus *pbus = pdev->bus;
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. Algorithm traverses available quirk list (static array), matches against <oem_id, oem_table_id, rev, domain, bus number range> and returns pci_config_window structure with fancy PCI config ops. oem_id, oem_table_id and rev come from MCFG table standard header.
It is possible to define custom init call which is responsible for setting up PCI config access accordingly to quirk requirements. If no custom init call defined, use standard pci_acpi_setup_ecam_mapping().
pci_generic_ecam_ops will be used for platforms free from quirks.
Signed-off-by: Tomasz Nowicki tn@semihalf.com Signed-off-by: Dongdong Liu liudongdong3@huawei.com Signed-off-by: Christopher Covington cov@codeaurora.org --- drivers/pci/host/Makefile | 1 + drivers/pci/host/mcfg-quirks.c | 88 ++++++++++++++++++++++++++++++++++++++++++ drivers/pci/host/mcfg-quirks.h | 20 ++++++++++ include/linux/pci-acpi.h | 2 + 4 files changed, 111 insertions(+) create mode 100644 drivers/pci/host/mcfg-quirks.c create mode 100644 drivers/pci/host/mcfg-quirks.h
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile index 5bc0af2..e3d55a0 100644 --- a/drivers/pci/host/Makefile +++ b/drivers/pci/host/Makefile @@ -30,3 +30,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o +obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c new file mode 100644 index 0000000..fb2b184 --- /dev/null +++ b/drivers/pci/host/mcfg-quirks.c @@ -0,0 +1,88 @@ +/* + * Copyright (C) 2016 Semihalf + * Author: Tomasz Nowicki tn@semihalf.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License, version 2, as + * published by the Free Software Foundation (the "GPL"). + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License version 2 (GPLv2) for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 (GPLv2) along with this source code. + */ + +#include <linux/kernel.h> +#include <linux/ioport.h> +#include <linux/pci.h> +#include <linux/pci-acpi.h> +#include <linux/pci-ecam.h> + +#include "mcfg-quirks.h" + +struct pci_cfg_fixup { + char *oem_id; + char *oem_table_id; + u32 oem_revision; + struct resource domain_range; + struct resource bus_range; + struct pci_ops *ops; + struct pci_config_window *(*init)(struct acpi_pci_root *root, + struct pci_ops *ops); +}; + +#define MCFG_DOM_RANGE(start, end) DEFINE_RES_NAMED((start), \ + ((end) - (start) + 1), NULL, 0) +#define MCFG_DOM_ANY MCFG_DOM_RANGE(0x0, 0xffff) +#define MCFG_BUS_RANGE(start, end) DEFINE_RES_NAMED((start), \ + ((end) - (start) + 1), \ + NULL, IORESOURCE_BUS) +#define MCFG_BUS_ANY MCFG_BUS_RANGE(0x0, 0xff) + +static struct pci_cfg_fixup mcfg_qurks[] __initconst = { +/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */ +}; + +static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f, + struct acpi_table_header *mcfg_header) +{ + int olen = min_t(u8, strlen(f->oem_id), ACPI_OEM_ID_SIZE); + int tlen = min_t(u8, strlen(f->oem_table_id), ACPI_OEM_TABLE_ID_SIZE); + + return (!strncmp(f->oem_id, mcfg_header->oem_id, olen) && + !strncmp(f->oem_table_id, mcfg_header->oem_table_id, tlen) && + f->oem_revision == mcfg_header->oem_revision); +} + +struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root) +{ + struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment); + struct resource *bus_res = &root->secondary; + struct pci_cfg_fixup *f = mcfg_qurks; + struct acpi_table_header *mcfg_header; + acpi_status status; + int i; + + status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header); + if (ACPI_FAILURE(status)) + return NULL; + + /* + * First match against PCI topology domain:bus then use OEM ID, OEM + * table ID, and OEM revision from MCFG table standard header. + */ + for (i = 0; i < ARRAY_SIZE(mcfg_qurks); i++, f++) { + if (resource_contains(&f->domain_range, &dom_res) && + resource_contains(&f->bus_range, bus_res) && + pci_mcfg_fixup_match(f, mcfg_header)) { + pr_info("Handling %s %s r%d PCI MCFG quirks\n", + f->oem_id, f->oem_table_id, f->oem_revision); + return f->init ? f->init(root, f->ops) : + pci_acpi_setup_ecam_mapping(root, f->ops); + } + } + return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops); +} diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h new file mode 100644 index 0000000..45cbd16 --- /dev/null +++ b/drivers/pci/host/mcfg-quirks.h @@ -0,0 +1,20 @@ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License, version 2, as + * published by the Free Software Foundation (the "GPL"). + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License version 2 (GPLv2) for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 (GPLv2) along with this source code. + */ + +#ifndef __MCFG_QUIRKS_H__ +#define __MCFG_QUIRKS_H__ + +/* MCFG quirks initialize call list */ + +#endif /* __MCFG_QUIRKS_H__ */ diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h index e9bfe00..28cdce4 100644 --- a/include/linux/pci-acpi.h +++ b/include/linux/pci-acpi.h @@ -25,6 +25,8 @@ static inline acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev) extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle);
extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res); +extern struct pci_config_window * +pci_mcfg_match_quirks(struct acpi_pci_root *root);
extern struct pci_config_window * pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
Hi Tomasz,
On 06/28/2016 03:54 AM, Tomasz Nowicki wrote:
diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c new file mode 100644 index 0000000..fb2b184 --- /dev/null +++ b/drivers/pci/host/mcfg-quirks.c @@ -0,0 +1,88 @@
+static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
struct acpi_table_header *mcfg_header)+{
- int olen = min_t(u8, strlen(f->oem_id), ACPI_OEM_ID_SIZE);
- int tlen = min_t(u8, strlen(f->oem_table_id), ACPI_OEM_TABLE_ID_SIZE);
- return (!strncmp(f->oem_id, mcfg_header->oem_id, olen) &&
!strncmp(f->oem_table_id, mcfg_header->oem_table_id, tlen) &&f->oem_revision == mcfg_header->oem_revision);+}
Ard's comments on v3 included:
"... exact OEM table/rev id matches ..." "... substring match ... out of the question ..."
I originally advocated the substring match approach because space-padding the input strings was unfamiliar. But given that some vendors have a "PLAT " then "PLAT2 " naming scheme, where the former needs quirks and the latter (hopefully) doesn't, I agree with Ard and think space-padded inputs is the better way to go. Sorry for the lack of foresight.
(I'm happy to rip it out, test, and communicate the delta however you'd prefer--just let me know.)
Regards, Cov
On Tue, Jun 28, 2016 at 6:04 AM, Christopher Covington cov@codeaurora.org wrote:
Hi Tomasz,
On 06/28/2016 03:54 AM, Tomasz Nowicki wrote:
diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c new file mode 100644 index 0000000..fb2b184 --- /dev/null +++ b/drivers/pci/host/mcfg-quirks.c @@ -0,0 +1,88 @@
+static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
struct acpi_table_header *mcfg_header)+{
int olen = min_t(u8, strlen(f->oem_id), ACPI_OEM_ID_SIZE);int tlen = min_t(u8, strlen(f->oem_table_id), ACPI_OEM_TABLE_ID_SIZE);return (!strncmp(f->oem_id, mcfg_header->oem_id, olen) &&!strncmp(f->oem_table_id, mcfg_header->oem_table_id, tlen) &&f->oem_revision == mcfg_header->oem_revision);+}
Ard's comments on v3 included:
"... exact OEM table/rev id matches ..." "... substring match ... out of the question ..."
I originally advocated the substring match approach because space-padding the input strings was unfamiliar. But given that some vendors have a "PLAT " then "PLAT2 " naming scheme, where the former needs quirks and the latter (hopefully) doesn't, I agree with Ard and think space-padded inputs is the better way to go. Sorry for the lack of foresight.
I think having OEM Table ID as "PLAT " and then "PLAT2 " (the the next version of the SoC) is common. So yes, matching full string is better as we can use "PLAT2 " in MCFG table and not worry about the "PLAT" sub-string match causes the quirk to be applied unintentionally.
(I'm happy to rip it out, test, and communicate the delta however you'd prefer--just let me know.)
Regards, Cov
-- Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Regards, Duc Dang.
On 28.06.2016 18:12, Duc Dang wrote:
On Tue, Jun 28, 2016 at 6:04 AM, Christopher Covington cov@codeaurora.org wrote:
Hi Tomasz,
On 06/28/2016 03:54 AM, Tomasz Nowicki wrote:
diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c new file mode 100644 index 0000000..fb2b184 --- /dev/null +++ b/drivers/pci/host/mcfg-quirks.c @@ -0,0 +1,88 @@
+static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
struct acpi_table_header *mcfg_header)+{
int olen = min_t(u8, strlen(f->oem_id), ACPI_OEM_ID_SIZE);int tlen = min_t(u8, strlen(f->oem_table_id), ACPI_OEM_TABLE_ID_SIZE);return (!strncmp(f->oem_id, mcfg_header->oem_id, olen) &&!strncmp(f->oem_table_id, mcfg_header->oem_table_id, tlen) &&f->oem_revision == mcfg_header->oem_revision);+}
Ard's comments on v3 included:
"... exact OEM table/rev id matches ..." "... substring match ... out of the question ..."
I originally advocated the substring match approach because space-padding the input strings was unfamiliar. But given that some vendors have a "PLAT " then "PLAT2 " naming scheme, where the former needs quirks and the latter (hopefully) doesn't, I agree with Ard and think space-padded inputs is the better way to go. Sorry for the lack of foresight.
I think having OEM Table ID as "PLAT " and then "PLAT2 " (the the next version of the SoC) is common. So yes, matching full string is better as we can use "PLAT2 " in MCFG table and not worry about the "PLAT" sub-string match causes the quirk to be applied unintentionally.
Sorry, I forgot to address Ard's comment.
Note that platforms already shipped where OEM string has no padding will have change the firmware or add 0 padding to our quirk array IDs. So how about:
static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f, struct acpi_table_header *mcfg_header) { int olen = strnlen(mcfg_header->oem_id, ACPI_OEM_ID_SIZE); int tlen = strnlen(mcfg_header->oem_table_id, ACPI_OEM_TABLE_ID_SIZE);
if (olen != strlen(f->oem_id) || tlen != strlen(f->oem_table_id)) return false;
return (!strncmp(f->oem_id, mcfg_header->oem_id, olen) && !strncmp(f->oem_table_id, mcfg_header->oem_table_id, tlen) && f->oem_revision == mcfg_header->oem_revision); }
This should work for all cases: 1. "PLAT" 2. "PLAT " padding 3. No need to change existing firmware
Tomasz
Hi Tomasz,
On 06/29/2016 06:48 AM, Tomasz Nowicki wrote:
On 28.06.2016 18:12, Duc Dang wrote:
On Tue, Jun 28, 2016 at 6:04 AM, Christopher Covington cov@codeaurora.org wrote:
Hi Tomasz,
Ard's comments on v3 included:
"... exact OEM table/rev id matches ..." "... substring match ... out of the question ..."
Digging through the archives I see Jon Master commented earlier to "be careful with substring match".
I think having OEM Table ID as "PLAT " and then "PLAT2 " (the the next version of the SoC) is common. So yes, matching full string is better as we can use "PLAT2 " in MCFG table and not worry about the "PLAT" sub-string match causes the quirk to be applied unintentionally.
Note that platforms already shipped where OEM string has no padding will
I'm confused by this statement. OEMID is defined as 6 bytes long and OEM Table ID as 8 bytes long in the ACPI specification. As far as I can tell, if your string isn't exactly that long, padding up to that length is required.
have change the firmware or add 0 padding to our quirk array IDs.
The fixed 6 or 8 character string compare, as used v2 of this patchset, will be compatible with existing firmware as best I can tell. Adding padding to the quirk array IDs is exactly what I'm suggesting, although all the strings I've seen are space padded rather than null padded.
Matches: {"APM ", "XGENE ", 1} {"CAVIUM", "THUNDERX", 1} {"HISI ", "HISI-D02", 1} {"HISI ", "HISI-D03", 1} {"QCOM ", "QDF2432 ", 1}
Given the above tuples, won't accidentally match: (guessing at possible future ids) {"APM ", "XGENEi ", 1} {"CAVIUM", "THUNDERX", i} i != 1 {"CAVIUM", "THUNDERi", 1} {"CAVIUM", "THUNDRXi", 1} {"HISI ", "HISI-D0i", 1} i != 2 && i != 3 {"QCOM ", "QDF24ij ", 1} i != 3 && j != 2
References for APM, HiSilicon IDs: https://lists.linaro.org/pipermail/linaro-acpi/2016-June/007108.html https://lists.linaro.org/pipermail/linaro-acpi/2016-June/007043.html
Thanks, Cov
On 29.06.2016 15:34, Christopher Covington wrote:
I'm confused by this statement. OEMID is defined as 6 bytes long and OEM Table ID as 8 bytes long in the ACPI specification. As far as I can tell, if your string isn't exactly that long, padding up to that length is required.
Well, I cannot find that requirement in ACPI spec. but I might missed something.
I dumped my x86 machine ACPI tables and here is an example of MCFG:
$ cat mcfg.dsl /* * Intel ACPI Component Architecture * AML/ASL+ Disassembler version 20160108-64 * Copyright (c) 2000 - 2016 Intel Corporation * * Disassembly of mcfg.dat, Wed Jun 29 15:48:16 2016 * * ACPI Data Table [MCFG] * * Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue */
[000h 0000 4] Signature : "MCFG" [Memory Mapped Configuration table] [004h 0004 4] Table Length : 0000003C [008h 0008 1] Revision : 01 [009h 0009 1] Checksum : A9 [00Ah 0010 6] Oem ID : "ALASKA" [010h 0016 8] Oem Table ID : "A M I" [018h 0024 4] Oem Revision : 01072009 [01Ch 0028 4] Asl Compiler ID : "MSFT" [020h 0032 4] Asl Compiler Revision : 00000097
[024h 0036 8] Reserved : 0000000000000000
[02Ch 0044 8] Base Address : 00000000F8000000 [034h 0052 2] Segment Group Number : 0000 [036h 0054 1] Start Bus Number : 00 [037h 0055 1] End Bus Number : 3F [038h 0056 4] Reserved : 00000000
Raw Table Data: Length 60 (0x3C)
00000000 4d 43 46 47 3c 00 00 00 01 a9 41 4c 41 53 4b 41 |MCFG<.....ALASKA| 00000010 41 20 4d 20 49 00 00 00 09 20 07 01 4d 53 46 54 |A M I.... ..MSFT| 00000020 97 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f8 |................| 00000030 00 00 00 00 00 00 00 3f 00 00 00 00 |.......?....|
So in this example I have OEM table ID "A M I" 6 character long and 0 padding.
Tomasz
On 29 June 2016 at 15:52, Tomasz Nowicki tn@semihalf.com wrote:
On 29.06.2016 15:34, Christopher Covington wrote:
I'm confused by this statement. OEMID is defined as 6 bytes long and OEM Table ID as 8 bytes long in the ACPI specification. As far as I can tell, if your string isn't exactly that long, padding up to that length is required.
Well, I cannot find that requirement in ACPI spec. but I might missed something.
This has *nothing* to do with the ACPI spec.
There exist a couple of machines that have known quirks. Those machines have certain 6 or 8 byte long strings in their MCFG tables that we can match against. If any of those fields contains spaces, whether you call that padding or not is totally irrelevant. The same applies to NUL bytes.
So for a 6 byte field, our table contains the six characters that we know is in the quirky hardware's table. The same for an 8 byte field.
That means there is no need for strnlen, strncmp() with variable size etc. Each match can be implemented using memcmp() [since the table fields are not NUL terminated] with a constant argument for the length parameter.
Regards, Ard.
I dumped my x86 machine ACPI tables and here is an example of MCFG:
$ cat mcfg.dsl /*
- Intel ACPI Component Architecture
- AML/ASL+ Disassembler version 20160108-64
- Copyright (c) 2000 - 2016 Intel Corporation
- Disassembly of mcfg.dat, Wed Jun 29 15:48:16 2016
- ACPI Data Table [MCFG]
- Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue
*/
[000h 0000 4] Signature : "MCFG" [Memory Mapped Configuration table] [004h 0004 4] Table Length : 0000003C [008h 0008 1] Revision : 01 [009h 0009 1] Checksum : A9 [00Ah 0010 6] Oem ID : "ALASKA" [010h 0016 8] Oem Table ID : "A M I" [018h 0024 4] Oem Revision : 01072009 [01Ch 0028 4] Asl Compiler ID : "MSFT" [020h 0032 4] Asl Compiler Revision : 00000097
[024h 0036 8] Reserved : 0000000000000000
[02Ch 0044 8] Base Address : 00000000F8000000 [034h 0052 2] Segment Group Number : 0000 [036h 0054 1] Start Bus Number : 00 [037h 0055 1] End Bus Number : 3F [038h 0056 4] Reserved : 00000000
Raw Table Data: Length 60 (0x3C)
00000000 4d 43 46 47 3c 00 00 00 01 a9 41 4c 41 53 4b 41 |MCFG<.....ALASKA| 00000010 41 20 4d 20 49 00 00 00 09 20 07 01 4d 53 46 54 |A M I.... ..MSFT| 00000020 97 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f8 |................| 00000030 00 00 00 00 00 00 00 3f 00 00 00 00 |.......?....|
So in this example I have OEM table ID "A M I" 6 character long and 0 padding.
Tomasz
On 6/29/2016 7:52 AM, Tomasz Nowicki wrote:
On 29.06.2016 15:34, Christopher Covington wrote:
I'm confused by this statement. OEMID is defined as 6 bytes long and OEM Table ID as 8 bytes long in the ACPI specification. As far as I can tell, if your string isn't exactly that long, padding up to that length is required.
Well, I cannot find that requirement in ACPI spec. but I might missed something.
Have a look at table 5-29 in Section 5.2.6 which starts on page 114 of ACPI 6.1
I dumped my x86 machine ACPI tables and here is an example of MCFG:
$ cat mcfg.dsl /*
- Intel ACPI Component Architecture
- AML/ASL+ Disassembler version 20160108-64
- Copyright (c) 2000 - 2016 Intel Corporation
- Disassembly of mcfg.dat, Wed Jun 29 15:48:16 2016
- ACPI Data Table [MCFG]
- Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue
*/
[000h 0000 4] Signature : "MCFG" [Memory Mapped Configuration table] [004h 0004 4] Table Length : 0000003C [008h 0008 1] Revision : 01 [009h 0009 1] Checksum : A9 [00Ah 0010 6] Oem ID : "ALASKA" [010h 0016 8] Oem Table ID : "A M I" [018h 0024 4] Oem Revision : 01072009 [01Ch 0028 4] Asl Compiler ID : "MSFT" [020h 0032 4] Asl Compiler Revision : 00000097
[024h 0036 8] Reserved : 0000000000000000
[02Ch 0044 8] Base Address : 00000000F8000000 [034h 0052 2] Segment Group Number : 0000 [036h 0054 1] Start Bus Number : 00 [037h 0055 1] End Bus Number : 3F [038h 0056 4] Reserved : 00000000
Raw Table Data: Length 60 (0x3C)
00000000 4d 43 46 47 3c 00 00 00 01 a9 41 4c 41 53 4b 41 |MCFG<.....ALASKA| 00000010 41 20 4d 20 49 00 00 00 09 20 07 01 4d 53 46 54 |A M I.... ..MSFT| 00000020 97 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f8 |................| 00000030 00 00 00 00 00 00 00 3f 00 00 00 00 |.......?....|
So in this example I have OEM table ID "A M I" 6 character long and 0 padding.
Tomasz
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 29 June 2016 at 15:34, Christopher Covington cov@codeaurora.org wrote:
Hi Tomasz,
On 06/29/2016 06:48 AM, Tomasz Nowicki wrote:
On 28.06.2016 18:12, Duc Dang wrote:
On Tue, Jun 28, 2016 at 6:04 AM, Christopher Covington cov@codeaurora.org wrote:
Hi Tomasz,
Ard's comments on v3 included:
"... exact OEM table/rev id matches ..." "... substring match ... out of the question ..."
Digging through the archives I see Jon Master commented earlier to "be careful with substring match".
I think having OEM Table ID as "PLAT " and then "PLAT2 " (the the next version of the SoC) is common. So yes, matching full string is better as we can use "PLAT2 " in MCFG table and not worry about the "PLAT" sub-string match causes the quirk to be applied unintentionally.
Note that platforms already shipped where OEM string has no padding will
I'm confused by this statement. OEMID is defined as 6 bytes long and OEM Table ID as 8 bytes long in the ACPI specification. As far as I can tell, if your string isn't exactly that long, padding up to that length is required.
have change the firmware or add 0 padding to our quirk array IDs.
The fixed 6 or 8 character string compare, as used v2 of this patchset, will be compatible with existing firmware as best I can tell. Adding padding to the quirk array IDs is exactly what I'm suggesting, although all the strings I've seen are space padded rather than null padded.
I don't think any interpretation of the 6 or 8 byte wide OEM fields is necessary to be able to match it against a list of known values as used by the quirky platforms. We need an exact match against whatever we know is in the table of an affected system, and whether a space qualifies as padding or as a character is irrelevant.
Matches: {"APM ", "XGENE ", 1} {"CAVIUM", "THUNDERX", 1} {"HISI ", "HISI-D02", 1} {"HISI ", "HISI-D03", 1} {"QCOM ", "QDF2432 ", 1}
I would not mind listing these as
{ { 'A','P','M',' ',' ',' ',' '}, {'X','G','E','N','E',' ',' ',' '}, 1} ...
just to stress that we are not dealing with C strings (and to avoid having to deal with the implicit NUL terminator). That also means memcmp() with a fixed length is the most appropriate to perform the comparison
Given the above tuples, won't accidentally match: (guessing at possible future ids) {"APM ", "XGENEi ", 1} {"CAVIUM", "THUNDERX", i} i != 1 {"CAVIUM", "THUNDERi", 1} {"CAVIUM", "THUNDRXi", 1} {"HISI ", "HISI-D0i", 1} i != 2 && i != 3 {"QCOM ", "QDF24ij ", 1} i != 3 && j != 2
References for APM, HiSilicon IDs: https://lists.linaro.org/pipermail/linaro-acpi/2016-June/007108.html https://lists.linaro.org/pipermail/linaro-acpi/2016-June/007043.html
Thanks, Cov
-- Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
On 29.06.16 15:56:50, Ard Biesheuvel wrote:
On 29 June 2016 at 15:34, Christopher Covington cov@codeaurora.org wrote:
Hi Tomasz,
On 06/29/2016 06:48 AM, Tomasz Nowicki wrote:
On 28.06.2016 18:12, Duc Dang wrote:
On Tue, Jun 28, 2016 at 6:04 AM, Christopher Covington cov@codeaurora.org wrote:
Hi Tomasz,
Ard's comments on v3 included:
"... exact OEM table/rev id matches ..." "... substring match ... out of the question ..."
Digging through the archives I see Jon Master commented earlier to "be careful with substring match".
I think having OEM Table ID as "PLAT " and then "PLAT2 " (the the next version of the SoC) is common. So yes, matching full string is better as we can use "PLAT2 " in MCFG table and not worry about the "PLAT" sub-string match causes the quirk to be applied unintentionally.
Note that platforms already shipped where OEM string has no padding will
I'm confused by this statement. OEMID is defined as 6 bytes long and OEM Table ID as 8 bytes long in the ACPI specification. As far as I can tell, if your string isn't exactly that long, padding up to that length is required.
have change the firmware or add 0 padding to our quirk array IDs.
The fixed 6 or 8 character string compare, as used v2 of this patchset, will be compatible with existing firmware as best I can tell. Adding padding to the quirk array IDs is exactly what I'm suggesting, although all the strings I've seen are space padded rather than null padded.
I don't think any interpretation of the 6 or 8 byte wide OEM fields is necessary to be able to match it against a list of known values as used by the quirky platforms. We need an exact match against whatever we know is in the table of an affected system, and whether a space qualifies as padding or as a character is irrelevant.
Matches: {"APM ", "XGENE ", 1} {"CAVIUM", "THUNDERX", 1} {"HISI ", "HISI-D02", 1} {"HISI ", "HISI-D03", 1} {"QCOM ", "QDF2432 ", 1}
I would not mind listing these as
{ { 'A','P','M',' ',' ',' ',' '}, {'X','G','E','N','E',' ',' ',' '}, 1} ...
just to stress that we are not dealing with C strings (and to avoid having to deal with the implicit NUL terminator). That also means memcmp() with a fixed length is the most appropriate to perform the comparison
I still would go with memcmp but have the char arrays null terminated in addition. This first makes string handling easier, and fixes some unterminated %s printfs bugs in the code.
Thus, I would prefer to go with:
struct pci_cfg_fixup { char oem_id[ACPI_OEM_ID_SIZE + 1]; char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1]; ...
static struct pci_cfg_quirks mcfg_qurks[] __initconst = { /* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */ #ifdef CONFIG_PCI_HOST_THUNDER_PEM /* Pass2.0 */ { "CAVIUM", "THUNDERX", 1, ...
This is also no "pain in the eyes". :)
If there are zero bytes in then just use \0, e.g.:
{ "foo\0\0\0", "foobar\0\0", ... }
For comparisation still use memcmp accordingly:
memcmp(..., ACPI_OEM_ID_SIZE); memcmp(..., ACPI_OEM_TABLE_ID_SIZE);
The following would be fixed too as strings are now null terminated:
pr_info("Handling %s %s r%d PCI MCFG quirks\n", f->oem_id, f->oem_table_id, f->oem_revision);
Btw, use dev_info(&root->device->dev, ...) here for pr_info() and modify message text, e.g.:
acpi PNP0A08:04: Applying PCI MCFG quirks for CAVIUM THUNDERX rev: 1
And, we should support some sort of MCFG_OEM_REVISION_ANY to move the rev handling optional to pci_cfg_fixup::init().
Plus one spelling fix: mcfg_qurks -> mcfg_quirks
Thanks,
-Robert
Given the above tuples, won't accidentally match: (guessing at possible future ids) {"APM ", "XGENEi ", 1} {"CAVIUM", "THUNDERX", i} i != 1 {"CAVIUM", "THUNDERi", 1} {"CAVIUM", "THUNDRXi", 1} {"HISI ", "HISI-D0i", 1} i != 2 && i != 3 {"QCOM ", "QDF24ij ", 1} i != 3 && j != 2
References for APM, HiSilicon IDs: https://lists.linaro.org/pipermail/linaro-acpi/2016-June/007108.html https://lists.linaro.org/pipermail/linaro-acpi/2016-June/007043.html
Thanks, Cov
-- Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
On 22 July 2016 at 13:38, Robert Richter rric@kernel.org wrote:
On 29.06.16 15:56:50, Ard Biesheuvel wrote:
On 29 June 2016 at 15:34, Christopher Covington cov@codeaurora.org wrote:
Hi Tomasz,
On 06/29/2016 06:48 AM, Tomasz Nowicki wrote:
On 28.06.2016 18:12, Duc Dang wrote:
On Tue, Jun 28, 2016 at 6:04 AM, Christopher Covington cov@codeaurora.org wrote:
Hi Tomasz,
Ard's comments on v3 included:
"... exact OEM table/rev id matches ..." "... substring match ... out of the question ..."
Digging through the archives I see Jon Master commented earlier to "be careful with substring match".
I think having OEM Table ID as "PLAT " and then "PLAT2 " (the the next version of the SoC) is common. So yes, matching full string is better as we can use "PLAT2 " in MCFG table and not worry about the "PLAT" sub-string match causes the quirk to be applied unintentionally.
Note that platforms already shipped where OEM string has no padding will
I'm confused by this statement. OEMID is defined as 6 bytes long and OEM Table ID as 8 bytes long in the ACPI specification. As far as I can tell, if your string isn't exactly that long, padding up to that length is required.
have change the firmware or add 0 padding to our quirk array IDs.
The fixed 6 or 8 character string compare, as used v2 of this patchset, will be compatible with existing firmware as best I can tell. Adding padding to the quirk array IDs is exactly what I'm suggesting, although all the strings I've seen are space padded rather than null padded.
I don't think any interpretation of the 6 or 8 byte wide OEM fields is necessary to be able to match it against a list of known values as used by the quirky platforms. We need an exact match against whatever we know is in the table of an affected system, and whether a space qualifies as padding or as a character is irrelevant.
Matches: {"APM ", "XGENE ", 1} {"CAVIUM", "THUNDERX", 1} {"HISI ", "HISI-D02", 1} {"HISI ", "HISI-D03", 1} {"QCOM ", "QDF2432 ", 1}
I would not mind listing these as
{ { 'A','P','M',' ',' ',' ',' '}, {'X','G','E','N','E',' ',' ',' '}, 1} ...
just to stress that we are not dealing with C strings (and to avoid having to deal with the implicit NUL terminator). That also means memcmp() with a fixed length is the most appropriate to perform the comparison
I still would go with memcmp but have the char arrays null terminated in addition. This first makes string handling easier, and fixes some unterminated %s printfs bugs in the code.
Thus, I would prefer to go with:
struct pci_cfg_fixup { char oem_id[ACPI_OEM_ID_SIZE + 1]; char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1]; ...
static struct pci_cfg_quirks mcfg_qurks[] __initconst = { /* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */ #ifdef CONFIG_PCI_HOST_THUNDER_PEM /* Pass2.0 */ { "CAVIUM", "THUNDERX", 1, ...
This is also no "pain in the eyes". :)
If there are zero bytes in then just use \0, e.g.:
{ "foo\0\0\0", "foobar\0\0", ... }
For comparisation still use memcmp accordingly:
memcmp(..., ACPI_OEM_ID_SIZE); memcmp(..., ACPI_OEM_TABLE_ID_SIZE);
The following would be fixed too as strings are now null terminated:
pr_info("Handling %s %s r%d PCI MCFG quirks\n", f->oem_id, f->oem_table_id, f->oem_revision);
This looks like a clear improvement to me.
Btw, use dev_info(&root->device->dev, ...) here for pr_info() and modify message text, e.g.:
acpi PNP0A08:04: Applying PCI MCFG quirks for CAVIUM THUNDERX rev: 1
And, we should support some sort of MCFG_OEM_REVISION_ANY to move the rev handling optional to pci_cfg_fixup::init().
xxx_ANY implies 'wildcard', which we don't want in this code. The set of quirky hardware we intend to support is known, and wildcard matching makes it easier to circumvent our policy that from here on, i.e., that all ACPI/arm64 supported hardware shall adhere to the spec.
On 22.07.16 14:00:42, Ard Biesheuvel wrote:
On 22 July 2016 at 13:38, Robert Richter rric@kernel.org wrote:
And, we should support some sort of MCFG_OEM_REVISION_ANY to move the rev handling optional to pci_cfg_fixup::init().
xxx_ANY implies 'wildcard', which we don't want in this code. The set of quirky hardware we intend to support is known, and wildcard matching makes it easier to circumvent our policy that from here on, i.e., that all ACPI/arm64 supported hardware shall adhere to the spec.
Fine with me, there shouln't be to many revs around to have a quirk per rev.
-Robert
On Tue, 2016-06-28 at 09:54 +0200, 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. Algorithm traverses available quirk list (static array), matches against <oem_id, oem_table_id, rev, domain, bus number range> and returns pci_config_window structure with fancy PCI config ops. oem_id, oem_table_id and rev come from MCFG table standard header.
It is possible to define custom init call which is responsible for setting up PCI config access accordingly to quirk requirements. If no custom init call defined, use standard pci_acpi_setup_ecam_mapping().
pci_generic_ecam_ops will be used for platforms free from quirks.
Signed-off-by: Tomasz Nowicki tn@semihalf.com Signed-off-by: Dongdong Liu liudongdong3@huawei.com Signed-off-by: Christopher Covington cov@codeaurora.org
drivers/pci/host/Makefile | 1 + drivers/pci/host/mcfg-quirks.c | 88 ++++++++++++++++++++++++++++++++++++++++++ drivers/pci/host/mcfg-quirks.h | 20 ++++++++++ include/linux/pci-acpi.h | 2 + 4 files changed, 111 insertions(+) create mode 100644 drivers/pci/host/mcfg-quirks.c create mode 100644 drivers/pci/host/mcfg-quirks.h
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile index 5bc0af2..e3d55a0 100644 --- a/drivers/pci/host/Makefile +++ b/drivers/pci/host/Makefile @@ -30,3 +30,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o +obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c new file mode 100644 index 0000000..fb2b184 --- /dev/null +++ b/drivers/pci/host/mcfg-quirks.c @@ -0,0 +1,88 @@ +/*
- Copyright (C) 2016 Semihalf
- Author: Tomasz Nowicki tn@semihalf.com
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License, version 2, as
- published by the Free Software Foundation (the "GPL").
- This program is distributed in the hope that it will be useful, but
- WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- General Public License version 2 (GPLv2) for more details.
- You should have received a copy of the GNU General Public License
- version 2 (GPLv2) along with this source code.
- */
+#include <linux/kernel.h> +#include <linux/ioport.h> +#include <linux/pci.h> +#include <linux/pci-acpi.h> +#include <linux/pci-ecam.h>
+#include "mcfg-quirks.h"
+struct pci_cfg_fixup {
- char *oem_id;
- char *oem_table_id;
- u32 oem_revision;
- struct resource domain_range;
- struct resource bus_range;
- struct pci_ops *ops;
- struct pci_config_window *(*init)(struct acpi_pci_root *root,
struct pci_ops *ops);+};
+#define MCFG_DOM_RANGE(start, end) DEFINE_RES_NAMED((start), \
((end) - (start) + 1), NULL, 0)+#define MCFG_DOM_ANY MCFG_DOM_RANGE(0x0, 0xffff) +#define MCFG_BUS_RANGE(start, end) DEFINE_RES_NAMED((start), \
((end) - (start) + 1), \NULL, IORESOURCE_BUS)+#define MCFG_BUS_ANY MCFG_BUS_RANGE(0x0, 0xff)
+static struct pci_cfg_fixup mcfg_qurks[] __initconst = {
^^^^^^^^^^ mcfg_quirks
+/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */ +};
+static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
struct acpi_table_header *mcfg_header)+{
- int olen = min_t(u8, strlen(f->oem_id), ACPI_OEM_ID_SIZE);
- int tlen = min_t(u8, strlen(f->oem_table_id), ACPI_OEM_TABLE_ID_SIZE);
- return (!strncmp(f->oem_id, mcfg_header->oem_id, olen) &&
!strncmp(f->oem_table_id, mcfg_header->oem_table_id, tlen) &&f->oem_revision == mcfg_header->oem_revision);+}
+struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root) +{
- struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
- struct resource *bus_res = &root->secondary;
- struct pci_cfg_fixup *f = mcfg_qurks;
- struct acpi_table_header *mcfg_header;
- acpi_status status;
- int i;
- status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
- if (ACPI_FAILURE(status))
return NULL;- /*
- * First match against PCI topology domain:bus then use OEM ID, OEM
- * table ID, and OEM revision from MCFG table standard header.
- */
- for (i = 0; i < ARRAY_SIZE(mcfg_qurks); i++, f++) {
if (resource_contains(&f->domain_range, &dom_res) &&resource_contains(&f->bus_range, bus_res) &&pci_mcfg_fixup_match(f, mcfg_header)) {pr_info("Handling %s %s r%d PCI MCFG quirks\n",f->oem_id, f->oem_table_id, f->oem_revision);return f->init ? f->init(root, f->ops) :pci_acpi_setup_ecam_mapping(root, f->ops);}- }
- return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);
+} diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h new file mode 100644 index 0000000..45cbd16 --- /dev/null +++ b/drivers/pci/host/mcfg-quirks.h @@ -0,0 +1,20 @@ +/*
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License, version 2, as
- published by the Free Software Foundation (the "GPL").
- This program is distributed in the hope that it will be useful, but
- WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- General Public License version 2 (GPLv2) for more details.
- You should have received a copy of the GNU General Public License
- version 2 (GPLv2) along with this source code.
- */
+#ifndef __MCFG_QUIRKS_H__ +#define __MCFG_QUIRKS_H__
+/* MCFG quirks initialize call list */
+#endif /* __MCFG_QUIRKS_H__ */ diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h index e9bfe00..28cdce4 100644 --- a/include/linux/pci-acpi.h +++ b/include/linux/pci-acpi.h @@ -25,6 +25,8 @@ static inline acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev) extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle); extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res); +extern struct pci_config_window * +pci_mcfg_match_quirks(struct acpi_pci_root *root); extern struct pci_config_window * pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
Since there are platforms which have non-compliant ECAM space we need to override these accessors prior to PCI buses enumeration. In order to do that we call pci_mcfg_match_quirks() to retrieve custom pci_config_window structure. If no correlated quirk on list, use fully ECAM compliant generic PCI config accessors.
Signed-off-by: Tomasz Nowicki tn@semihalf.com --- arch/arm64/kernel/pci.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index addf00c..ef142b8 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -152,8 +152,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) if (!ri) return NULL;
- ri->cfg = pci_acpi_setup_ecam_mapping(root, - &pci_generic_ecam_ops.pci_ops); + ri->cfg = pci_mcfg_match_quirks(root); if (!ri->cfg) { kfree(ri); return NULL;
Add ThunderX PEM specific infrastructure to support configuration access for ACPI based PCI host controller. This involves: 1. New initialization call thunder_pem_cfg_init() to create configuration space mapping with hardcoded range address 2. thunder_pem_init() ACPI extension to get hardcoded address for PEM specific register range 3. New entry in common quirk array.
Signed-off-by: Tomasz Nowicki tn@semihalf.com --- drivers/pci/host/mcfg-quirks.c | 7 +++ drivers/pci/host/mcfg-quirks.h | 4 ++ drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------ 3 files changed, 94 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c index fb2b184..a4bb76a 100644 --- a/drivers/pci/host/mcfg-quirks.c +++ b/drivers/pci/host/mcfg-quirks.c @@ -44,6 +44,13 @@ struct pci_cfg_fixup {
static struct pci_cfg_fixup mcfg_qurks[] __initconst = { /* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */ +#ifdef CONFIG_PCI_HOST_THUNDER_PEM + /* Pass2.0 */ + { "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(4, 9), MCFG_BUS_ANY, + NULL, thunder_pem_cfg_init}, + { "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, + NULL, thunder_pem_cfg_init}, +#endif };
static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f, diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h index 45cbd16..411c667 100644 --- a/drivers/pci/host/mcfg-quirks.h +++ b/drivers/pci/host/mcfg-quirks.h @@ -16,5 +16,9 @@ #define __MCFG_QUIRKS_H__
/* MCFG quirks initialize call list */ +#ifdef CONFIG_PCI_HOST_THUNDER_PEM +struct pci_config_window * +thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops); +#endif
#endif /* __MCFG_QUIRKS_H__ */ diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c index 91f6fc6..73ac537 100644 --- a/drivers/pci/host/pci-thunder-pem.c +++ b/drivers/pci/host/pci-thunder-pem.c @@ -18,9 +18,12 @@ #include <linux/module.h> #include <linux/of_address.h> #include <linux/of_pci.h> +#include <linux/pci-acpi.h> #include <linux/pci-ecam.h> #include <linux/platform_device.h>
+#include "mcfg-quirks.h" + #define PEM_CFG_WR 0x28 #define PEM_CFG_RD 0x30
@@ -284,6 +287,37 @@ static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn, return pci_generic_config_write(bus, devfn, where, size, val); }
+#ifdef CONFIG_ACPI + +static struct resource thunder_pem_reg_res[] = { + [4] = DEFINE_RES_MEM(0x87e0c0000000UL, SZ_16M), + [5] = DEFINE_RES_MEM(0x87e0c1000000UL, SZ_16M), + [6] = DEFINE_RES_MEM(0x87e0c2000000UL, SZ_16M), + [7] = DEFINE_RES_MEM(0x87e0c3000000UL, SZ_16M), + [8] = DEFINE_RES_MEM(0x87e0c4000000UL, SZ_16M), + [9] = DEFINE_RES_MEM(0x87e0c5000000UL, SZ_16M), + [14] = DEFINE_RES_MEM(0x97e0c0000000UL, SZ_16M), + [15] = DEFINE_RES_MEM(0x97e0c1000000UL, SZ_16M), + [16] = DEFINE_RES_MEM(0x97e0c2000000UL, SZ_16M), + [17] = DEFINE_RES_MEM(0x97e0c3000000UL, SZ_16M), + [18] = DEFINE_RES_MEM(0x97e0c4000000UL, SZ_16M), + [19] = DEFINE_RES_MEM(0x97e0c5000000UL, SZ_16M), +}; + +static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg) +{ + struct acpi_device *adev = to_acpi_device(cfg->parent); + struct acpi_pci_root *root = acpi_driver_data(adev); + + return &thunder_pem_reg_res[root->segment]; +} +#else +static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg) +{ + return NULL; +} +#endif + static int thunder_pem_init(struct pci_config_window *cfg) { struct device *dev = cfg->parent; @@ -292,24 +326,24 @@ static int thunder_pem_init(struct pci_config_window *cfg) struct thunder_pem_pci *pem_pci; struct platform_device *pdev;
- /* Only OF support for now */ - if (!dev->of_node) - return -EINVAL; - pem_pci = devm_kzalloc(dev, sizeof(*pem_pci), GFP_KERNEL); if (!pem_pci) return -ENOMEM;
- pdev = to_platform_device(dev); - - /* - * The second register range is the PEM bridge to the PCIe - * bus. It has a different config access method than those - * devices behind the bridge. - */ - res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1); + if (acpi_disabled) { + pdev = to_platform_device(dev); + + /* + * The second register range is the PEM bridge to the PCIe + * bus. It has a different config access method than those + * devices behind the bridge. + */ + res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1); + } else { + res_pem = thunder_pem_acpi_res(cfg); + } if (!res_pem) { - dev_err(dev, "missing "reg[1]"property\n"); + dev_err(dev, "missing configuration region\n"); return -EINVAL; }
@@ -362,5 +396,41 @@ static struct platform_driver thunder_pem_driver = { }; module_platform_driver(thunder_pem_driver);
+#ifdef CONFIG_ACPI + +static struct resource thunder_pem_cfg_res[] = { + [4] = DEFINE_RES_MEM(0x88001f000000UL, 0x39 * SZ_16M), + [5] = DEFINE_RES_MEM(0x884057000000UL, 0x39 * SZ_16M), + [6] = DEFINE_RES_MEM(0x88808f000000UL, 0x39 * SZ_16M), + [7] = DEFINE_RES_MEM(0x89001f000000UL, 0x39 * SZ_16M), + [8] = DEFINE_RES_MEM(0x894057000000UL, 0x39 * SZ_16M), + [9] = DEFINE_RES_MEM(0x89808f000000UL, 0x39 * SZ_16M), + [14] = DEFINE_RES_MEM(0x98001f000000UL, 0x39 * SZ_16M), + [15] = DEFINE_RES_MEM(0x984057000000UL, 0x39 * SZ_16M), + [16] = DEFINE_RES_MEM(0x98808f000000UL, 0x39 * SZ_16M), + [17] = DEFINE_RES_MEM(0x99001f000000UL, 0x39 * SZ_16M), + [18] = DEFINE_RES_MEM(0x994057000000UL, 0x39 * SZ_16M), + [19] = DEFINE_RES_MEM(0x99808f000000UL, 0x39 * SZ_16M), +}; + +struct pci_config_window * +thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops) +{ + struct resource *bus_res = &root->secondary; + u16 seg = root->segment; + struct pci_config_window *cfg; + + cfg = pci_ecam_create(&root->device->dev, &thunder_pem_cfg_res[seg], + bus_res, &pci_thunder_pem_ops); + if (IS_ERR(cfg)) { + dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n", + seg, bus_res, PTR_ERR(cfg)); + return NULL; + } + + return cfg; +} +#endif + MODULE_DESCRIPTION("Thunder PEM PCIe host driver"); MODULE_LICENSE("GPL v2");
-
Ard Biesheuvel -
Christopher Covington -
Duc Dang -
Jeffrey Hugo -
Mark Salter -
Robert Richter -
Tomasz Nowicki