From the functionality point of view this series might be split into two logic parts:
1. Making MMCONFIG code arch-agnostic which allows all architectures to collect PCI config regions and used when necessary. 2. Using generic MMCONFIG code and introducing ACPI based PCI hostbridge initialization for ARM64
Patches has been built on top of: [Patch v7 0/7] Consolidate ACPI PCI root common code into ACPI core https://lkml.org/lkml/2015/10/14/31 Git branch can be found here: https://git.linaro.org/leg/acpi/acpi.git/shortlog/refs/heads/pci-acpi-upstre...
This has been tested on Cavium ThunderX 1 socket server. Any help in reviewing and testing is very appreciated.
Hanjun Guo (1): XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y
Tomasz Nowicki (10): x86, pci: Reorder logic of pci_mmconfig_insert() function x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory pci, acpi, mcfg: Provide generic implementation of MCFG code initialization. x86, pci: mmconfig_{32,64}.c code refactoring - remove code duplication. x86, pci, ecam: mmconfig_64.c becomes default implementation for ECAM driver. pci, acpi, mcfg: Provide default RAW ACPI PCI config space accessors. pci, acpi, ecam: Add flag to indicate whether ECAM region was hot added or not. x86, pci: Use previously added ECAM hot_added flag to remove ECAM regions. pci, acpi: Provide generic way to assign bus domain number. arm64, pci, acpi: Support for ACPI based PCI hostbridge init
arch/arm64/Kconfig | 6 + arch/arm64/kernel/pci.c | 208 ++++++++++++++++++++++++++++++++-- arch/x86/Kconfig | 4 + arch/x86/include/asm/pci_x86.h | 28 +---- arch/x86/pci/acpi.c | 17 +-- arch/x86/pci/mmconfig-shared.c | 250 +++++++---------------------------------- arch/x86/pci/mmconfig_32.c | 11 +- arch/x86/pci/mmconfig_64.c | 67 +---------- arch/x86/pci/numachip.c | 1 + drivers/acpi/Makefile | 1 + drivers/acpi/mcfg.c | 104 +++++++++++++++++ drivers/acpi/pci_root.c | 2 +- drivers/pci/Kconfig | 10 ++ drivers/pci/Makefile | 5 + drivers/pci/ecam.c | 234 ++++++++++++++++++++++++++++++++++++++ drivers/pci/pci.c | 30 ++++- drivers/xen/pci.c | 7 +- include/linux/acpi.h | 2 + include/linux/ecam.h | 44 ++++++++ 19 files changed, 691 insertions(+), 340 deletions(-) create mode 100644 drivers/acpi/mcfg.c create mode 100644 drivers/pci/ecam.c create mode 100644 include/linux/ecam.h
This patch is the first step for MMCONFIG refactoring process.
Code that uses pci_mmcfg_lock will be moved to common file and become accessible for all architectures. pci_mmconfig_insert() cannot be moved so easily since it is mixing generic mmconfig code with x86 specific logic inside of mutual exclusive block guarded by pci_mmcfg_lock.
To get rid of that constraint, we reorder actions as follow: 1. sanity check for mmconfig region presence, if we already have such region it doesn't make snese to alloc new mmconfig list entry 2. mmconfig entry allocation, no need to lock 3. insertion to iomem_resource has its own lock, no need to wrap it into mutex 4. insertion to mmconfig list can be done as the final step in separate function (candidate for further refactoring) and needs another mmconfig lookup to avoid race condition.
Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org --- arch/x86/pci/mmconfig-shared.c | 101 +++++++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 43 deletions(-)
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index dd30b7e..c8bb9b0 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -720,6 +720,38 @@ static int __init pci_mmcfg_late_insert_resources(void) */ late_initcall(pci_mmcfg_late_insert_resources);
+static int pci_mmconfig_inject(struct pci_mmcfg_region *cfg) +{ + struct pci_mmcfg_region *cfg_conflict; + int err = 0; + + mutex_lock(&pci_mmcfg_lock); + cfg_conflict = pci_mmconfig_lookup(cfg->segment, cfg->start_bus); + if (cfg_conflict) { + if (cfg_conflict->end_bus < cfg->end_bus) + pr_info(FW_INFO "MMCONFIG for " + "domain %04x [bus %02x-%02x] " + "only partially covers this bridge\n", + cfg_conflict->segment, cfg_conflict->start_bus, + cfg_conflict->end_bus); + err = -EEXIST; + goto out; + } + + if (pci_mmcfg_arch_map(cfg)) { + pr_warn("fail to map MMCONFIG %pR.\n", &cfg->res); + err = -ENOMEM; + goto out; + } else { + list_add_sorted(cfg); + pr_info("MMCONFIG at %pR (base %#lx)\n", + &cfg->res, (unsigned long)cfg->address); + } +out: + mutex_unlock(&pci_mmcfg_lock); + return err; +} + /* Add MMCFG information for host bridges */ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, phys_addr_t addr) @@ -734,63 +766,46 @@ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, if (start > end) return -EINVAL;
- mutex_lock(&pci_mmcfg_lock); + rcu_read_lock(); cfg = pci_mmconfig_lookup(seg, start); - if (cfg) { - if (cfg->end_bus < end) - dev_info(dev, FW_INFO - "MMCONFIG for " - "domain %04x [bus %02x-%02x] " - "only partially covers this bridge\n", - cfg->segment, cfg->start_bus, cfg->end_bus); - mutex_unlock(&pci_mmcfg_lock); + rcu_read_unlock(); + if (cfg) return -EEXIST; - }
- if (!addr) { - mutex_unlock(&pci_mmcfg_lock); + if (!addr) return -EINVAL; - }
- rc = -EBUSY; cfg = pci_mmconfig_alloc(seg, start, end, addr); - if (cfg == NULL) { + if (!cfg) { dev_warn(dev, "fail to add MMCONFIG (out of memory)\n"); - rc = -ENOMEM; - } else if (!pci_mmcfg_check_reserved(dev, cfg, 0)) { + return -ENOMEM; + } + + rc = -EBUSY; + if (!pci_mmcfg_check_reserved(dev, cfg, 0)) { dev_warn(dev, FW_BUG "MMCONFIG %pR isn't reserved\n", &cfg->res); - } else { - /* Insert resource if it's not in boot stage */ - if (pci_mmcfg_running_state) - tmp = insert_resource_conflict(&iomem_resource, - &cfg->res); - - if (tmp) { - dev_warn(dev, - "MMCONFIG %pR conflicts with " - "%s %pR\n", - &cfg->res, tmp->name, tmp); - } else if (pci_mmcfg_arch_map(cfg)) { - dev_warn(dev, "fail to map MMCONFIG %pR.\n", - &cfg->res); - } else { - list_add_sorted(cfg); - dev_info(dev, "MMCONFIG at %pR (base %#lx)\n", - &cfg->res, (unsigned long)addr); - cfg = NULL; - rc = 0; - } + goto error; }
- if (cfg) { - if (cfg->res.parent) - release_resource(&cfg->res); - kfree(cfg); + /* Insert resource if it's not in boot stage */ + if (pci_mmcfg_running_state) + tmp = insert_resource_conflict(&iomem_resource, &cfg->res); + + if (tmp) { + dev_warn(dev, "MMCONFIG %pR conflicts with %s %pR\n", + &cfg->res, tmp->name, tmp); + goto error; }
- mutex_unlock(&pci_mmcfg_lock); + rc = pci_mmconfig_inject(cfg); + if (!rc) + return 0;
+error: + if (cfg->res.parent) + release_resource(&cfg->res); + kfree(cfg); return rc; }
ECAM standard and MCFG table are architecture independent and it makes sense to share common code across all architectures. Both are going to corresponding files - ecam.c and mcfg.c
While we are here, rename pci_parse_mcfg to acpi_parse_mcfg. We already have acpi_parse_mcfg prototype which is used nowhere. At the same time, we need pci_parse_mcfg been global so acpi_parse_mcfg can be used perfectly here.
Signed-off-by: Tomasz Nowicki tn@semihalf.com --- arch/x86/Kconfig | 3 + arch/x86/include/asm/pci_x86.h | 21 ----- arch/x86/pci/acpi.c | 1 + arch/x86/pci/mmconfig-shared.c | 205 +---------------------------------------- arch/x86/pci/mmconfig_32.c | 1 + arch/x86/pci/mmconfig_64.c | 1 + arch/x86/pci/numachip.c | 1 + drivers/acpi/Makefile | 1 + drivers/acpi/mcfg.c | 59 ++++++++++++ drivers/pci/Kconfig | 7 ++ drivers/pci/Makefile | 5 + drivers/pci/ecam.c | 175 +++++++++++++++++++++++++++++++++++ drivers/xen/pci.c | 1 + include/linux/acpi.h | 2 + include/linux/ecam.h | 37 ++++++++ 15 files changed, 299 insertions(+), 221 deletions(-) create mode 100644 drivers/acpi/mcfg.c create mode 100644 drivers/pci/ecam.c create mode 100644 include/linux/ecam.h
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 96d058a..4ca8134 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -128,6 +128,7 @@ config X86 select HAVE_MIXED_BREAKPOINTS_REGS select HAVE_OPROFILE select HAVE_OPTPROBES + select HAVE_PCI_ECAM select HAVE_PCSPKR_PLATFORM select HAVE_PERF_EVENTS select HAVE_PERF_EVENTS_NMI @@ -2332,6 +2333,7 @@ config PCI_DIRECT
config PCI_MMCONFIG def_bool y + select PCI_ECAM depends on X86_32 && PCI && (ACPI || SFI) && (PCI_GOMMCONFIG || PCI_GOANY)
config PCI_OLPC @@ -2349,6 +2351,7 @@ config PCI_DOMAINS
config PCI_MMCONFIG bool "Support mmconfig PCI config space access" + select PCI_ECAM depends on X86_64 && PCI && ACPI
config PCI_CNB20LE_QUIRK diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h index fa1195d..64cb514 100644 --- a/arch/x86/include/asm/pci_x86.h +++ b/arch/x86/include/asm/pci_x86.h @@ -122,33 +122,12 @@ extern int pci_legacy_init(void); extern void pcibios_fixup_irqs(void);
/* pci-mmconfig.c */ - -/* "PCI MMCONFIG %04x [bus %02x-%02x]" */ -#define PCI_MMCFG_RESOURCE_NAME_LEN (22 + 4 + 2 + 2) - -struct pci_mmcfg_region { - struct list_head list; - struct resource res; - u64 address; - char __iomem *virt; - u16 segment; - u8 start_bus; - u8 end_bus; - char name[PCI_MMCFG_RESOURCE_NAME_LEN]; -}; - extern int __init pci_mmcfg_arch_init(void); extern void __init pci_mmcfg_arch_free(void); extern int pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg); extern void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg); extern int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, phys_addr_t addr); -extern int pci_mmconfig_delete(u16 seg, u8 start, u8 end); -extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus); - -extern struct list_head pci_mmcfg_list; - -#define PCI_MMCFG_BUS_OFFSET(bus) ((bus) << 20)
/* * AMD Fam10h CPUs are buggy, and cannot access MMIO config space diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c index 3cd6983..28c9cd6 100644 --- a/arch/x86/pci/acpi.c +++ b/arch/x86/pci/acpi.c @@ -5,6 +5,7 @@ #include <linux/dmi.h> #include <linux/slab.h> #include <linux/pci-acpi.h> +#include <linux/ecam.h> #include <asm/numa.h> #include <asm/pci_x86.h>
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index c8bb9b0..ce2c2e4 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -18,6 +18,7 @@ #include <linux/slab.h> #include <linux/mutex.h> #include <linux/rculist.h> +#include <linux/ecam.h> #include <asm/e820.h> #include <asm/pci_x86.h> #include <asm/acpi.h> @@ -27,103 +28,6 @@ /* Indicate if the mmcfg resources have been placed into the resource table. */ static bool pci_mmcfg_running_state; static bool pci_mmcfg_arch_init_failed; -static DEFINE_MUTEX(pci_mmcfg_lock); - -LIST_HEAD(pci_mmcfg_list); - -static void __init pci_mmconfig_remove(struct pci_mmcfg_region *cfg) -{ - if (cfg->res.parent) - release_resource(&cfg->res); - list_del(&cfg->list); - kfree(cfg); -} - -static void __init free_all_mmcfg(void) -{ - struct pci_mmcfg_region *cfg, *tmp; - - pci_mmcfg_arch_free(); - list_for_each_entry_safe(cfg, tmp, &pci_mmcfg_list, list) - pci_mmconfig_remove(cfg); -} - -static void list_add_sorted(struct pci_mmcfg_region *new) -{ - struct pci_mmcfg_region *cfg; - - /* keep list sorted by segment and starting bus number */ - list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) { - if (cfg->segment > new->segment || - (cfg->segment == new->segment && - cfg->start_bus >= new->start_bus)) { - list_add_tail_rcu(&new->list, &cfg->list); - return; - } - } - list_add_tail_rcu(&new->list, &pci_mmcfg_list); -} - -static struct pci_mmcfg_region *pci_mmconfig_alloc(int segment, int start, - int end, u64 addr) -{ - struct pci_mmcfg_region *new; - struct resource *res; - - if (addr == 0) - return NULL; - - new = kzalloc(sizeof(*new), GFP_KERNEL); - if (!new) - return NULL; - - new->address = addr; - new->segment = segment; - new->start_bus = start; - new->end_bus = end; - - res = &new->res; - res->start = addr + PCI_MMCFG_BUS_OFFSET(start); - res->end = addr + PCI_MMCFG_BUS_OFFSET(end + 1) - 1; - res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; - snprintf(new->name, PCI_MMCFG_RESOURCE_NAME_LEN, - "PCI MMCONFIG %04x [bus %02x-%02x]", segment, start, end); - res->name = new->name; - - return new; -} - -static struct pci_mmcfg_region *__init pci_mmconfig_add(int segment, int start, - int end, u64 addr) -{ - struct pci_mmcfg_region *new; - - new = pci_mmconfig_alloc(segment, start, end, addr); - if (new) { - mutex_lock(&pci_mmcfg_lock); - list_add_sorted(new); - mutex_unlock(&pci_mmcfg_lock); - - pr_info(PREFIX - "MMCONFIG for domain %04x [bus %02x-%02x] at %pR " - "(base %#lx)\n", - segment, start, end, &new->res, (unsigned long)addr); - } - - return new; -} - -struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus) -{ - struct pci_mmcfg_region *cfg; - - list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) - if (cfg->segment == segment && - cfg->start_bus <= bus && bus <= cfg->end_bus) - return cfg; - - return NULL; -}
static const char *__init pci_mmcfg_e7520(void) { @@ -543,8 +447,8 @@ static void __init pci_mmcfg_reject_broken(int early) } }
-static int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg, - struct acpi_mcfg_allocation *cfg) +int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg, + struct acpi_mcfg_allocation *cfg) { int year;
@@ -566,50 +470,6 @@ static int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg, return -EINVAL; }
-static int __init pci_parse_mcfg(struct acpi_table_header *header) -{ - struct acpi_table_mcfg *mcfg; - struct acpi_mcfg_allocation *cfg_table, *cfg; - unsigned long i; - int entries; - - if (!header) - return -EINVAL; - - mcfg = (struct acpi_table_mcfg *)header; - - /* how many config structures do we have */ - free_all_mmcfg(); - entries = 0; - i = header->length - sizeof(struct acpi_table_mcfg); - while (i >= sizeof(struct acpi_mcfg_allocation)) { - entries++; - i -= sizeof(struct acpi_mcfg_allocation); - } - if (entries == 0) { - pr_err(PREFIX "MMCONFIG has no entries\n"); - return -ENODEV; - } - - cfg_table = (struct acpi_mcfg_allocation *) &mcfg[1]; - for (i = 0; i < entries; i++) { - cfg = &cfg_table[i]; - if (acpi_mcfg_check_entry(mcfg, cfg)) { - free_all_mmcfg(); - return -ENODEV; - } - - if (pci_mmconfig_add(cfg->pci_segment, cfg->start_bus_number, - cfg->end_bus_number, cfg->address) == NULL) { - pr_warn(PREFIX "no memory for MCFG entries\n"); - free_all_mmcfg(); - return -ENOMEM; - } - } - - return 0; -} - #ifdef CONFIG_ACPI_APEI extern int (*arch_apei_filter_addr)(int (*func)(__u64 start, __u64 size, void *data), void *data); @@ -668,7 +528,7 @@ void __init pci_mmcfg_early_init(void) if (pci_mmcfg_check_hostbridge()) known_bridge = 1; else - acpi_sfi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg); + acpi_sfi_table_parse(ACPI_SIG_MCFG, acpi_parse_mcfg); __pci_mmcfg_init(1);
set_apei_filter(); @@ -686,7 +546,7 @@ void __init pci_mmcfg_late_init(void)
/* MMCONFIG hasn't been enabled yet, try again */ if (pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF) { - acpi_sfi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg); + acpi_sfi_table_parse(ACPI_SIG_MCFG, acpi_parse_mcfg); __pci_mmcfg_init(0); } } @@ -720,38 +580,6 @@ static int __init pci_mmcfg_late_insert_resources(void) */ late_initcall(pci_mmcfg_late_insert_resources);
-static int pci_mmconfig_inject(struct pci_mmcfg_region *cfg) -{ - struct pci_mmcfg_region *cfg_conflict; - int err = 0; - - mutex_lock(&pci_mmcfg_lock); - cfg_conflict = pci_mmconfig_lookup(cfg->segment, cfg->start_bus); - if (cfg_conflict) { - if (cfg_conflict->end_bus < cfg->end_bus) - pr_info(FW_INFO "MMCONFIG for " - "domain %04x [bus %02x-%02x] " - "only partially covers this bridge\n", - cfg_conflict->segment, cfg_conflict->start_bus, - cfg_conflict->end_bus); - err = -EEXIST; - goto out; - } - - if (pci_mmcfg_arch_map(cfg)) { - pr_warn("fail to map MMCONFIG %pR.\n", &cfg->res); - err = -ENOMEM; - goto out; - } else { - list_add_sorted(cfg); - pr_info("MMCONFIG at %pR (base %#lx)\n", - &cfg->res, (unsigned long)cfg->address); - } -out: - mutex_unlock(&pci_mmcfg_lock); - return err; -} - /* Add MMCFG information for host bridges */ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, phys_addr_t addr) @@ -808,26 +636,3 @@ error: kfree(cfg); return rc; } - -/* Delete MMCFG information for host bridges */ -int pci_mmconfig_delete(u16 seg, u8 start, u8 end) -{ - struct pci_mmcfg_region *cfg; - - mutex_lock(&pci_mmcfg_lock); - list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) - if (cfg->segment == seg && cfg->start_bus == start && - cfg->end_bus == end) { - list_del_rcu(&cfg->list); - synchronize_rcu(); - pci_mmcfg_arch_unmap(cfg); - if (cfg->res.parent) - release_resource(&cfg->res); - mutex_unlock(&pci_mmcfg_lock); - kfree(cfg); - return 0; - } - mutex_unlock(&pci_mmcfg_lock); - - return -ENOENT; -} diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c index 43984bc..246f135 100644 --- a/arch/x86/pci/mmconfig_32.c +++ b/arch/x86/pci/mmconfig_32.c @@ -12,6 +12,7 @@ #include <linux/pci.h> #include <linux/init.h> #include <linux/rcupdate.h> +#include <linux/ecam.h> #include <asm/e820.h> #include <asm/pci_x86.h>
diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c index bea5249..b14fcd3 100644 --- a/arch/x86/pci/mmconfig_64.c +++ b/arch/x86/pci/mmconfig_64.c @@ -10,6 +10,7 @@ #include <linux/acpi.h> #include <linux/bitmap.h> #include <linux/rcupdate.h> +#include <linux/ecam.h> #include <asm/e820.h> #include <asm/pci_x86.h>
diff --git a/arch/x86/pci/numachip.c b/arch/x86/pci/numachip.c index 2e565e6..55fbd18 100644 --- a/arch/x86/pci/numachip.c +++ b/arch/x86/pci/numachip.c @@ -13,6 +13,7 @@ * */
+#include <linux/ecam.h> #include <linux/pci.h> #include <asm/pci_x86.h>
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 962f98e..607a879 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -65,6 +65,7 @@ obj-$(CONFIG_ACPI_BUTTON) += button.o obj-$(CONFIG_ACPI_FAN) += fan.o obj-$(CONFIG_ACPI_VIDEO) += video.o obj-$(CONFIG_ACPI_PCI_SLOT) += pci_slot.o +obj-$(CONFIG_PCI_MMCONFIG) += mcfg.o obj-$(CONFIG_ACPI_PROCESSOR) += processor.o obj-y += container.o obj-$(CONFIG_ACPI_THERMAL) += thermal.o diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c new file mode 100644 index 0000000..5ecef20 --- /dev/null +++ b/drivers/acpi/mcfg.c @@ -0,0 +1,59 @@ +/* + * MCFG ACPI table parser. + * + * 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. + * + */ + +#include <linux/acpi.h> +#include <linux/ecam.h> + +#include <asm/pci_x86.h> /* Temp hack before refactoring arch-specific calls */ + +#define PREFIX "MCFG: " + +int __init acpi_parse_mcfg(struct acpi_table_header *header) +{ + struct acpi_table_mcfg *mcfg; + struct acpi_mcfg_allocation *cfg_table, *cfg; + unsigned long i; + int entries; + + if (!header) + return -EINVAL; + + mcfg = (struct acpi_table_mcfg *)header; + + /* how many config structures do we have */ + free_all_mmcfg(); + entries = 0; + i = header->length - sizeof(struct acpi_table_mcfg); + while (i >= sizeof(struct acpi_mcfg_allocation)) { + entries++; + i -= sizeof(struct acpi_mcfg_allocation); + } + if (entries == 0) { + pr_err(PREFIX "MCFG table has no entries\n"); + return -ENODEV; + } + + cfg_table = (struct acpi_mcfg_allocation *) &mcfg[1]; + for (i = 0; i < entries; i++) { + cfg = &cfg_table[i]; + if (acpi_mcfg_check_entry(mcfg, cfg)) { + free_all_mmcfg(); + return -ENODEV; + } + + if (pci_mmconfig_add(cfg->pci_segment, cfg->start_bus_number, + cfg->end_bus_number, cfg->address) == NULL) { + pr_warn(PREFIX "no memory for MCFG entries\n"); + free_all_mmcfg(); + return -ENOMEM; + } + } + + return 0; +} diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 73de4ef..9950248 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -26,6 +26,13 @@ config PCI_MSI_IRQ_DOMAIN depends on PCI_MSI select GENERIC_MSI_IRQ_DOMAIN
+config PCI_ECAM + bool "Enhanced Configuration Access Mechanism (ECAM)" + depends on PCI && HAVE_PCI_ECAM + +config HAVE_PCI_ECAM + bool + config PCI_DEBUG bool "PCI Debugging" depends on PCI && DEBUG_KERNEL diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index be3f631..eb574f8 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -42,6 +42,11 @@ obj-$(CONFIG_SPARC_LEON) += setup-irq.o obj-$(CONFIG_M68K) += setup-irq.o
# +# Enhanced Configuration Access Mechanism (ECAM) +# +obj-$(CONFIG_PCI_ECAM) += ecam.o + +# # ACPI Related PCI FW Functions # ACPI _DSM provided firmware instance and string name # diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c new file mode 100644 index 0000000..d221dba --- /dev/null +++ b/drivers/pci/ecam.c @@ -0,0 +1,175 @@ +/* + * Arch agnostic direct PCI config space access via + * ECAM (Enhanced Configuration Access Mechanism) + * + * Per-architecture code takes care of the mappings, region validation and + * accesses themselves. + * + * 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. + * + */ + +#include <linux/mutex.h> +#include <linux/rculist.h> +#include <linux/ecam.h> + +#include <asm/io.h> +#include <asm/pci_x86.h> /* Temp hack before refactoring arch-specific calls */ + +#define PREFIX "PCI: " + +static DEFINE_MUTEX(pci_mmcfg_lock); + +LIST_HEAD(pci_mmcfg_list); + +static void __init pci_mmconfig_remove(struct pci_mmcfg_region *cfg) +{ + if (cfg->res.parent) + release_resource(&cfg->res); + list_del(&cfg->list); + kfree(cfg); +} + +void __init free_all_mmcfg(void) +{ + struct pci_mmcfg_region *cfg, *tmp; + + pci_mmcfg_arch_free(); + list_for_each_entry_safe(cfg, tmp, &pci_mmcfg_list, list) + pci_mmconfig_remove(cfg); +} + +void list_add_sorted(struct pci_mmcfg_region *new) +{ + struct pci_mmcfg_region *cfg; + + /* keep list sorted by segment and starting bus number */ + list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) { + if (cfg->segment > new->segment || + (cfg->segment == new->segment && + cfg->start_bus >= new->start_bus)) { + list_add_tail_rcu(&new->list, &cfg->list); + return; + } + } + list_add_tail_rcu(&new->list, &pci_mmcfg_list); +} + +struct pci_mmcfg_region *pci_mmconfig_alloc(int segment, int start, + int end, u64 addr) +{ + struct pci_mmcfg_region *new; + struct resource *res; + + if (addr == 0) + return NULL; + + new = kzalloc(sizeof(*new), GFP_KERNEL); + if (!new) + return NULL; + + new->address = addr; + new->segment = segment; + new->start_bus = start; + new->end_bus = end; + + res = &new->res; + res->start = addr + PCI_MMCFG_BUS_OFFSET(start); + res->end = addr + PCI_MMCFG_BUS_OFFSET(end + 1) - 1; + res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; + snprintf(new->name, PCI_MMCFG_RESOURCE_NAME_LEN, + "PCI MMCONFIG %04x [bus %02x-%02x]", segment, start, end); + res->name = new->name; + + return new; +} + +struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start, + int end, u64 addr) +{ + struct pci_mmcfg_region *new; + + new = pci_mmconfig_alloc(segment, start, end, addr); + if (new) { + mutex_lock(&pci_mmcfg_lock); + list_add_sorted(new); + mutex_unlock(&pci_mmcfg_lock); + + pr_info(PREFIX + "MMCONFIG for domain %04x [bus %02x-%02x] at %pR " + "(base %#lx)\n", + segment, start, end, &new->res, (unsigned long)addr); + } + + return new; +} + +struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus) +{ + struct pci_mmcfg_region *cfg; + + list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) + if (cfg->segment == segment && + cfg->start_bus <= bus && bus <= cfg->end_bus) + return cfg; + + return NULL; +} + +/* Delete MMCFG information for host bridges */ +int pci_mmconfig_delete(u16 seg, u8 start, u8 end) +{ + struct pci_mmcfg_region *cfg; + + mutex_lock(&pci_mmcfg_lock); + list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) + if (cfg->segment == seg && cfg->start_bus == start && + cfg->end_bus == end) { + list_del_rcu(&cfg->list); + synchronize_rcu(); + pci_mmcfg_arch_unmap(cfg); + if (cfg->res.parent) + release_resource(&cfg->res); + mutex_unlock(&pci_mmcfg_lock); + kfree(cfg); + return 0; + } + mutex_unlock(&pci_mmcfg_lock); + + return -ENOENT; +} + +int pci_mmconfig_inject(struct pci_mmcfg_region *cfg) +{ + struct pci_mmcfg_region *cfg_conflict; + int err = 0; + + mutex_lock(&pci_mmcfg_lock); + cfg_conflict = pci_mmconfig_lookup(cfg->segment, cfg->start_bus); + if (cfg_conflict) { + if (cfg_conflict->end_bus < cfg->end_bus) + pr_info(FW_INFO "MMCONFIG for " + "domain %04x [bus %02x-%02x] " + "only partially covers this bridge\n", + cfg_conflict->segment, cfg_conflict->start_bus, + cfg_conflict->end_bus); + err = -EEXIST; + goto out; + } + + if (pci_mmcfg_arch_map(cfg)) { + pr_warn("fail to map MMCONFIG %pR.\n", &cfg->res); + err = -ENOMEM; + goto out; + } else { + list_add_sorted(cfg); + pr_info("MMCONFIG at %pR (base %#lx)\n", + &cfg->res, (unsigned long)cfg->address); + + } +out: + mutex_unlock(&pci_mmcfg_lock); + return err; +} diff --git a/drivers/xen/pci.c b/drivers/xen/pci.c index 7494dbe..6785ebb 100644 --- a/drivers/xen/pci.c +++ b/drivers/xen/pci.c @@ -20,6 +20,7 @@ #include <linux/pci.h> #include <linux/acpi.h> #include <linux/pci-acpi.h> +#include <linux/ecam.h> #include <xen/xen.h> #include <xen/interface/physdev.h> #include <xen/interface/xen.h> diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 127d68b..98f2ce4 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -162,6 +162,8 @@ int acpi_table_parse_madt(enum acpi_madt_type id, acpi_tbl_entry_handler handler, unsigned int max_entries); int acpi_parse_mcfg (struct acpi_table_header *header); +int acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg, + struct acpi_mcfg_allocation *cfg); void acpi_table_print_madt_entry (struct acpi_subtable_header *madt);
/* the following four functions are architecture-dependent */ diff --git a/include/linux/ecam.h b/include/linux/ecam.h new file mode 100644 index 0000000..dec3b52 --- /dev/null +++ b/include/linux/ecam.h @@ -0,0 +1,37 @@ +#ifndef __ECAM_H +#define __ECAM_H +#ifdef __KERNEL__ + +#include <linux/types.h> +#include <linux/acpi.h> + +/* "PCI MMCONFIG %04x [bus %02x-%02x]" */ +#define PCI_MMCFG_RESOURCE_NAME_LEN (22 + 4 + 2 + 2) + +struct pci_mmcfg_region { + struct list_head list; + struct resource res; + u64 address; + char __iomem *virt; + u16 segment; + u8 start_bus; + u8 end_bus; + char name[PCI_MMCFG_RESOURCE_NAME_LEN]; +}; + +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); +int pci_mmconfig_inject(struct pci_mmcfg_region *cfg); +struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start, + int end, u64 addr); +void list_add_sorted(struct pci_mmcfg_region *new); +void free_all_mmcfg(void); +int pci_mmconfig_delete(u16 seg, u8 start, u8 end); + +extern struct list_head pci_mmcfg_list; + +#define PCI_MMCFG_BUS_OFFSET(bus) ((bus) << 20) + +#endif /* __KERNEL__ */ +#endif /* __ECAM_H */
First function acpi_mcfg_check_entry() does not apply any quirks by default.
Last two functions are required by ACPI subsystem to make PCI config space accessible. Generic code assume to do nothing for early init call but late init call does as follow: - parse MCFG table and add regions to ECAM resource list - map regions - add regions to iomem_resource
Signed-off-by: Tomasz Nowicki tn@semihalf.com --- drivers/acpi/mcfg.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c index 5ecef20..fad9917 100644 --- a/drivers/acpi/mcfg.c +++ b/drivers/acpi/mcfg.c @@ -57,3 +57,29 @@ int __init acpi_parse_mcfg(struct acpi_table_header *header)
return 0; } + +int __init __weak acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg, + struct acpi_mcfg_allocation *cfg) +{ + return 0; +} + +void __init __weak pci_mmcfg_early_init(void) +{ + +} + +void __init __weak pci_mmcfg_late_init(void) +{ + struct pci_mmcfg_region *cfg; + + acpi_table_parse(ACPI_SIG_MCFG, acpi_parse_mcfg); + + if (list_empty(&pci_mmcfg_list)) + return; + if (!pci_mmcfg_arch_init()) + free_all_mmcfg(); + + list_for_each_entry(cfg, &pci_mmcfg_list, list) + insert_resource(&iomem_resource, &cfg->res); +}
mmconfig_64.c version is going to be default implementation for low-level operation on mmconfig regions. However, now it initializes raw_pci_ext_ops pointer which is specific for x86 only. Moreover, mmconfig_32.c is doing the same thing at the same time. So lets move it to mmconfig_shared.c so it becomes common for both and mmconfig_64.c turns out to be purely arch agnostic.
Signed-off-by: Tomasz Nowicki tn@semihalf.com --- arch/x86/include/asm/pci_x86.h | 5 +++++ arch/x86/pci/mmconfig-shared.c | 10 ++++++++-- arch/x86/pci/mmconfig_32.c | 10 ++-------- arch/x86/pci/mmconfig_64.c | 11 ++--------- 4 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h index 64cb514..039f69e 100644 --- a/arch/x86/include/asm/pci_x86.h +++ b/arch/x86/include/asm/pci_x86.h @@ -129,6 +129,11 @@ extern void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg); extern int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, phys_addr_t addr);
+int pci_mmcfg_read(unsigned int seg, unsigned int bus, unsigned int devfn, + int reg, int len, u32 *value); +int pci_mmcfg_write(unsigned int seg, unsigned int bus, unsigned int devfn, + int reg, int len, u32 value); + /* * AMD Fam10h CPUs are buggy, and cannot access MMIO config space * on their northbrige except through the * %eax register. As such, you MUST diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index ce2c2e4..980f304 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -29,6 +29,11 @@ static bool pci_mmcfg_running_state; static bool pci_mmcfg_arch_init_failed;
+const struct pci_raw_ops pci_mmcfg = { + .read = pci_mmcfg_read, + .write = pci_mmcfg_write, +}; + static const char *__init pci_mmcfg_e7520(void) { u32 win; @@ -512,9 +517,10 @@ static void __init __pci_mmcfg_init(int early) } }
- if (pci_mmcfg_arch_init()) + if (pci_mmcfg_arch_init()) { + raw_pci_ext_ops = &pci_mmcfg; pci_probe = (pci_probe & ~PCI_PROBE_MASK) | PCI_PROBE_MMCONF; - else { + } else { free_all_mmcfg(); pci_mmcfg_arch_init_failed = true; } diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c index 246f135..2ded56f 100644 --- a/arch/x86/pci/mmconfig_32.c +++ b/arch/x86/pci/mmconfig_32.c @@ -50,7 +50,7 @@ static void pci_exp_set_dev_base(unsigned int base, int bus, int devfn) } }
-static int pci_mmcfg_read(unsigned int seg, unsigned int bus, +int pci_mmcfg_read(unsigned int seg, unsigned int bus, unsigned int devfn, int reg, int len, u32 *value) { unsigned long flags; @@ -89,7 +89,7 @@ err: *value = -1; return 0; }
-static int pci_mmcfg_write(unsigned int seg, unsigned int bus, +int pci_mmcfg_write(unsigned int seg, unsigned int bus, unsigned int devfn, int reg, int len, u32 value) { unsigned long flags; @@ -126,15 +126,9 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus, return 0; }
-const struct pci_raw_ops pci_mmcfg = { - .read = pci_mmcfg_read, - .write = pci_mmcfg_write, -}; - int __init pci_mmcfg_arch_init(void) { printk(KERN_INFO "PCI: Using MMCONFIG for extended config space\n"); - raw_pci_ext_ops = &pci_mmcfg; return 1; }
diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c index b14fcd3..d0c48eb 100644 --- a/arch/x86/pci/mmconfig_64.c +++ b/arch/x86/pci/mmconfig_64.c @@ -25,7 +25,7 @@ static char __iomem *pci_dev_base(unsigned int seg, unsigned int bus, unsigned i return NULL; }
-static int pci_mmcfg_read(unsigned int seg, unsigned int bus, +int pci_mmcfg_read(unsigned int seg, unsigned int bus, unsigned int devfn, int reg, int len, u32 *value) { char __iomem *addr; @@ -59,7 +59,7 @@ err: *value = -1; return 0; }
-static int pci_mmcfg_write(unsigned int seg, unsigned int bus, +int pci_mmcfg_write(unsigned int seg, unsigned int bus, unsigned int devfn, int reg, int len, u32 value) { char __iomem *addr; @@ -91,11 +91,6 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus, return 0; }
-const struct pci_raw_ops pci_mmcfg = { - .read = pci_mmcfg_read, - .write = pci_mmcfg_write, -}; - static void __iomem *mcfg_ioremap(struct pci_mmcfg_region *cfg) { void __iomem *addr; @@ -121,8 +116,6 @@ int __init pci_mmcfg_arch_init(void) return 0; }
- raw_pci_ext_ops = &pci_mmcfg; - return 1; }
Hosts with custom ECAM hooks (like 32bit x86) should select ARCH_HAS_CUSTOM_PCI_ECAM. Otherwise, host will use generic version provided by this patch (like 64bit x86 does).
Note, we leaved x86-specific PCI config accessors in corresponding files.
Signed-off-by: Tomasz Nowicki tn@semihalf.com --- arch/x86/Kconfig | 1 + arch/x86/include/asm/pci_x86.h | 4 --- arch/x86/pci/mmconfig_64.c | 55 --------------------------------------- drivers/acpi/mcfg.c | 2 -- drivers/pci/Kconfig | 3 +++ drivers/pci/ecam.c | 59 +++++++++++++++++++++++++++++++++++++++++- include/linux/ecam.h | 6 +++++ 7 files changed, 68 insertions(+), 62 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 4ca8134..b4b68eb 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -30,6 +30,7 @@ config X86 select ARCH_HAS_PMEM_API if X86_64 select ARCH_HAS_MMIO_FLUSH select ARCH_HAS_SG_CHAIN + select ARCH_HAS_CUSTOM_PCI_ECAM if X86_32 select ARCH_HAVE_NMI_SAFE_CMPXCHG select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI select ARCH_MIGHT_HAVE_PC_PARPORT diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h index 039f69e..c9bda33 100644 --- a/arch/x86/include/asm/pci_x86.h +++ b/arch/x86/include/asm/pci_x86.h @@ -122,10 +122,6 @@ extern int pci_legacy_init(void); extern void pcibios_fixup_irqs(void);
/* pci-mmconfig.c */ -extern int __init pci_mmcfg_arch_init(void); -extern void __init pci_mmcfg_arch_free(void); -extern int pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg); -extern void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg); extern int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, phys_addr_t addr);
diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c index d0c48eb..fd356cc 100644 --- a/arch/x86/pci/mmconfig_64.c +++ b/arch/x86/pci/mmconfig_64.c @@ -90,58 +90,3 @@ int pci_mmcfg_write(unsigned int seg, unsigned int bus,
return 0; } - -static void __iomem *mcfg_ioremap(struct pci_mmcfg_region *cfg) -{ - void __iomem *addr; - u64 start, size; - int num_buses; - - start = cfg->address + PCI_MMCFG_BUS_OFFSET(cfg->start_bus); - num_buses = cfg->end_bus - cfg->start_bus + 1; - size = PCI_MMCFG_BUS_OFFSET(num_buses); - addr = ioremap_nocache(start, size); - if (addr) - addr -= PCI_MMCFG_BUS_OFFSET(cfg->start_bus); - return addr; -} - -int __init pci_mmcfg_arch_init(void) -{ - struct pci_mmcfg_region *cfg; - - list_for_each_entry(cfg, &pci_mmcfg_list, list) - if (pci_mmcfg_arch_map(cfg)) { - pci_mmcfg_arch_free(); - return 0; - } - - return 1; -} - -void __init pci_mmcfg_arch_free(void) -{ - struct pci_mmcfg_region *cfg; - - list_for_each_entry(cfg, &pci_mmcfg_list, list) - pci_mmcfg_arch_unmap(cfg); -} - -int pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg) -{ - cfg->virt = mcfg_ioremap(cfg); - if (!cfg->virt) { - pr_err(PREFIX "can't map MMCONFIG at %pR\n", &cfg->res); - return -ENOMEM; - } - - return 0; -} - -void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg) -{ - if (cfg && cfg->virt) { - iounmap(cfg->virt + PCI_MMCFG_BUS_OFFSET(cfg->start_bus)); - cfg->virt = NULL; - } -} diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c index fad9917..745b83e 100644 --- a/drivers/acpi/mcfg.c +++ b/drivers/acpi/mcfg.c @@ -10,8 +10,6 @@ #include <linux/acpi.h> #include <linux/ecam.h>
-#include <asm/pci_x86.h> /* Temp hack before refactoring arch-specific calls */ - #define PREFIX "MCFG: "
int __init acpi_parse_mcfg(struct acpi_table_header *header) diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 9950248..b2e27c8 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -33,6 +33,9 @@ config PCI_ECAM config HAVE_PCI_ECAM bool
+config ARCH_HAS_CUSTOM_PCI_ECAM + bool + config PCI_DEBUG bool "PCI Debugging" depends on PCI && DEBUG_KERNEL diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c index d221dba..8a5eef7 100644 --- a/drivers/pci/ecam.c +++ b/drivers/pci/ecam.c @@ -16,7 +16,6 @@ #include <linux/ecam.h>
#include <asm/io.h> -#include <asm/pci_x86.h> /* Temp hack before refactoring arch-specific calls */
#define PREFIX "PCI: "
@@ -24,6 +23,64 @@ static DEFINE_MUTEX(pci_mmcfg_lock);
LIST_HEAD(pci_mmcfg_list);
+#ifndef CONFIG_ARCH_HAS_CUSTOM_PCI_ECAM + +static void __iomem *mcfg_ioremap(struct pci_mmcfg_region *cfg) +{ + void __iomem *addr; + u64 start, size; + int num_buses; + + start = cfg->address + PCI_MMCFG_BUS_OFFSET(cfg->start_bus); + num_buses = cfg->end_bus - cfg->start_bus + 1; + size = PCI_MMCFG_BUS_OFFSET(num_buses); + addr = ioremap_nocache(start, size); + if (addr) + addr -= PCI_MMCFG_BUS_OFFSET(cfg->start_bus); + return addr; +} + +int __init pci_mmcfg_arch_init(void) +{ + struct pci_mmcfg_region *cfg; + + list_for_each_entry(cfg, &pci_mmcfg_list, list) + if (pci_mmcfg_arch_map(cfg)) { + pci_mmcfg_arch_free(); + return 0; + } + + return 1; +} + +void __init pci_mmcfg_arch_free(void) +{ + struct pci_mmcfg_region *cfg; + + list_for_each_entry(cfg, &pci_mmcfg_list, list) + pci_mmcfg_arch_unmap(cfg); +} + +int pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg) +{ + cfg->virt = mcfg_ioremap(cfg); + if (!cfg->virt) { + pr_err(PREFIX "can't map MMCONFIG at %pR\n", &cfg->res); + return -ENOMEM; + } + + return 0; +} + +void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg) +{ + if (cfg && cfg->virt) { + iounmap(cfg->virt + PCI_MMCFG_BUS_OFFSET(cfg->start_bus)); + cfg->virt = NULL; + } +} +#endif + static void __init pci_mmconfig_remove(struct pci_mmcfg_region *cfg) { if (cfg->res.parent) diff --git a/include/linux/ecam.h b/include/linux/ecam.h index dec3b52..813acd1 100644 --- a/include/linux/ecam.h +++ b/include/linux/ecam.h @@ -29,6 +29,12 @@ void list_add_sorted(struct pci_mmcfg_region *new); void free_all_mmcfg(void); int pci_mmconfig_delete(u16 seg, u8 start, u8 end);
+/* Arch specific calls */ +int pci_mmcfg_arch_init(void); +void pci_mmcfg_arch_free(void); +int pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg); +void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg); + extern struct list_head pci_mmcfg_list;
#define PCI_MMCFG_BUS_OFFSET(bus) ((bus) << 20)
Lets keep RAW ACPI PCI config space accessors empty by default, since we are note sure if they are necessary accross all archs. Once we sort this out, we can provide generic version or let architectures to overwrite, like now x86.
Signed-off-by: Tomasz Nowicki tn@semihalf.com --- drivers/acpi/mcfg.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c index 745b83e..3e1e7be 100644 --- a/drivers/acpi/mcfg.c +++ b/drivers/acpi/mcfg.c @@ -9,9 +9,30 @@
#include <linux/acpi.h> #include <linux/ecam.h> +#include <linux/pci.h>
#define PREFIX "MCFG: "
+/* + * raw_pci_read/write - raw ACPI PCI config space accessors. + * + * By defauly (__weak) these accessors are empty and should be overwritten + * by architectures which support operations on ACPI PCI_Config regions, + * see osl.c file. + */ + +int __weak raw_pci_read(unsigned int domain, unsigned int bus, + unsigned int devfn, int reg, int len, u32 *val) +{ + return PCIBIOS_DEVICE_NOT_FOUND; +} + +int __weak raw_pci_write(unsigned int domain, unsigned int bus, + unsigned int devfn, int reg, int len, u32 val) +{ + return PCIBIOS_DEVICE_NOT_FOUND; +} + int __init acpi_parse_mcfg(struct acpi_table_header *header) { struct acpi_table_mcfg *mcfg;
From: Hanjun Guo hanjun.guo@linaro.org
In drivers/xen/pci.c, there are arch x86 dependent codes when CONFIG_PCI_MMCONFIG is enabled, since CONFIG_PCI_MMCONFIG depends on ACPI, so this will prevent XEN PCI running on other architectures using ACPI with PCI_MMCONFIG enabled (such as ARM64).
Fortunatly, it can be sloved in a simple way. In drivers/xen/pci.c, the only x86 dependent code is if ((pci_probe & PCI_PROBE_MMCONF) == 0), and it's defined in asm/pci_x86.h, the code means that if the PCI resource is not probed in PCI_PROBE_MMCONF way, just ingnore the xen mcfg init. Actually this is duplicate, because if PCI resource is not probed in PCI_PROBE_MMCONF way, the pci_mmconfig_list will be empty, and the if (list_empty()) after it will do the same job.
So just remove the arch related code and the head file, this will be no functional change for x86, and also makes xen/pci.c usable for other architectures.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org CC: Konrad Rzeszutek Wilk konrad.wilk@oracle.com CC: Boris Ostrovsky boris.ostrovsky@oracle.com --- drivers/xen/pci.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/xen/pci.c b/drivers/xen/pci.c index 6785ebb..9a8dbe3 100644 --- a/drivers/xen/pci.c +++ b/drivers/xen/pci.c @@ -28,9 +28,6 @@ #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h> #include "../pci/pci.h" -#ifdef CONFIG_PCI_MMCONFIG -#include <asm/pci_x86.h> -#endif
static bool __read_mostly pci_seg_supported = true;
@@ -222,9 +219,6 @@ static int __init xen_mcfg_late(void) if (!xen_initial_domain()) return 0;
- if ((pci_probe & PCI_PROBE_MMCONF) == 0) - return 0; - if (list_empty(&pci_mmcfg_list)) return 0;
+ Stefano
On 27.10.2015 17:38, Tomasz Nowicki wrote:
From: Hanjun Guo hanjun.guo@linaro.org
In drivers/xen/pci.c, there are arch x86 dependent codes when CONFIG_PCI_MMCONFIG is enabled, since CONFIG_PCI_MMCONFIG depends on ACPI, so this will prevent XEN PCI running on other architectures using ACPI with PCI_MMCONFIG enabled (such as ARM64).
Fortunatly, it can be sloved in a simple way. In drivers/xen/pci.c, the only x86 dependent code is if ((pci_probe & PCI_PROBE_MMCONF) == 0), and it's defined in asm/pci_x86.h, the code means that if the PCI resource is not probed in PCI_PROBE_MMCONF way, just ingnore the xen mcfg init. Actually this is duplicate, because if PCI resource is not probed in PCI_PROBE_MMCONF way, the pci_mmconfig_list will be empty, and the if (list_empty()) after it will do the same job.
So just remove the arch related code and the head file, this will be no functional change for x86, and also makes xen/pci.c usable for other architectures.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org CC: Konrad Rzeszutek Wilk konrad.wilk@oracle.com CC: Boris Ostrovsky boris.ostrovsky@oracle.com
drivers/xen/pci.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/xen/pci.c b/drivers/xen/pci.c index 6785ebb..9a8dbe3 100644 --- a/drivers/xen/pci.c +++ b/drivers/xen/pci.c @@ -28,9 +28,6 @@ #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h> #include "../pci/pci.h" -#ifdef CONFIG_PCI_MMCONFIG -#include <asm/pci_x86.h> -#endif
static bool __read_mostly pci_seg_supported = true;
@@ -222,9 +219,6 @@ static int __init xen_mcfg_late(void) if (!xen_initial_domain()) return 0;
- if ((pci_probe & PCI_PROBE_MMCONF) == 0)
return 0;- if (list_empty(&pci_mmcfg_list)) return 0;
On 10/27/2015 12:47 PM, Tomasz Nowicki wrote:
- Stefano
On 27.10.2015 17:38, Tomasz Nowicki wrote:
From: Hanjun Guo hanjun.guo@linaro.org
In drivers/xen/pci.c, there are arch x86 dependent codes when CONFIG_PCI_MMCONFIG is enabled, since CONFIG_PCI_MMCONFIG depends on ACPI, so this will prevent XEN PCI running on other architectures using ACPI with PCI_MMCONFIG enabled (such as ARM64).
Fortunatly, it can be sloved in a simple way. In drivers/xen/pci.c, the only x86 dependent code is if ((pci_probe & PCI_PROBE_MMCONF) == 0), and it's defined in asm/pci_x86.h, the code means that if the PCI resource is not probed in PCI_PROBE_MMCONF way, just ingnore the xen mcfg init. Actually this is duplicate, because if PCI resource is not probed in PCI_PROBE_MMCONF way, the pci_mmconfig_list will be empty, and the if (list_empty()) after it will do the same job.
So just remove the arch related code and the head file, this will be no functional change for x86, and also makes xen/pci.c usable for other architectures.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org CC: Konrad Rzeszutek Wilk konrad.wilk@oracle.com CC: Boris Ostrovsky boris.ostrovsky@oracle.com
drivers/xen/pci.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/xen/pci.c b/drivers/xen/pci.c index 6785ebb..9a8dbe3 100644 --- a/drivers/xen/pci.c +++ b/drivers/xen/pci.c @@ -28,9 +28,6 @@ #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h> #include "../pci/pci.h" -#ifdef CONFIG_PCI_MMCONFIG -#include <asm/pci_x86.h> -#endif
Assuming this still compiles on x86 now that this include file is removed
Reviewed-by: Boris Ostrovsky boris.ostrovsky@oracle.com
static bool __read_mostly pci_seg_supported = true;
@@ -222,9 +219,6 @@ static int __init xen_mcfg_late(void) if (!xen_initial_domain()) return 0;
- if ((pci_probe & PCI_PROBE_MMCONF) == 0)
return 0;if (list_empty(&pci_mmcfg_list)) return 0;
On Tue, 27 Oct 2015, Boris Ostrovsky wrote:
On 10/27/2015 12:47 PM, Tomasz Nowicki wrote:
- Stefano
On 27.10.2015 17:38, Tomasz Nowicki wrote:
From: Hanjun Guo hanjun.guo@linaro.org
In drivers/xen/pci.c, there are arch x86 dependent codes when CONFIG_PCI_MMCONFIG is enabled, since CONFIG_PCI_MMCONFIG depends on ACPI, so this will prevent XEN PCI running on other architectures using ACPI with PCI_MMCONFIG enabled (such as ARM64).
Fortunatly, it can be sloved in a simple way. In drivers/xen/pci.c, the only x86 dependent code is if ((pci_probe & PCI_PROBE_MMCONF) == 0), and it's defined in asm/pci_x86.h, the code means that if the PCI resource is not probed in PCI_PROBE_MMCONF way, just ingnore the xen mcfg init. Actually this is duplicate, because if PCI resource is not probed in PCI_PROBE_MMCONF way, the pci_mmconfig_list will be empty, and the if (list_empty()) after it will do the same job.
So just remove the arch related code and the head file, this will be no functional change for x86, and also makes xen/pci.c usable for other architectures.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org CC: Konrad Rzeszutek Wilk konrad.wilk@oracle.com CC: Boris Ostrovsky boris.ostrovsky@oracle.com
drivers/xen/pci.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/xen/pci.c b/drivers/xen/pci.c index 6785ebb..9a8dbe3 100644 --- a/drivers/xen/pci.c +++ b/drivers/xen/pci.c @@ -28,9 +28,6 @@ #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h> #include "../pci/pci.h" -#ifdef CONFIG_PCI_MMCONFIG -#include <asm/pci_x86.h> -#endif
Assuming this still compiles on x86 now that this include file is removed
Reviewed-by: Boris Ostrovsky boris.ostrovsky@oracle.com
I think it does not:
drivers/xen/pci.c: In function āxen_mcfg_lateā: drivers/xen/pci.c:221:18: error: āpci_mmcfg_listā undeclared (first use in this function) drivers/xen/pci.c:221:18: note: each undeclared identifier is reported only once for each f
static bool __read_mostly pci_seg_supported = true;
@@ -222,9 +219,6 @@ static int __init xen_mcfg_late(void) if (!xen_initial_domain()) return 0;
- if ((pci_probe & PCI_PROBE_MMCONF) == 0)
return 0;if (list_empty(&pci_mmcfg_list)) return 0;
On 28.10.2015 11:49, Stefano Stabellini wrote:
On Tue, 27 Oct 2015, Boris Ostrovsky wrote:
On 10/27/2015 12:47 PM, Tomasz Nowicki wrote:
- Stefano
On 27.10.2015 17:38, Tomasz Nowicki wrote:
From: Hanjun Guo hanjun.guo@linaro.org
In drivers/xen/pci.c, there are arch x86 dependent codes when CONFIG_PCI_MMCONFIG is enabled, since CONFIG_PCI_MMCONFIG depends on ACPI, so this will prevent XEN PCI running on other architectures using ACPI with PCI_MMCONFIG enabled (such as ARM64).
Fortunatly, it can be sloved in a simple way. In drivers/xen/pci.c, the only x86 dependent code is if ((pci_probe & PCI_PROBE_MMCONF) == 0), and it's defined in asm/pci_x86.h, the code means that if the PCI resource is not probed in PCI_PROBE_MMCONF way, just ingnore the xen mcfg init. Actually this is duplicate, because if PCI resource is not probed in PCI_PROBE_MMCONF way, the pci_mmconfig_list will be empty, and the if (list_empty()) after it will do the same job.
So just remove the arch related code and the head file, this will be no functional change for x86, and also makes xen/pci.c usable for other architectures.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org CC: Konrad Rzeszutek Wilk konrad.wilk@oracle.com CC: Boris Ostrovsky boris.ostrovsky@oracle.com
drivers/xen/pci.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/xen/pci.c b/drivers/xen/pci.c index 6785ebb..9a8dbe3 100644 --- a/drivers/xen/pci.c +++ b/drivers/xen/pci.c @@ -28,9 +28,6 @@ #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h> #include "../pci/pci.h" -#ifdef CONFIG_PCI_MMCONFIG -#include <asm/pci_x86.h> -#endif
Assuming this still compiles on x86 now that this include file is removed
Reviewed-by: Boris Ostrovsky boris.ostrovsky@oracle.com
I think it does not:
drivers/xen/pci.c: In function āxen_mcfg_lateā: drivers/xen/pci.c:221:18: error: āpci_mmcfg_listā undeclared (first use in this function) drivers/xen/pci.c:221:18: note: each undeclared identifier is reported only once for each f
Right, we need: +#include <linux/ecam.h>
Will fix this in next version, thanks!
Tomasz
static bool __read_mostly pci_seg_supported = true;
@@ -222,9 +219,6 @@ static int __init xen_mcfg_late(void) if (!xen_initial_domain()) return 0;
- if ((pci_probe & PCI_PROBE_MMCONF) == 0)
return 0;if (list_empty(&pci_mmcfg_list)) return 0;
On 10/28/2015 06:56 PM, Tomasz Nowicki wrote:
On 28.10.2015 11:49, Stefano Stabellini wrote:
On Tue, 27 Oct 2015, Boris Ostrovsky wrote:
On 10/27/2015 12:47 PM, Tomasz Nowicki wrote:
- Stefano
On 27.10.2015 17:38, Tomasz Nowicki wrote:
From: Hanjun Guo hanjun.guo@linaro.org
In drivers/xen/pci.c, there are arch x86 dependent codes when CONFIG_PCI_MMCONFIG is enabled, since CONFIG_PCI_MMCONFIG depends on ACPI, so this will prevent XEN PCI running on other architectures using ACPI with PCI_MMCONFIG enabled (such as ARM64).
Fortunatly, it can be sloved in a simple way. In drivers/xen/pci.c, the only x86 dependent code is if ((pci_probe & PCI_PROBE_MMCONF) == 0), and it's defined in asm/pci_x86.h, the code means that if the PCI resource is not probed in PCI_PROBE_MMCONF way, just ingnore the xen mcfg init. Actually this is duplicate, because if PCI resource is not probed in PCI_PROBE_MMCONF way, the pci_mmconfig_list will be empty, and the if (list_empty()) after it will do the same job.
So just remove the arch related code and the head file, this will be no functional change for x86, and also makes xen/pci.c usable for other architectures.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org CC: Konrad Rzeszutek Wilk konrad.wilk@oracle.com CC: Boris Ostrovsky boris.ostrovsky@oracle.com
drivers/xen/pci.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/xen/pci.c b/drivers/xen/pci.c index 6785ebb..9a8dbe3 100644 --- a/drivers/xen/pci.c +++ b/drivers/xen/pci.c @@ -28,9 +28,6 @@ #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h> #include "../pci/pci.h" -#ifdef CONFIG_PCI_MMCONFIG -#include <asm/pci_x86.h> -#endif
Assuming this still compiles on x86 now that this include file is removed
Reviewed-by: Boris Ostrovsky boris.ostrovsky@oracle.com
I think it does not:
drivers/xen/pci.c: In function āxen_mcfg_lateā: drivers/xen/pci.c:221:18: error: āpci_mmcfg_listā undeclared (first use in this function) drivers/xen/pci.c:221:18: note: each undeclared identifier is reported only once for each f
Right, we need: +#include <linux/ecam.h>
Will fix this in next version, thanks!
Hmm, I think we just missed the head file, but the logic of this patch is still right.
Thanks Hanjun
On 10/28/2015 09:45 AM, Hanjun Guo wrote:
On 10/28/2015 06:56 PM, Tomasz Nowicki wrote:
On 28.10.2015 11:49, Stefano Stabellini wrote:
On Tue, 27 Oct 2015, Boris Ostrovsky wrote:
On 10/27/2015 12:47 PM, Tomasz Nowicki wrote:
- Stefano
On 27.10.2015 17:38, Tomasz Nowicki wrote:
From: Hanjun Guo hanjun.guo@linaro.org
In drivers/xen/pci.c, there are arch x86 dependent codes when CONFIG_PCI_MMCONFIG is enabled, since CONFIG_PCI_MMCONFIG depends on ACPI, so this will prevent XEN PCI running on other architectures using ACPI with PCI_MMCONFIG enabled (such as ARM64).
Fortunatly, it can be sloved in a simple way. In drivers/xen/pci.c, the only x86 dependent code is if ((pci_probe & PCI_PROBE_MMCONF) == 0), and it's defined in asm/pci_x86.h, the code means that if the PCI resource is not probed in PCI_PROBE_MMCONF way, just ingnore the xen mcfg init. Actually this is duplicate, because if PCI resource is not probed in PCI_PROBE_MMCONF way, the pci_mmconfig_list will be empty, and the if (list_empty()) after it will do the same job.
So just remove the arch related code and the head file, this will be no functional change for x86, and also makes xen/pci.c usable for other architectures.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org CC: Konrad Rzeszutek Wilk konrad.wilk@oracle.com CC: Boris Ostrovsky boris.ostrovsky@oracle.com
drivers/xen/pci.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/xen/pci.c b/drivers/xen/pci.c index 6785ebb..9a8dbe3 100644 --- a/drivers/xen/pci.c +++ b/drivers/xen/pci.c @@ -28,9 +28,6 @@ #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h> #include "../pci/pci.h" -#ifdef CONFIG_PCI_MMCONFIG -#include <asm/pci_x86.h> -#endif
Assuming this still compiles on x86 now that this include file is removed
Reviewed-by: Boris Ostrovsky boris.ostrovsky@oracle.com
I think it does not:
drivers/xen/pci.c: In function āxen_mcfg_lateā: drivers/xen/pci.c:221:18: error: āpci_mmcfg_listā undeclared (first use in this function) drivers/xen/pci.c:221:18: note: each undeclared identifier is reported only once for each f
Right, we need: +#include <linux/ecam.h>
Will fix this in next version, thanks!
Hmm, I think we just missed the head file, but the logic of this patch is still right.
Yes, removing the test should be safe.
-boris
There are two ways we can get ECAM (aka MCFG) regions using ACPI, from MCFG static table and from _CBA method. We cannot remove static regions, however regions coming from _CBA should be removed while removing bridge device.
In the light of above we need flag to mark hot added ECAM entries so that user should use pci_mmconfig_inject while adding regions from _CBA method.
Signed-off-by: Tomasz Nowicki tn@semihalf.com --- drivers/pci/ecam.c | 2 ++ include/linux/ecam.h | 1 + 2 files changed, 3 insertions(+)
diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c index 8a5eef7..da35b4c 100644 --- a/drivers/pci/ecam.c +++ b/drivers/pci/ecam.c @@ -131,6 +131,7 @@ struct pci_mmcfg_region *pci_mmconfig_alloc(int segment, int start, new->segment = segment; new->start_bus = start; new->end_bus = end; + new->hot_added = false;
res = &new->res; res->start = addr + PCI_MMCFG_BUS_OFFSET(start); @@ -221,6 +222,7 @@ int pci_mmconfig_inject(struct pci_mmcfg_region *cfg) err = -ENOMEM; goto out; } else { + cfg->hot_added = true; list_add_sorted(cfg); pr_info("MMCONFIG at %pR (base %#lx)\n", &cfg->res, (unsigned long)cfg->address); diff --git a/include/linux/ecam.h b/include/linux/ecam.h index 813acd1..e0f322e 100644 --- a/include/linux/ecam.h +++ b/include/linux/ecam.h @@ -17,6 +17,7 @@ struct pci_mmcfg_region { u8 start_bus; u8 end_bus; char name[PCI_MMCFG_RESOURCE_NAME_LEN]; + bool hot_added; };
struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus);
Now that we have hot_added flag we can get rid of arch specific mcfg_added.
Signed-off-by: Tomasz Nowicki tn@semihalf.com --- arch/x86/pci/acpi.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c index 28c9cd6..b4ef761 100644 --- a/arch/x86/pci/acpi.c +++ b/arch/x86/pci/acpi.c @@ -13,7 +13,6 @@ struct pci_root_info { struct acpi_pci_root_info common; struct pci_sysdata sd; #ifdef CONFIG_PCI_MMCONFIG - bool mcfg_added; u8 start_bus; u8 end_bus; #endif @@ -188,7 +187,6 @@ static int setup_mcfg_map(struct acpi_pci_root_info *ci) info = container_of(ci, struct pci_root_info, common); info->start_bus = (u8)root->secondary.start; info->end_bus = (u8)root->secondary.end; - info->mcfg_added = false; seg = info->sd.domain;
/* return success if MMCFG is not in use */ @@ -204,7 +202,6 @@ static int setup_mcfg_map(struct acpi_pci_root_info *ci) /* enable MMCFG if it hasn't been enabled yet */ if (raw_pci_ext_ops == NULL) raw_pci_ext_ops = &pci_mmcfg; - info->mcfg_added = true; } else if (result != -EEXIST) return check_segment(seg, dev, "fail to add MMCONFIG information,"); @@ -214,14 +211,17 @@ static int setup_mcfg_map(struct acpi_pci_root_info *ci)
static void teardown_mcfg_map(struct acpi_pci_root_info *ci) { + struct pci_mmcfg_region *cfg; struct pci_root_info *info;
info = container_of(ci, struct pci_root_info, common); - if (info->mcfg_added) { - pci_mmconfig_delete(info->sd.domain, - info->start_bus, info->end_bus); - info->mcfg_added = false; - } + cfg = pci_mmconfig_lookup(info->sd.domain, info->start_bus); + if (!cfg) + return; + + if (cfg->hot_added) + pci_mmconfig_delete(info->sd.domain, info->start_bus, + info->end_bus); } #else static int setup_mcfg_map(struct acpi_pci_root_info *ci)
Architectures which support PCI_DOMAINS_GENERIC (like ARM64) cannot call pci_bus_assign_domain_nr along ACPI PCI host bridge initialization since this function needs valid parent device reference to be able to retrieve domain number (aka segment).
We can omit that blocker and pass down host bridge device via pci_create_root_bus parameter and then be able to evaluate _SEG method being in pci_bus_assign_domain_nr.
Note that _SEG method is optional, therefore _SEG absence means that all PCI buses belong to domain 0.
Signed-off-by: Tomasz Nowicki tn@semihalf.com --- drivers/acpi/pci_root.c | 2 +- drivers/pci/pci.c | 32 +++++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 850d7bf..e682dc6 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -839,7 +839,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
pci_acpi_root_add_resources(info); pci_add_resource(&info->resources, &root->secondary); - bus = pci_create_root_bus(NULL, busnum, ops->pci_ops, + bus = pci_create_root_bus(&device->dev, busnum, ops->pci_ops, sysdata, &info->resources); if (!bus) goto out_release_info; diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 6a9a111..17d1857 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -25,6 +25,7 @@ #include <linux/device.h> #include <linux/pm_runtime.h> #include <linux/pci_hotplug.h> +#include <linux/acpi.h> #include <asm-generic/pci-bridge.h> #include <asm/setup.h> #include "pci.h" @@ -4501,7 +4502,7 @@ int pci_get_new_domain_nr(void) void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) { static int use_dt_domains = -1; - int domain = of_get_pci_domain_nr(parent->of_node); + int domain;
/* * Check DT domain and use_dt_domains values. @@ -4523,14 +4524,35 @@ void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) * API and update the use_dt_domains value to keep track of method we * are using to assign domain numbers (use_dt_domains = 0). * + * IF ACPI, we expect non-DT method (use_dt_domains == -1) + * and call _SEG method for corresponding host bridge device. + * If _SEG method does not exist, following ACPI spec (6.5.6) + * all PCI buses belong to domain 0. + * * All other combinations imply we have a platform that is trying - * to mix domain numbers obtained from DT and pci_get_new_domain_nr(), - * which is a recipe for domain mishandling and it is prevented by - * invalidating the domain value (domain = -1) and printing a - * corresponding error. + * to mix domain numbers obtained from DT, ACPI and + * pci_get_new_domain_nr(), which is a recipe for domain mishandling and + * it is prevented by invalidating the domain value (domain = -1) and + * printing a corresponding error. */ + + domain = of_get_pci_domain_nr(parent->of_node); if (domain >= 0 && use_dt_domains) { use_dt_domains = 1; +#ifdef CONFIG_ACPI + } else if (!acpi_disabled && use_dt_domains == -1) { + struct acpi_device *acpi_dev = to_acpi_device(parent); + unsigned long long segment = 0; + acpi_status status; + + status = acpi_evaluate_integer(acpi_dev->handle, + METHOD_NAME__SEG, NULL, + &segment); + if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) + dev_err(&acpi_dev->dev, "can't evaluate _SEG\n"); + + domain = segment; +#endif } else if (domain < 0 && use_dt_domains != 1) { use_dt_domains = 0; domain = pci_get_new_domain_nr();
On Tue, Oct 27, 2015 at 05:38:41PM +0100, Tomasz Nowicki wrote:
Architectures which support PCI_DOMAINS_GENERIC (like ARM64) cannot call pci_bus_assign_domain_nr along ACPI PCI host bridge initialization since this function needs valid parent device reference to be able to retrieve domain number (aka segment).
We can omit that blocker and pass down host bridge device via pci_create_root_bus parameter and then be able to evaluate _SEG method being in pci_bus_assign_domain_nr.
Note that _SEG method is optional, therefore _SEG absence means that all PCI buses belong to domain 0.
Signed-off-by: Tomasz Nowicki tn@semihalf.com
drivers/acpi/pci_root.c | 2 +- drivers/pci/pci.c | 32 +++++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 850d7bf..e682dc6 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -839,7 +839,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, pci_acpi_root_add_resources(info); pci_add_resource(&info->resources, &root->secondary);
- bus = pci_create_root_bus(NULL, busnum, ops->pci_ops,
- bus = pci_create_root_bus(&device->dev, busnum, ops->pci_ops, sysdata, &info->resources);
Not sure this change should be in this patch, I don't see the relation.
To put it differently: I think the patch should introduce the retrieval of the domain number from _SEG method and leave the passing of a valid host bridge device to a more appropriate patch.
if (!bus) goto out_release_info; diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 6a9a111..17d1857 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -25,6 +25,7 @@ #include <linux/device.h> #include <linux/pm_runtime.h> #include <linux/pci_hotplug.h> +#include <linux/acpi.h> #include <asm-generic/pci-bridge.h> #include <asm/setup.h> #include "pci.h" @@ -4501,7 +4502,7 @@ int pci_get_new_domain_nr(void) void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) { static int use_dt_domains = -1;
- int domain = of_get_pci_domain_nr(parent->of_node);
- int domain;
/* * Check DT domain and use_dt_domains values. @@ -4523,14 +4524,35 @@ void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) * API and update the use_dt_domains value to keep track of method we * are using to assign domain numbers (use_dt_domains = 0). *
* IF ACPI, we expect non-DT method (use_dt_domains == -1)* and call _SEG method for corresponding host bridge device.* If _SEG method does not exist, following ACPI spec (6.5.6)* all PCI buses belong to domain 0.*
- All other combinations imply we have a platform that is trying
* to mix domain numbers obtained from DT and pci_get_new_domain_nr(),* which is a recipe for domain mishandling and it is prevented by* invalidating the domain value (domain = -1) and printing a* corresponding error.
* to mix domain numbers obtained from DT, ACPI and* pci_get_new_domain_nr(), which is a recipe for domain mishandling and* it is prevented by invalidating the domain value (domain = -1) and* printing a corresponding error.- domain = of_get_pci_domain_nr(parent->of_node);
Not sure what you've got here by splitting the original line into two other than an increase in the change count.
Otherwise, it looks sensible.
Reviewed-by: Liviu Dudau Liviu.Dudau@arm.com
if (domain >= 0 && use_dt_domains) { use_dt_domains = 1; +#ifdef CONFIG_ACPI
- } else if (!acpi_disabled && use_dt_domains == -1) {
struct acpi_device *acpi_dev = to_acpi_device(parent);unsigned long long segment = 0;acpi_status status;status = acpi_evaluate_integer(acpi_dev->handle,METHOD_NAME__SEG, NULL,&segment);if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)dev_err(&acpi_dev->dev, "can't evaluate _SEG\n");domain = segment;+#endif } else if (domain < 0 && use_dt_domains != 1) { use_dt_domains = 0; domain = pci_get_new_domain_nr(); -- 1.9.1
Hi Liviu,
On 28.10.2015 12:38, Liviu.Dudau@arm.com wrote:
On Tue, Oct 27, 2015 at 05:38:41PM +0100, Tomasz Nowicki wrote:
Architectures which support PCI_DOMAINS_GENERIC (like ARM64) cannot call pci_bus_assign_domain_nr along ACPI PCI host bridge initialization since this function needs valid parent device reference to be able to retrieve domain number (aka segment).
We can omit that blocker and pass down host bridge device via pci_create_root_bus parameter and then be able to evaluate _SEG method being in pci_bus_assign_domain_nr.
Note that _SEG method is optional, therefore _SEG absence means that all PCI buses belong to domain 0.
Signed-off-by: Tomasz Nowicki tn@semihalf.com
drivers/acpi/pci_root.c | 2 +- drivers/pci/pci.c | 32 +++++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 850d7bf..e682dc6 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -839,7 +839,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
pci_acpi_root_add_resources(info); pci_add_resource(&info->resources, &root->secondary);
- bus = pci_create_root_bus(NULL, busnum, ops->pci_ops,
- bus = pci_create_root_bus(&device->dev, busnum, ops->pci_ops, sysdata, &info->resources);
Not sure this change should be in this patch, I don't see the relation.
To put it differently: I think the patch should introduce the retrieval of the domain number from _SEG method and leave the passing of a valid host bridge device to a more appropriate patch.
I wanted to highlight that ACPI kernel using PCI_DOMAINS_GENERIC needs to have both in place: 1. Obtaining domain from _SEG method 2. And host bridge device reference for which we can call _SEG. But you are right, it will be more clear if I split up patch and describe it in separate changelog.
if (!bus) goto out_release_info; diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 6a9a111..17d1857 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -25,6 +25,7 @@ #include <linux/device.h> #include <linux/pm_runtime.h> #include <linux/pci_hotplug.h> +#include <linux/acpi.h> #include <asm-generic/pci-bridge.h> #include <asm/setup.h> #include "pci.h" @@ -4501,7 +4502,7 @@ int pci_get_new_domain_nr(void) void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) { static int use_dt_domains = -1;
- int domain = of_get_pci_domain_nr(parent->of_node);
int domain;
/*
- Check DT domain and use_dt_domains values.
@@ -4523,14 +4524,35 @@ void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) * API and update the use_dt_domains value to keep track of method we * are using to assign domain numbers (use_dt_domains = 0). *
* IF ACPI, we expect non-DT method (use_dt_domains == -1)* and call _SEG method for corresponding host bridge device.* If _SEG method does not exist, following ACPI spec (6.5.6)* all PCI buses belong to domain 0.*
- All other combinations imply we have a platform that is trying
* to mix domain numbers obtained from DT and pci_get_new_domain_nr(),* which is a recipe for domain mishandling and it is prevented by* invalidating the domain value (domain = -1) and printing a* corresponding error.
* to mix domain numbers obtained from DT, ACPI and* pci_get_new_domain_nr(), which is a recipe for domain mishandling and* it is prevented by invalidating the domain value (domain = -1) and* printing a corresponding error.- domain = of_get_pci_domain_nr(parent->of_node);
Not sure what you've got here by splitting the original line into two other than an increase in the change count.
Yes, it does not make sense to split the original line. I will fix that.
Otherwise, it looks sensible.
Reviewed-by: Liviu Dudau Liviu.Dudau@arm.com
Thanks Liviu!
Regards, Tomasz
On Tue, Oct 27, 2015 at 05:38:41PM +0100, Tomasz Nowicki wrote:
Architectures which support PCI_DOMAINS_GENERIC (like ARM64) cannot call pci_bus_assign_domain_nr along ACPI PCI host bridge initialization since this function needs valid parent device reference to be able to retrieve domain number (aka segment).
We can omit that blocker and pass down host bridge device via pci_create_root_bus parameter and then be able to evaluate _SEG method being in pci_bus_assign_domain_nr.
Note that _SEG method is optional, therefore _SEG absence means that all PCI buses belong to domain 0.
Signed-off-by: Tomasz Nowicki tn@semihalf.com
drivers/acpi/pci_root.c | 2 +- drivers/pci/pci.c | 32 +++++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 850d7bf..e682dc6 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -839,7 +839,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, pci_acpi_root_add_resources(info); pci_add_resource(&info->resources, &root->secondary);
- bus = pci_create_root_bus(NULL, busnum, ops->pci_ops,
- bus = pci_create_root_bus(&device->dev, busnum, ops->pci_ops, sysdata, &info->resources);
If I read x86 code correctly, they rely on the first argument to be NULL, I think you would break x86 by doing that, see:
arch/x86/pci/acpi.c (pcibios_root_bridge_prepare())
By the way, can't we move the code setting up the ACPI_COMPANION to core PCI code and stop relying on sysdata for that ?
Thanks, Lorenzo
if (!bus) goto out_release_info; diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 6a9a111..17d1857 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -25,6 +25,7 @@ #include <linux/device.h> #include <linux/pm_runtime.h> #include <linux/pci_hotplug.h> +#include <linux/acpi.h> #include <asm-generic/pci-bridge.h> #include <asm/setup.h> #include "pci.h" @@ -4501,7 +4502,7 @@ int pci_get_new_domain_nr(void) void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) { static int use_dt_domains = -1;
- int domain = of_get_pci_domain_nr(parent->of_node);
- int domain;
/* * Check DT domain and use_dt_domains values. @@ -4523,14 +4524,35 @@ void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) * API and update the use_dt_domains value to keep track of method we * are using to assign domain numbers (use_dt_domains = 0). *
* IF ACPI, we expect non-DT method (use_dt_domains == -1)* and call _SEG method for corresponding host bridge device.* If _SEG method does not exist, following ACPI spec (6.5.6)* all PCI buses belong to domain 0.*
- All other combinations imply we have a platform that is trying
* to mix domain numbers obtained from DT and pci_get_new_domain_nr(),* which is a recipe for domain mishandling and it is prevented by* invalidating the domain value (domain = -1) and printing a* corresponding error.
* to mix domain numbers obtained from DT, ACPI and* pci_get_new_domain_nr(), which is a recipe for domain mishandling and* it is prevented by invalidating the domain value (domain = -1) and* printing a corresponding error.- domain = of_get_pci_domain_nr(parent->of_node); if (domain >= 0 && use_dt_domains) { use_dt_domains = 1;
+#ifdef CONFIG_ACPI
- } else if (!acpi_disabled && use_dt_domains == -1) {
struct acpi_device *acpi_dev = to_acpi_device(parent);unsigned long long segment = 0;acpi_status status;status = acpi_evaluate_integer(acpi_dev->handle,METHOD_NAME__SEG, NULL,&segment);if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)dev_err(&acpi_dev->dev, "can't evaluate _SEG\n");domain = segment;+#endif } else if (domain < 0 && use_dt_domains != 1) { use_dt_domains = 0; domain = pci_get_new_domain_nr(); -- 1.9.1
On 03.11.2015 17:10, Lorenzo Pieralisi wrote:
On Tue, Oct 27, 2015 at 05:38:41PM +0100, Tomasz Nowicki wrote:
Architectures which support PCI_DOMAINS_GENERIC (like ARM64) cannot call pci_bus_assign_domain_nr along ACPI PCI host bridge initialization since this function needs valid parent device reference to be able to retrieve domain number (aka segment).
We can omit that blocker and pass down host bridge device via pci_create_root_bus parameter and then be able to evaluate _SEG method being in pci_bus_assign_domain_nr.
Note that _SEG method is optional, therefore _SEG absence means that all PCI buses belong to domain 0.
Signed-off-by: Tomasz Nowicki tn@semihalf.com
drivers/acpi/pci_root.c | 2 +- drivers/pci/pci.c | 32 +++++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 850d7bf..e682dc6 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -839,7 +839,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
pci_acpi_root_add_resources(info); pci_add_resource(&info->resources, &root->secondary);
- bus = pci_create_root_bus(NULL, busnum, ops->pci_ops,
- bus = pci_create_root_bus(&device->dev, busnum, ops->pci_ops, sysdata, &info->resources);
If I read x86 code correctly, they rely on the first argument to be NULL, I think you would break x86 by doing that, see:
arch/x86/pci/acpi.c (pcibios_root_bridge_prepare())
By the way, can't we move the code setting up the ACPI_COMPANION to core PCI code and stop relying on sysdata for that ?
Yes, that will break x86&ia64 but as you said it may be overcome with consolidation of ACPI_COMPANION into the core code.
Tomasz
Because of two patch series: 1. Jiang Liu's common interface to support PCI host bridge init 2. Refactoring of MMCONFIG, part of this patch set now we can think about PCI buses enumeration for ARM64 and ACPI tables.
This patch introduce ACPI based PCI hostbridge init calls which use information from MCFG table (PCI config space regions) and _CRS (IO/irq resources) to initialize PCI hostbridge.
Signed-off-by: Tomasz Nowicki tn@semihalf.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com CC: Arnd Bergmann arnd@arndb.de CC: Catalin Marinas catalin.marinas@arm.com CC: Liviu Dudau Liviu.Dudau@arm.com CC: Lorenzo Pieralisi Lorenzo.Pieralisi@arm.com CC: Will Deacon will.deacon@arm.com --- arch/arm64/Kconfig | 6 ++ arch/arm64/kernel/pci.c | 208 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 202 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 07d1811..bbcc6b1 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -89,6 +89,7 @@ config ARM64 select SPARSE_IRQ select SYSCTL_EXCEPTION_TRACE select HAVE_CONTEXT_TRACKING + select HAVE_PCI_ECAM help ARM 64-bit (AArch64) Linux support.
@@ -202,6 +203,11 @@ source "drivers/pci/Kconfig" source "drivers/pci/pcie/Kconfig" source "drivers/pci/hotplug/Kconfig"
+config PCI_MMCONFIG + def_bool y + select PCI_ECAM + depends on ACPI + endmenu
menu "Kernel Features" diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index b3d098b..66cc1ae 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -11,12 +11,15 @@ */
#include <linux/acpi.h> +#include <linux/ecam.h> #include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/mm.h> +#include <linux/of_address.h> #include <linux/of_pci.h> #include <linux/of_platform.h> +#include <linux/pci-acpi.h> #include <linux/slab.h>
#include <asm/pci-bridge.h> @@ -52,35 +55,216 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) }
/* - * Try to assign the IRQ number from DT when adding a new device + * Try to assign the IRQ number from DT/ACPI when adding a new device */ int pcibios_add_device(struct pci_dev *dev) { - dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); + if (acpi_disabled) + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); +#ifdef CONFIG_ACPI + else + acpi_pci_irq_enable(dev); +#endif
return 0; }
+#ifdef CONFIG_ACPI +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{ + struct acpi_pci_root *root = bridge->bus->sysdata; + + ACPI_COMPANION_SET(&bridge->dev, root->device); + return 0; +} + +void pcibios_add_bus(struct pci_bus *bus) +{ + acpi_pci_add_bus(bus); +} + +void pcibios_remove_bus(struct pci_bus *bus) +{ + acpi_pci_remove_bus(bus); +} + +static int __init pcibios_assign_resources(void) +{ + if (acpi_disabled) + return 0; + + pci_assign_unassigned_resources(); + return 0; +} /* - * raw_pci_read/write - Platform-specific PCI config space access. + * rootfs_initcall comes after subsys_initcall and fs_initcall_sync, + * so we know acpi scan and PCI_FIXUP_FINAL quirks have both run. */ -int raw_pci_read(unsigned int domain, unsigned int bus, - unsigned int devfn, int reg, int len, u32 *val) +rootfs_initcall(pcibios_assign_resources); + +static void __iomem * +pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) { - return -ENXIO; + struct pci_mmcfg_region *cfg; + + cfg = pci_mmconfig_lookup(pci_domain_nr(bus), bus->number); + if (cfg && cfg->virt) + return cfg->virt + + (PCI_MMCFG_BUS_OFFSET(bus->number) | (devfn << 12)) + + offset; + return NULL; }
-int raw_pci_write(unsigned int domain, unsigned int bus, - unsigned int devfn, int reg, int len, u32 val) +struct pci_ops pci_root_ops = { + .map_bus = pci_mcfg_dev_base, + .read = pci_generic_config_read, + .write = pci_generic_config_write, +}; + +#ifdef CONFIG_PCI_MMCONFIG +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) { - return -ENXIO; + struct pci_mmcfg_region *cfg; + struct acpi_pci_root *root; + int seg, start, end, err; + + root = ci->root; + seg = root->segment; + start = root->secondary.start; + end = root->secondary.end; + + cfg = pci_mmconfig_lookup(seg, start); + if (cfg) + return 0; + + cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr); + if (!cfg) + return -ENOMEM; + + err = pci_mmconfig_inject(cfg); + return err; }
-#ifdef CONFIG_ACPI +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) +{ + struct acpi_pci_root *root = ci->root; + struct pci_mmcfg_region *cfg; + + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start); + if (cfg) + return; + + if (cfg->hot_added) + pci_mmconfig_delete(root->segment, root->secondary.start, + root->secondary.end); +} +#else +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) +{ + return 0; +} + +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) { } +#endif + +static int pci_acpi_root_init_info(struct acpi_pci_root_info *ci) +{ + return pci_add_mmconfig_region(ci); +} + +static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci) +{ + pci_remove_mmconfig_region(ci); + kfree(ci); +} + +static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) +{ + struct resource_entry *entry, *tmp; + int ret; + + ret = acpi_pci_probe_root_resources(ci); + if (ret < 0) + return ret; + + resource_list_for_each_entry_safe(entry, tmp, &ci->resources) { + struct resource *res = entry->res; + + /* + * Special handling for ARM IO range + * TODO: need to move pci_register_io_range() function out + * of drivers/of/address.c for both used by DT and ACPI + */ + if (res->flags & IORESOURCE_IO) { + unsigned long port; + int err; + resource_size_t length = res->end - res->start; + + err = pci_register_io_range(res->start, length); + if (err) { + resource_list_destroy_entry(entry); + continue; + } + + port = pci_address_to_pio(res->start); + if (port == (unsigned long)-1) { + resource_list_destroy_entry(entry); + continue; + } + + res->start = port; + res->end = res->start + length - 1; + + if (pci_remap_iospace(res, res->start) < 0) + resource_list_destroy_entry(entry); + } + } + + return 0; +} + +static struct acpi_pci_root_ops acpi_pci_root_ops = { + .pci_ops = &pci_root_ops, + .init_info = pci_acpi_root_init_info, + .release_info = pci_acpi_root_release_info, + .prepare_resources = pci_acpi_root_prepare_resources, +}; + /* Root bridge scanning */ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) { - /* TODO: Should be revisited when implementing PCI on ACPI */ - return NULL; + int node = acpi_get_node(root->device->handle); + int domain = root->segment; + int busnum = root->secondary.start; + struct acpi_pci_root_info *info; + struct pci_bus *bus; + + if (domain && !pci_domains_supported) { + pr_warn("PCI %04x:%02x: multiple domains not supported.\n", + domain, busnum); + return NULL; + } + + info = kzalloc_node(sizeof(*info), GFP_KERNEL, node); + if (!info) { + dev_err(&root->device->dev, + "pci_bus %04x:%02x: ignored (out of memory)\n", + domain, busnum); + return NULL; + } + + bus = acpi_pci_root_create(root, &acpi_pci_root_ops, info, root); + + /* After the PCI-E bus has been walked and all devices discovered, + * configure any settings of the fabric that might be necessary. + */ + if (bus) { + struct pci_bus *child; + + list_for_each_entry(child, &bus->children, node) + pcie_bus_configure_settings(child); + } + + return bus; } #endif
On Tue, Oct 27, 2015 at 05:38:42PM +0100, Tomasz Nowicki wrote:
Because of two patch series:
- Jiang Liu's common interface to support PCI host bridge init
- Refactoring of MMCONFIG, part of this patch set
now we can think about PCI buses enumeration for ARM64 and ACPI tables.
This patch introduce ACPI based PCI hostbridge init calls which use information from MCFG table (PCI config space regions) and _CRS (IO/irq resources) to initialize PCI hostbridge.
Signed-off-by: Tomasz Nowicki tn@semihalf.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com CC: Arnd Bergmann arnd@arndb.de CC: Catalin Marinas catalin.marinas@arm.com CC: Liviu Dudau Liviu.Dudau@arm.com CC: Lorenzo Pieralisi Lorenzo.Pieralisi@arm.com CC: Will Deacon will.deacon@arm.com
arch/arm64/Kconfig | 6 ++ arch/arm64/kernel/pci.c | 208 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 202 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 07d1811..bbcc6b1 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -89,6 +89,7 @@ config ARM64 select SPARSE_IRQ select SYSCTL_EXCEPTION_TRACE select HAVE_CONTEXT_TRACKING
- select HAVE_PCI_ECAM help ARM 64-bit (AArch64) Linux support.
@@ -202,6 +203,11 @@ source "drivers/pci/Kconfig" source "drivers/pci/pcie/Kconfig" source "drivers/pci/hotplug/Kconfig" +config PCI_MMCONFIG
- def_bool y
- select PCI_ECAM
- depends on ACPI
endmenu menu "Kernel Features" diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index b3d098b..66cc1ae 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -11,12 +11,15 @@ */ #include <linux/acpi.h> +#include <linux/ecam.h> #include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/mm.h> +#include <linux/of_address.h> #include <linux/of_pci.h> #include <linux/of_platform.h> +#include <linux/pci-acpi.h> #include <linux/slab.h> #include <asm/pci-bridge.h> @@ -52,35 +55,216 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) } /*
- Try to assign the IRQ number from DT when adding a new device
*/
- Try to assign the IRQ number from DT/ACPI when adding a new device
int pcibios_add_device(struct pci_dev *dev) {
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
- if (acpi_disabled)
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);+#ifdef CONFIG_ACPI
- else
acpi_pci_irq_enable(dev);+#endif return 0; } +#ifdef CONFIG_ACPI +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- struct acpi_pci_root *root = bridge->bus->sysdata;
- ACPI_COMPANION_SET(&bridge->dev, root->device);
- return 0;
+}
+void pcibios_add_bus(struct pci_bus *bus) +{
- acpi_pci_add_bus(bus);
+}
+void pcibios_remove_bus(struct pci_bus *bus) +{
- acpi_pci_remove_bus(bus);
+}
+static int __init pcibios_assign_resources(void) +{
- if (acpi_disabled)
return 0;- pci_assign_unassigned_resources();
- return 0;
You can change this function into: { if (!acpi_disabled) pci_assign_unassigned_resources();
return 0; }
as the equivalent but shorter form.
+} /*
- raw_pci_read/write - Platform-specific PCI config space access.
- rootfs_initcall comes after subsys_initcall and fs_initcall_sync,
*/
- so we know acpi scan and PCI_FIXUP_FINAL quirks have both run.
-int raw_pci_read(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *val)
What happened with raw_pci_{read,write} ? Why do you remove them?
+rootfs_initcall(pcibios_assign_resources);
Would you be so kind and explain to me why you need this initcall? Can you not set the PCI_REASSIGN_ALL_RSRC flag before calling pci_scan_root_bus() ?
I haven't focused on ACPI before so I'm a bit hazy on the init order when that is enabled. That being said, I don't like adding in the architecture code initcall hooks just to fix up some dependency orders that could / should be fixed some other way.
+static void __iomem * +pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(pci_domain_nr(bus), bus->number);
- if (cfg && cfg->virt)
return cfg->virt +(PCI_MMCFG_BUS_OFFSET(bus->number) | (devfn << 12)) +offset;- return NULL;
} -int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
+};
+#ifdef CONFIG_PCI_MMCONFIG +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- struct acpi_pci_root *root;
- int seg, start, end, err;
- root = ci->root;
- seg = root->segment;
- start = root->secondary.start;
- end = root->secondary.end;
- cfg = pci_mmconfig_lookup(seg, start);
- if (cfg)
return 0;- cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
- if (!cfg)
return -ENOMEM;- err = pci_mmconfig_inject(cfg);
- return err;
} -#ifdef CONFIG_ACPI +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) +{
- struct acpi_pci_root *root = ci->root;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
- if (cfg)
return;- if (cfg->hot_added)
pci_mmconfig_delete(root->segment, root->secondary.start,root->secondary.end);+} +#else +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) +{
- return 0;
+}
+static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) { } +#endif
+static int pci_acpi_root_init_info(struct acpi_pci_root_info *ci) +{
- return pci_add_mmconfig_region(ci);
+}
+static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci) +{
- pci_remove_mmconfig_region(ci);
- kfree(ci);
+}
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) +{
- struct resource_entry *entry, *tmp;
- int ret;
- ret = acpi_pci_probe_root_resources(ci);
- if (ret < 0)
return ret;- resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
struct resource *res = entry->res;/** Special handling for ARM IO range
There is nothing ARM specific here. It should apply to any memory mapped IO range.
* TODO: need to move pci_register_io_range() function out* of drivers/of/address.c for both used by DT and ACPI*/if (res->flags & IORESOURCE_IO) {unsigned long port;int err;resource_size_t length = res->end - res->start;err = pci_register_io_range(res->start, length);if (err) {resource_list_destroy_entry(entry);continue;}port = pci_address_to_pio(res->start);if (port == (unsigned long)-1) {resource_list_destroy_entry(entry);continue;}res->start = port;res->end = res->start + length - 1;if (pci_remap_iospace(res, res->start) < 0)resource_list_destroy_entry(entry);}- }
- return 0;
+}
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .pci_ops = &pci_root_ops,
- .init_info = pci_acpi_root_init_info,
- .release_info = pci_acpi_root_release_info,
- .prepare_resources = pci_acpi_root_prepare_resources,
+};
/* Root bridge scanning */ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) {
- /* TODO: Should be revisited when implementing PCI on ACPI */
- return NULL;
- int node = acpi_get_node(root->device->handle);
- int domain = root->segment;
- int busnum = root->secondary.start;
- struct acpi_pci_root_info *info;
- struct pci_bus *bus;
- if (domain && !pci_domains_supported) {
pr_warn("PCI %04x:%02x: multiple domains not supported.\n",domain, busnum);return NULL;- }
- info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
- if (!info) {
dev_err(&root->device->dev,"pci_bus %04x:%02x: ignored (out of memory)\n",domain, busnum);return NULL;- }
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, info, root);
- /* After the PCI-E bus has been walked and all devices discovered,
* configure any settings of the fabric that might be necessary.*/- if (bus) {
struct pci_bus *child;list_for_each_entry(child, &bus->children, node)pcie_bus_configure_settings(child);- }
- return bus;
}
#endif
1.9.1
On 28.10.2015 12:49, Liviu.Dudau@arm.com wrote:
On Tue, Oct 27, 2015 at 05:38:42PM +0100, Tomasz Nowicki wrote:
Because of two patch series:
- Jiang Liu's common interface to support PCI host bridge init
- Refactoring of MMCONFIG, part of this patch set
now we can think about PCI buses enumeration for ARM64 and ACPI tables.
This patch introduce ACPI based PCI hostbridge init calls which use information from MCFG table (PCI config space regions) and _CRS (IO/irq resources) to initialize PCI hostbridge.
Signed-off-by: Tomasz Nowicki tn@semihalf.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com CC: Arnd Bergmann arnd@arndb.de CC: Catalin Marinas catalin.marinas@arm.com CC: Liviu Dudau Liviu.Dudau@arm.com CC: Lorenzo Pieralisi Lorenzo.Pieralisi@arm.com CC: Will Deacon will.deacon@arm.com
arch/arm64/Kconfig | 6 ++ arch/arm64/kernel/pci.c | 208 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 202 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 07d1811..bbcc6b1 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -89,6 +89,7 @@ config ARM64 select SPARSE_IRQ select SYSCTL_EXCEPTION_TRACE select HAVE_CONTEXT_TRACKING
- select HAVE_PCI_ECAM help ARM 64-bit (AArch64) Linux support.
@@ -202,6 +203,11 @@ source "drivers/pci/Kconfig" source "drivers/pci/pcie/Kconfig" source "drivers/pci/hotplug/Kconfig"
+config PCI_MMCONFIG
def_bool y
select PCI_ECAM
depends on ACPI
endmenu
menu "Kernel Features"
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index b3d098b..66cc1ae 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -11,12 +11,15 @@ */
#include <linux/acpi.h> +#include <linux/ecam.h> #include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/mm.h> +#include <linux/of_address.h> #include <linux/of_pci.h> #include <linux/of_platform.h> +#include <linux/pci-acpi.h> #include <linux/slab.h>
#include <asm/pci-bridge.h> @@ -52,35 +55,216 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) }
/*
- Try to assign the IRQ number from DT when adding a new device
*/ int pcibios_add_device(struct pci_dev *dev) {
- Try to assign the IRQ number from DT/ACPI when adding a new device
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
- if (acpi_disabled)
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);+#ifdef CONFIG_ACPI
- else
acpi_pci_irq_enable(dev);+#endif
return 0; }
+#ifdef CONFIG_ACPI +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- struct acpi_pci_root *root = bridge->bus->sysdata;
- ACPI_COMPANION_SET(&bridge->dev, root->device);
- return 0;
+}
+void pcibios_add_bus(struct pci_bus *bus) +{
- acpi_pci_add_bus(bus);
+}
+void pcibios_remove_bus(struct pci_bus *bus) +{
- acpi_pci_remove_bus(bus);
+}
+static int __init pcibios_assign_resources(void) +{
- if (acpi_disabled)
return 0;- pci_assign_unassigned_resources();
- return 0;
You can change this function into: { if (!acpi_disabled) pci_assign_unassigned_resources();
return 0; }
as the equivalent but shorter form.
Sure, will do.
+} /*
- raw_pci_read/write - Platform-specific PCI config space access.
- rootfs_initcall comes after subsys_initcall and fs_initcall_sync,
*/
- so we know acpi scan and PCI_FIXUP_FINAL quirks have both run.
-int raw_pci_read(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *val)What happened with raw_pci_{read,write} ? Why do you remove them?
We do not need raw_pci_{read,write} any more, we will use empty raw_pci_{read,write} from mcfg.c introduced in patch 6/11.
I think this is another candidate to split up, first I will remove these raw_pci_{read,write} accessors and then introduce ACPI PCI hostbridge init, it will be easier to review, what do you think?
+rootfs_initcall(pcibios_assign_resources);
Would you be so kind and explain to me why you need this initcall? Can you not set the PCI_REASSIGN_ALL_RSRC flag before calling pci_scan_root_bus() ?
I haven't focused on ACPI before so I'm a bit hazy on the init order when that is enabled. That being said, I don't like adding in the architecture code initcall hooks just to fix up some dependency orders that could / should be fixed some other way.
My idea was to defer resource reassigning to give a chance for running DECLARE_PCI_FIXUP_FINAL. I used DECLARE_PCI_FIXUP_FINAL to set IORESOURCE_PCI_FIXED for some PCI devices. Now I am not sure we need to defer it, my DECLARE_PCI_FIXUP_FINALs might be done in firmware, let me get back to you on this.
+static void __iomem * +pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(pci_domain_nr(bus), bus->number);
- if (cfg && cfg->virt)
return cfg->virt +(PCI_MMCFG_BUS_OFFSET(bus->number) | (devfn << 12)) +offset;- return NULL; }
-int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
+};
+#ifdef CONFIG_PCI_MMCONFIG +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- struct acpi_pci_root *root;
- int seg, start, end, err;
- root = ci->root;
- seg = root->segment;
- start = root->secondary.start;
- end = root->secondary.end;
- cfg = pci_mmconfig_lookup(seg, start);
- if (cfg)
return 0;- cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
- if (!cfg)
return -ENOMEM;- err = pci_mmconfig_inject(cfg);
- return err; }
-#ifdef CONFIG_ACPI +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) +{
- struct acpi_pci_root *root = ci->root;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
- if (cfg)
return;- if (cfg->hot_added)
pci_mmconfig_delete(root->segment, root->secondary.start,root->secondary.end);+} +#else +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) +{
- return 0;
+}
+static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) { } +#endif
+static int pci_acpi_root_init_info(struct acpi_pci_root_info *ci) +{
- return pci_add_mmconfig_region(ci);
+}
+static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci) +{
- pci_remove_mmconfig_region(ci);
- kfree(ci);
+}
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) +{
- struct resource_entry *entry, *tmp;
- int ret;
- ret = acpi_pci_probe_root_resources(ci);
- if (ret < 0)
return ret;- resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
struct resource *res = entry->res;/** Special handling for ARM IO rangeThere is nothing ARM specific here. It should apply to any memory mapped IO range.
OK, I will remove that comment.
* TODO: need to move pci_register_io_range() function out* of drivers/of/address.c for both used by DT and ACPI*/if (res->flags & IORESOURCE_IO) {unsigned long port;int err;resource_size_t length = res->end - res->start;err = pci_register_io_range(res->start, length);if (err) {resource_list_destroy_entry(entry);continue;}port = pci_address_to_pio(res->start);if (port == (unsigned long)-1) {resource_list_destroy_entry(entry);continue;}res->start = port;res->end = res->start + length - 1;if (pci_remap_iospace(res, res->start) < 0)resource_list_destroy_entry(entry);}- }
- return 0;
+}
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .pci_ops = &pci_root_ops,
- .init_info = pci_acpi_root_init_info,
- .release_info = pci_acpi_root_release_info,
- .prepare_resources = pci_acpi_root_prepare_resources,
+};
- /* Root bridge scanning */ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) {
- /* TODO: Should be revisited when implementing PCI on ACPI */
- return NULL;
- int node = acpi_get_node(root->device->handle);
- int domain = root->segment;
- int busnum = root->secondary.start;
- struct acpi_pci_root_info *info;
- struct pci_bus *bus;
- if (domain && !pci_domains_supported) {
pr_warn("PCI %04x:%02x: multiple domains not supported.\n",domain, busnum);return NULL;- }
- info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
- if (!info) {
dev_err(&root->device->dev,"pci_bus %04x:%02x: ignored (out of memory)\n",domain, busnum);return NULL;- }
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, info, root);
- /* After the PCI-E bus has been walked and all devices discovered,
* configure any settings of the fabric that might be necessary.*/- if (bus) {
struct pci_bus *child;list_for_each_entry(child, &bus->children, node)pcie_bus_configure_settings(child);- }
- return bus; } #endif
-- 1.9.1
On Wed, Oct 28, 2015 at 02:42:30PM +0100, Tomasz Nowicki wrote:
On 28.10.2015 12:49, Liviu.Dudau@arm.com wrote:
On Tue, Oct 27, 2015 at 05:38:42PM +0100, Tomasz Nowicki wrote:
Because of two patch series:
- Jiang Liu's common interface to support PCI host bridge init
- Refactoring of MMCONFIG, part of this patch set
now we can think about PCI buses enumeration for ARM64 and ACPI tables.
This patch introduce ACPI based PCI hostbridge init calls which use information from MCFG table (PCI config space regions) and _CRS (IO/irq resources) to initialize PCI hostbridge.
Signed-off-by: Tomasz Nowicki tn@semihalf.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com CC: Arnd Bergmann arnd@arndb.de CC: Catalin Marinas catalin.marinas@arm.com CC: Liviu Dudau Liviu.Dudau@arm.com CC: Lorenzo Pieralisi Lorenzo.Pieralisi@arm.com CC: Will Deacon will.deacon@arm.com
arch/arm64/Kconfig | 6 ++ arch/arm64/kernel/pci.c | 208 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 202 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 07d1811..bbcc6b1 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -89,6 +89,7 @@ config ARM64 select SPARSE_IRQ select SYSCTL_EXCEPTION_TRACE select HAVE_CONTEXT_TRACKING
- select HAVE_PCI_ECAM help ARM 64-bit (AArch64) Linux support.
@@ -202,6 +203,11 @@ source "drivers/pci/Kconfig" source "drivers/pci/pcie/Kconfig" source "drivers/pci/hotplug/Kconfig"
+config PCI_MMCONFIG
- def_bool y
- select PCI_ECAM
- depends on ACPI
endmenu
menu "Kernel Features" diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index b3d098b..66cc1ae 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -11,12 +11,15 @@ */
#include <linux/acpi.h> +#include <linux/ecam.h> #include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/mm.h> +#include <linux/of_address.h> #include <linux/of_pci.h> #include <linux/of_platform.h> +#include <linux/pci-acpi.h> #include <linux/slab.h>
#include <asm/pci-bridge.h> @@ -52,35 +55,216 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) }
/*
- Try to assign the IRQ number from DT when adding a new device
*/
- Try to assign the IRQ number from DT/ACPI when adding a new device
int pcibios_add_device(struct pci_dev *dev) {
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
- if (acpi_disabled)
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);+#ifdef CONFIG_ACPI
- else
acpi_pci_irq_enable(dev);+#endif
return 0; }
+#ifdef CONFIG_ACPI +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- struct acpi_pci_root *root = bridge->bus->sysdata;
- ACPI_COMPANION_SET(&bridge->dev, root->device);
- return 0;
+}
+void pcibios_add_bus(struct pci_bus *bus) +{
- acpi_pci_add_bus(bus);
+}
+void pcibios_remove_bus(struct pci_bus *bus) +{
- acpi_pci_remove_bus(bus);
+}
+static int __init pcibios_assign_resources(void) +{
- if (acpi_disabled)
return 0;- pci_assign_unassigned_resources();
- return 0;
You can change this function into: { if (!acpi_disabled) pci_assign_unassigned_resources();
return 0; }
as the equivalent but shorter form.
Sure, will do.
+} /*
- raw_pci_read/write - Platform-specific PCI config space access.
- rootfs_initcall comes after subsys_initcall and fs_initcall_sync,
*/
- so we know acpi scan and PCI_FIXUP_FINAL quirks have both run.
-int raw_pci_read(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *val)What happened with raw_pci_{read,write} ? Why do you remove them?
We do not need raw_pci_{read,write} any more, we will use empty raw_pci_{read,write} from mcfg.c introduced in patch 6/11.
I think this is another candidate to split up, first I will remove these raw_pci_{read,write} accessors and then introduce ACPI PCI hostbridge init, it will be easier to review, what do you think?
Yes, I like that. If the calls were redundant after 6/11 then they should not have been kept until this patch to be removed.
+rootfs_initcall(pcibios_assign_resources);
Would you be so kind and explain to me why you need this initcall? Can you not set the PCI_REASSIGN_ALL_RSRC flag before calling pci_scan_root_bus() ?
I haven't focused on ACPI before so I'm a bit hazy on the init order when that is enabled. That being said, I don't like adding in the architecture code initcall hooks just to fix up some dependency orders that could / should be fixed some other way.
My idea was to defer resource reassigning to give a chance for running DECLARE_PCI_FIXUP_FINAL. I used DECLARE_PCI_FIXUP_FINAL to set IORESOURCE_PCI_FIXED for some PCI devices. Now I am not sure we need to defer it, my DECLARE_PCI_FIXUP_FINALs might be done in firmware, let me get back to you on this.
If fixups can be done in firmware then that is the preferred direction nowadays.
+static void __iomem * +pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(pci_domain_nr(bus), bus->number);
- if (cfg && cfg->virt)
return cfg->virt +(PCI_MMCFG_BUS_OFFSET(bus->number) | (devfn << 12)) +offset;- return NULL;
}
-int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
+};
+#ifdef CONFIG_PCI_MMCONFIG +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- struct acpi_pci_root *root;
- int seg, start, end, err;
- root = ci->root;
- seg = root->segment;
- start = root->secondary.start;
- end = root->secondary.end;
- cfg = pci_mmconfig_lookup(seg, start);
- if (cfg)
return 0;- cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
- if (!cfg)
return -ENOMEM;- err = pci_mmconfig_inject(cfg);
- return err;
}
-#ifdef CONFIG_ACPI +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) +{
- struct acpi_pci_root *root = ci->root;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
- if (cfg)
return;- if (cfg->hot_added)
pci_mmconfig_delete(root->segment, root->secondary.start,root->secondary.end);+} +#else +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) +{
- return 0;
+}
+static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) { } +#endif
+static int pci_acpi_root_init_info(struct acpi_pci_root_info *ci) +{
- return pci_add_mmconfig_region(ci);
+}
+static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci) +{
- pci_remove_mmconfig_region(ci);
- kfree(ci);
+}
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) +{
- struct resource_entry *entry, *tmp;
- int ret;
- ret = acpi_pci_probe_root_resources(ci);
- if (ret < 0)
return ret;- resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
struct resource *res = entry->res;/** Special handling for ARM IO rangeThere is nothing ARM specific here. It should apply to any memory mapped IO range.
OK, I will remove that comment.
Thanks!
Liviu
* TODO: need to move pci_register_io_range() function out* of drivers/of/address.c for both used by DT and ACPI*/if (res->flags & IORESOURCE_IO) {unsigned long port;int err;resource_size_t length = res->end - res->start;err = pci_register_io_range(res->start, length);if (err) {resource_list_destroy_entry(entry);continue;}port = pci_address_to_pio(res->start);if (port == (unsigned long)-1) {resource_list_destroy_entry(entry);continue;}res->start = port;res->end = res->start + length - 1;if (pci_remap_iospace(res, res->start) < 0)resource_list_destroy_entry(entry);}- }
- return 0;
+}
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .pci_ops = &pci_root_ops,
- .init_info = pci_acpi_root_init_info,
- .release_info = pci_acpi_root_release_info,
- .prepare_resources = pci_acpi_root_prepare_resources,
+};
/* Root bridge scanning */ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) {
- /* TODO: Should be revisited when implementing PCI on ACPI */
- return NULL;
- int node = acpi_get_node(root->device->handle);
- int domain = root->segment;
- int busnum = root->secondary.start;
- struct acpi_pci_root_info *info;
- struct pci_bus *bus;
- if (domain && !pci_domains_supported) {
pr_warn("PCI %04x:%02x: multiple domains not supported.\n",domain, busnum);return NULL;- }
- info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
- if (!info) {
dev_err(&root->device->dev,"pci_bus %04x:%02x: ignored (out of memory)\n",domain, busnum);return NULL;- }
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, info, root);
- /* After the PCI-E bus has been walked and all devices discovered,
* configure any settings of the fabric that might be necessary.*/- if (bus) {
struct pci_bus *child;list_for_each_entry(child, &bus->children, node)pcie_bus_configure_settings(child);- }
- return bus;
}
#endif
1.9.1
On Wed, Oct 28, 2015 at 11:49:40AM +0000, Liviu.Dudau@arm.com wrote:
On Tue, Oct 27, 2015 at 05:38:42PM +0100, Tomasz Nowicki wrote:
[...]
+static int __init pcibios_assign_resources(void) +{
- if (acpi_disabled)
return 0;- pci_assign_unassigned_resources();
- return 0;
You can change this function into: { if (!acpi_disabled) pci_assign_unassigned_resources();
return 0; }
as the equivalent but shorter form.
I do not think it is a matter of code style here, it is a matter of understanding when and if we want to reassign resources on ACPI systems, it is an open question on ARM64 that must be sorted out (ie we ignore FW/BARs set-up entirely).
+} /*
- raw_pci_read/write - Platform-specific PCI config space access.
- rootfs_initcall comes after subsys_initcall and fs_initcall_sync,
*/
- so we know acpi scan and PCI_FIXUP_FINAL quirks have both run.
-int raw_pci_read(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *val)What happened with raw_pci_{read,write} ? Why do you remove them?
+rootfs_initcall(pcibios_assign_resources);
Would you be so kind and explain to me why you need this initcall? Can you not set the PCI_REASSIGN_ALL_RSRC flag before calling pci_scan_root_bus() ?
On what basis ? BTW, PCI core code does not assign unassigned resources anyway even if that flag is set, so some policy has to be defined here.
Thanks, Lorenzo
I haven't focused on ACPI before so I'm a bit hazy on the init order when that is enabled. That being said, I don't like adding in the architecture code initcall hooks just to fix up some dependency orders that could / should be fixed some other way.
+static void __iomem * +pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(pci_domain_nr(bus), bus->number);
- if (cfg && cfg->virt)
return cfg->virt +(PCI_MMCFG_BUS_OFFSET(bus->number) | (devfn << 12)) +offset;- return NULL;
} -int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
+};
+#ifdef CONFIG_PCI_MMCONFIG +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- struct acpi_pci_root *root;
- int seg, start, end, err;
- root = ci->root;
- seg = root->segment;
- start = root->secondary.start;
- end = root->secondary.end;
- cfg = pci_mmconfig_lookup(seg, start);
- if (cfg)
return 0;- cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
- if (!cfg)
return -ENOMEM;- err = pci_mmconfig_inject(cfg);
- return err;
} -#ifdef CONFIG_ACPI +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) +{
- struct acpi_pci_root *root = ci->root;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
- if (cfg)
return;- if (cfg->hot_added)
pci_mmconfig_delete(root->segment, root->secondary.start,root->secondary.end);+} +#else +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) +{
- return 0;
+}
+static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) { } +#endif
+static int pci_acpi_root_init_info(struct acpi_pci_root_info *ci) +{
- return pci_add_mmconfig_region(ci);
+}
+static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci) +{
- pci_remove_mmconfig_region(ci);
- kfree(ci);
+}
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) +{
- struct resource_entry *entry, *tmp;
- int ret;
- ret = acpi_pci_probe_root_resources(ci);
- if (ret < 0)
return ret;- resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
struct resource *res = entry->res;/** Special handling for ARM IO rangeThere is nothing ARM specific here. It should apply to any memory mapped IO range.
* TODO: need to move pci_register_io_range() function out* of drivers/of/address.c for both used by DT and ACPI*/if (res->flags & IORESOURCE_IO) {unsigned long port;int err;resource_size_t length = res->end - res->start;err = pci_register_io_range(res->start, length);if (err) {resource_list_destroy_entry(entry);continue;}port = pci_address_to_pio(res->start);if (port == (unsigned long)-1) {resource_list_destroy_entry(entry);continue;}res->start = port;res->end = res->start + length - 1;if (pci_remap_iospace(res, res->start) < 0)resource_list_destroy_entry(entry);}- }
- return 0;
+}
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .pci_ops = &pci_root_ops,
- .init_info = pci_acpi_root_init_info,
- .release_info = pci_acpi_root_release_info,
- .prepare_resources = pci_acpi_root_prepare_resources,
+};
/* Root bridge scanning */ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) {
- /* TODO: Should be revisited when implementing PCI on ACPI */
- return NULL;
- int node = acpi_get_node(root->device->handle);
- int domain = root->segment;
- int busnum = root->secondary.start;
- struct acpi_pci_root_info *info;
- struct pci_bus *bus;
- if (domain && !pci_domains_supported) {
pr_warn("PCI %04x:%02x: multiple domains not supported.\n",domain, busnum);return NULL;- }
- info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
- if (!info) {
dev_err(&root->device->dev,"pci_bus %04x:%02x: ignored (out of memory)\n",domain, busnum);return NULL;- }
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, info, root);
- /* After the PCI-E bus has been walked and all devices discovered,
* configure any settings of the fabric that might be necessary.*/- if (bus) {
struct pci_bus *child;list_for_each_entry(child, &bus->children, node)pcie_bus_configure_settings(child);- }
- return bus;
}
#endif
1.9.1
--
| I would like to | | fix the world, | | but they're not | | giving me the | \ source code! /
ĀÆ\_(ć)_/ĀÆ
On Tue, Nov 03, 2015 at 02:32:14PM +0000, Lorenzo Pieralisi wrote:
On Wed, Oct 28, 2015 at 11:49:40AM +0000, Liviu.Dudau@arm.com wrote:
On Tue, Oct 27, 2015 at 05:38:42PM +0100, Tomasz Nowicki wrote:
[...]
+static int __init pcibios_assign_resources(void) +{
- if (acpi_disabled)
return 0;- pci_assign_unassigned_resources();
- return 0;
You can change this function into: { if (!acpi_disabled) pci_assign_unassigned_resources();
return 0; }
as the equivalent but shorter form.
I do not think it is a matter of code style here, it is a matter of understanding when and if we want to reassign resources on ACPI systems, it is an open question on ARM64 that must be sorted out (ie we ignore FW/BARs set-up entirely).
I was reviewing the code here, not doing any technical guidance, I'm leaving that to you as you are more involved around ACPI.
+} /*
- raw_pci_read/write - Platform-specific PCI config space access.
- rootfs_initcall comes after subsys_initcall and fs_initcall_sync,
*/
- so we know acpi scan and PCI_FIXUP_FINAL quirks have both run.
-int raw_pci_read(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *val)What happened with raw_pci_{read,write} ? Why do you remove them?
+rootfs_initcall(pcibios_assign_resources);
Would you be so kind and explain to me why you need this initcall? Can you not set the PCI_REASSIGN_ALL_RSRC flag before calling pci_scan_root_bus() ?
On what basis ? BTW, PCI core code does not assign unassigned resources anyway even if that flag is set, so some policy has to be defined here.
I was thinking that ACPI code can do that if they seem to depend on the resources being assigned during root bus scan. I was not implying that PCI core code enforces that.
Best regards, Liviu
Thanks, Lorenzo
I haven't focused on ACPI before so I'm a bit hazy on the init order when that is enabled. That being said, I don't like adding in the architecture code initcall hooks just to fix up some dependency orders that could / should be fixed some other way.
+static void __iomem * +pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(pci_domain_nr(bus), bus->number);
- if (cfg && cfg->virt)
return cfg->virt +(PCI_MMCFG_BUS_OFFSET(bus->number) | (devfn << 12)) +offset;- return NULL;
} -int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
+};
+#ifdef CONFIG_PCI_MMCONFIG +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- struct acpi_pci_root *root;
- int seg, start, end, err;
- root = ci->root;
- seg = root->segment;
- start = root->secondary.start;
- end = root->secondary.end;
- cfg = pci_mmconfig_lookup(seg, start);
- if (cfg)
return 0;- cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
- if (!cfg)
return -ENOMEM;- err = pci_mmconfig_inject(cfg);
- return err;
} -#ifdef CONFIG_ACPI +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) +{
- struct acpi_pci_root *root = ci->root;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
- if (cfg)
return;- if (cfg->hot_added)
pci_mmconfig_delete(root->segment, root->secondary.start,root->secondary.end);+} +#else +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) +{
- return 0;
+}
+static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) { } +#endif
+static int pci_acpi_root_init_info(struct acpi_pci_root_info *ci) +{
- return pci_add_mmconfig_region(ci);
+}
+static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci) +{
- pci_remove_mmconfig_region(ci);
- kfree(ci);
+}
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) +{
- struct resource_entry *entry, *tmp;
- int ret;
- ret = acpi_pci_probe_root_resources(ci);
- if (ret < 0)
return ret;- resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
struct resource *res = entry->res;/** Special handling for ARM IO rangeThere is nothing ARM specific here. It should apply to any memory mapped IO range.
* TODO: need to move pci_register_io_range() function out* of drivers/of/address.c for both used by DT and ACPI*/if (res->flags & IORESOURCE_IO) {unsigned long port;int err;resource_size_t length = res->end - res->start;err = pci_register_io_range(res->start, length);if (err) {resource_list_destroy_entry(entry);continue;}port = pci_address_to_pio(res->start);if (port == (unsigned long)-1) {resource_list_destroy_entry(entry);continue;}res->start = port;res->end = res->start + length - 1;if (pci_remap_iospace(res, res->start) < 0)resource_list_destroy_entry(entry);}- }
- return 0;
+}
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .pci_ops = &pci_root_ops,
- .init_info = pci_acpi_root_init_info,
- .release_info = pci_acpi_root_release_info,
- .prepare_resources = pci_acpi_root_prepare_resources,
+};
/* Root bridge scanning */ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) {
- /* TODO: Should be revisited when implementing PCI on ACPI */
- return NULL;
- int node = acpi_get_node(root->device->handle);
- int domain = root->segment;
- int busnum = root->secondary.start;
- struct acpi_pci_root_info *info;
- struct pci_bus *bus;
- if (domain && !pci_domains_supported) {
pr_warn("PCI %04x:%02x: multiple domains not supported.\n",domain, busnum);return NULL;- }
- info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
- if (!info) {
dev_err(&root->device->dev,"pci_bus %04x:%02x: ignored (out of memory)\n",domain, busnum);return NULL;- }
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, info, root);
- /* After the PCI-E bus has been walked and all devices discovered,
* configure any settings of the fabric that might be necessary.*/- if (bus) {
struct pci_bus *child;list_for_each_entry(child, &bus->children, node)pcie_bus_configure_settings(child);- }
- return bus;
}
#endif
1.9.1
--
| I would like to | | fix the world, | | but they're not | | giving me the | \ source code! /
ĀÆ\_(ć)_/ĀÆ-- 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 10/27/2015 12:38 PM, Tomasz Nowicki wrote:
Because of two patch series:
- Jiang Liu's common interface to support PCI host bridge init
- Refactoring of MMCONFIG, part of this patch set
now we can think about PCI buses enumeration for ARM64 and ACPI tables.
This patch introduce ACPI based PCI hostbridge init calls which use information from MCFG table (PCI config space regions) and _CRS (IO/irq resources) to initialize PCI hostbridge.
Signed-off-by: Tomasz Nowicki tn@semihalf.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com CC: Arnd Bergmann arnd@arndb.de CC: Catalin Marinas catalin.marinas@arm.com CC: Liviu Dudau Liviu.Dudau@arm.com CC: Lorenzo Pieralisi Lorenzo.Pieralisi@arm.com CC: Will Deacon will.deacon@arm.com
arch/arm64/Kconfig | 6 ++ arch/arm64/kernel/pci.c | 208 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 202 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 07d1811..bbcc6b1 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -89,6 +89,7 @@ config ARM64 select SPARSE_IRQ select SYSCTL_EXCEPTION_TRACE select HAVE_CONTEXT_TRACKING
- select HAVE_PCI_ECAM help ARM 64-bit (AArch64) Linux support.
@@ -202,6 +203,11 @@ source "drivers/pci/Kconfig" source "drivers/pci/pcie/Kconfig" source "drivers/pci/hotplug/Kconfig"
+config PCI_MMCONFIG
def_bool y
select PCI_ECAM
depends on ACPI
endmenu
menu "Kernel Features"
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index b3d098b..66cc1ae 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -11,12 +11,15 @@ */
#include <linux/acpi.h> +#include <linux/ecam.h> #include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/mm.h> +#include <linux/of_address.h> #include <linux/of_pci.h> #include <linux/of_platform.h> +#include <linux/pci-acpi.h> #include <linux/slab.h>
#include <asm/pci-bridge.h> @@ -52,35 +55,216 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) }
/*
- Try to assign the IRQ number from DT when adding a new device
*/ int pcibios_add_device(struct pci_dev *dev) {
- Try to assign the IRQ number from DT/ACPI when adding a new device
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
- if (acpi_disabled)
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);+#ifdef CONFIG_ACPI
- else
acpi_pci_irq_enable(dev);+#endif
return 0; }
+#ifdef CONFIG_ACPI +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- struct acpi_pci_root *root = bridge->bus->sysdata;
- ACPI_COMPANION_SET(&bridge->dev, root->device);
- return 0;
+}
+void pcibios_add_bus(struct pci_bus *bus) +{
- acpi_pci_add_bus(bus);
+}
+void pcibios_remove_bus(struct pci_bus *bus) +{
- acpi_pci_remove_bus(bus);
+}
+static int __init pcibios_assign_resources(void) +{
- if (acpi_disabled)
return 0;- pci_assign_unassigned_resources();
- return 0;
+} /*
- raw_pci_read/write - Platform-specific PCI config space access.
- rootfs_initcall comes after subsys_initcall and fs_initcall_sync,
*/
- so we know acpi scan and PCI_FIXUP_FINAL quirks have both run.
-int raw_pci_read(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *val)+rootfs_initcall(pcibios_assign_resources);
+static void __iomem * +pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(pci_domain_nr(bus), bus->number);
- if (cfg && cfg->virt)
return cfg->virt +(PCI_MMCFG_BUS_OFFSET(bus->number) | (devfn << 12)) +offset;- return NULL; }
-int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
Can you change these with pci_generic_config_read32 and pci_generic_config_write32? We have some targets that can only do 32 bits PCI config space access.
+};
+#ifdef CONFIG_PCI_MMCONFIG +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- struct acpi_pci_root *root;
- int seg, start, end, err;
- root = ci->root;
- seg = root->segment;
- start = root->secondary.start;
- end = root->secondary.end;
- cfg = pci_mmconfig_lookup(seg, start);
- if (cfg)
return 0;- cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
- if (!cfg)
return -ENOMEM;- err = pci_mmconfig_inject(cfg);
- return err; }
-#ifdef CONFIG_ACPI +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) +{
- struct acpi_pci_root *root = ci->root;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
- if (cfg)
return;- if (cfg->hot_added)
pci_mmconfig_delete(root->segment, root->secondary.start,root->secondary.end);+} +#else +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) +{
- return 0;
+}
+static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) { } +#endif
+static int pci_acpi_root_init_info(struct acpi_pci_root_info *ci) +{
- return pci_add_mmconfig_region(ci);
+}
+static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci) +{
- pci_remove_mmconfig_region(ci);
- kfree(ci);
+}
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) +{
- struct resource_entry *entry, *tmp;
- int ret;
- ret = acpi_pci_probe_root_resources(ci);
- if (ret < 0)
return ret;- resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
struct resource *res = entry->res;/** Special handling for ARM IO range* TODO: need to move pci_register_io_range() function out* of drivers/of/address.c for both used by DT and ACPI*/if (res->flags & IORESOURCE_IO) {unsigned long port;int err;resource_size_t length = res->end - res->start;err = pci_register_io_range(res->start, length);if (err) {resource_list_destroy_entry(entry);continue;}port = pci_address_to_pio(res->start);if (port == (unsigned long)-1) {resource_list_destroy_entry(entry);continue;}res->start = port;res->end = res->start + length - 1;if (pci_remap_iospace(res, res->start) < 0)resource_list_destroy_entry(entry);}- }
- return 0;
+}
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .pci_ops = &pci_root_ops,
- .init_info = pci_acpi_root_init_info,
- .release_info = pci_acpi_root_release_info,
- .prepare_resources = pci_acpi_root_prepare_resources,
+};
- /* Root bridge scanning */ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) {
- /* TODO: Should be revisited when implementing PCI on ACPI */
- return NULL;
- int node = acpi_get_node(root->device->handle);
- int domain = root->segment;
- int busnum = root->secondary.start;
- struct acpi_pci_root_info *info;
- struct pci_bus *bus;
- if (domain && !pci_domains_supported) {
pr_warn("PCI %04x:%02x: multiple domains not supported.\n",domain, busnum);return NULL;- }
- info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
- if (!info) {
dev_err(&root->device->dev,"pci_bus %04x:%02x: ignored (out of memory)\n",domain, busnum);return NULL;- }
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, info, root);
- /* After the PCI-E bus has been walked and all devices discovered,
* configure any settings of the fabric that might be necessary.*/- if (bus) {
struct pci_bus *child;list_for_each_entry(child, &bus->children, node)pcie_bus_configure_settings(child);- }
- return bus; } #endif
On 10/28/2015 2:46 PM, Sinan Kaya wrote:
On 10/27/2015 12:38 PM, Tomasz Nowicki wrote:
Because of two patch series:
- Jiang Liu's common interface to support PCI host bridge init
- Refactoring of MMCONFIG, part of this patch set
now we can think about PCI buses enumeration for ARM64 and ACPI tables.
This patch introduce ACPI based PCI hostbridge init calls which use information from MCFG table (PCI config space regions) and _CRS (IO/irq resources) to initialize PCI hostbridge.
Signed-off-by: Tomasz Nowicki tn@semihalf.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com CC: Arnd Bergmann arnd@arndb.de CC: Catalin Marinas catalin.marinas@arm.com CC: Liviu Dudau Liviu.Dudau@arm.com CC: Lorenzo Pieralisi Lorenzo.Pieralisi@arm.com CC: Will Deacon will.deacon@arm.com
arch/arm64/Kconfig | 6 ++ arch/arm64/kernel/pci.c | 208 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 202 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 07d1811..bbcc6b1 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -89,6 +89,7 @@ config ARM64 select SPARSE_IRQ select SYSCTL_EXCEPTION_TRACE select HAVE_CONTEXT_TRACKING
- select HAVE_PCI_ECAM help ARM 64-bit (AArch64) Linux support.
@@ -202,6 +203,11 @@ source "drivers/pci/Kconfig" source "drivers/pci/pcie/Kconfig" source "drivers/pci/hotplug/Kconfig"
+config PCI_MMCONFIG
def_bool y
select PCI_ECAM
depends on ACPI
endmenu
menu "Kernel Features"
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index b3d098b..66cc1ae 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -11,12 +11,15 @@ */
#include <linux/acpi.h> +#include <linux/ecam.h> #include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/mm.h> +#include <linux/of_address.h> #include <linux/of_pci.h> #include <linux/of_platform.h> +#include <linux/pci-acpi.h> #include <linux/slab.h>
#include <asm/pci-bridge.h> @@ -52,35 +55,216 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) }
/*
- Try to assign the IRQ number from DT when adding a new device
*/ int pcibios_add_device(struct pci_dev *dev) {
- Try to assign the IRQ number from DT/ACPI when adding a new device
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
- if (acpi_disabled)
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);+#ifdef CONFIG_ACPI
- else
acpi_pci_irq_enable(dev);+#endif
return 0;}
+#ifdef CONFIG_ACPI +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- struct acpi_pci_root *root = bridge->bus->sysdata;
- ACPI_COMPANION_SET(&bridge->dev, root->device);
- return 0;
+}
+void pcibios_add_bus(struct pci_bus *bus) +{
- acpi_pci_add_bus(bus);
+}
+void pcibios_remove_bus(struct pci_bus *bus) +{
- acpi_pci_remove_bus(bus);
+}
+static int __init pcibios_assign_resources(void) +{
- if (acpi_disabled)
return 0;- pci_assign_unassigned_resources();
- return 0;
+} /*
- raw_pci_read/write - Platform-specific PCI config space access.
- rootfs_initcall comes after subsys_initcall and fs_initcall_sync,
*/
- so we know acpi scan and PCI_FIXUP_FINAL quirks have both run.
-int raw_pci_read(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *val)+rootfs_initcall(pcibios_assign_resources);
+static void __iomem * +pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(pci_domain_nr(bus), bus->number);
- if (cfg && cfg->virt)
return cfg->virt +(PCI_MMCFG_BUS_OFFSET(bus->number) | (devfn << 12)) +offset;- return NULL; }
-int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
Can you change these with pci_generic_config_read32 and pci_generic_config_write32? We have some targets that can only do 32 bits PCI config space access.
+};
+#ifdef CONFIG_PCI_MMCONFIG +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- struct acpi_pci_root *root;
- int seg, start, end, err;
- root = ci->root;
- seg = root->segment;
- start = root->secondary.start;
- end = root->secondary.end;
- cfg = pci_mmconfig_lookup(seg, start);
- if (cfg)
return 0;- cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
- if (!cfg)
return -ENOMEM;- err = pci_mmconfig_inject(cfg);
- return err; }
-#ifdef CONFIG_ACPI +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) +{
- struct acpi_pci_root *root = ci->root;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
- if (cfg)
return;- if (cfg->hot_added)
pci_mmconfig_delete(root->segment, root->secondary.start,root->secondary.end);+} +#else +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) +{
- return 0;
+}
+static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) { } +#endif
+static int pci_acpi_root_init_info(struct acpi_pci_root_info *ci) +{
- return pci_add_mmconfig_region(ci);
+}
+static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci) +{
- pci_remove_mmconfig_region(ci);
- kfree(ci);
+}
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) +{
- struct resource_entry *entry, *tmp;
- int ret;
- ret = acpi_pci_probe_root_resources(ci);
- if (ret < 0)
return ret;- resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
struct resource *res = entry->res;/** Special handling for ARM IO range* TODO: need to move pci_register_io_range() function out* of drivers/of/address.c for both used by DT and ACPI*/if (res->flags & IORESOURCE_IO) {unsigned long port;int err;resource_size_t length = res->end - res->start;err = pci_register_io_range(res->start, length);if (err) {resource_list_destroy_entry(entry);continue;}port = pci_address_to_pio(res->start);if (port == (unsigned long)-1) {resource_list_destroy_entry(entry);continue;}res->start = port;res->end = res->start + length - 1;if (pci_remap_iospace(res, res->start) < 0)resource_list_destroy_entry(entry);}- }
- return 0;
+}
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .pci_ops = &pci_root_ops,
- .init_info = pci_acpi_root_init_info,
- .release_info = pci_acpi_root_release_info,
- .prepare_resources = pci_acpi_root_prepare_resources,
+};
- /* Root bridge scanning */ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) {
- /* TODO: Should be revisited when implementing PCI on ACPI */
- return NULL;
- int node = acpi_get_node(root->device->handle);
- int domain = root->segment;
- int busnum = root->secondary.start;
- struct acpi_pci_root_info *info;
- struct pci_bus *bus;
- if (domain && !pci_domains_supported) {
pr_warn("PCI %04x:%02x: multiple domains not supported.\n",domain, busnum);return NULL;- }
- info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
- if (!info) {
dev_err(&root->device->dev,"pci_bus %04x:%02x: ignored (out of memory)\n",domain, busnum);return NULL;- }
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, info, root);
- /* After the PCI-E bus has been walked and all devices discovered,
* configure any settings of the fabric that might be necessary.*/- if (bus) {
struct pci_bus *child;list_for_each_entry(child, &bus->children, node)pcie_bus_configure_settings(child);- }
- return bus; } #endif
Tomasz, I was asked to test your new patchset on our platform. With the new patchset, I'm seeing two problems.
1. ACPI code is unable to discover the interrupt numbers when objects are ordered as follows in the ACPI file
PNP0A08 object PNP0C0F INTA object PNP0C0F INTB object PNP0C0F INTC object PNP0C0F INTD object
This gives me invalid link context error.
pci 0000:00:00.0: PCI INT A: no GSI pci 0000:01:00.0: Derived GSI for 0000:01:00.0 INT A from 0000:00:00.0 acpi PNP0C0F:00: Invalid link context
If I order it like this in the ACPI file,
PNP0C0F INTA object PNP0C0F INTB object PNP0C0F INTC object PNP0C0F INTD object PNP0A08 object
then, the legacy interrupt numbers can be discovered properly.
2. The second problem is about the PCIe resources.
pci_0000:01:00.0:_can't_enable_device:_BAR_0_[mem_0x80100100000-0x80100101fff_64bit]_not_claimed pci 0000:01:00.0: Can't enable PCI device, BIOS handoff failed. pci 0000:00:00.0: BAR 14: assigned [mem 0x80100100000-0x801003fffff] pci 0000:00:00.0: BAR 13: no space for [io size 0x1000] pci 0000:00:00.0: BAR 13: failed to assign [io size 0x1000] pci 0000:00:00.0: BAR 13: no space for [io size 0x1000] pci 0000:00:00.0: BAR 13: failed to assign [io size 0x1000] pci 0000:01:00.0: BAR 0: assigned [mem 0x80100100000-0x80100101fff 64bit]
For some reason, the kernel is unable to assign resources.
I appreciate any pointers you might give.
On 28.10.2015 21:36, Sinan Kaya wrote:
- ACPI code is unable to discover the interrupt numbers when objects
are ordered as follows in the ACPI file
PNP0A08 object PNP0C0F INTA object PNP0C0F INTB object PNP0C0F INTC object PNP0C0F INTD object
This gives me invalid link context error.
pci 0000:00:00.0: PCI INT A: no GSI pci 0000:01:00.0: Derived GSI for 0000:01:00.0 INT A from 0000:00:00.0 acpi PNP0C0F:00: Invalid link context
If I order it like this in the ACPI file,
PNP0C0F INTA object PNP0C0F INTB object PNP0C0F INTC object PNP0C0F INTD object PNP0A08 object
then, the legacy interrupt numbers can be discovered properly.
Can you show full content of your PNP0C0F and PNP0A08 objects?
Regards, Tomasz
On 10/29/2015 7:38 AM, Tomasz Nowicki wrote:
On 28.10.2015 21:36, Sinan Kaya wrote:
- ACPI code is unable to discover the interrupt numbers when objects
are ordered as follows in the ACPI file
PNP0A08 object PNP0C0F INTA object PNP0C0F INTB object PNP0C0F INTC object PNP0C0F INTD object
This gives me invalid link context error.
pci 0000:00:00.0: PCI INT A: no GSI pci 0000:01:00.0: Derived GSI for 0000:01:00.0 INT A from 0000:00:00.0 acpi PNP0C0F:00: Invalid link context
If I order it like this in the ACPI file,
PNP0C0F INTA object PNP0C0F INTB object PNP0C0F INTC object PNP0C0F INTD object PNP0A08 object
then, the legacy interrupt numbers can be discovered properly.
Can you show full content of your PNP0C0F and PNP0A08 objects?
ACPI table is considered proprietary. I don't think I can get the legal approval in time. I can give you pieces though.
Here is the _PRT Device (PCI0) { // PCIe port 0 Name(_HID, EISAID("PNP0A08")) // PCI express Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge { .... Name(_PRT, Package(){ Package(){0x0FFFF, 0, _SB.LN0A, 0}, // Slot 0, INTA Package(){0x0FFFF, 1, _SB.LN0B, 0}, // Slot 0, INTB Package(){0x0FFFF, 2, _SB.LN0C, 0}, // Slot 0, INTC Package(){0x0FFFF, 3, _SB.LN0D, 0} // Slot 0, INTD }) }
Here is the PNP0C0F
Device(LN0A){ Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link Name(_UID, 1) Name(_PRS, ResourceTemplate(){ Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive, , ,) {0xE8} }) Method(_DIS) {} Method(_CRS) { Return (_PRS) } Method(_SRS, 1) {} }
Regards, 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 29.10.2015 16:01, Sinan Kaya wrote:
On 10/29/2015 7:38 AM, Tomasz Nowicki wrote:
On 28.10.2015 21:36, Sinan Kaya wrote:
- ACPI code is unable to discover the interrupt numbers when objects
are ordered as follows in the ACPI file
PNP0A08 object PNP0C0F INTA object PNP0C0F INTB object PNP0C0F INTC object PNP0C0F INTD object
This gives me invalid link context error.
pci 0000:00:00.0: PCI INT A: no GSI pci 0000:01:00.0: Derived GSI for 0000:01:00.0 INT A from 0000:00:00.0 acpi PNP0C0F:00: Invalid link context
If I order it like this in the ACPI file,
PNP0C0F INTA object PNP0C0F INTB object PNP0C0F INTC object PNP0C0F INTD object PNP0A08 object
then, the legacy interrupt numbers can be discovered properly.
Can you show full content of your PNP0C0F and PNP0A08 objects?
ACPI table is considered proprietary. I don't think I can get the legal approval in time. I can give you pieces though.
Here is the _PRT Device (PCI0) { // PCIe port 0 Name(_HID, EISAID("PNP0A08")) // PCI express Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge { .... Name(_PRT, Package(){ Package(){0x0FFFF, 0, _SB.LN0A, 0}, // Slot 0, INTA Package(){0x0FFFF, 1, _SB.LN0B, 0}, // Slot 0, INTB Package(){0x0FFFF, 2, _SB.LN0C, 0}, // Slot 0, INTC Package(){0x0FFFF, 3, _SB.LN0D, 0} // Slot 0, INTD }) }
Here is the PNP0C0F
Device(LN0A){ Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link Name(_UID, 1) Name(_PRS, ResourceTemplate(){ Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive, , ,) {0xE8} }) Method(_DIS) {} Method(_CRS) { Return (_PRS) } Method(_SRS, 1) {} }
Can you please apply patch below:
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index fec1c91..fe34415 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -48,10 +48,19 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, */ int pcibios_enable_device(struct pci_dev *dev, int mask) { + int ret; + if (pci_has_flag(PCI_PROBE_ONLY)) return 0;
- return pci_enable_resources(dev, mask); + ret = pci_enable_resources(dev, mask); + if (ret < 0) + return ret; +#ifdef CONFIG_ACPI + if (!dev->msi_enabled) + return acpi_pci_irq_enable(dev); +#endif + return 0; }
/* @@ -61,10 +70,6 @@ int pcibios_add_device(struct pci_dev *dev) { if (acpi_disabled) dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); -#ifdef CONFIG_ACPI - else - acpi_pci_irq_enable(dev); -#endif
return 0; }
and let me know if the order still matter?
Regards, Tomasz
On 10/29/2015 11:53 AM, Tomasz Nowicki wrote:
On 29.10.2015 16:01, Sinan Kaya wrote:
On 10/29/2015 7:38 AM, Tomasz Nowicki wrote:
On 28.10.2015 21:36, Sinan Kaya wrote:
- ACPI code is unable to discover the interrupt numbers when objects
are ordered as follows in the ACPI file
PNP0A08 object PNP0C0F INTA object PNP0C0F INTB object PNP0C0F INTC object PNP0C0F INTD object
This gives me invalid link context error.
pci 0000:00:00.0: PCI INT A: no GSI pci 0000:01:00.0: Derived GSI for 0000:01:00.0 INT A from 0000:00:00.0 acpi PNP0C0F:00: Invalid link context
If I order it like this in the ACPI file,
PNP0C0F INTA object PNP0C0F INTB object PNP0C0F INTC object PNP0C0F INTD object PNP0A08 object
then, the legacy interrupt numbers can be discovered properly.
Can you show full content of your PNP0C0F and PNP0A08 objects?
ACPI table is considered proprietary. I don't think I can get the legal approval in time. I can give you pieces though.
Here is the _PRT Device (PCI0) { // PCIe port 0 Name(_HID, EISAID("PNP0A08")) // PCI express Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge { .... Name(_PRT, Package(){ Package(){0x0FFFF, 0, _SB.LN0A, 0}, // Slot 0, INTA Package(){0x0FFFF, 1, _SB.LN0B, 0}, // Slot 0, INTB Package(){0x0FFFF, 2, _SB.LN0C, 0}, // Slot 0, INTC Package(){0x0FFFF, 3, _SB.LN0D, 0} // Slot 0, INTD }) }
Here is the PNP0C0F
Device(LN0A){ Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link Name(_UID, 1) Name(_PRS, ResourceTemplate(){ Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive, , ,) {0xE8} }) Method(_DIS) {} Method(_CRS) { Return (_PRS) } Method(_SRS, 1) {} }
Can you please apply patch below:
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index fec1c91..fe34415 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -48,10 +48,19 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, */ int pcibios_enable_device(struct pci_dev *dev, int mask) {
int ret;if (pci_has_flag(PCI_PROBE_ONLY)) return 0;
return pci_enable_resources(dev, mask);
ret = pci_enable_resources(dev, mask);if (ret < 0)return ret;+#ifdef CONFIG_ACPI
if (!dev->msi_enabled)return acpi_pci_irq_enable(dev);+#endif
return 0;}
/*
@@ -61,10 +70,6 @@ int pcibios_add_device(struct pci_dev *dev) { if (acpi_disabled) dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); -#ifdef CONFIG_ACPI
elseacpi_pci_irq_enable(dev);-#endif
return 0;}
and let me know if the order still matter?
Regards, 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
Thanks, This seems to have fixed the ACPI table order problem.
On 10/28/2015 4:36 PM, Sinan Kaya wrote:
- The second problem is about the PCIe resources.
pci_0000:01:00.0:_can't_enable_device:_BAR_0_[mem_0x80100100000-0x80100101fff_64bit]_not_claimed
pci 0000:01:00.0: Can't enable PCI device, BIOS handoff failed. pci 0000:00:00.0: BAR 14: assigned [mem 0x80100100000-0x801003fffff] pci 0000:00:00.0: BAR 13: no space for [io size 0x1000] pci 0000:00:00.0: BAR 13: failed to assign [io size 0x1000] pci 0000:00:00.0: BAR 13: no space for [io size 0x1000] pci 0000:00:00.0: BAR 13: failed to assign [io size 0x1000] pci 0000:01:00.0: BAR 0: assigned [mem 0x80100100000-0x80100101fff 64bit]
For some reason, the kernel is unable to assign resources.
I appreciate any pointers you might give.
Tomasz, I debugged this part of the problem yesterday night.
I have one set up with 4.2 kernel and Mark Salter's original ACPI PCIE patch. His patchwork works fine with our ACPI table.
This is what IO resource looks like in our table. PCI IO is supported using the ACPI translation attribute on ARM systems.
QWORDIO( // produced resource ResourceProducer, // bit 0 of general flags is 0 MinFixed, // Range is fixed MaxFixed, // Range is fixed PosDecode, EntireRange, 0x0000, // Granularity 0x1000, // Min, 0xFFFF, // Max 0x8FFFFFEF000, // Translation 0xF000, // Range Length ,, PI00 )
It looks like you are missing the translation support for the IO aperture and Mark Salter's patch has this support.
Yours is missing in pci_acpi_root_prepare_resources function here. Mark's patch uses the offset argument to reorganize the resource.
+ resource_size_t length = res->end - res->start; + + err = pci_register_io_range(res->start, length); + if (err) { + resource_list_destroy_entry(entry); + continue; + } + + port = pci_address_to_pio(res->start); + if (port == (unsigned long)-1) { + resource_list_destroy_entry(entry); + continue; + } + + res->start = port; + res->end = res->start + length - 1; + + if (pci_remap_iospace(res, res->start) < 0) + resource_list_destroy_entry(entry);
pci_0000:01:00.0:_can't_enable_device:_BAR_0_[mem_0x80100100000-0x80100101fff_64bit]_not_claimed
pci 0000:01:00.0: Can't enable PCI device, BIOS handoff failed.
I also found out this problem. This is a resource reclaim order problem with respect to quirks. The card I'm testing is a PCIe USB card. It has a quirk associated with it and it tries to enable and then disable the device here.
DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_SERIAL_USB, 8, quirk_usb_early_handoff);
The resources get assigned after this quirk. That's why I see these can't enable messages. I looked at 4.2 kernel. The execution order is reversed.
pci 0000:00:00.0: BAR 14: assigned [mem 0x80100100000-0x801003fffff] pci 0000:00:00.0: BAR 13: no space for [io size 0x1000] pci 0000:00:00.0: BAR 13: failed to assign [io size 0x1000] pci 0000:00:00.0: BAR 13: no space for [io size 0x1000] pci 0000:00:00.0: BAR 13: failed to assign [io size 0x1000] pci 0000:01:00.0: BAR 0: assigned [mem 0x80100100000-0x80100101fff 64bit]
On 29.10.2015 15:57, Sinan Kaya wrote:
On 10/28/2015 4:36 PM, Sinan Kaya wrote:
- The second problem is about the PCIe resources.
pci_0000:01:00.0:_can't_enable_device:_BAR_0_[mem_0x80100100000-0x80100101fff_64bit]_not_claimed
pci 0000:01:00.0: Can't enable PCI device, BIOS handoff failed. pci 0000:00:00.0: BAR 14: assigned [mem 0x80100100000-0x801003fffff] pci 0000:00:00.0: BAR 13: no space for [io size 0x1000] pci 0000:00:00.0: BAR 13: failed to assign [io size 0x1000] pci 0000:00:00.0: BAR 13: no space for [io size 0x1000] pci 0000:00:00.0: BAR 13: failed to assign [io size 0x1000] pci 0000:01:00.0: BAR 0: assigned [mem 0x80100100000-0x80100101fff 64bit]
For some reason, the kernel is unable to assign resources.
I appreciate any pointers you might give.
Tomasz, I debugged this part of the problem yesterday night.
I have one set up with 4.2 kernel and Mark Salter's original ACPI PCIE patch. His patchwork works fine with our ACPI table.
This is what IO resource looks like in our table. PCI IO is supported using the ACPI translation attribute on ARM systems.
QWORDIO( // produced resource ResourceProducer, // bit 0 of general flags is 0 MinFixed, // Range is fixed MaxFixed, // Range is fixed PosDecode, EntireRange, 0x0000, // Granularity 0x1000, // Min, 0xFFFF, // Max 0x8FFFFFEF000, // Translation 0xF000, // Range Length ,, PI00 )
It looks like you are missing the translation support for the IO aperture and Mark Salter's patch has this support.
Yours is missing in pci_acpi_root_prepare_resources function here. Mark's patch uses the offset argument to reorganize the resource.
resource_size_t length = res->end - res->start;err = pci_register_io_range(res->start, length);if (err) {resource_list_destroy_entry(entry);continue;}port = pci_address_to_pio(res->start);if (port == (unsigned long)-1) {resource_list_destroy_entry(entry);continue;}res->start = port;res->end = res->start + length - 1;if (pci_remap_iospace(res, res->start) < 0)resource_list_destroy_entry(entry);
Thanks for the heads up, working on it.
Regards, Tomasz
On Wed, Oct 28, 2015 at 02:46:37PM -0400, Sinan Kaya wrote:
[...]
-int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
Can you change these with pci_generic_config_read32 and pci_generic_config_write32? We have some targets that can only do 32 bits PCI config space access.
No.
http://www.spinics.net/lists/linux-pci/msg44869.html
Can you be a bit more specific please ?
Sigh. Looks like we have to start adding platform specific quirks even before we merged the generic ACPI PCIe host controller implementation.
Lorenzo
+};
+#ifdef CONFIG_PCI_MMCONFIG +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- struct acpi_pci_root *root;
- int seg, start, end, err;
- root = ci->root;
- seg = root->segment;
- start = root->secondary.start;
- end = root->secondary.end;
- cfg = pci_mmconfig_lookup(seg, start);
- if (cfg)
return 0;- cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
- if (!cfg)
return -ENOMEM;- err = pci_mmconfig_inject(cfg);
- return err;
}
-#ifdef CONFIG_ACPI +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) +{
- struct acpi_pci_root *root = ci->root;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
- if (cfg)
return;- if (cfg->hot_added)
pci_mmconfig_delete(root->segment, root->secondary.start,root->secondary.end);+} +#else +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) +{
- return 0;
+}
+static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) { } +#endif
+static int pci_acpi_root_init_info(struct acpi_pci_root_info *ci) +{
- return pci_add_mmconfig_region(ci);
+}
+static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci) +{
- pci_remove_mmconfig_region(ci);
- kfree(ci);
+}
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) +{
- struct resource_entry *entry, *tmp;
- int ret;
- ret = acpi_pci_probe_root_resources(ci);
- if (ret < 0)
return ret;- resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
struct resource *res = entry->res;/** Special handling for ARM IO range* TODO: need to move pci_register_io_range() function out* of drivers/of/address.c for both used by DT and ACPI*/if (res->flags & IORESOURCE_IO) {unsigned long port;int err;resource_size_t length = res->end - res->start;err = pci_register_io_range(res->start, length);if (err) {resource_list_destroy_entry(entry);continue;}port = pci_address_to_pio(res->start);if (port == (unsigned long)-1) {resource_list_destroy_entry(entry);continue;}res->start = port;res->end = res->start + length - 1;if (pci_remap_iospace(res, res->start) < 0)resource_list_destroy_entry(entry);}- }
- return 0;
+}
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .pci_ops = &pci_root_ops,
- .init_info = pci_acpi_root_init_info,
- .release_info = pci_acpi_root_release_info,
- .prepare_resources = pci_acpi_root_prepare_resources,
+};
/* Root bridge scanning */ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) {
- /* TODO: Should be revisited when implementing PCI on ACPI */
- return NULL;
- int node = acpi_get_node(root->device->handle);
- int domain = root->segment;
- int busnum = root->secondary.start;
- struct acpi_pci_root_info *info;
- struct pci_bus *bus;
- if (domain && !pci_domains_supported) {
pr_warn("PCI %04x:%02x: multiple domains not supported.\n",domain, busnum);return NULL;- }
- info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
- if (!info) {
dev_err(&root->device->dev,"pci_bus %04x:%02x: ignored (out of memory)\n",domain, busnum);return NULL;- }
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, info, root);
- /* After the PCI-E bus has been walked and all devices discovered,
* configure any settings of the fabric that might be necessary.*/- if (bus) {
struct pci_bus *child;list_for_each_entry(child, &bus->children, node)pcie_bus_configure_settings(child);- }
- return bus;
} #endif
-- Sinan Kaya Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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/
On 03.11.2015 15:15, Lorenzo Pieralisi wrote:
On Wed, Oct 28, 2015 at 02:46:37PM -0400, Sinan Kaya wrote:
[...]
-int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
Can you change these with pci_generic_config_read32 and pci_generic_config_write32? We have some targets that can only do 32 bits PCI config space access.
No.
http://www.spinics.net/lists/linux-pci/msg44869.html
Can you be a bit more specific please ?
Sigh. Looks like we have to start adding platform specific quirks even before we merged the generic ACPI PCIe host controller implementation.
The sad reality... But my next version will be still generic. Once that one appear to be in good shape then we can add quirks.
Regards, Tomasz
On 11/3/2015 9:39 AM, Tomasz Nowicki wrote:
+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
Can you change these with pci_generic_config_read32 and pci_generic_config_write32? We have some targets that can only do 32 bits PCI config space access.
No.
http://www.spinics.net/lists/linux-pci/msg44869.html
Can you be a bit more specific please ?
Sigh. Looks like we have to start adding platform specific quirks even before we merged the generic ACPI PCIe host controller implementation.
The sad reality... But my next version will be still generic. Once that one appear to be in good shape then we can add quirks.
Thanks.
I don't see anywhere in the SBSA spec addendum that the PCI configuration space section that unaligned accesses *MUST* be supported.
If this is required, please have this info added to the spec. I can work with the designers for the next chip.
Unaligned access on the current hardware returns incomplete values or can cause bus faults. The behavior is undefined.
On Tuesday 03 November 2015 10:10:21 Sinan Kaya wrote:
I don't see anywhere in the SBSA spec addendum that the PCI configuration space section that unaligned accesses *MUST* be supported.
If this is required, please have this info added to the spec. I can work with the designers for the next chip.
Unaligned access on the current hardware returns incomplete values or can cause bus faults. The behavior is undefined.
Unaligned accesses are not allowed, but any PCI compliant device must support aligned 1, 2 or 4 byte accesses on its configuration space, though the byte-enable mechanism. In an ECAM host bridge, those are mapped to load/store accesses from the CPU with the respective width and natural alignment.
Arnd
On 11/3/2015 10:59 AM, Arnd Bergmann wrote:
On Tuesday 03 November 2015 10:10:21 Sinan Kaya wrote:
I don't see anywhere in the SBSA spec addendum that the PCI configuration space section that unaligned accesses *MUST* be supported.
If this is required, please have this info added to the spec. I can work with the designers for the next chip.
Unaligned access on the current hardware returns incomplete values or can cause bus faults. The behavior is undefined.
Unaligned accesses are not allowed, but any PCI compliant device must support aligned 1, 2 or 4 byte accesses on its configuration space, though the byte-enable mechanism. In an ECAM host bridge, those are mapped to load/store accesses from the CPU with the respective width and natural alignment.
Arnd
As far as I see, the endpoints do not have any problems with unaligned accesses. It is the host bridge itself (stuff that doesn't get on the PCIe bus and uses traditional AXI kind bus internally) has problems with alignment.
If Linux is expecting all HW vendors to implement alignment support, this needs to be put in the SBSA spec as a hard requirement.
On Tuesday 03 November 2015 11:33:18 Sinan Kaya wrote:
On 11/3/2015 10:59 AM, Arnd Bergmann wrote:
On Tuesday 03 November 2015 10:10:21 Sinan Kaya wrote:
I don't see anywhere in the SBSA spec addendum that the PCI configuration space section that unaligned accesses *MUST* be supported.
If this is required, please have this info added to the spec. I can work with the designers for the next chip.
Unaligned access on the current hardware returns incomplete values or can cause bus faults. The behavior is undefined.
Unaligned accesses are not allowed, but any PCI compliant device must support aligned 1, 2 or 4 byte accesses on its configuration space, though the byte-enable mechanism. In an ECAM host bridge, those are mapped to load/store accesses from the CPU with the respective width and natural alignment.
As far as I see, the endpoints do not have any problems with unaligned accesses. It is the host bridge itself (stuff that doesn't get on the PCIe bus and uses traditional AXI kind bus internally) has problems with alignment.
If Linux is expecting all HW vendors to implement alignment support, this needs to be put in the SBSA spec as a hard requirement.
As I said, it's not unaligned accesses at all, just 1-byte and aligned 2-byte accesses, and it's not Linux mandating this but the PCI spec. Please read Russell's email again, it is not possible for PCI to work according to the specification unless the host bridge allows sub-32-bit accesses.
You can probably work around this by using the legacy I/O port method rather than ECAM, if the PCI host bridge itself is functional and just the host bus it is connected to is buggy.
Arnd
On 11/3/2015 11:55 AM, Arnd Bergmann wrote:
On Tuesday 03 November 2015 11:33:18 Sinan Kaya wrote:
On 11/3/2015 10:59 AM, Arnd Bergmann wrote:
On Tuesday 03 November 2015 10:10:21 Sinan Kaya wrote:
I don't see anywhere in the SBSA spec addendum that the PCI configuration space section that unaligned accesses *MUST* be supported.
If this is required, please have this info added to the spec. I can work with the designers for the next chip.
Unaligned access on the current hardware returns incomplete values or can cause bus faults. The behavior is undefined.
Unaligned accesses are not allowed, but any PCI compliant device must support aligned 1, 2 or 4 byte accesses on its configuration space, though the byte-enable mechanism. In an ECAM host bridge, those are mapped to load/store accesses from the CPU with the respective width and natural alignment.
As far as I see, the endpoints do not have any problems with unaligned accesses. It is the host bridge itself (stuff that doesn't get on the PCIe bus and uses traditional AXI kind bus internally) has problems with alignment.
If Linux is expecting all HW vendors to implement alignment support, this needs to be put in the SBSA spec as a hard requirement.
As I said, it's not unaligned accesses at all, just 1-byte and aligned 2-byte accesses, and it's not Linux mandating this but the PCI spec. Please read Russell's email again, it is not possible for PCI to work according to the specification unless the host bridge allows sub-32-bit accesses.
I'll check back with the hardware designers. Seeing readb/readw/readl made me nervous that we are trying unaligned access from any boundaries.
In any case, the hardware document says 32 bit configuration space access to the host bridge only. I'll get more clarification.
You can probably work around this by using the legacy I/O port method rather than ECAM, if the PCI host bridge itself is functional and just the host bus it is connected to is buggy.
From the sounds of it, we'll need a quirk for config space. We support legacy I/O only to make the endpoints happy. Some endpoints do not get initialized if they don't have a BAR address assigned to all the BAR resources.
I just saw David Daney's email. I like his idea. I think this chip will fit into the same category.
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 11/3/2015 12:43 PM, Sinan Kaya wrote:
In any case, the hardware document says 32 bit configuration space access to the host bridge only. I'll get more clarification.
I got confirmation this morning that this chip supports 32 bit access to the root complex configuration space. 8/16/32 bits accesses to the endpoints are supported.
You can probably work around this by using the legacy I/O port method rather than ECAM, if the PCI host bridge itself is functional and just the host bus it is connected to is buggy.
From the sounds of it, we'll need a quirk for config space. We support legacy I/O only to make the endpoints happy. Some endpoints do not get initialized if they don't have a BAR address assigned to all the BAR resources.
We'll need an MCFG fix up.
On Tue, Oct 27, 2015 at 05:38:42PM +0100, Tomasz Nowicki wrote:
[...]
menu "Kernel Features" diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index b3d098b..66cc1ae 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -11,12 +11,15 @@ */ #include <linux/acpi.h> +#include <linux/ecam.h> #include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/mm.h> +#include <linux/of_address.h> #include <linux/of_pci.h> #include <linux/of_platform.h> +#include <linux/pci-acpi.h> #include <linux/slab.h> #include <asm/pci-bridge.h> @@ -52,35 +55,216 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) } /*
- Try to assign the IRQ number from DT when adding a new device
*/
- Try to assign the IRQ number from DT/ACPI when adding a new device
int pcibios_add_device(struct pci_dev *dev) {
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
- if (acpi_disabled)
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);+#ifdef CONFIG_ACPI
- else
acpi_pci_irq_enable(dev);+#endif
This series:
http://www.spinics.net/lists/linux-pci/msg45950.html
will allow us to initialize the irq mapping function according to the boot method, code above is getting cumbersome and it is already overriden when booting with DT, so we will remove it altogether.
return 0; } +#ifdef CONFIG_ACPI +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- struct acpi_pci_root *root = bridge->bus->sysdata;
- ACPI_COMPANION_SET(&bridge->dev, root->device);
- return 0;
This should be made part of core code IMO.
+}
+void pcibios_add_bus(struct pci_bus *bus) +{
- acpi_pci_add_bus(bus);
+}
+void pcibios_remove_bus(struct pci_bus *bus) +{
- acpi_pci_remove_bus(bus);
+}
Two functions above are identical for arm64, ia64 and x86, I do not think they belong in arch code.
+static int __init pcibios_assign_resources(void) +{
- if (acpi_disabled)
return 0;- pci_assign_unassigned_resources();
- return 0;
Already commented on this.
+} /*
- raw_pci_read/write - Platform-specific PCI config space access.
- rootfs_initcall comes after subsys_initcall and fs_initcall_sync,
*/
- so we know acpi scan and PCI_FIXUP_FINAL quirks have both run.
-int raw_pci_read(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *val)+rootfs_initcall(pcibios_assign_resources);
+static void __iomem * +pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(pci_domain_nr(bus), bus->number);
- if (cfg && cfg->virt)
return cfg->virt +(PCI_MMCFG_BUS_OFFSET(bus->number) | (devfn << 12)) +offset;- return NULL;
Why is this code arm64 specific ?
} -int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
+};
+#ifdef CONFIG_PCI_MMCONFIG +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- struct acpi_pci_root *root;
- int seg, start, end, err;
- root = ci->root;
- seg = root->segment;
- start = root->secondary.start;
- end = root->secondary.end;
- cfg = pci_mmconfig_lookup(seg, start);
- if (cfg)
return 0;- cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
- if (!cfg)
return -ENOMEM;- err = pci_mmconfig_inject(cfg);
- return err;
} -#ifdef CONFIG_ACPI +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) +{
- struct acpi_pci_root *root = ci->root;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
- if (cfg)
return;- if (cfg->hot_added)
pci_mmconfig_delete(root->segment, root->secondary.start,root->secondary.end);+} +#else +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) +{
- return 0;
+}
+static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) { } +#endif
Ditto.
+static int pci_acpi_root_init_info(struct acpi_pci_root_info *ci) +{
- return pci_add_mmconfig_region(ci);
+}
+static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci) +{
- pci_remove_mmconfig_region(ci);
- kfree(ci);
+}
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) +{
- struct resource_entry *entry, *tmp;
- int ret;
- ret = acpi_pci_probe_root_resources(ci);
- if (ret < 0)
return ret;
Code above is identical on arm64, ia64 and x86.
- resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
struct resource *res = entry->res;/** Special handling for ARM IO range* TODO: need to move pci_register_io_range() function out* of drivers/of/address.c for both used by DT and ACPI*/if (res->flags & IORESOURCE_IO) {unsigned long port;int err;resource_size_t length = res->end - res->start;err = pci_register_io_range(res->start, length);if (err) {resource_list_destroy_entry(entry);continue;}port = pci_address_to_pio(res->start);if (port == (unsigned long)-1) {resource_list_destroy_entry(entry);continue;}res->start = port;res->end = res->start + length - 1;if (pci_remap_iospace(res, res->start) < 0)resource_list_destroy_entry(entry);}- }
- return 0;
+}
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .pci_ops = &pci_root_ops,
- .init_info = pci_acpi_root_init_info,
- .release_info = pci_acpi_root_release_info,
- .prepare_resources = pci_acpi_root_prepare_resources,
+};
/* Root bridge scanning */ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) {
- /* TODO: Should be revisited when implementing PCI on ACPI */
- return NULL;
- int node = acpi_get_node(root->device->handle);
- int domain = root->segment;
- int busnum = root->secondary.start;
- struct acpi_pci_root_info *info;
- struct pci_bus *bus;
- if (domain && !pci_domains_supported) {
pr_warn("PCI %04x:%02x: multiple domains not supported.\n",domain, busnum);return NULL;- }
- info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
- if (!info) {
dev_err(&root->device->dev,"pci_bus %04x:%02x: ignored (out of memory)\n",domain, busnum);return NULL;- }
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, info, root);
- /* After the PCI-E bus has been walked and all devices discovered,
* configure any settings of the fabric that might be necessary.*/- if (bus) {
struct pci_bus *child;list_for_each_entry(child, &bus->children, node)pcie_bus_configure_settings(child);- }
- return bus;
Code above is entirely arch agnostic (apart from what data is passed to sysdata) and I think there is room for further consolidation with x86 and ia64, I will have a look into this.
Thanks, Lorenzo
On 11/3/2015 11:55 AM, Lorenzo Pieralisi wrote:
/*
- Try to assign the IRQ number from DT when adding a new device
*/
- Try to assign the IRQ number from DT/ACPI when adding a new device
int pcibios_add_device(struct pci_dev *dev) {
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
- if (acpi_disabled)
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);+#ifdef CONFIG_ACPI
- else
acpi_pci_irq_enable(dev);+#endif
This series:
http://www.spinics.net/lists/linux-pci/msg45950.html
will allow us to initialize the irq mapping function according to the boot method, code above is getting cumbersome and it is already overriden when booting with DT, so we will remove it altogether.
Having some interrupt related issues. I wondered if the above patchset will fix this problem.
ACPI link object unfortunately is limited to 256 interrupts and I'd like to use the _PRT table without the link object for the full SPI interrupt range.
My previous ACPI table was as follows:
Device (PCI0) { // PCIe port 0 Name(_HID, EISAID("PNP0A08")) // PCI express Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge { .... Name(_PRT, Package(){ Package(){0x0FFFF, 0, _SB.LN0A, 0}, // Slot 0, INTA Package(){0x0FFFF, 1, _SB.LN0B, 0}, // Slot 0, INTB Package(){0x0FFFF, 2, _SB.LN0C, 0}, // Slot 0, INTC Package(){0x0FFFF, 3, _SB.LN0D, 0} // Slot 0, INTD }) }
Here is the PNP0C0F
Device(LN0A){ Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link Name(_UID, 1) Name(_PRS, ResourceTemplate(){ Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive, , ,) {0xE8} }) Method(_DIS) {} Method(_CRS) { Return (_PRS) } Method(_SRS, 1) {} }
Now, I changed to the _PRT object to move to GIC model rather than sticking with the PIC model The new _PRT table as follows:
Name(_PRT, Package(){ Package(){0x0FFFF, 0, 0, 0xE8}, // Slot 0, INTA })
This is causing problems to the GICv3 ACPI driver.
[ 8.826409] genirq: Setting trigger mode 8 for irq 105 failed (gic_set_type+0x0/0x80) [ 8.850685] genirq: Setting trigger mode 8 for irq 105 failed (gic_set_type+0x0/0x80) [ 9.330358] genirq: Setting trigger mode 8 for irq 108 failed (gic_set_type+0x0/0x80) [ 9.378252] genirq: Setting trigger mode 8 for irq 110 failed (gic_set_type+0x0/0x80) [ 11.749632] genirq: Setting trigger mode 8 for irq 108 failed (gic_set_type+0x0/0x80) [ 19.340412] genirq: Setting trigger mode 8 for irq 110 failed (gic_set_type+0x0/0x80)
GICv3 driver supports only IRQ_TYPE_EDGE_RISING and IRQ_TYPE_LEVEL_HIGH interrupt types.
The ACPI PCI code assume active low which doesn't work for GICv3.
int acpi_pci_irq_enable(struct pci_dev *dev) { struct acpi_prt_entry *entry; int gsi; u8 pin; int triggering = ACPI_LEVEL_SENSITIVE; int polarity = ACPI_ACTIVE_LOW;
Do we need ARM64 specific code here?
On 04.11.2015 03:23, Sinan Kaya wrote:
ACPI link object unfortunately is limited to 256 interrupts and I'd like to use the _PRT table without the link object for the full SPI interrupt range.
Can you point me to spec 256 interrupts limitation and explain whether you see it per link object or per whole system? Also, can you use more than 256 different ACPI link objects ?
Tomasz
On 11/4/2015 6:33 AM, Tomasz Nowicki wrote:
On 04.11.2015 03:23, Sinan Kaya wrote:
ACPI link object unfortunately is limited to 256 interrupts and I'd like to use the _PRT table without the link object for the full SPI interrupt range.
Can you point me to spec 256 interrupts limitation and explain whether you see it per link object or per whole system? Also, can you use more than 256 different ACPI link objects ?
Tomasz
The limitation is not the number of interrupts. The limitation is the interrupt Id. You can reproduce this easily by specifying a SPI interrupt number bigger than 256.
Something like this
Device(LN0A){ Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link Name(_UID, 1) Name(_PRS, ResourceTemplate(){ Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive, , ,) {0x160} }) Method(_DIS) {} Method(_CRS) { Return (_PRS) } Method(_SRS, 1) {} }
The limitation is here in pci_link.c driver. See u8 below
struct acpi_pci_link_irq { u8 active; /* Current IRQ */ u8 triggering; /* All IRQs */ u8 polarity; /* All IRQs */
I patched this file locally for testing. See my attachment.
Then, I talked to a couple of people from the Intel world that has ACPI experience.
They questioned why I'm using ACPI_LINK object at all. "ACPI_LINK object has been designed for older intel systems with PIC interrupt controller where the interrupt numbers were limited to 256. Newer systems with IOAPIC declare their interrupts directly inside the _PRT. This also removes the 256 interrupt restriction"
The point is that both interrupt declarations are correct. ARM64 only works with ACPI LINK object. The default implementation in _PRT assumes ACTIVE_LOW which is incompatible with GICv3 driver.
I think we should have Rafael's opinion on this.
On 04.11.2015 15:47, Sinan Kaya wrote:
On 11/4/2015 6:33 AM, Tomasz Nowicki wrote:
On 04.11.2015 03:23, Sinan Kaya wrote:
ACPI link object unfortunately is limited to 256 interrupts and I'd like to use the _PRT table without the link object for the full SPI interrupt range.
Can you point me to spec 256 interrupts limitation and explain whether you see it per link object or per whole system? Also, can you use more than 256 different ACPI link objects ?
Tomasz
The limitation is not the number of interrupts. The limitation is the interrupt Id. You can reproduce this easily by specifying a SPI interrupt number bigger than 256.
Something like this
Device(LN0A){ Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link Name(_UID, 1) Name(_PRS, ResourceTemplate(){ Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive, , ,) {0x160} }) Method(_DIS) {} Method(_CRS) { Return (_PRS) } Method(_SRS, 1) {} }
The limitation is here in pci_link.c driver. See u8 below
struct acpi_pci_link_irq { u8 active; /* Current IRQ */ u8 triggering; /* All IRQs */ u8 polarity; /* All IRQs */
I patched this file locally for testing. See my attachment.
Then, I talked to a couple of people from the Intel world that has ACPI experience.
They questioned why I'm using ACPI_LINK object at all. "ACPI_LINK object has been designed for older intel systems with PIC interrupt controller where the interrupt numbers were limited to 256. Newer systems with IOAPIC declare their interrupts directly inside the _PRT. This also removes the 256 interrupt restriction"
The point is that both interrupt declarations are correct. ARM64 only works with ACPI LINK object. The default implementation in _PRT assumes ACTIVE_LOW which is incompatible with GICv3 driver.
I think we should have Rafael's opinion on this.
OK, now I see your point and 256 limitation, your patch won't help to solve that issue because spec defines: struct acpi_resource_irq { u8 descriptor_length; u8 triggering; u8 polarity; u8 sharable; u8 wake_capable; u8 interrupt_count; u8 interrupts[1]; <---- 1byte size };
I think we should use _PRT and adjust default ACTIVE_LOW to be more flexible. But it is always good to ask maintainer.
Regards, Tomasz
On 11/4/2015 10:09 AM, Tomasz Nowicki wrote:
OK, now I see your point and 256 limitation, your patch won't help to solve that issue because spec defines: struct acpi_resource_irq { u8 descriptor_length; u8 triggering; u8 polarity; u8 sharable; u8 wake_capable; u8 interrupt_count; u8 interrupts[1]; <---- 1byte size };
My code works... Tested against real hardware. The format used with extended IRQ is this. See u32 below.
struct acpi_resource_extended_irq { u8 producer_consumer; u8 triggering; u8 polarity; u8 sharable; u8 wake_capable; u8 interrupt_count; struct acpi_resource_source resource_source; u32 interrupts[1]; };
The pci_link.c already supports parsing extended IRQ range. It is just limited to 256 due to common data structure types (u8) used.
case ACPI_RESOURCE_TYPE_EXTENDED_IRQ: { struct acpi_resource_extended_irq *p = &resource->data.extended_irq; if (!p || !p->interrupt_count) {
I think we need support for both. Both interrupt declarations are correct. The ACPI spec does not limit the interrupt value to 256 and the declaration of interrupt numbers inside _PRT is also correct.
On 04.11.2015 16:47, Sinan Kaya wrote:
On 11/4/2015 10:09 AM, Tomasz Nowicki wrote:
OK, now I see your point and 256 limitation, your patch won't help to solve that issue because spec defines: struct acpi_resource_irq { u8 descriptor_length; u8 triggering; u8 polarity; u8 sharable; u8 wake_capable; u8 interrupt_count; u8 interrupts[1]; <---- 1byte size };
My code works... Tested against real hardware. The format used with extended IRQ is this. See u32 below.
struct acpi_resource_extended_irq { u8 producer_consumer; u8 triggering; u8 polarity; u8 sharable; u8 wake_capable; u8 interrupt_count; struct acpi_resource_source resource_source; u32 interrupts[1]; };
The pci_link.c already supports parsing extended IRQ range. It is just limited to 256 due to common data structure types (u8) used.
case ACPI_RESOURCE_TYPE_EXTENDED_IRQ: { struct acpi_resource_extended_irq *p = &resource->data.extended_irq; if (!p || !p->interrupt_count) {
I think we need support for both. Both interrupt declarations are correct. The ACPI spec does not limit the interrupt value to 256 and the declaration of interrupt numbers inside _PRT is also correct.
Yep, EXTENDED_IRQ does the job, so conclusion is that we need two kernel patches, one for _PRT and one for link object.
Tomasz
On 03.11.2015 17:55, Lorenzo Pieralisi wrote:
On Tue, Oct 27, 2015 at 05:38:42PM +0100, Tomasz Nowicki wrote:
[...]
menu "Kernel Features" diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index b3d098b..66cc1ae 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -11,12 +11,15 @@ */
#include <linux/acpi.h> +#include <linux/ecam.h> #include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/mm.h> +#include <linux/of_address.h> #include <linux/of_pci.h> #include <linux/of_platform.h> +#include <linux/pci-acpi.h> #include <linux/slab.h>
#include <asm/pci-bridge.h> @@ -52,35 +55,216 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) }
/*
- Try to assign the IRQ number from DT when adding a new device
*/ int pcibios_add_device(struct pci_dev *dev) {
- Try to assign the IRQ number from DT/ACPI when adding a new device
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
- if (acpi_disabled)
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);+#ifdef CONFIG_ACPI
- else
acpi_pci_irq_enable(dev);+#endif
This series:
http://www.spinics.net/lists/linux-pci/msg45950.html
will allow us to initialize the irq mapping function according to the boot method, code above is getting cumbersome and it is already overriden when booting with DT, so we will remove it altogether.
return 0; }
+#ifdef CONFIG_ACPI +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- struct acpi_pci_root *root = bridge->bus->sysdata;
- ACPI_COMPANION_SET(&bridge->dev, root->device);
- return 0;
This should be made part of core code IMO.
+}
+void pcibios_add_bus(struct pci_bus *bus) +{
- acpi_pci_add_bus(bus);
+}
+void pcibios_remove_bus(struct pci_bus *bus) +{
- acpi_pci_remove_bus(bus);
+}
Two functions above are identical for arm64, ia64 and x86, I do not think they belong in arch code.
+static int __init pcibios_assign_resources(void) +{
- if (acpi_disabled)
return 0;- pci_assign_unassigned_resources();
- return 0;
Already commented on this.
+} /*
- raw_pci_read/write - Platform-specific PCI config space access.
- rootfs_initcall comes after subsys_initcall and fs_initcall_sync,
*/
- so we know acpi scan and PCI_FIXUP_FINAL quirks have both run.
-int raw_pci_read(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *val)+rootfs_initcall(pcibios_assign_resources);
+static void __iomem * +pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(pci_domain_nr(bus), bus->number);
- if (cfg && cfg->virt)
return cfg->virt +(PCI_MMCFG_BUS_OFFSET(bus->number) | (devfn << 12)) +offset;- return NULL;
Why is this code arm64 specific ?
}
-int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)+struct pci_ops pci_root_ops = {
- .map_bus = pci_mcfg_dev_base,
- .read = pci_generic_config_read,
- .write = pci_generic_config_write,
+};
+#ifdef CONFIG_PCI_MMCONFIG +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- struct acpi_pci_root *root;
- int seg, start, end, err;
- root = ci->root;
- seg = root->segment;
- start = root->secondary.start;
- end = root->secondary.end;
- cfg = pci_mmconfig_lookup(seg, start);
- if (cfg)
return 0;- cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
- if (!cfg)
return -ENOMEM;- err = pci_mmconfig_inject(cfg);
- return err; }
-#ifdef CONFIG_ACPI +static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) +{
- struct acpi_pci_root *root = ci->root;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
- if (cfg)
return;- if (cfg->hot_added)
pci_mmconfig_delete(root->segment, root->secondary.start,root->secondary.end);+} +#else +static int pci_add_mmconfig_region(struct acpi_pci_root_info *ci) +{
- return 0;
+}
+static void pci_remove_mmconfig_region(struct acpi_pci_root_info *ci) { } +#endif
Ditto.
+static int pci_acpi_root_init_info(struct acpi_pci_root_info *ci) +{
- return pci_add_mmconfig_region(ci);
+}
+static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci) +{
- pci_remove_mmconfig_region(ci);
- kfree(ci);
+}
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) +{
- struct resource_entry *entry, *tmp;
- int ret;
- ret = acpi_pci_probe_root_resources(ci);
- if (ret < 0)
return ret;Code above is identical on arm64, ia64 and x86.
- resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
struct resource *res = entry->res;/** Special handling for ARM IO range* TODO: need to move pci_register_io_range() function out* of drivers/of/address.c for both used by DT and ACPI*/if (res->flags & IORESOURCE_IO) {unsigned long port;int err;resource_size_t length = res->end - res->start;err = pci_register_io_range(res->start, length);if (err) {resource_list_destroy_entry(entry);continue;}port = pci_address_to_pio(res->start);if (port == (unsigned long)-1) {resource_list_destroy_entry(entry);continue;}res->start = port;res->end = res->start + length - 1;if (pci_remap_iospace(res, res->start) < 0)resource_list_destroy_entry(entry);}- }
- return 0;
+}
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .pci_ops = &pci_root_ops,
- .init_info = pci_acpi_root_init_info,
- .release_info = pci_acpi_root_release_info,
- .prepare_resources = pci_acpi_root_prepare_resources,
+};
- /* Root bridge scanning */ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) {
- /* TODO: Should be revisited when implementing PCI on ACPI */
- return NULL;
- int node = acpi_get_node(root->device->handle);
- int domain = root->segment;
- int busnum = root->secondary.start;
- struct acpi_pci_root_info *info;
- struct pci_bus *bus;
- if (domain && !pci_domains_supported) {
pr_warn("PCI %04x:%02x: multiple domains not supported.\n",domain, busnum);return NULL;- }
- info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
- if (!info) {
dev_err(&root->device->dev,"pci_bus %04x:%02x: ignored (out of memory)\n",domain, busnum);return NULL;- }
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, info, root);
- /* After the PCI-E bus has been walked and all devices discovered,
* configure any settings of the fabric that might be necessary.*/- if (bus) {
struct pci_bus *child;list_for_each_entry(child, &bus->children, node)pcie_bus_configure_settings(child);- }
- return bus;
Code above is entirely arch agnostic (apart from what data is passed to sysdata) and I think there is room for further consolidation with x86 and ia64, I will have a look into this.
I agree with your comments, currently I do not know how much of it can be consolidated but I will rework my next version in this direction.
Thanks, Tomasz
On 03.11.2015 17:55, Lorenzo Pieralisi wrote:
On Tue, Oct 27, 2015 at 05:38:42PM +0100, Tomasz Nowicki wrote:
[...]
menu "Kernel Features" diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index b3d098b..66cc1ae 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -11,12 +11,15 @@ */
#include <linux/acpi.h> +#include <linux/ecam.h> #include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/mm.h> +#include <linux/of_address.h> #include <linux/of_pci.h> #include <linux/of_platform.h> +#include <linux/pci-acpi.h> #include <linux/slab.h>
#include <asm/pci-bridge.h> @@ -52,35 +55,216 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) }
/*
- Try to assign the IRQ number from DT when adding a new device
*/ int pcibios_add_device(struct pci_dev *dev) {
- Try to assign the IRQ number from DT/ACPI when adding a new device
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
- if (acpi_disabled)
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);+#ifdef CONFIG_ACPI
- else
acpi_pci_irq_enable(dev);+#endif
This series:
http://www.spinics.net/lists/linux-pci/msg45950.html
will allow us to initialize the irq mapping function according to the boot method, code above is getting cumbersome and it is already overriden when booting with DT, so we will remove it altogether.
return 0; }
+#ifdef CONFIG_ACPI +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- struct acpi_pci_root *root = bridge->bus->sysdata;
- ACPI_COMPANION_SET(&bridge->dev, root->device);
- return 0;
This should be made part of core code IMO.
+}
+void pcibios_add_bus(struct pci_bus *bus) +{
- acpi_pci_add_bus(bus);
+}
+void pcibios_remove_bus(struct pci_bus *bus) +{
- acpi_pci_remove_bus(bus);
+}
Two functions above are identical for arm64, ia64 and x86, I do not think they belong in arch code.
+static int __init pcibios_assign_resources(void) +{
- if (acpi_disabled)
return 0;- pci_assign_unassigned_resources();
- return 0;
Already commented on this.
+} /*
- raw_pci_read/write - Platform-specific PCI config space access.
- rootfs_initcall comes after subsys_initcall and fs_initcall_sync,
*/
- so we know acpi scan and PCI_FIXUP_FINAL quirks have both run.
-int raw_pci_read(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *val)+rootfs_initcall(pcibios_assign_resources);
+static void __iomem * +pci_mcfg_dev_base(struct pci_bus *bus, unsigned int devfn, int offset) {
- return -ENXIO;
- struct pci_mmcfg_region *cfg;
- cfg = pci_mmconfig_lookup(pci_domain_nr(bus), bus->number);
- if (cfg && cfg->virt)
return cfg->virt +(PCI_MMCFG_BUS_OFFSET(bus->number) | (devfn << 12)) +offset;- return NULL;
Why is this code arm64 specific ?
It is not, I will move it out of here, probably to mcfg.c file where we can apply quirks.
Thanks, Tomasz
Hi Tomasz,
Thanks for posting this series.
On 10/27/2015 12:38 PM, Tomasz Nowicki wrote:
From the functionality point of view this series might be split into two logic parts:
- Making MMCONFIG code arch-agnostic which allows all architectures to collect PCI config regions and used when necessary.
- Using generic MMCONFIG code and introducing ACPI based PCI hostbridge initialization for ARM64
Can I confirm that the intention here is that this replaces Hanjun's previous patch series? Here's the previous one we were tracking:
https://lkml.kernel.org/g/1432644564-24746-1-git-send-email-hanjun.guo@linar...
Assuming that is the case, then we will ping a number of folks to conduct testing and provide acks (this has already been discussed in a number of conversations with semiconductors over the past few days).
Thanks,
Jon.
Hi Jon,
On 2015/10/30 12:07, Jon Masters wrote:
Hi Tomasz,
Thanks for posting this series.
On 10/27/2015 12:38 PM, Tomasz Nowicki wrote:
From the functionality point of view this series might be split into two logic parts:
- Making MMCONFIG code arch-agnostic which allows all architectures to collect PCI config regions and used when necessary.
- Using generic MMCONFIG code and introducing ACPI based PCI hostbridge initialization for ARM64
Can I confirm that the intention here is that this replaces Hanjun's previous patch series? Here's the previous one we were tracking:
https://lkml.kernel.org/g/1432644564-24746-1-git-send-email-hanjun.guo@linar...
Yes, that's the new version according to the discussion from my version.
Assuming that is the case, then we will ping a number of folks to conduct testing and provide acks (this has already been discussed in a number of conversations with semiconductors over the past few days).
Much appreciated.
Thanks Hanjun
On 10/30/2015 05:07 AM, Jon Masters wrote:
Hi Tomasz,
Thanks for posting this series.
On 10/27/2015 12:38 PM, Tomasz Nowicki wrote:
From the functionality point of view this series might be split into two logic parts:
- Making MMCONFIG code arch-agnostic which allows all architectures to collect PCI config regions and used when necessary.
- Using generic MMCONFIG code and introducing ACPI based PCI hostbridge initialization for ARM64
Can I confirm that the intention here is that this replaces Hanjun's previous patch series? Here's the previous one we were tracking:
https://lkml.kernel.org/g/1432644564-24746-1-git-send-email-hanjun.guo@linar...
Indeed, we can call it next version.
Assuming that is the case, then we will ping a number of folks to conduct testing and provide acks (this has already been discussed in a number of conversations with semiconductors over the past few days).
Yes, please, thanks!
Tomasz
Tested on AMD Seattle platform (Overdrive revB) w/ both MSI and legacy interrupt.
Tested-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com
Thanks, Suravee
On 10/27/15 11:38, Tomasz Nowicki wrote:
From the functionality point of view this series might be split into two logic parts:
- Making MMCONFIG code arch-agnostic which allows all architectures to collect PCI config regions and used when necessary.
- Using generic MMCONFIG code and introducing ACPI based PCI hostbridge initialization for ARM64
Patches has been built on top of: [Patch v7 0/7] Consolidate ACPI PCI root common code into ACPI core https://lkml.org/lkml/2015/10/14/31 Git branch can be found here: https://git.linaro.org/leg/acpi/acpi.git/shortlog/refs/heads/pci-acpi-upstre...
This has been tested on Cavium ThunderX 1 socket server. Any help in reviewing and testing is very appreciated.
Hanjun Guo (1): XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y
Tomasz Nowicki (10): x86, pci: Reorder logic of pci_mmconfig_insert() function x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory pci, acpi, mcfg: Provide generic implementation of MCFG code initialization. x86, pci: mmconfig_{32,64}.c code refactoring - remove code duplication. x86, pci, ecam: mmconfig_64.c becomes default implementation for ECAM driver. pci, acpi, mcfg: Provide default RAW ACPI PCI config space accessors. pci, acpi, ecam: Add flag to indicate whether ECAM region was hot added or not. x86, pci: Use previously added ECAM hot_added flag to remove ECAM regions. pci, acpi: Provide generic way to assign bus domain number. arm64, pci, acpi: Support for ACPI based PCI hostbridge init
arch/arm64/Kconfig | 6 + arch/arm64/kernel/pci.c | 208 ++++++++++++++++++++++++++++++++-- arch/x86/Kconfig | 4 + arch/x86/include/asm/pci_x86.h | 28 +---- arch/x86/pci/acpi.c | 17 +-- arch/x86/pci/mmconfig-shared.c | 250 +++++++---------------------------------- arch/x86/pci/mmconfig_32.c | 11 +- arch/x86/pci/mmconfig_64.c | 67 +---------- arch/x86/pci/numachip.c | 1 + drivers/acpi/Makefile | 1 + drivers/acpi/mcfg.c | 104 +++++++++++++++++ drivers/acpi/pci_root.c | 2 +- drivers/pci/Kconfig | 10 ++ drivers/pci/Makefile | 5 + drivers/pci/ecam.c | 234 ++++++++++++++++++++++++++++++++++++++ drivers/pci/pci.c | 30 ++++- drivers/xen/pci.c | 7 +- include/linux/acpi.h | 2 + include/linux/ecam.h | 44 ++++++++ 19 files changed, 691 insertions(+), 340 deletions(-) create mode 100644 drivers/acpi/mcfg.c create mode 100644 drivers/pci/ecam.c create mode 100644 include/linux/ecam.h
On Tue, Oct 27, 2015 at 05:38:31PM +0100, Tomasz Nowicki wrote:
From the functionality point of view this series might be split into two logic parts:
- Making MMCONFIG code arch-agnostic which allows all architectures to collect PCI config regions and used when necessary.
- Using generic MMCONFIG code and introducing ACPI based PCI hostbridge initialization for ARM64
Patches has been built on top of: [Patch v7 0/7] Consolidate ACPI PCI root common code into ACPI core https://lkml.org/lkml/2015/10/14/31 Git branch can be found here: https://git.linaro.org/leg/acpi/acpi.git/shortlog/refs/heads/pci-acpi-upstre...
This has been tested on Cavium ThunderX 1 socket server. Any help in reviewing and testing is very appreciated.
Hanjun Guo (1): XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y
Tomasz Nowicki (10): x86, pci: Reorder logic of pci_mmconfig_insert() function x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory pci, acpi, mcfg: Provide generic implementation of MCFG code initialization. x86, pci: mmconfig_{32,64}.c code refactoring - remove code duplication. x86, pci, ecam: mmconfig_64.c becomes default implementation for ECAM driver. pci, acpi, mcfg: Provide default RAW ACPI PCI config space accessors. pci, acpi, ecam: Add flag to indicate whether ECAM region was hot added or not. x86, pci: Use previously added ECAM hot_added flag to remove ECAM regions. pci, acpi: Provide generic way to assign bus domain number. arm64, pci, acpi: Support for ACPI based PCI hostbridge init
arch/arm64/Kconfig | 6 + arch/arm64/kernel/pci.c | 208 ++++++++++++++++++++++++++++++++-- arch/x86/Kconfig | 4 + arch/x86/include/asm/pci_x86.h | 28 +---- arch/x86/pci/acpi.c | 17 +-- arch/x86/pci/mmconfig-shared.c | 250 +++++++---------------------------------- arch/x86/pci/mmconfig_32.c | 11 +- arch/x86/pci/mmconfig_64.c | 67 +---------- arch/x86/pci/numachip.c | 1 + drivers/acpi/Makefile | 1 + drivers/acpi/mcfg.c | 104 +++++++++++++++++ drivers/acpi/pci_root.c | 2 +- drivers/pci/Kconfig | 10 ++ drivers/pci/Makefile | 5 + drivers/pci/ecam.c | 234 ++++++++++++++++++++++++++++++++++++++ drivers/pci/pci.c | 30 ++++- drivers/xen/pci.c | 7 +- include/linux/acpi.h | 2 + include/linux/ecam.h | 44 ++++++++ 19 files changed, 691 insertions(+), 340 deletions(-) create mode 100644 drivers/acpi/mcfg.c create mode 100644 drivers/pci/ecam.c create mode 100644 include/linux/ecam.h
Waiting for an updated version after all the discussion.
Bjorn
[trimmed the cc list a bit] On Mon, Dec 07, 2015 at 02:31:17PM -0600, Bjorn Helgaas wrote:
On Tue, Oct 27, 2015 at 05:38:31PM +0100, Tomasz Nowicki wrote:
[...]
Tomasz Nowicki (10): x86, pci: Reorder logic of pci_mmconfig_insert() function x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory pci, acpi, mcfg: Provide generic implementation of MCFG code initialization. x86, pci: mmconfig_{32,64}.c code refactoring - remove code duplication. x86, pci, ecam: mmconfig_64.c becomes default implementation for ECAM driver. pci, acpi, mcfg: Provide default RAW ACPI PCI config space accessors. pci, acpi, ecam: Add flag to indicate whether ECAM region was hot added or not. x86, pci: Use previously added ECAM hot_added flag to remove ECAM regions.
[...]
Waiting for an updated version after all the discussion.
Hi Bjorn,
After looking thru the code, I don't think moving the mmconfig code out of x86 and using it in ARM64 is a good idea.
The code maintains a list of mapping of ECAM regions which can modified at run time, and ends doing this sequence for doing a config access:
rcu_read_lock(); addr = pci_dev_base(seg, bus, devfn); list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) ...find mapping... do pci operation rcu_read_unlock();
A lot of the complexity in mmconfig_*.c is to maintain and validate the pci_mmcfg_list which seems to be unecessary.
The better way would be to keep the mapping of ECAM region in the PCI host bridge sysdata like pci-host-generic and directly access using map_bus/read/write.
I see that early raw_pci_read/raw_pci_write may need to be supported. I am not sure if this is needed in ARM64 - and if it is, we can handle this by taking an early mapping for ECAM regions until the PCI host bridges are setup, and dropping the mapping at that point.
Thanks, JC.
On Wed, Dec 09, 2015 at 03:31:41PM +0530, Jayachandran C. wrote:
[trimmed the cc list a bit] On Mon, Dec 07, 2015 at 02:31:17PM -0600, Bjorn Helgaas wrote:
On Tue, Oct 27, 2015 at 05:38:31PM +0100, Tomasz Nowicki wrote:
[...]
Tomasz Nowicki (10): x86, pci: Reorder logic of pci_mmconfig_insert() function x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory pci, acpi, mcfg: Provide generic implementation of MCFG code initialization. x86, pci: mmconfig_{32,64}.c code refactoring - remove code duplication. x86, pci, ecam: mmconfig_64.c becomes default implementation for ECAM driver. pci, acpi, mcfg: Provide default RAW ACPI PCI config space accessors. pci, acpi, ecam: Add flag to indicate whether ECAM region was hot added or not. x86, pci: Use previously added ECAM hot_added flag to remove ECAM regions.
[...]
Waiting for an updated version after all the discussion.
Hi Bjorn,
After looking thru the code, I don't think moving the mmconfig code out of x86 and using it in ARM64 is a good idea.
The code maintains a list of mapping of ECAM regions which can modified at run time, and ends doing this sequence for doing a config access:
rcu_read_lock(); addr = pci_dev_base(seg, bus, devfn); list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) ...find mapping...do pci operation rcu_read_unlock();
A lot of the complexity in mmconfig_*.c is to maintain and validate the pci_mmcfg_list which seems to be unecessary.
That list is there to manage hotplug bridges, what makes you think it is not necessary ? Jiang (in CC) can certainly comment on that and how that list handling can be updated/simplified, if possible.
The better way would be to keep the mapping of ECAM region in the PCI host bridge sysdata like pci-host-generic and directly access using map_bus/read/write.
I see that early raw_pci_read/raw_pci_write may need to be supported. I am not sure if this is needed in ARM64 - and if it is, we can handle this by taking an early mapping for ECAM regions until the PCI host bridges are setup, and dropping the mapping at that point.
I do not think it is needed (and if it is it can be added later, it should not block this series), this was already debated in previous threads (and in the ASWG, where basically nobody could provide a reason why those raw accessors in ACPICA are meant to exist, they are there for AML to access config space, why that has to be happen before PCI busses are enumerated is still a mystery to me).
Lorenzo
On Wed, Dec 09, 2015 at 03:55:11PM +0000, Lorenzo Pieralisi wrote:
On Wed, Dec 09, 2015 at 03:31:41PM +0530, Jayachandran C. wrote:
[trimmed the cc list a bit] On Mon, Dec 07, 2015 at 02:31:17PM -0600, Bjorn Helgaas wrote:
On Tue, Oct 27, 2015 at 05:38:31PM +0100, Tomasz Nowicki wrote:
[...]
Tomasz Nowicki (10): x86, pci: Reorder logic of pci_mmconfig_insert() function x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory pci, acpi, mcfg: Provide generic implementation of MCFG code initialization. x86, pci: mmconfig_{32,64}.c code refactoring - remove code duplication. x86, pci, ecam: mmconfig_64.c becomes default implementation for ECAM driver. pci, acpi, mcfg: Provide default RAW ACPI PCI config space accessors. pci, acpi, ecam: Add flag to indicate whether ECAM region was hot added or not. x86, pci: Use previously added ECAM hot_added flag to remove ECAM regions.
[...]
Waiting for an updated version after all the discussion.
Hi Bjorn,
After looking thru the code, I don't think moving the mmconfig code out of x86 and using it in ARM64 is a good idea.
The code maintains a list of mapping of ECAM regions which can modified at run time, and ends doing this sequence for doing a config access:
rcu_read_lock(); addr = pci_dev_base(seg, bus, devfn); list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) ...find mapping...do pci operation rcu_read_unlock();
A lot of the complexity in mmconfig_*.c is to maintain and validate the pci_mmcfg_list which seems to be unecessary.
That list is there to manage hotplug bridges, what makes you think it is not necessary ? Jiang (in CC) can certainly comment on that and how that list handling can be updated/simplified, if possible.
Looking thru the code, I think moving the MCFG code to common can be done in a simpler way. I have posted a new patchset for this. (And I don't see how the hotplug case needs the list, but that is not relevant now)
The better way would be to keep the mapping of ECAM region in the PCI host bridge sysdata like pci-host-generic and directly access using map_bus/read/write.
I see that early raw_pci_read/raw_pci_write may need to be supported. I am not sure if this is needed in ARM64 - and if it is, we can handle this by taking an early mapping for ECAM regions until the PCI host bridges are setup, and dropping the mapping at that point.
I do not think it is needed (and if it is it can be added later, it should not block this series), this was already debated in previous threads (and in the ASWG, where basically nobody could provide a reason why those raw accessors in ACPICA are meant to exist, they are there for AML to access config space, why that has to be happen before PCI busses are enumerated is still a mystery to me).
The posted patchset adds raw_pci_read/raw_pci_write as well.
JC.
On Wed, Dec 16, 2015 at 06:21:37PM +0530, Jayachandran C. wrote:
[...]
That list is there to manage hotplug bridges, what makes you think it is not necessary ? Jiang (in CC) can certainly comment on that and how that list handling can be updated/simplified, if possible.
Looking thru the code, I think moving the MCFG code to common can be done in a simpler way. I have posted a new patchset for this. (And I don't see how the hotplug case needs the list, but that is not relevant now)
You must work with Tomasz and come up with a unified patchset that implements ACPI PCIe support for ARM64. We do not need more churn, we need a working set, period. Posting separate RFCs just adds to the confusion and to maintainers' review backlog, with the end result of confusing everyone and achieving nothing.
Lorenzo
On 10/27/2015 11:38 AM, Tomasz Nowicki wrote:
From the functionality point of view this series might be split into two logic parts:
- Making MMCONFIG code arch-agnostic which allows all architectures to collect PCI config regions and used when necessary.
- Using generic MMCONFIG code and introducing ACPI based PCI hostbridge initialization for ARM64
Tested with legacy interrupts on ARM Juno R1, HP m400 (APM Potenza), and APM mustang. The latter two platforms have an additional pci quirk patch applied (to be posted shortly) to fix their problems with the config space accessors.
Tested-by: Jeremy Linton jeremy.linton@arm.com
Thanks!
-
Arnd Bergmann -
Bjorn Helgaas -
Boris Ostrovsky -
Hanjun Guo -
Hanjun Guo -
Jayachandran C. -
Jeremy Linton -
Jon Masters -
Liviu.Dudauļ¼ arm.com -
Lorenzo Pieralisi -
Sinan Kaya -
Stefano Stabellini -
Suravee Suthikulpanit -
Tomasz Nowicki -
Tomasz Nowicki