[RFC PATCH 0/3] ACPI PCI support for arm64

[RFC PATCH 0/3] ACPI PCI support for arm64

[Jayachandran C]

This is a very simple and generic implementation of a PCI host controller
based on ACPI. This approach does not pull in the MMCONFIG and ECAM code
from x86.

It is important for us to have a working ACPI based PCI host controller
implementation for arm64, so I thought I would post this as a simple
and less disruptive alternative.

This is tested with arm64 QEMU and OVMF. Comments are very welcome.

Thanks,
JC.

Jayachandran C (3):
  arm64: pci: Add ACPI support
  pci: Handle NULL parent in pci_bus_assign_domain_nr
  pci/host : Add a generic ACPI based host controller

 arch/arm64/kernel/pci.c          |  47 ++++++++-
 drivers/pci/host/Kconfig         |   7 ++
 drivers/pci/host/Makefile        |   1 +
 drivers/pci/host/pci-host-acpi.c | 211 +++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.c                |   7 +-
 5 files changed, 270 insertions(+), 3 deletions(-)
 create mode 100644 drivers/pci/host/pci-host-acpi.c

-- 
1.9.1

[PATCH 1/3] arm64: pci: Add ACPI support

[Jayachandran C]

Add functions needed for ACPI support.

pci_acpi_scan_root(struct acpi_pci_root *root) is marked as weak so
that it can be implemented by the generic acpi pci driver.
pcibios_root_bridge_prepare assumes that the sysdata is struct
acpi_pci_root_info and sets up the domain and ACPI companion

The functions pcibios_enable_device and pcibios_disable_device have
been update to handle ACPI irq enable and disable. And
pcibios_add_bus and pcibios_remove_bus have been added call the
corresponding ACPI functions.

Signed-off-by: Jayachandran C <jchandra@broadcom.com>
---
 arch/arm64/kernel/pci.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index b3d098b..8597624 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -17,6 +17,7 @@
 #include <linux/mm.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>
@@ -48,9 +49,22 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
 	if (pci_has_flag(PCI_PROBE_ONLY))
 		return 0;
 
+#ifdef CONFIG_ACPI
+	if (acpi_find_root_bridge_handle(dev))
+		acpi_pci_irq_enable(dev);
+#endif
+
 	return pci_enable_resources(dev, mask);
 }
 
+void pcibios_disable_device(struct pci_dev *dev)
+{
+#ifdef CONFIG_ACPI
+	if (acpi_find_root_bridge_handle(dev))
+		acpi_pci_irq_disable(dev);
+#endif
+}
+
 /*
  * Try to assign the IRQ number from DT when adding a new device
  */
@@ -78,9 +92,38 @@ int raw_pci_write(unsigned int domain, unsigned int bus,
 
 #ifdef CONFIG_ACPI
 /* Root bridge scanning */
-struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
+struct pci_bus *__weak pci_acpi_scan_root(struct acpi_pci_root *root)
 {
-	/* TODO: Should be revisited when implementing PCI on ACPI */
 	return NULL;
 }
+
+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);
+}
+
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	if (!acpi_disabled) {
+		struct pci_bus *b = bridge->bus;
+		struct acpi_pci_root_info *ci = b->sysdata;
+
+		ACPI_COMPANION_SET(&bridge->dev, ci->bridge);
+		b->domain_nr = ci->root->segment;
+	}
+	return 0;
+}
+
+static int __init pcibios_assign_resources(void)
+{
+	pci_assign_unassigned_resources();
+	return 0;
+}
+
+fs_initcall(pcibios_assign_resources);
 #endif
-- 
1.9.1

[PATCH 2/3] pci: Handle NULL parent in pci_bus_assign_domain_nr

[Jayachandran C]

pci_create_root_bus is called with NULL  parent from ACPI. On arm64,
this ends up calling pci_bus_assign_domain_nr, which crashes when
dereferencing parent.

To fix this, we update pci_bus_assign_domain_nr to return if parent
is NULL. Setting up the domain number will be handled from
pcibios_root_bridge_prepare on arm64 when booted with ACPI.

Signed-off-by: Jayachandran C <jchandra@broadcom.com>
---
 drivers/pci/pci.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 314db8c..a96c356 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4772,8 +4772,13 @@ 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;
 
+	/* in case of ACPI, parent is NULL */
+	if (parent == NULL)
+		return;
+
+	domain = of_get_pci_domain_nr(parent->of_node);
 	/*
 	 * Check DT domain and use_dt_domains values.
 	 *
-- 
1.9.1

[PATCH 3/3] pci/host : Add a generic ACPI based host controller

[Jayachandran C]

Add a simple ACPI based PCI host controller. This is done by
providing a simple implementation of pci_acpi_scan_root().
The MCFG table is parsed early and saved so that it can be
mapped when ACPI calls pci_acpi_scan_root.

This is enabled only for ARM64 now.

Signed-off-by: Jayachandran C <jchandra@broadcom.com>
---
 drivers/pci/host/Kconfig         |   7 ++
 drivers/pci/host/Makefile        |   1 +
 drivers/pci/host/pci-host-acpi.c | 211 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 219 insertions(+)
 create mode 100644 drivers/pci/host/pci-host-acpi.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index f131ba9..8321efc 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -53,6 +53,13 @@ config PCI_RCAR_GEN2_PCIE
 	help
 	  Say Y here if you want PCIe controller support on R-Car Gen2 SoCs.
 
+config PCI_HOST_GENERIC_ACPI
+	bool "Generic ACPI PCI host controller"
+	depends on ARM64 && ACPI
+	help
+	  Say Y here if you want to support a simple generic ACPI PCI host
+	  controller.
+
 config PCI_HOST_GENERIC
 	bool "Generic PCI host controller"
 	depends on (ARM || ARM64) && OF
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 9d4d3c6..bc31852 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
 obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
 obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
 obj-$(CONFIG_PCI_RCAR_GEN2_PCIE) += pcie-rcar.o
+obj-$(CONFIG_PCI_HOST_GENERIC_ACPI) += pci-host-acpi.o
 obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
 obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
 obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
diff --git a/drivers/pci/host/pci-host-acpi.c b/drivers/pci/host/pci-host-acpi.c
new file mode 100644
index 0000000..3aade43
--- /dev/null
+++ b/drivers/pci/host/pci-host-acpi.c
@@ -0,0 +1,211 @@
+/*
+ * Generic PCI host controller driver for ACPI based systems
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright (c) 2015 Broadcom Corporation
+ *
+ * Based on drivers/pci/host/pci-host-generic.c
+ * Copyright (C) 2014 ARM Limited
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/acpi.h>
+#include <linux/pci-acpi.h>
+#include <linux/sfi_acpi.h>
+#include <linux/slab.h>
+
+#define PREFIX			"pci-host-acpi:"
+#define MCFG_NAMELEN		32
+#define MCFG_SHIFT		20
+
+/* ECFG window for this root bus */
+struct gen_mcfg_window {
+	struct resource		res;
+	unsigned int		domain_nr;
+	unsigned int		bus_start;
+	unsigned int		bus_end;
+	char			name[MCFG_NAMELEN];
+	void __iomem		*win;
+};
+
+/* sysdata pointer is ->root_info */
+struct gen_acpi_pci {
+	struct acpi_pci_root_info	root_info;
+	struct gen_mcfg_window		cfg;
+};
+
+/* MCFG entries  */
+struct mcfg_entry {
+	int			segment;
+	int			bus_start;
+	int			bus_end;
+	u64			addr;
+};
+
+static struct mcfg_entries {
+	int			size;
+	struct mcfg_entry	*entries;
+} mcfg_sav;
+
+/* find mapping of a MCFG area */
+static void __iomem *gen_acpi_map_cfg_bus(struct pci_bus *bus,
+			unsigned int devfn, int where)
+{
+	struct gen_acpi_pci *pci = bus->sysdata;
+	struct gen_mcfg_window *cfg = &pci->cfg;
+
+	if (bus->number < cfg->bus_start || bus->number > cfg->bus_end)
+		return NULL;
+
+	return cfg->win + ((bus->number - cfg->bus_start) << MCFG_SHIFT) +
+			((devfn << 12) | where);
+}
+
+/* Map the ECFG area for a root bus */
+static int gen_acpi_pci_map_mcfg(struct acpi_pci_root *root,
+				struct gen_acpi_pci *pci)
+{
+	struct gen_mcfg_window *cfg = &pci->cfg;
+	struct acpi_device *device = root->device;
+	void __iomem  *vaddr;
+
+	/* if there is info from _CBA, use that, otherwise use MCFG table */
+	if (root->mcfg_addr) {
+		cfg->bus_start = root->secondary.start;
+		cfg->bus_end = root->secondary.end;
+		cfg->res.start = root->mcfg_addr;
+	} else {
+		struct mcfg_entry *e = mcfg_sav.entries;
+		int i, n = mcfg_sav.size;
+
+		for (i = 0; i < n; i++, e++)
+			if (e->segment == root->segment)
+				break;
+		if (i >= n)
+			return -ENODEV;
+		cfg->bus_start = e->bus_start;
+		cfg->bus_end = e->bus_end;
+		cfg->res.start = e->addr;
+	}
+
+	cfg->res.flags = IORESOURCE_MEM;
+	cfg->res.name = cfg->name;
+	cfg->res.end = cfg->res.start +
+			((cfg->bus_end - cfg->bus_start + 1) << MCFG_SHIFT) - 1;
+	snprintf(cfg->name, MCFG_NAMELEN, "PCI MMCONFIG %04x [bus %02x-%02x]",
+		 root->segment, cfg->bus_start, cfg->bus_end);
+
+	/* map ECFG space for the bus range */
+	vaddr = devm_ioremap_resource(&device->dev, &cfg->res);
+	if (IS_ERR(vaddr))
+		return PTR_ERR(vaddr);
+
+	cfg->win = vaddr;
+	return 0;
+}
+
+static struct pci_ops gen_acpi_pci_ops = {
+	.map_bus	= gen_acpi_map_cfg_bus,
+	.read		= pci_generic_config_read,
+	.write		= pci_generic_config_write,
+};
+
+static struct acpi_pci_root_ops pci_acpi_root_ops = {
+	.pci_ops = &gen_acpi_pci_ops,
+};
+
+struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
+{
+	struct acpi_device *device = root->device;
+	struct gen_acpi_pci *pci;
+	struct pci_bus *bus, *child;
+	int err;
+
+	pci = devm_kzalloc(&device->dev, sizeof(*pci), GFP_KERNEL);
+	if (!pci) {
+		dev_err(&device->dev,
+			"pci_bus %04x:%02x: ignored (out of memory)\n",
+			root->segment, (int)root->secondary.start);
+		return NULL;
+	}
+
+	err = gen_acpi_pci_map_mcfg(root, pci);
+	if (err) {
+		dev_err(&device->dev, "MCFG lookup for domain %d failed",
+			root->segment);
+		return NULL;
+	}
+	bus =  acpi_pci_root_create(root, &pci_acpi_root_ops,
+					&pci->root_info, pci);
+	if (!bus) {
+		dev_err(&device->dev, "Scanning rootbus failed");
+		return NULL;
+	}
+
+	list_for_each_entry(child, &bus->children, node)
+		pcie_bus_configure_settings(child);
+
+	pci_bus_add_devices(bus);
+	return bus;
+}
+
+/* save MCFG entries */
+static __init int handle_mcfg(struct acpi_table_header *header)
+{
+	struct acpi_table_mcfg *mcfg;
+	struct acpi_mcfg_allocation *mptr;
+	struct mcfg_entry *e;
+	int i, n;
+
+	if (!header)
+		return -EINVAL;
+
+	mcfg = (struct acpi_table_mcfg *)header;
+	mptr = (struct acpi_mcfg_allocation *) &mcfg[1];
+	n = (header->length - sizeof(*mcfg)) / sizeof(*mptr);
+	if (n <= 0 || n > 255) {
+		pr_err(PREFIX " MCFG has incorrect entries (%d).\n", n);
+		return -EINVAL;
+	}
+	mcfg_sav.entries = e = kcalloc(n, sizeof(*e), GFP_KERNEL);
+	if (e == NULL)
+		return -ENOMEM;
+	mcfg_sav.size = n;
+	for (i = 0; i < n; i++, mptr++) {
+		e->segment = mptr->pci_segment;
+		e->bus_start = mptr->start_bus_number;
+		e->bus_end = mptr->end_bus_number;
+		e->addr = mptr->address;
+	}
+	return 0;
+}
+
+static __init int parse_save_mcfg(void)
+{
+	int err;
+
+	err = acpi_sfi_table_parse(ACPI_SIG_MCFG, handle_mcfg);
+	if (err) {
+		pr_err(PREFIX " Failed to parse MCFG (%d)\n", err);
+		mcfg_sav.size = -1;
+	} else {
+		pr_info(PREFIX " MCFG table at %p, %d entries.\n",
+			mcfg_sav.entries, mcfg_sav.size);
+	}
+	return err;
+}
+
+arch_initcall(parse_save_mcfg);
-- 
1.9.1

[RFC PATCH 0/3] ACPI PCI support for arm64

[Lorenzo Pieralisi]

[Resend, LAKML copied-in]

On Thu, Dec 03, 2015 at 10:56:28AM +0000, Lorenzo Pieralisi wrote:
> [CC'ing Tomasz]
> 
> On Thu, Dec 03, 2015 at 03:54:43AM +0530, Jayachandran C wrote:
> > This is a very simple and generic implementation of a PCI host controller
> > based on ACPI. This approach does not pull in the MMCONFIG and ECAM code
> > from x86.
> 
> Why ? Tomasz's patchset does not move MMCONFIG and ECAM code to the generic
> PCI layer for fun, it is generic code and should be shared by all
> architectures and most importantly we should not add more churn on
> top of it which would complicate consolidation even further.
> 
> > It is important for us to have a working ACPI based PCI host controller
> > implementation for arm64, so I thought I would post this as a simple
> > and less disruptive alternative.
> 
> It is important for everyone but that's not a reason granting shortcuts.
> 
> > This is tested with arm64 QEMU and OVMF. Comments are very welcome.
> 
> Tomasz's patch went through several review cycles, please help review
> it and test it, that's my comment.
> 
> A new version should be posted soon, previous version here:
> 
> https://lkml.org/lkml/2015/10/27/504
> 
> Thanks,
> Lorenzo
> 
> > 
> > Thanks,
> > JC.
> > 
> > Jayachandran C (3):
> >   arm64: pci: Add ACPI support
> >   pci: Handle NULL parent in pci_bus_assign_domain_nr
> >   pci/host : Add a generic ACPI based host controller
> > 
> >  arch/arm64/kernel/pci.c          |  47 ++++++++-
> >  drivers/pci/host/Kconfig         |   7 ++
> >  drivers/pci/host/Makefile        |   1 +
> >  drivers/pci/host/pci-host-acpi.c | 211 +++++++++++++++++++++++++++++++++++++++
> >  drivers/pci/pci.c                |   7 +-
> >  5 files changed, 270 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/pci/host/pci-host-acpi.c
> > 
> > -- 
> > 1.9.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 

[RFC PATCH 0/3] ACPI PCI support for arm64

[Jayachandran C.]

On Thu, Dec 03, 2015 at 10:56:28AM +0000, Lorenzo Pieralisi wrote:
> [CC'ing Tomasz]
> 
> On Thu, Dec 03, 2015 at 03:54:43AM +0530, Jayachandran C wrote:
> > This is a very simple and generic implementation of a PCI host controller
> > based on ACPI. This approach does not pull in the MMCONFIG and ECAM code
> > from x86.
> 
> Why ? Tomasz's patchset does not move MMCONFIG and ECAM code to the generic
> PCI layer for fun,

Honestly, Lorenzo, I can see that it was not fun :)

> it is generic code and should be shared by all
> architectures and most importantly we should not add more churn on
> top of it which would complicate consolidation even further.

There is no need for such a common/generic infrastructure to
walk thru a simple table. In my opinion, trying to do this added
a lot of complexity into what should be a very simple patchset.

The x86 code has to deal with a lot more fw/hw issues and I fail
to see why that mess should be brought into the a new architecture.

> > It is important for us to have a working ACPI based PCI host controller
> > implementation for arm64, so I thought I would post this as a simple
> > and less disruptive alternative.
> 
> It is important for everyone but that's not a reason granting shortcuts.
 
I am suggesting a simpler (and maybe better) implementation, which
is much easier to review and merge - not a shortcut.

> > This is tested with arm64 QEMU and OVMF. Comments are very welcome.
> 
> Tomasz's patch went through several review cycles, please help review
> it and test it, that's my comment.
> 
> A new version should be posted soon, previous version here:
> 
> https://lkml.org/lkml/2015/10/27/504

It seems to be v1, I did not see any ACKs from maintainers either -
sorry if I am missing something here.

JC.

[RFC PATCH 0/3] ACPI PCI support for arm64

[Lorenzo Pieralisi]

On Fri, Dec 04, 2015 at 12:11:30AM +0530, Jayachandran C. wrote:
> On Thu, Dec 03, 2015 at 10:56:28AM +0000, Lorenzo Pieralisi wrote:
> > [CC'ing Tomasz]
> > 
> > On Thu, Dec 03, 2015 at 03:54:43AM +0530, Jayachandran C wrote:
> > > This is a very simple and generic implementation of a PCI host controller
> > > based on ACPI. This approach does not pull in the MMCONFIG and ECAM code
> > > from x86.
> > 
> > Why ? Tomasz's patchset does not move MMCONFIG and ECAM code to the generic
> > PCI layer for fun,
> 
> Honestly, Lorenzo, I can see that it was not fun :)

Ok, maybe you have not noticed it was done on Bjorn's request to
consolidate ECAM (ie pulling that code out of x86) instead of reinventing
the wheel for arm64.

> > it is generic code and should be shared by all
> > architectures and most importantly we should not add more churn on
> > top of it which would complicate consolidation even further.
> 
> There is no need for such a common/generic infrastructure to
> walk thru a simple table. In my opinion, trying to do this added
> a lot of complexity into what should be a very simple patchset.
> 
> The x86 code has to deal with a lot more fw/hw issues and I fail
> to see why that mess should be brought into the a new architecture.

See above. We can't keep adding code that does the same thing as
code already in the mainline.

> > > It is important for us to have a working ACPI based PCI host controller
> > > implementation for arm64, so I thought I would post this as a simple
> > > and less disruptive alternative.
> > 
> > It is important for everyone but that's not a reason granting shortcuts.
>  
> I am suggesting a simpler (and maybe better) implementation, which
> is much easier to review and merge - not a shortcut.

You are suggesting adding code to parse the MCFG table, it exists today
and should be consolidated instead of adding on top of it.

> > > This is tested with arm64 QEMU and OVMF. Comments are very welcome.
> > 
> > Tomasz's patch went through several review cycles, please help review
> > it and test it, that's my comment.
> > 
> > A new version should be posted soon, previous version here:
> > 
> > https://lkml.org/lkml/2015/10/27/504
> 
> It seems to be v1, I did not see any ACKs from maintainers either -
> sorry if I am missing something here.

That patchset went through several reviews already, they just reset the
version. I agree with you it is time we got this done, with Tomasz's code,
your code or any combination thereof, please help us get this through,
thanks.

Lorenzo