* [PATCH V1 01/11] x86, pci: Reorder logic of pci_mmconfig_insert() function
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
@ 2015-10-27 16:38 ` Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 02/11] x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory Tomasz Nowicki
` (13 subsequent siblings)
14 siblings, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:38 UTC (permalink / raw)
To: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi,
Tomasz Nowicki, Tomasz Nowicki
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;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V1 02/11] x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 01/11] x86, pci: Reorder logic of pci_mmconfig_insert() function Tomasz Nowicki
@ 2015-10-27 16:38 ` Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 03/11] pci, acpi, mcfg: Provide generic implementation of MCFG code initialization Tomasz Nowicki
` (12 subsequent siblings)
14 siblings, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:38 UTC (permalink / raw)
To: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi,
Tomasz Nowicki
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 */
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V1 03/11] pci, acpi, mcfg: Provide generic implementation of MCFG code initialization.
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 01/11] x86, pci: Reorder logic of pci_mmconfig_insert() function Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 02/11] x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory Tomasz Nowicki
@ 2015-10-27 16:38 ` Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 04/11] x86, pci: mmconfig_{32,64}.c code refactoring - remove code duplication Tomasz Nowicki
` (11 subsequent siblings)
14 siblings, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:38 UTC (permalink / raw)
To: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi,
Tomasz Nowicki
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);
+}
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V1 04/11] x86, pci: mmconfig_{32,64}.c code refactoring - remove code duplication.
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (2 preceding siblings ...)
2015-10-27 16:38 ` [PATCH V1 03/11] pci, acpi, mcfg: Provide generic implementation of MCFG code initialization Tomasz Nowicki
@ 2015-10-27 16:38 ` Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 05/11] x86, pci, ecam: mmconfig_64.c becomes default implementation for ECAM driver Tomasz Nowicki
` (10 subsequent siblings)
14 siblings, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:38 UTC (permalink / raw)
To: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi,
Tomasz Nowicki
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;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V1 05/11] x86, pci, ecam: mmconfig_64.c becomes default implementation for ECAM driver.
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (3 preceding siblings ...)
2015-10-27 16:38 ` [PATCH V1 04/11] x86, pci: mmconfig_{32,64}.c code refactoring - remove code duplication Tomasz Nowicki
@ 2015-10-27 16:38 ` Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 06/11] pci, acpi, mcfg: Provide default RAW ACPI PCI config space accessors Tomasz Nowicki
` (9 subsequent siblings)
14 siblings, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:38 UTC (permalink / raw)
To: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi,
Tomasz Nowicki
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)
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V1 06/11] pci, acpi, mcfg: Provide default RAW ACPI PCI config space accessors.
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (4 preceding siblings ...)
2015-10-27 16:38 ` [PATCH V1 05/11] x86, pci, ecam: mmconfig_64.c becomes default implementation for ECAM driver Tomasz Nowicki
@ 2015-10-27 16:38 ` Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 07/11] XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y Tomasz Nowicki
` (8 subsequent siblings)
14 siblings, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:38 UTC (permalink / raw)
To: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi,
Tomasz Nowicki
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;
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V1 07/11] XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (5 preceding siblings ...)
2015-10-27 16:38 ` [PATCH V1 06/11] pci, acpi, mcfg: Provide default RAW ACPI PCI config space accessors Tomasz Nowicki
@ 2015-10-27 16:38 ` Tomasz Nowicki
2015-10-27 16:47 ` [Linaro-acpi] " Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 08/11] pci, acpi, ecam: Add flag to indicate whether ECAM region was hot added or not Tomasz Nowicki
` (7 subsequent siblings)
14 siblings, 1 reply; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:38 UTC (permalink / raw)
To: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi,
Konrad Rzeszutek Wilk, Boris Ostrovsky
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;
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [Linaro-acpi] [PATCH V1 07/11] XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y
2015-10-27 16:38 ` [PATCH V1 07/11] XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y Tomasz Nowicki
@ 2015-10-27 16:47 ` Tomasz Nowicki
2015-10-27 17:25 ` Boris Ostrovsky
0 siblings, 1 reply; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:47 UTC (permalink / raw)
To: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, hanjun.guo, Lorenzo.Pieralisi
Cc: linux-kernel, linaro-acpi, Boris Ostrovsky, linux-pci,
Konrad Rzeszutek Wilk, Liviu.Dudau, ddaney, Narinder.Dhillon,
linux-acpi, robert.richter, wangyijing, tglx, jiang.liu,
linux-arm-kernel, Stefano.Stabellini
+ 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;
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Linaro-acpi] [PATCH V1 07/11] XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y
2015-10-27 16:47 ` [Linaro-acpi] " Tomasz Nowicki
@ 2015-10-27 17:25 ` Boris Ostrovsky
2015-10-28 10:49 ` Stefano Stabellini
0 siblings, 1 reply; 50+ messages in thread
From: Boris Ostrovsky @ 2015-10-27 17:25 UTC (permalink / raw)
To: Tomasz Nowicki, Tomasz Nowicki, bhelgaas, arnd, will.deacon,
catalin.marinas, rjw, hanjun.guo, Lorenzo.Pieralisi
Cc: linux-kernel, linaro-acpi, linux-pci, Konrad Rzeszutek Wilk,
Liviu.Dudau, ddaney, Narinder.Dhillon, linux-acpi,
robert.richter, wangyijing, tglx, jiang.liu, linux-arm-kernel,
Stefano.Stabellini
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;
>>
>>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Linaro-acpi] [PATCH V1 07/11] XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y
2015-10-27 17:25 ` Boris Ostrovsky
@ 2015-10-28 10:49 ` Stefano Stabellini
2015-10-28 10:56 ` Tomasz Nowicki
0 siblings, 1 reply; 50+ messages in thread
From: Stefano Stabellini @ 2015-10-28 10:49 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: Tomasz Nowicki, Tomasz Nowicki, bhelgaas, arnd, will.deacon,
catalin.marinas, rjw, hanjun.guo, Lorenzo.Pieralisi,
linux-kernel, linaro-acpi, linux-pci, Konrad Rzeszutek Wilk,
Liviu.Dudau, ddaney, Narinder.Dhillon, linux-acpi,
robert.richter, wangyijing, tglx, jiang.liu, linux-arm-kernel,
Stefano.Stabellini
[-- Attachment #1: Type: text/plain, Size: 2618 bytes --]
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;
> > >
> > >
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Linaro-acpi] [PATCH V1 07/11] XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y
2015-10-28 10:49 ` Stefano Stabellini
@ 2015-10-28 10:56 ` Tomasz Nowicki
2015-10-28 13:45 ` Hanjun Guo
0 siblings, 1 reply; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-28 10:56 UTC (permalink / raw)
To: Stefano Stabellini, Boris Ostrovsky
Cc: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, hanjun.guo, Lorenzo.Pieralisi, linux-kernel, linaro-acpi,
linux-pci, Konrad Rzeszutek Wilk, Liviu.Dudau, ddaney,
Narinder.Dhillon, linux-acpi, robert.richter, wangyijing, tglx,
jiang.liu, linux-arm-kernel
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;
>>>>
>>>>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Linaro-acpi] [PATCH V1 07/11] XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y
2015-10-28 10:56 ` Tomasz Nowicki
@ 2015-10-28 13:45 ` Hanjun Guo
2015-10-28 14:07 ` Boris Ostrovsky
0 siblings, 1 reply; 50+ messages in thread
From: Hanjun Guo @ 2015-10-28 13:45 UTC (permalink / raw)
To: Tomasz Nowicki, Stefano Stabellini, Boris Ostrovsky
Cc: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, Lorenzo.Pieralisi, linux-kernel, linaro-acpi, linux-pci,
Konrad Rzeszutek Wilk, Liviu.Dudau, ddaney, Narinder.Dhillon,
linux-acpi, robert.richter, wangyijing, tglx, jiang.liu,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Linaro-acpi] [PATCH V1 07/11] XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y
2015-10-28 13:45 ` Hanjun Guo
@ 2015-10-28 14:07 ` Boris Ostrovsky
0 siblings, 0 replies; 50+ messages in thread
From: Boris Ostrovsky @ 2015-10-28 14:07 UTC (permalink / raw)
To: Hanjun Guo, Tomasz Nowicki, Stefano Stabellini
Cc: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, Lorenzo.Pieralisi, linux-kernel, linaro-acpi, linux-pci,
Konrad Rzeszutek Wilk, Liviu.Dudau, ddaney, Narinder.Dhillon,
linux-acpi, robert.richter, wangyijing, tglx, jiang.liu,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH V1 08/11] pci, acpi, ecam: Add flag to indicate whether ECAM region was hot added or not.
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (6 preceding siblings ...)
2015-10-27 16:38 ` [PATCH V1 07/11] XEN / PCI: Remove the dependence on arch x86 when PCI_MMCONFIG=y Tomasz Nowicki
@ 2015-10-27 16:38 ` Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 09/11] x86, pci: Use previously added ECAM hot_added flag to remove ECAM regions Tomasz Nowicki
` (6 subsequent siblings)
14 siblings, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:38 UTC (permalink / raw)
To: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi,
Tomasz Nowicki
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);
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V1 09/11] x86, pci: Use previously added ECAM hot_added flag to remove ECAM regions.
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (7 preceding siblings ...)
2015-10-27 16:38 ` [PATCH V1 08/11] pci, acpi, ecam: Add flag to indicate whether ECAM region was hot added or not Tomasz Nowicki
@ 2015-10-27 16:38 ` Tomasz Nowicki
2015-10-27 16:38 ` [PATCH V1 10/11] pci, acpi: Provide generic way to assign bus domain number Tomasz Nowicki
` (5 subsequent siblings)
14 siblings, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:38 UTC (permalink / raw)
To: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi,
Tomasz Nowicki
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)
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V1 10/11] pci, acpi: Provide generic way to assign bus domain number.
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (8 preceding siblings ...)
2015-10-27 16:38 ` [PATCH V1 09/11] x86, pci: Use previously added ECAM hot_added flag to remove ECAM regions Tomasz Nowicki
@ 2015-10-27 16:38 ` Tomasz Nowicki
2015-10-28 11:38 ` Liviu.Dudau
2015-11-03 16:10 ` Lorenzo Pieralisi
2015-10-27 16:38 ` [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init Tomasz Nowicki
` (4 subsequent siblings)
14 siblings, 2 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:38 UTC (permalink / raw)
To: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi,
Tomasz Nowicki
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();
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH V1 10/11] pci, acpi: Provide generic way to assign bus domain number.
2015-10-27 16:38 ` [PATCH V1 10/11] pci, acpi: Provide generic way to assign bus domain number Tomasz Nowicki
@ 2015-10-28 11:38 ` Liviu.Dudau
2015-10-28 12:47 ` [Linaro-acpi] " Tomasz Nowicki
2015-11-03 16:10 ` Lorenzo Pieralisi
1 sibling, 1 reply; 50+ messages in thread
From: Liviu.Dudau @ 2015-10-28 11:38 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi, jiang.liu, robert.richter, Narinder.Dhillon,
ddaney, tglx, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi
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
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Linaro-acpi] [PATCH V1 10/11] pci, acpi: Provide generic way to assign bus domain number.
2015-10-28 11:38 ` Liviu.Dudau
@ 2015-10-28 12:47 ` Tomasz Nowicki
0 siblings, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-28 12:47 UTC (permalink / raw)
To: Liviu.Dudau, Tomasz Nowicki
Cc: rjw, linux-kernel, arnd, linux-acpi, linux-pci, catalin.marinas,
linaro-acpi, will.deacon, ddaney, Narinder.Dhillon, wangyijing,
robert.richter, bhelgaas, tglx, jiang.liu, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 10/11] pci, acpi: Provide generic way to assign bus domain number.
2015-10-27 16:38 ` [PATCH V1 10/11] pci, acpi: Provide generic way to assign bus domain number Tomasz Nowicki
2015-10-28 11:38 ` Liviu.Dudau
@ 2015-11-03 16:10 ` Lorenzo Pieralisi
2015-11-04 10:04 ` [Linaro-acpi] " Tomasz Nowicki
1 sibling, 1 reply; 50+ messages in thread
From: Lorenzo Pieralisi @ 2015-11-03 16:10 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi
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
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Linaro-acpi] [PATCH V1 10/11] pci, acpi: Provide generic way to assign bus domain number.
2015-11-03 16:10 ` Lorenzo Pieralisi
@ 2015-11-04 10:04 ` Tomasz Nowicki
0 siblings, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-11-04 10:04 UTC (permalink / raw)
To: Lorenzo Pieralisi, Tomasz Nowicki
Cc: rjw, linux-kernel, arnd, linux-acpi, linux-pci, catalin.marinas,
linaro-acpi, will.deacon, ddaney, Narinder.Dhillon, wangyijing,
robert.richter, tglx, bhelgaas, Liviu.Dudau, jiang.liu,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (9 preceding siblings ...)
2015-10-27 16:38 ` [PATCH V1 10/11] pci, acpi: Provide generic way to assign bus domain number Tomasz Nowicki
@ 2015-10-27 16:38 ` Tomasz Nowicki
2015-10-28 11:49 ` Liviu.Dudau
` (2 more replies)
2015-10-30 4:07 ` [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Jon Masters
` (3 subsequent siblings)
14 siblings, 3 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-27 16:38 UTC (permalink / raw)
To: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi,
Tomasz Nowicki
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
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-10-27 16:38 ` [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init Tomasz Nowicki
@ 2015-10-28 11:49 ` Liviu.Dudau
2015-10-28 13:42 ` Tomasz Nowicki
2015-11-03 14:32 ` Lorenzo Pieralisi
2015-10-28 18:46 ` Sinan Kaya
2015-11-03 16:55 ` Lorenzo Pieralisi
2 siblings, 2 replies; 50+ messages in thread
From: Liviu.Dudau @ 2015-10-28 11:49 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi, jiang.liu, robert.richter, Narinder.Dhillon,
ddaney, tglx, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi
On Tue, Oct 27, 2015 at 05:38:42PM +0100, Tomasz Nowicki wrote:
> 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;
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
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-10-28 11:49 ` Liviu.Dudau
@ 2015-10-28 13:42 ` Tomasz Nowicki
2015-10-28 13:51 ` Liviu.Dudau
2015-11-03 14:32 ` Lorenzo Pieralisi
1 sibling, 1 reply; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-28 13:42 UTC (permalink / raw)
To: Liviu.Dudau
Cc: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi, jiang.liu, robert.richter, Narinder.Dhillon,
ddaney, tglx, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi
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:
>> 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;
>
> 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 range
>
> There 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
>>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-10-28 13:42 ` Tomasz Nowicki
@ 2015-10-28 13:51 ` Liviu.Dudau
0 siblings, 0 replies; 50+ messages in thread
From: Liviu.Dudau @ 2015-10-28 13:51 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi, jiang.liu, robert.richter, Narinder.Dhillon,
ddaney, tglx, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi
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:
> >>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;
> >
> >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 range
> >
> >There 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
> >>
> >
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-10-28 11:49 ` Liviu.Dudau
2015-10-28 13:42 ` Tomasz Nowicki
@ 2015-11-03 14:32 ` Lorenzo Pieralisi
2015-11-03 16:28 ` Liviu.Dudau
1 sibling, 1 reply; 50+ messages in thread
From: Lorenzo Pieralisi @ 2015-11-03 14:32 UTC (permalink / raw)
To: Liviu.Dudau
Cc: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, hanjun.guo, jiang.liu, robert.richter, Narinder.Dhillon,
ddaney, tglx, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi
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 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
> >
>
> --
> ====================
> | I would like to |
> | fix the world, |
> | but they're not |
> | giving me the |
> \ source code! /
> ---------------
> ¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 14:32 ` Lorenzo Pieralisi
@ 2015-11-03 16:28 ` Liviu.Dudau
0 siblings, 0 replies; 50+ messages in thread
From: Liviu.Dudau @ 2015-11-03 16:28 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, hanjun.guo, jiang.liu, robert.richter, Narinder.Dhillon,
ddaney, tglx, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi
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 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
> > >
> >
> > --
> > ====================
> > | 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
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-10-27 16:38 ` [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init Tomasz Nowicki
2015-10-28 11:49 ` Liviu.Dudau
@ 2015-10-28 18:46 ` Sinan Kaya
2015-11-03 14:15 ` Lorenzo Pieralisi
2015-11-03 16:55 ` Lorenzo Pieralisi
2 siblings, 1 reply; 50+ messages in thread
From: Sinan Kaya @ 2015-10-28 18:46 UTC (permalink / raw)
To: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, hanjun.guo, Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi
On 10/27/2015 12:38 PM, Tomasz Nowicki wrote:
> 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,
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
>
--
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-10-28 18:46 ` Sinan Kaya
@ 2015-11-03 14:15 ` Lorenzo Pieralisi
2015-11-03 14:39 ` Tomasz Nowicki
2015-11-03 15:19 ` Hanjun Guo
0 siblings, 2 replies; 50+ messages in thread
From: Lorenzo Pieralisi @ 2015-11-03 14:15 UTC (permalink / raw)
To: Sinan Kaya
Cc: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, hanjun.guo, jiang.liu, robert.richter, Narinder.Dhillon,
ddaney, Liviu.Dudau, tglx, wangyijing, Suravee.Suthikulpanit,
msalter, linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi
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/
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 14:15 ` Lorenzo Pieralisi
@ 2015-11-03 14:39 ` Tomasz Nowicki
2015-11-03 15:10 ` Sinan Kaya
2015-11-03 15:19 ` Hanjun Guo
1 sibling, 1 reply; 50+ messages in thread
From: Tomasz Nowicki @ 2015-11-03 14:39 UTC (permalink / raw)
To: Lorenzo Pieralisi, Sinan Kaya
Cc: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 14:39 ` Tomasz Nowicki
@ 2015-11-03 15:10 ` Sinan Kaya
2015-11-03 15:59 ` Arnd Bergmann
0 siblings, 1 reply; 50+ messages in thread
From: Sinan Kaya @ 2015-11-03 15:10 UTC (permalink / raw)
To: Tomasz Nowicki, Lorenzo Pieralisi
Cc: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi
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.
--
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 15:10 ` Sinan Kaya
@ 2015-11-03 15:59 ` Arnd Bergmann
2015-11-03 16:33 ` Sinan Kaya
0 siblings, 1 reply; 50+ messages in thread
From: Arnd Bergmann @ 2015-11-03 15:59 UTC (permalink / raw)
To: Sinan Kaya
Cc: Tomasz Nowicki, Lorenzo Pieralisi, bhelgaas, will.deacon,
catalin.marinas, rjw, hanjun.guo, jiang.liu, robert.richter,
Narinder.Dhillon, ddaney, Liviu.Dudau, tglx, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 15:59 ` Arnd Bergmann
@ 2015-11-03 16:33 ` Sinan Kaya
2015-11-03 16:55 ` Arnd Bergmann
0 siblings, 1 reply; 50+ messages in thread
From: Sinan Kaya @ 2015-11-03 16:33 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Tomasz Nowicki, Lorenzo Pieralisi, bhelgaas, will.deacon,
catalin.marinas, rjw, hanjun.guo, jiang.liu, robert.richter,
Narinder.Dhillon, ddaney, Liviu.Dudau, tglx, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi
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.
--
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 16:33 ` Sinan Kaya
@ 2015-11-03 16:55 ` Arnd Bergmann
2015-11-03 17:43 ` Sinan Kaya
0 siblings, 1 reply; 50+ messages in thread
From: Arnd Bergmann @ 2015-11-03 16:55 UTC (permalink / raw)
To: Sinan Kaya
Cc: Tomasz Nowicki, Lorenzo Pieralisi, bhelgaas, will.deacon,
catalin.marinas, rjw, hanjun.guo, jiang.liu, robert.richter,
Narinder.Dhillon, ddaney, Liviu.Dudau, tglx, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 16:55 ` Arnd Bergmann
@ 2015-11-03 17:43 ` Sinan Kaya
2015-11-05 14:48 ` [Linaro-acpi] " Sinan Kaya
0 siblings, 1 reply; 50+ messages in thread
From: Sinan Kaya @ 2015-11-03 17:43 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Tomasz Nowicki, Lorenzo Pieralisi, bhelgaas, will.deacon,
catalin.marinas, rjw, hanjun.guo, jiang.liu, robert.richter,
Narinder.Dhillon, ddaney, Liviu.Dudau, tglx, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi
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
>
--
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Linaro-acpi] [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 17:43 ` Sinan Kaya
@ 2015-11-05 14:48 ` Sinan Kaya
0 siblings, 0 replies; 50+ messages in thread
From: Sinan Kaya @ 2015-11-05 14:48 UTC (permalink / raw)
To: Arnd Bergmann
Cc: rjw, linux-kernel, linux-acpi, linux-pci, will.deacon, ddaney,
Narinder.Dhillon, linaro-acpi, wangyijing, robert.richter, tglx,
catalin.marinas, bhelgaas, Liviu.Dudau, jiang.liu,
linux-arm-kernel
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.
--
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 14:15 ` Lorenzo Pieralisi
2015-11-03 14:39 ` Tomasz Nowicki
@ 2015-11-03 15:19 ` Hanjun Guo
2015-11-03 17:39 ` David Daney
1 sibling, 1 reply; 50+ messages in thread
From: Hanjun Guo @ 2015-11-03 15:19 UTC (permalink / raw)
To: Lorenzo Pieralisi, Sinan Kaya
Cc: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, jiang.liu, robert.richter, Narinder.Dhillon, ddaney,
Liviu.Dudau, tglx, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
Gabriele Paoloni, Wangzhou (B), liudongdong (C)
On 11/03/2015 10:15 PM, 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.
Cc Gab, Zhou, and Dondong who upstream the hip05 (designware) PCIe host
support.
I think so, some platform may not support ECAM for root complex,
which needs special handling of access config space, we may need
to consider those cases.
Thanks
Hanjun
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 15:19 ` Hanjun Guo
@ 2015-11-03 17:39 ` David Daney
2015-11-03 18:00 ` Gabriele Paoloni
0 siblings, 1 reply; 50+ messages in thread
From: David Daney @ 2015-11-03 17:39 UTC (permalink / raw)
To: Hanjun Guo
Cc: Lorenzo Pieralisi, Sinan Kaya, Tomasz Nowicki, bhelgaas, arnd,
will.deacon, catalin.marinas, rjw, jiang.liu, robert.richter,
Narinder.Dhillon, Liviu.Dudau, tglx, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, Gabriele Paoloni, Wangzhou (B),
liudongdong (C)
On 11/03/2015 07:19 AM, Hanjun Guo wrote:
> On 11/03/2015 10:15 PM, 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.
>
> Cc Gab, Zhou, and Dondong who upstream the hip05 (designware) PCIe host
> support.
>
> I think so, some platform may not support ECAM for root complex,
> which needs special handling of access config space, we may need
> to consider those cases.
>
Yes, it is indeed true. For example, some Cavium ThunderX processors
fall into this category.
Some options I thought of are:
o Use DECLARE_ACPI_MCFG_FIXUP() in the kernel to supply the needed
config space accessors.
o Define additional root_device_ids that imply the needed config space
accessors.
> Thanks
> Hanjun
^ permalink raw reply [flat|nested] 50+ messages in thread
* RE: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 17:39 ` David Daney
@ 2015-11-03 18:00 ` Gabriele Paoloni
0 siblings, 0 replies; 50+ messages in thread
From: Gabriele Paoloni @ 2015-11-03 18:00 UTC (permalink / raw)
To: David Daney, Hanjun Guo
Cc: Lorenzo Pieralisi, Sinan Kaya, Tomasz Nowicki, bhelgaas, arnd,
will.deacon, catalin.marinas, rjw, jiang.liu, robert.richter,
Narinder.Dhillon, Liviu.Dudau, tglx, Wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, Wangzhou (B), liudongdong (C)
Hi David
> -----Original Message-----
> From: David Daney [mailto:ddaney@caviumnetworks.com]
> Sent: 03 November 2015 17:39
> To: Hanjun Guo
> Cc: Lorenzo Pieralisi; Sinan Kaya; Tomasz Nowicki; bhelgaas@google.com;
> arnd@arndb.de; will.deacon@arm.com; catalin.marinas@arm.com; rjw@rjwysocki.net;
> jiang.liu@linux.intel.com; robert.richter@caviumnetworks.com;
> Narinder.Dhillon@caviumnetworks.com; Liviu.Dudau@arm.com; tglx@linutronix.de;
> Wangyijing; Suravee.Suthikulpanit@amd.com; msalter@redhat.com; linux-
> pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> acpi@vger.kernel.org; linux-kernel@vger.kernel.org; Gabriele Paoloni; Wangzhou
> (B); liudongdong (C)
> Subject: Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI
> hostbridge init
>
> On 11/03/2015 07:19 AM, Hanjun Guo wrote:
> > On 11/03/2015 10:15 PM, 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.
> >
> > Cc Gab, Zhou, and Dondong who upstream the hip05 (designware) PCIe host
> > support.
> >
> > I think so, some platform may not support ECAM for root complex,
> > which needs special handling of access config space, we may need
> > to consider those cases.
> >
>
> Yes, it is indeed true. For example, some Cavium ThunderX processors
> fall into this category.
>
> Some options I thought of are:
>
> o Use DECLARE_ACPI_MCFG_FIXUP() in the kernel to supply the needed
> config space accessors.
>
> o Define additional root_device_ids that imply the needed config space
> accessors.
Yes I like this
it would fit designware too
Gab
>
>
> > Thanks
> > Hanjun
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-10-27 16:38 ` [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init Tomasz Nowicki
2015-10-28 11:49 ` Liviu.Dudau
2015-10-28 18:46 ` Sinan Kaya
@ 2015-11-03 16:55 ` Lorenzo Pieralisi
2015-11-04 9:59 ` Tomasz Nowicki
2015-11-04 10:11 ` Tomasz Nowicki
2 siblings, 2 replies; 50+ messages in thread
From: Lorenzo Pieralisi @ 2015-11-03 16:55 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 16:55 ` Lorenzo Pieralisi
@ 2015-11-04 9:59 ` Tomasz Nowicki
2015-11-04 10:11 ` Tomasz Nowicki
1 sibling, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-11-04 9:59 UTC (permalink / raw)
To: Lorenzo Pieralisi, Tomasz Nowicki
Cc: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi
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
>> + * 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.
>
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init
2015-11-03 16:55 ` Lorenzo Pieralisi
2015-11-04 9:59 ` Tomasz Nowicki
@ 2015-11-04 10:11 ` Tomasz Nowicki
1 sibling, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-11-04 10:11 UTC (permalink / raw)
To: Lorenzo Pieralisi, Tomasz Nowicki
Cc: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi
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
>> + * 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 ?
It is not, I will move it out of here, probably to mcfg.c file where we
can apply quirks.
Thanks,
Tomasz
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (10 preceding siblings ...)
2015-10-27 16:38 ` [PATCH V1 11/11] arm64, pci, acpi: Support for ACPI based PCI hostbridge init Tomasz Nowicki
@ 2015-10-30 4:07 ` Jon Masters
2015-10-30 4:50 ` Hanjun Guo
2015-10-30 8:26 ` Tomasz Nowicki
2015-10-30 16:38 ` Suravee Suthikulpanit
` (2 subsequent siblings)
14 siblings, 2 replies; 50+ messages in thread
From: Jon Masters @ 2015-10-30 4:07 UTC (permalink / raw)
To: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, hanjun.guo, Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi
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:
> 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
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@linaro.org
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.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI
2015-10-30 4:07 ` [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Jon Masters
@ 2015-10-30 4:50 ` Hanjun Guo
2015-10-30 8:26 ` Tomasz Nowicki
1 sibling, 0 replies; 50+ messages in thread
From: Hanjun Guo @ 2015-10-30 4:50 UTC (permalink / raw)
To: Jon Masters, Tomasz Nowicki, bhelgaas, arnd, will.deacon,
catalin.marinas, rjw, hanjun.guo, Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi
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:
>> 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
> 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@linaro.org
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI
2015-10-30 4:07 ` [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Jon Masters
2015-10-30 4:50 ` Hanjun Guo
@ 2015-10-30 8:26 ` Tomasz Nowicki
1 sibling, 0 replies; 50+ messages in thread
From: Tomasz Nowicki @ 2015-10-30 8:26 UTC (permalink / raw)
To: Jon Masters, bhelgaas, arnd, will.deacon, catalin.marinas, rjw,
hanjun.guo, Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi
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:
>> 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
> 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@linaro.org
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (11 preceding siblings ...)
2015-10-30 4:07 ` [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Jon Masters
@ 2015-10-30 16:38 ` Suravee Suthikulpanit
2015-12-07 20:31 ` Bjorn Helgaas
2015-12-08 17:43 ` Jeremy Linton
14 siblings, 0 replies; 50+ messages in thread
From: Suravee Suthikulpanit @ 2015-10-30 16:38 UTC (permalink / raw)
To: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, hanjun.guo, Lorenzo.Pieralisi
Cc: jiang.liu, robert.richter, Narinder.Dhillon, ddaney, Liviu.Dudau,
tglx, wangyijing, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi
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:
> 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-upstream
>
> 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
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (12 preceding siblings ...)
2015-10-30 16:38 ` Suravee Suthikulpanit
@ 2015-12-07 20:31 ` Bjorn Helgaas
[not found] ` <20151209100125.GA12632@jayachandranc.netlogicmicro.com>
2015-12-08 17:43 ` Jeremy Linton
14 siblings, 1 reply; 50+ messages in thread
From: Bjorn Helgaas @ 2015-12-07 20:31 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: bhelgaas, arnd, will.deacon, catalin.marinas, rjw, hanjun.guo,
Lorenzo.Pieralisi, jiang.liu, robert.richter, Narinder.Dhillon,
ddaney, Liviu.Dudau, tglx, wangyijing, Suravee.Suthikulpanit,
msalter, linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi
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:
> 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-upstream
>
> 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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI
2015-10-27 16:38 [PATCH V1 00/11] MMCONFIG refactoring and ARM64 PCI hostbridge init based on ACPI Tomasz Nowicki
` (13 preceding siblings ...)
2015-12-07 20:31 ` Bjorn Helgaas
@ 2015-12-08 17:43 ` Jeremy Linton
14 siblings, 0 replies; 50+ messages in thread
From: Jeremy Linton @ 2015-12-08 17:43 UTC (permalink / raw)
To: Tomasz Nowicki, bhelgaas, arnd, will.deacon, catalin.marinas,
rjw, hanjun.guo, Lorenzo.Pieralisi
Cc: linux-kernel, linaro-acpi, linux-pci, Liviu.Dudau, ddaney,
Narinder.Dhillon, linux-acpi, robert.richter,
Suravee.Suthikulpanit, msalter, wangyijing, tglx, jiang.liu,
linux-arm-kernel
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:
> 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
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!
^ permalink raw reply [flat|nested] 50+ messages in thread