linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m
@ 2015-07-13  9:14 Suravee Suthikulpanit
  2015-07-13  9:14 ` [RFCv2 PATCH 1/8] irqdomain: Introduce irq_domain_ops.init_alloc_info Suravee Suthikulpanit
                   ` (8 more replies)
  0 siblings, 9 replies; 13+ messages in thread
From: Suravee Suthikulpanit @ 2015-07-13  9:14 UTC (permalink / raw)
  To: tglx, marc.zyngier, lorenzo.pieralisi, hanjun.guo, tomasz.nowicki
  Cc: rjw, al.stone, catalin.marinas, will.deacon, msalter,
	grant.likely, leo.duran, sherry.hurwitz, linux-arm-kernel,
	linux-kernel, linux-acpi, Suravee Suthikulpanit

ACPI core patches for ARM64 are now upstreamed in 4.1. The PCI support
patches for ARM64 ACPI are also in progress. I am sending out this RFC to
introduce ACPI support for GICv2m. This would allow MSI to work when
booting ACPI.

There are some modifications to the irq_domain and acpi/gsi code.

Due to a large number of prerequisite patches, I have put together a branch
on GitHub for review and testing:

	https://github.com/ssuthiku/linux.git acpi-pci-msi-rfc2

This branch has been tested on AMD Seattle Platform. Any feedback and
comments are appreciated.

Thank you in advance,

Suravee

Changes in RFC2:
    - Rebased to V3 of Introducing per-device MSI domain
      (from Marc Zyngier)
    - Rebased to V2 of Add seld-probe infrastructure and stacked
      irqdomain support for APCI based GICv2/3 init
      (from Hanjun Guo)
    - Introduce GIC MSI frame handle.

Suravee Suthikulpanit (8):
  irqdomain: Introduce irq_domain_ops.init_alloc_info
  gic: Introduce gic_init_irq_alloc_info()
  gicv2m: Convert to use GIC irq_domain_ops.init_alloc_info
  acpi: gsi: Adding acpi_init_irq_alloc_info() hook
  arm64: Adding arch-specific acpi_init_irq_alloc_info
  gic: acpi: Introduce GIC MSI frame handle and helper functions
  gicv2m: Introducing gicv2m_acpi_init()
  pci: acpi: Bind GICv2m MSI frame to PCI host bridge

 arch/arm64/kernel/acpi.c             |  13 ++++
 drivers/acpi/gsi.c                   |  36 ++++++++--
 drivers/irqchip/irq-gic-acpi.c       |  78 +++++++++++++++++++++
 drivers/irqchip/irq-gic-v2m.c        | 129 ++++++++++++++++++++++++++++-------
 drivers/irqchip/irq-gic.c            |  85 +++++++++++++++++++----
 drivers/pci/pci-acpi.c               |  35 ++++++++++
 drivers/pci/probe.c                  |   2 +
 include/linux/irqchip/arm-gic-acpi.h |   3 +
 include/linux/irqchip/arm-gic.h      |  24 +++++++
 include/linux/irqdomain.h            |   2 +
 include/linux/pci-acpi.h             |   1 +
 kernel/irq/irqdomain.c               |  10 ++-
 12 files changed, 372 insertions(+), 46 deletions(-)

-- 
2.1.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFCv2 PATCH 1/8] irqdomain: Introduce irq_domain_ops.init_alloc_info
  2015-07-13  9:14 [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Suravee Suthikulpanit
@ 2015-07-13  9:14 ` Suravee Suthikulpanit
  2015-07-20 21:28   ` Thomas Gleixner
  2015-07-13  9:14 ` [RFCv2 PATCH 2/8] gic: Introduce gic_init_irq_alloc_info() Suravee Suthikulpanit
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 13+ messages in thread
From: Suravee Suthikulpanit @ 2015-07-13  9:14 UTC (permalink / raw)
  To: tglx, marc.zyngier, lorenzo.pieralisi, hanjun.guo, tomasz.nowicki
  Cc: rjw, al.stone, catalin.marinas, will.deacon, msalter,
	grant.likely, leo.duran, sherry.hurwitz, linux-arm-kernel,
	linux-kernel, linux-acpi, Suravee Suthikulpanit

Currently, when calling irq_domain_alloc_irqs() on ARM64, it uses
struct of_phandle_args to pass irq information. However, this is not
appropriate for ACPI since of_phandle_args is specific to DT.

Therefore, this patch introduces a new function pointer,
irq_domain_ops.init_alloc_info, which can be used by irqchips to provide
a way to initialize irqchip-specific data-structure for allocating IRQ.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
NOTE:
Similarly, x86 is currently using struct irq_alloc_info
(see arch/x86/include/asm/hw_irq.h) and each irq_domain has different
way of initializing this structure.

Patch 2 also has an example of how I am planning to use this new op.

Alternative would be to keep re-using of_phandle_args for ACPI as done
currently. Any suggestions / feedbacks here are appreciated.

Thanks,

Suravee

 include/linux/irqdomain.h |  2 ++
 kernel/irq/irqdomain.c    | 10 +++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index b4a74f7..1e51369 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -86,6 +86,8 @@ struct irq_domain_ops {
 	/* extended V2 interfaces to support hierarchy irq_domains */
 	int (*alloc)(struct irq_domain *d, unsigned int virq,
 		     unsigned int nr_irqs, void *arg);
+	int (*init_alloc_info)(uint32_t *data, int nr, void *ref,
+			       void **info);
 	void (*free)(struct irq_domain *d, unsigned int virq,
 		     unsigned int nr_irqs);
 	void (*activate)(struct irq_domain *d, struct irq_data *irq_data);
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 995d217..54434d2 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -478,6 +478,7 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
 	irq_hw_number_t hwirq;
 	unsigned int type = IRQ_TYPE_NONE;
 	int virq;
+	void *info;
 
 	domain = irq_data->np ? irq_find_host(irq_data->np) : irq_default_domain;
 	if (!domain) {
@@ -504,7 +505,14 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
 		if (virq)
 			return virq;
 
-		virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
+		if (domain->ops->init_alloc_info)
+			if (domain->ops->init_alloc_info(irq_data->args,
+							 irq_data->args_count,
+							 irq_data->np,
+							 &info))
+				return 0;
+
+		virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, info);
 		if (virq <= 0)
 			return 0;
 	} else {
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFCv2 PATCH 2/8] gic: Introduce gic_init_irq_alloc_info()
  2015-07-13  9:14 [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Suravee Suthikulpanit
  2015-07-13  9:14 ` [RFCv2 PATCH 1/8] irqdomain: Introduce irq_domain_ops.init_alloc_info Suravee Suthikulpanit
@ 2015-07-13  9:14 ` Suravee Suthikulpanit
  2015-07-13  9:14 ` [RFCv2 PATCH 3/8] gicv2m: Convert to use GIC irq_domain_ops.init_alloc_info Suravee Suthikulpanit
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Suravee Suthikulpanit @ 2015-07-13  9:14 UTC (permalink / raw)
  To: tglx, marc.zyngier, lorenzo.pieralisi, hanjun.guo, tomasz.nowicki
  Cc: rjw, al.stone, catalin.marinas, will.deacon, msalter,
	grant.likely, leo.duran, sherry.hurwitz, linux-arm-kernel,
	linux-kernel, linux-acpi, Suravee Suthikulpanit

Currently, gic_irq_domain_alloc assumes that the arg parameter must be
a pointer to of_phandle_args. This is not appropriate for using with ACPI.

Furthermore, there are several irq mappings (i.e. SPI, PPI, and GSI),
which can be used when allocating GIC irqs. This can be confusing
when used in multiple initialization path (i.e. DT, ACPI, MSI, etc.).

This patch introduces struct gic_irq_alloc_info, which replaces the
of_phandle_args when calling gic_irq_domain_alloc(). Also, it introduces
gic_init_irq_alloc_info(), a new helper function to help simplifying
GIC irq allocation, which hooks into the new
irq_domain_ops.init_alloc_info.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/irqchip/irq-gic.c       | 82 ++++++++++++++++++++++++++++++++++-------
 include/linux/irqchip/arm-gic.h | 11 ++++++
 2 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 7f943ef..8ac8ec4 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -852,22 +852,18 @@ static struct notifier_block gic_cpu_notifier = {
 static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
 				unsigned int nr_irqs, void *arg)
 {
-	int i;
+	int i, ret;
 	irq_hw_number_t hwirq;
+	unsigned int type = IRQ_TYPE_NONE;
+	struct gic_irq_alloc_info *info = arg;
+	u32 intspec[3];
 
-	if (acpi_disabled) {	/* DT case */
-		int ret;
-		unsigned int type = IRQ_TYPE_NONE;
-		struct of_phandle_args *irq_data = arg;
-
-		ret = gic_irq_domain_xlate(domain, irq_data->np,
-					irq_data->args,
-					irq_data->args_count, &hwirq, &type);
-		if (ret)
-			return ret;
-	} else {	/* ACPI case */
-		hwirq = (irq_hw_number_t)*(u32 *)arg;
-	}
+	intspec[0] = info->gic_int_type;
+	intspec[1] = info->hwirq;
+	intspec[2] = info->irq_type;
+	ret = gic_irq_domain_xlate(domain, info->ref, intspec, 3, &hwirq, &type);
+	if (ret)
+		return ret;
 
 	for (i = 0; i < nr_irqs; i++)
 		gic_irq_domain_map(domain, virq + i, hwirq + i);
@@ -875,10 +871,68 @@ static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
 	return 0;
 }
 
+static int gic_init_irq_alloc_info(uint32_t *data, int nr, void *ref,
+				   void **info)
+{
+	struct gic_irq_alloc_info *alloc_info;
+	unsigned int gic_int_type;
+	unsigned int hwirq;
+	unsigned int irq_type;
+
+	if (nr != 3)
+		return -EINVAL;
+
+	gic_int_type = data[0];
+	hwirq = data[1];
+	irq_type = data[2];
+
+	alloc_info = kzalloc(sizeof(struct gic_irq_alloc_info), GFP_KERNEL);
+	if (!alloc_info)
+		return -ENOMEM;
+
+	if ((irq_type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_HIGH &&
+	    (irq_type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_EDGE_RISING)
+		return -EINVAL;
+
+	alloc_info->irq_type = irq_type;
+	alloc_info->ref = ref;
+
+	/*
+	 * ACPI have no bindings to indicate SPI or PPI, so we
+	 * use different mappings from DT in ACPI.
+	 *
+	 * For FDT
+	 * PPI interrupt: in the range [0, 15];
+	 * SPI interrupt: in the range [0, 987];
+	 *
+	 * For ACPI, GSI should be unique so using
+	 * the hwirq directly for the mapping:
+	 * PPI interrupt: in the range [16, 31];
+	 * SPI interrupt: in the range [32, 1019];
+	 */
+
+	if (gic_int_type != GIC_INT_TYPE_GSI) {
+		alloc_info->gic_int_type = gic_int_type;
+		alloc_info->hwirq = hwirq;
+	} else {
+		if (hwirq < 32) {
+			alloc_info->gic_int_type = GIC_INT_TYPE_PPI;
+			alloc_info->hwirq = hwirq - 16;
+		} else {
+			alloc_info->gic_int_type = GIC_INT_TYPE_SPI;
+			alloc_info->hwirq = hwirq - 32;
+		}
+	}
+
+	*info = alloc_info;
+	return 0;
+}
+
 static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
 	.xlate = gic_irq_domain_xlate,
 	.alloc = gic_irq_domain_alloc,
 	.free = irq_domain_free_irqs_top,
+	.init_alloc_info = gic_init_irq_alloc_info,
 };
 
 static const struct irq_domain_ops gic_irq_domain_ops = {
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 9de976b..4808514 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -89,12 +89,23 @@
 #define GICH_MISR_EOI			(1 << 0)
 #define GICH_MISR_U			(1 << 1)
 
+#define GIC_INT_TYPE_SPI		0
+#define GIC_INT_TYPE_PPI		1
+#define GIC_INT_TYPE_GSI		~0U
+
 #ifndef __ASSEMBLY__
 
 #include <linux/irqdomain.h>
 
 struct device_node;
 
+struct gic_irq_alloc_info {
+	void *ref;
+	unsigned int irq_type;
+	unsigned int gic_int_type;
+	unsigned int hwirq;
+};
+
 void gic_set_irqchip_flags(unsigned long flags);
 void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
 		    u32 offset, struct device_node *);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFCv2 PATCH 3/8] gicv2m: Convert to use GIC irq_domain_ops.init_alloc_info
  2015-07-13  9:14 [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Suravee Suthikulpanit
  2015-07-13  9:14 ` [RFCv2 PATCH 1/8] irqdomain: Introduce irq_domain_ops.init_alloc_info Suravee Suthikulpanit
  2015-07-13  9:14 ` [RFCv2 PATCH 2/8] gic: Introduce gic_init_irq_alloc_info() Suravee Suthikulpanit
@ 2015-07-13  9:14 ` Suravee Suthikulpanit
  2015-07-13  9:14 ` [RFCv2 PATCH 4/8] acpi: gsi: Adding acpi_init_irq_alloc_info() hook Suravee Suthikulpanit
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Suravee Suthikulpanit @ 2015-07-13  9:14 UTC (permalink / raw)
  To: tglx, marc.zyngier, lorenzo.pieralisi, hanjun.guo, tomasz.nowicki
  Cc: rjw, al.stone, catalin.marinas, will.deacon, msalter,
	grant.likely, leo.duran, sherry.hurwitz, linux-arm-kernel,
	linux-kernel, linux-acpi, Suravee Suthikulpanit

This patch converts the current call to irq_domain_alloc_irqs_parent()
to use structure initilized by irq_domain_ops.init_alloc_info()
instead of of_phandle_args.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/irqchip/irq-gic-v2m.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index bf5082d..e4e2a92 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -22,6 +22,10 @@
 #include <linux/of_pci.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/irqchip/arm-gic.h>
+#ifdef CONFIG_ARM_GIC_ACPI
+#include <linux/irqchip/arm-gic-acpi.h>
+#endif
 
 /*
 * MSI_TYPER:
@@ -114,17 +118,20 @@ static int gicv2m_irq_gic_domain_alloc(struct irq_domain *domain,
 				       unsigned int virq,
 				       irq_hw_number_t hwirq)
 {
-	struct of_phandle_args args;
 	struct irq_data *d;
 	int err;
+	void *info;
+	struct irq_domain *parent = domain->parent;
+	uint32_t data[3] = {GIC_INT_TYPE_GSI, hwirq, IRQ_TYPE_EDGE_RISING};
+
+	if (parent->ops->init_alloc_info) {
+		err = parent->ops->init_alloc_info(data, 3, parent->of_node,
+						   &info);
+		if (err)
+			return err;
+	}
 
-	args.np = domain->parent->of_node;
-	args.args_count = 3;
-	args.args[0] = 0;
-	args.args[1] = hwirq - 32;
-	args.args[2] = IRQ_TYPE_EDGE_RISING;
-
-	err = irq_domain_alloc_irqs_parent(domain, virq, 1, &args);
+	err = irq_domain_alloc_irqs_parent(domain, virq, 1, info);
 	if (err)
 		return err;
 
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFCv2 PATCH 4/8] acpi: gsi: Adding acpi_init_irq_alloc_info() hook
  2015-07-13  9:14 [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Suravee Suthikulpanit
                   ` (2 preceding siblings ...)
  2015-07-13  9:14 ` [RFCv2 PATCH 3/8] gicv2m: Convert to use GIC irq_domain_ops.init_alloc_info Suravee Suthikulpanit
@ 2015-07-13  9:14 ` Suravee Suthikulpanit
  2015-07-13  9:14 ` [RFCv2 PATCH 5/8] arm64: Adding arch-specific acpi_init_irq_alloc_info Suravee Suthikulpanit
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Suravee Suthikulpanit @ 2015-07-13  9:14 UTC (permalink / raw)
  To: tglx, marc.zyngier, lorenzo.pieralisi, hanjun.guo, tomasz.nowicki
  Cc: rjw, al.stone, catalin.marinas, will.deacon, msalter,
	grant.likely, leo.duran, sherry.hurwitz, linux-arm-kernel,
	linux-kernel, linux-acpi, Suravee Suthikulpanit

This patch adds acpi_init_irq_alloc_info(), which is declared as
a weak symbol. This would allow arch-specific code to hook into
acpi_register_gsi().

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/acpi/gsi.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
index 02c8408..01a14c5 100644
--- a/drivers/acpi/gsi.c
+++ b/drivers/acpi/gsi.c
@@ -62,6 +62,13 @@ int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
 }
 EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
 
+int __weak
+acpi_init_irq_alloc_info(struct irq_domain *domain, u32 gsi,
+			 unsigned int irq_type, void **info)
+{
+	return 0;
+}
+
 /**
  * acpi_register_gsi() - Map a GSI to a linux IRQ number
  * @dev: device for which IRQ has to be mapped
@@ -75,22 +82,41 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
 int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
 		      int polarity)
 {
-	int irq;
+	unsigned int irq;
 	unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
+	struct irq_data *d = NULL;
+	void *info = NULL;
+	int err;
 
 	irq = irq_find_mapping(acpi_irq_domain, gsi);
 	if (irq > 0)
 		return irq;
 
-	irq = irq_domain_alloc_irqs(acpi_irq_domain, 1, dev_to_node(dev),
-				    &gsi);
+	err = acpi_init_irq_alloc_info(acpi_irq_domain, gsi, irq_type, &info);
+	if (err)
+		return err;
+
+	if (!info)
+		/* Default to pass gsi directly to allocate irq */
+		info = &gsi;
+
+	irq = irq_domain_alloc_irqs(acpi_irq_domain, 1, dev_to_node(dev), info);
 	if (irq <= 0)
 		return -EINVAL;
 
+	d = irq_domain_get_irq_data(acpi_irq_domain, irq);
+	if (!d)
+		return -EFAULT;
+
 	/* Set irq type if specified and different than the current one */
 	if (irq_type != IRQ_TYPE_NONE &&
-		irq_type != irq_get_trigger_type(irq))
-		irq_set_irq_type(irq, irq_type);
+	    irq_type != irq_get_trigger_type(irq)) {
+		if (d)
+			d->chip->irq_set_type(d, irq_type);
+		else
+			irq_set_irq_type(irq, irq_type);
+	}
+
 	return irq;
 }
 EXPORT_SYMBOL_GPL(acpi_register_gsi);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFCv2 PATCH 5/8] arm64: Adding arch-specific acpi_init_irq_alloc_info
  2015-07-13  9:14 [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Suravee Suthikulpanit
                   ` (3 preceding siblings ...)
  2015-07-13  9:14 ` [RFCv2 PATCH 4/8] acpi: gsi: Adding acpi_init_irq_alloc_info() hook Suravee Suthikulpanit
@ 2015-07-13  9:14 ` Suravee Suthikulpanit
  2015-07-13  9:14 ` [RFCv2 PATCH 6/8] gic: acpi: Introduce GIC MSI frame handle and helper functions Suravee Suthikulpanit
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Suravee Suthikulpanit @ 2015-07-13  9:14 UTC (permalink / raw)
  To: tglx, marc.zyngier, lorenzo.pieralisi, hanjun.guo, tomasz.nowicki
  Cc: rjw, al.stone, catalin.marinas, will.deacon, msalter,
	grant.likely, leo.duran, sherry.hurwitz, linux-arm-kernel,
	linux-kernel, linux-acpi, Suravee Suthikulpanit

This patch addes ARM64-specific hook for calling arch-specific
irq_domain_ops.ini_alloc_info() before allocating irqs.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 arch/arm64/kernel/acpi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index d6463bb..f3709dc 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -20,6 +20,7 @@
 #include <linux/cpumask.h>
 #include <linux/init.h>
 #include <linux/irq.h>
+#include <linux/irqchip/arm-gic.h>
 #include <linux/irqdomain.h>
 #include <linux/memblock.h>
 #include <linux/of_fdt.h>
@@ -205,3 +206,15 @@ void __init acpi_boot_table_init(void)
 			disable_acpi();
 	}
 }
+
+int acpi_init_irq_alloc_info(struct irq_domain *domain, u32 gsi, unsigned int irq_type,
+			 void **info)
+{
+	int ret;
+	uint32_t data[3] = {GIC_INT_TYPE_GSI, gsi, irq_type};
+
+	if (domain && domain->ops->init_alloc_info)
+		ret = domain->ops->init_alloc_info(data, 3, NULL, info);
+
+	return 0;
+}
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFCv2 PATCH 6/8] gic: acpi: Introduce GIC MSI frame handle and helper functions
  2015-07-13  9:14 [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Suravee Suthikulpanit
                   ` (4 preceding siblings ...)
  2015-07-13  9:14 ` [RFCv2 PATCH 5/8] arm64: Adding arch-specific acpi_init_irq_alloc_info Suravee Suthikulpanit
@ 2015-07-13  9:14 ` Suravee Suthikulpanit
  2015-07-13  9:14 ` [RFCv2 PATCH 7/8] gicv2m: Introducing gicv2m_acpi_init() Suravee Suthikulpanit
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Suravee Suthikulpanit @ 2015-07-13  9:14 UTC (permalink / raw)
  To: tglx, marc.zyngier, lorenzo.pieralisi, hanjun.guo, tomasz.nowicki
  Cc: rjw, al.stone, catalin.marinas, will.deacon, msalter,
	grant.likely, leo.duran, sherry.hurwitz, linux-arm-kernel,
	linux-kernel, linux-acpi, Suravee Suthikulpanit

This patch introdues struct gic_msi_frame_handle, which can be used
as a reference to GIC MSI frame in MADT. It also provides helper
functions to help parsing and getting reference to each MSI frame.
This avoids having to map and parse MADT multiple times.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/irqchip/irq-gic-acpi.c       | 78 ++++++++++++++++++++++++++++++++++++
 include/linux/irqchip/arm-gic-acpi.h |  3 ++
 2 files changed, 81 insertions(+)

diff --git a/drivers/irqchip/irq-gic-acpi.c b/drivers/irqchip/irq-gic-acpi.c
index f0772d9..6684a8f 100644
--- a/drivers/irqchip/irq-gic-acpi.c
+++ b/drivers/irqchip/irq-gic-acpi.c
@@ -16,6 +16,15 @@
 #include <linux/irqchip/arm-gic-acpi.h>
 #include <linux/irqchip/arm-gic-v3.h>
 
+struct gic_msi_frame_handle {
+	struct list_head list;
+	struct acpi_madt_generic_msi_frame *frame;
+};
+
+static LIST_HEAD(msi_frame_list);
+
+static int acpi_num_msi_frame;
+
 /* GIC version presented in MADT GIC distributor structure */
 static u8 gic_version __initdata = ACPI_MADT_GIC_VERSION_NONE;
 
@@ -141,6 +150,75 @@ static int __init acpi_gic_version_init(void)
 	return 0;
 }
 
+static int __init
+acpi_parse_madt_msi(struct acpi_subtable_header *header,
+			const unsigned long end)
+{
+	struct gic_msi_frame_handle *ms;
+	struct acpi_madt_generic_msi_frame *frame;
+
+	frame = (struct acpi_madt_generic_msi_frame *)header;
+	if (BAD_MADT_ENTRY(frame, end))
+		return -EINVAL;
+
+	ms = kzalloc(sizeof(struct gic_msi_frame_handle *), GFP_KERNEL);
+	if (!ms)
+		return -ENOMEM;
+
+	ms->frame = frame;
+
+	list_add(&ms->list, &msi_frame_list);
+
+	return 0;
+}
+
+inline int acpi_get_num_msi_frames(void)
+{
+	return acpi_num_msi_frame;
+}
+
+int __init acpi_madt_msi_frame_init(struct acpi_table_header *table)
+{
+	int ret = 0;
+
+	if (acpi_num_msi_frame > 0)
+		return ret;
+
+	ret = acpi_parse_entries(ACPI_SIG_MADT,
+				 sizeof(struct acpi_table_madt),
+				 acpi_parse_madt_msi, table,
+				 ACPI_MADT_TYPE_GENERIC_MSI_FRAME, 0);
+	if (ret == 0) {
+		pr_debug("No valid ACPI GIC MSI FRAME exist\n");
+		return ret;
+	}
+
+	acpi_num_msi_frame = ret;
+	return 0;
+}
+
+int acpi_get_msi_frame(int index, struct acpi_madt_generic_msi_frame **p)
+{
+	int i = 0;
+	struct gic_msi_frame_handle *m;
+
+	if (index >= acpi_num_msi_frame)
+		return -EINVAL;
+
+	list_for_each_entry(m, &msi_frame_list, list) {
+		if (i == index)
+			break;
+		i++;
+	}
+
+	if (i == acpi_num_msi_frame)
+		return -EINVAL;
+
+	*p = m->frame;
+	return  0;
+}
+
+
 /*
  * This special acpi_table_id is the sentinel at the end of the
  * acpi_table_id[] array of all irqchips. It is automatically placed at
diff --git a/include/linux/irqchip/arm-gic-acpi.h b/include/linux/irqchip/arm-gic-acpi.h
index 0d43f515..9fb3577 100644
--- a/include/linux/irqchip/arm-gic-acpi.h
+++ b/include/linux/irqchip/arm-gic-acpi.h
@@ -22,6 +22,9 @@
 #define ACPI_GICV3_DIST_MEM_SIZE	(SZ_64K)
 
 u8 acpi_gic_version(void);
+int acpi_madt_msi_frame_init(struct acpi_table_header *table);
+int acpi_get_msi_frame(int index, struct acpi_madt_generic_msi_frame **p);
+int acpi_get_num_msi_frames(void);
 
 #endif /* CONFIG_ACPI */
 #endif /* ARM_GIC_ACPI_H_ */
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFCv2 PATCH 7/8] gicv2m: Introducing gicv2m_acpi_init()
  2015-07-13  9:14 [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Suravee Suthikulpanit
                   ` (5 preceding siblings ...)
  2015-07-13  9:14 ` [RFCv2 PATCH 6/8] gic: acpi: Introduce GIC MSI frame handle and helper functions Suravee Suthikulpanit
@ 2015-07-13  9:14 ` Suravee Suthikulpanit
  2015-07-13  9:14 ` [RFCv2 PATCH 8/8] pci: acpi: Bind GICv2m MSI frame to PCI host bridge Suravee Suthikulpanit
  2015-07-17 15:46 ` [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Marc Zyngier
  8 siblings, 0 replies; 13+ messages in thread
From: Suravee Suthikulpanit @ 2015-07-13  9:14 UTC (permalink / raw)
  To: tglx, marc.zyngier, lorenzo.pieralisi, hanjun.guo, tomasz.nowicki
  Cc: rjw, al.stone, catalin.marinas, will.deacon, msalter,
	grant.likely, leo.duran, sherry.hurwitz, linux-arm-kernel,
	linux-kernel, linux-acpi, Suravee Suthikulpanit

This patch introduces gicv2m_acpi_init(), which parse MADT GIC MSI frames,
and use information to initialize GICv2m. It also refactors
gicv2m_init_one() to handle both DT and ACPI initialization path.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
NOTE:
Now that we are setting v2m->domain->bus_token to DOMAIN_BUS_PCI_MSI,
this limits the scope of this particular domain to just PCI-MSI-related
stuff. I am not sure if it would be acceptable to use MSI frame handle as
a reference to assign to v2m->domain->of_node. This same concern is also
applied to patch 8 where we bind the GIC MSI frame to PCI host bridge.

Any suggestions/feedbacks are welcomed.

Thanks,

Suravee

 drivers/irqchip/irq-gic-v2m.c   | 106 +++++++++++++++++++++++++++++++++-------
 drivers/irqchip/irq-gic.c       |   3 ++
 include/linux/irqchip/arm-gic.h |  13 +++++
 3 files changed, 104 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index e4e2a92..c0417e1 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -15,6 +15,7 @@
 
 #define pr_fmt(fmt) "GICv2m: " fmt
 
+#include <linux/acpi.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
@@ -219,8 +220,14 @@ static bool is_msi_spi_valid(u32 base, u32 num)
 	return true;
 }
 
-static int __init gicv2m_init_one(struct device_node *node,
-				  struct irq_domain *parent)
+char gicv2m_domain_name[] = "GICV2M";
+char gicv2m_msi_domain_name[] = "V2M-MSI";
+
+static int __init gicv2m_init_one(struct irq_domain *parent,
+				  u32 *spi_start, u32 *nr_spis,
+				  struct resource *res,
+				  struct device_node *node,
+				  void *msi_frame)
 {
 	int ret;
 	struct v2m_data *v2m;
@@ -232,23 +239,17 @@ static int __init gicv2m_init_one(struct device_node *node,
 		return -ENOMEM;
 	}
 
-	ret = of_address_to_resource(node, 0, &v2m->res);
-	if (ret) {
-		pr_err("Failed to allocate v2m resource.\n");
-		goto err_free_v2m;
-	}
-
-	v2m->base = ioremap(v2m->res.start, resource_size(&v2m->res));
+	v2m->base = ioremap(res->start, resource_size(res));
 	if (!v2m->base) {
 		pr_err("Failed to map GICv2m resource\n");
 		ret = -ENOMEM;
 		goto err_free_v2m;
 	}
+	memcpy(&v2m->res, res, sizeof(struct resource));
 
-	if (!of_property_read_u32(node, "arm,msi-base-spi", &v2m->spi_start) &&
-	    !of_property_read_u32(node, "arm,msi-num-spis", &v2m->nr_spis)) {
-		pr_info("Overriding V2M MSI_TYPER (base:%u, num:%u)\n",
-			v2m->spi_start, v2m->nr_spis);
+	if (*spi_start && *nr_spis) {
+		v2m->spi_start = *spi_start;
+		v2m->nr_spis = *nr_spis;
 	} else {
 		u32 typer = readl_relaxed(v2m->base + V2M_MSI_TYPER);
 
@@ -277,6 +278,8 @@ static int __init gicv2m_init_one(struct device_node *node,
 
 	inner_domain->bus_token = DOMAIN_BUS_PLATFORM_MSI;
 	inner_domain->parent = parent;
+	inner_domain->name = gicv2m_domain_name;
+
 	v2m->domain = pci_msi_create_irq_domain(node, &gicv2m_msi_domain_info,
 						inner_domain);
 	if (!v2m->domain) {
@@ -285,11 +288,11 @@ static int __init gicv2m_init_one(struct device_node *node,
 		goto err_free_domains;
 	}
 
-	spin_lock_init(&v2m->msi_cnt_lock);
+	v2m->domain->name = gicv2m_msi_domain_name;
+	if (!acpi_disabled)
+		v2m->domain->of_node = msi_frame;
 
-	pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
-		(unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
-		v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
+	spin_lock_init(&v2m->msi_cnt_lock);
 
 	return 0;
 
@@ -319,15 +322,82 @@ int __init gicv2m_of_init(struct device_node *node, struct irq_domain *parent)
 
 	for (child = of_find_matching_node(node, gicv2m_device_id); child;
 	     child = of_find_matching_node(child, gicv2m_device_id)) {
+		u32 spi_start = 0, nr_spis = 0;
+		struct resource res;
+
 		if (!of_find_property(child, "msi-controller", NULL))
 			continue;
 
-		ret = gicv2m_init_one(child, parent);
+		ret = of_address_to_resource(child, 0, &res);
+		if (ret) {
+			pr_err("Failed to allocate v2m resource.\n");
+			break;
+		}
+
+		if (!of_property_read_u32(child, "arm,msi-base-spi", &spi_start) &&
+		    !of_property_read_u32(child, "arm,msi-num-spis", &nr_spis))
+			pr_info("Overriding V2M MSI_TYPER (base:%u, num:%u)\n",
+				spi_start, nr_spis);
+
+		ret = gicv2m_init_one(parent, &spi_start, &nr_spis, &res,
+				      child, NULL);
 		if (ret) {
 			of_node_put(node);
 			break;
 		}
+
+		pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", child->name,
+			(unsigned long)res.start, (unsigned long)res.end,
+			spi_start, (spi_start + nr_spis));
 	}
 
 	return ret;
 }
+
+#ifdef CONFIG_ACPI
+int __init gicv2m_acpi_init(struct acpi_table_header *table,
+			    struct irq_domain *parent)
+{
+	int ret = 0;
+	int num, i;
+	struct acpi_madt_generic_msi_frame *cur, *msi_frame;
+
+	ret = acpi_madt_msi_frame_init(table);
+	if (ret)
+		return ret;
+
+	ret = acpi_get_msi_frame(0, &msi_frame);
+	if (ret)
+		return ret;
+
+	num = acpi_get_num_msi_frames();
+
+	for (i = 0, cur = msi_frame; i < num; i++, cur++) {
+		struct resource res;
+		u32 spi_start = 0, nr_spis = 0;
+
+		res.start = cur->base_address;
+		res.end = cur->base_address + 0x1000;
+
+		if (cur->flags & ACPI_MADT_OVERRIDE_SPI_VALUES) {
+			spi_start = cur->spi_base;
+			nr_spis = cur->spi_count;
+
+			pr_info("ACPI overriding V2M MSI_TYPER (base:%u, num:%u)\n",
+				spi_start, nr_spis);
+		}
+
+		ret = gicv2m_init_one(parent, &spi_start, &nr_spis, &res,
+				      NULL, msi_frame);
+		if (ret)
+			break;
+
+		pr_info("MSI frame ID %u: range[%#lx:%#lx], SPI[%d:%d]\n",
+			cur->msi_frame_id,
+			(unsigned long)res.start, (unsigned long)res.end,
+			spi_start, (spi_start + nr_spis));
+	}
+	return ret;
+}
+
+#endif /* CONFIG_ACPI */
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 8ac8ec4..5630e16 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1195,6 +1195,9 @@ gic_v2_acpi_init(struct acpi_table_header *table)
 	gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
 	set_acpi_irq_domain(gic_data[0].domain);
 
+	if (IS_ENABLED(CONFIG_ARM_GIC_V2M))
+		gicv2m_acpi_init(table, gic_data[0].domain);
+
 	acpi_irq_model = ACPI_IRQ_MODEL_GIC;
 	return 0;
 }
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 4808514..de1bd1e 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -118,6 +118,19 @@ static inline void gic_init(unsigned int nr, int start,
 	gic_init_bases(nr, start, dist, cpu, 0, NULL);
 }
 
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+
+int gicv2m_acpi_init(struct acpi_table_header *table,
+		     struct irq_domain *parent);
+#else
+int gicv2m_acpi_init(struct acpi_table_header *table,
+		     struct irq_domain *parent);
+{
+	return 0;
+}
+#endif
+
 int gicv2m_of_init(struct device_node *node, struct irq_domain *parent);
 
 void gic_send_sgi(unsigned int cpu_id, unsigned int irq);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFCv2 PATCH 8/8] pci: acpi: Bind GICv2m MSI frame to PCI host bridge
  2015-07-13  9:14 [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Suravee Suthikulpanit
                   ` (6 preceding siblings ...)
  2015-07-13  9:14 ` [RFCv2 PATCH 7/8] gicv2m: Introducing gicv2m_acpi_init() Suravee Suthikulpanit
@ 2015-07-13  9:14 ` Suravee Suthikulpanit
  2015-07-17 15:46 ` [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Marc Zyngier
  8 siblings, 0 replies; 13+ messages in thread
From: Suravee Suthikulpanit @ 2015-07-13  9:14 UTC (permalink / raw)
  To: tglx, marc.zyngier, lorenzo.pieralisi, hanjun.guo, tomasz.nowicki
  Cc: rjw, al.stone, catalin.marinas, will.deacon, msalter,
	grant.likely, leo.duran, sherry.hurwitz, linux-arm-kernel,
	linux-kernel, linux-acpi, Suravee Suthikulpanit

This patch introduces pci_set_phb_acpi_msi_domain(), which looks for
GIC MSI frame and bind the corresponded GICv2m irq_domain to the
PCI host-bridge.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/pci/pci-acpi.c   | 35 +++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c      |  2 ++
 include/linux/pci-acpi.h |  1 +
 3 files changed, 38 insertions(+)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 314a625..0da6b90 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -9,6 +9,7 @@
 
 #include <linux/delay.h>
 #include <linux/init.h>
+#include <linux/irqdomain.h>
 #include <linux/pci.h>
 #include <linux/pci_hotplug.h>
 #include <linux/module.h>
@@ -18,6 +19,10 @@
 #include <linux/pm_qos.h>
 #include "pci.h"
 
+#ifdef CONFIG_ARM_GIC_ACPI
+#include <linux/irqchip/arm-gic-acpi.h>
+#endif
+
 /*
  * The UUID is defined in the PCI Firmware Specification available here:
  * https://www.pcisig.com/members/downloads/pcifw_r3_1_13Dec10.pdf
@@ -681,6 +686,36 @@ static bool pci_acpi_bus_match(struct device *dev)
 	return dev_is_pci(dev);
 }
 
+#ifdef CONFIG_PCI_MSI
+void pci_set_phb_acpi_msi_domain(struct pci_bus *bus)
+{
+	int err;
+	struct irq_domain *d;
+	struct acpi_madt_generic_msi_frame *msi_frame;
+
+	/**
+	* Since ACPI 5.1 currently does not define
+	* a way to associate MSI frame ID to a device,
+	* we can only support single MSI frame (index 0)
+	* at the moment.
+	*/
+	err = acpi_get_msi_frame(0, &msi_frame);
+	if (err)
+		return;
+
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	d = irq_find_matching_host((struct device_node *)msi_frame,
+					DOMAIN_BUS_PCI_MSI);
+	if (!d) {
+		pr_debug("Fail to find domain for MSI\n");
+		return;
+	}
+
+	dev_set_msi_domain(&bus->dev, d);
+#endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
+}
+#endif /* CONFIG_PCI_MSI */
+
 static struct acpi_bus_type acpi_pci_bus = {
 	.name = "PCI",
 	.match = pci_acpi_bus_match,
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index f305b78..c93474f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/cpumask.h>
 #include <linux/pci-aspm.h>
+#include <linux/pci-acpi.h>
 #include <linux/acpi.h>
 #include <linux/property.h>
 #include <asm-generic/pci-bridge.h>
@@ -683,6 +684,7 @@ static void pci_set_bus_speed(struct pci_bus *bus)
 void __weak pcibios_set_host_bridge_msi_domain(struct pci_bus *bus)
 {
 	pci_set_phb_of_msi_domain(bus);
+	pci_set_phb_acpi_msi_domain(bus);
 }
 
 static void pci_set_bus_msi_domain(struct pci_bus *bus)
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index a76cb6f..96a19c7 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -99,6 +99,7 @@ static inline void acpiphp_enumerate_slots(struct pci_bus *bus) { }
 static inline void acpiphp_remove_slots(struct pci_bus *bus) { }
 static inline void acpiphp_check_host_bridge(struct acpi_device *adev) { }
 #endif
+void pci_set_phb_acpi_msi_domain(struct pci_bus *bus);
 
 extern const u8 pci_acpi_dsm_uuid[];
 #define DEVICE_LABEL_DSM	0x07
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m
  2015-07-13  9:14 [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Suravee Suthikulpanit
                   ` (7 preceding siblings ...)
  2015-07-13  9:14 ` [RFCv2 PATCH 8/8] pci: acpi: Bind GICv2m MSI frame to PCI host bridge Suravee Suthikulpanit
@ 2015-07-17 15:46 ` Marc Zyngier
  2015-07-23  6:49   ` Suravee Suthikulpanit
  8 siblings, 1 reply; 13+ messages in thread
From: Marc Zyngier @ 2015-07-17 15:46 UTC (permalink / raw)
  To: Suravee Suthikulpanit, tglx, Lorenzo Pieralisi, hanjun.guo,
	tomasz.nowicki
  Cc: rjw, al.stone, Catalin Marinas, Will Deacon, msalter,
	grant.likely, leo.duran, sherry.hurwitz, linux-arm-kernel,
	linux-kernel, linux-acpi

Hi Suravee,

On 13/07/15 10:14, Suravee Suthikulpanit wrote:
> ACPI core patches for ARM64 are now upstreamed in 4.1. The PCI support
> patches for ARM64 ACPI are also in progress. I am sending out this RFC to
> introduce ACPI support for GICv2m. This would allow MSI to work when
> booting ACPI.
> 
> There are some modifications to the irq_domain and acpi/gsi code.
> 
> Due to a large number of prerequisite patches, I have put together a branch
> on GitHub for review and testing:
> 
> 	https://github.com/ssuthiku/linux.git acpi-pci-msi-rfc2
> 
> This branch has been tested on AMD Seattle Platform. Any feedback and
> comments are appreciated.

I've had a look at this, and mostly the init_alloc_info method you
introduce. I have a few issues with the concept:

- The first thing that annoys me a tiny bit is that the bottom irqchip
(the GIC in your case) is allocating memory on the behalf of all the
others in the stack, while the actual users are sitting on top. It feels
really backward. Why can't this allocation be performed at the top of
the stack? The order of the request goes from top to bottom anyway, so
what am I missing?

- This gic_irq_alloc_info structure is completely GIC specific, and
contains things that don't make much sense to most domains. Here, it is
only useful to the GICv2m driver, but not to the top MSI layer. So why
should this structure be passed around across domains that don't care?

So I'd like to get back to the intent: why do you need to turn the logic
around? I understand that of_phandle_args is not ideal for ACPI, and I'm
happy to find ways around its limitations. But why do we need to reverse
the allocation logic and make this structure global along the stack,
rather than keeping it for local interaction at the frontier of two domains?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFCv2 PATCH 1/8] irqdomain: Introduce irq_domain_ops.init_alloc_info
  2015-07-13  9:14 ` [RFCv2 PATCH 1/8] irqdomain: Introduce irq_domain_ops.init_alloc_info Suravee Suthikulpanit
@ 2015-07-20 21:28   ` Thomas Gleixner
  2015-07-23  6:50     ` Suravee Suthikulpanit
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Gleixner @ 2015-07-20 21:28 UTC (permalink / raw)
  To: Suravee Suthikulpanit
  Cc: marc.zyngier, lorenzo.pieralisi, hanjun.guo, tomasz.nowicki, rjw,
	al.stone, catalin.marinas, will.deacon, msalter, grant.likely,
	leo.duran, sherry.hurwitz, linux-arm-kernel, linux-kernel,
	linux-acpi

On Mon, 13 Jul 2015, Suravee Suthikulpanit wrote:

> Currently, when calling irq_domain_alloc_irqs() on ARM64, it uses
> struct of_phandle_args to pass irq information. However, this is not
> appropriate for ACPI since of_phandle_args is specific to DT.
> 
> Therefore, this patch introduces a new function pointer,
> irq_domain_ops.init_alloc_info, which can be used by irqchips to provide
> a way to initialize irqchip-specific data-structure for allocating IRQ.
> 
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> ---
> NOTE:
> Similarly, x86 is currently using struct irq_alloc_info
> (see arch/x86/include/asm/hw_irq.h) and each irq_domain has different
> way of initializing this structure.

And why don't you use the same mechanism on ARM and have a private
irq_alloc_info implementation which can carry either DT or ACPI
information?
 
Thanks,

	tglx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m
  2015-07-17 15:46 ` [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Marc Zyngier
@ 2015-07-23  6:49   ` Suravee Suthikulpanit
  0 siblings, 0 replies; 13+ messages in thread
From: Suravee Suthikulpanit @ 2015-07-23  6:49 UTC (permalink / raw)
  To: Marc Zyngier, tglx, Lorenzo Pieralisi, hanjun.guo, tomasz.nowicki
  Cc: rjw, al.stone, Catalin Marinas, Will Deacon, msalter,
	grant.likely, leo.duran, sherry.hurwitz, linux-arm-kernel,
	linux-kernel, linux-acpi

Hi Marc,

Sorry for delay reply. Please see my comments below.

On 7/17/15 22:46, Marc Zyngier wrote:
> Hi Suravee,
>
> On 13/07/15 10:14, Suravee Suthikulpanit wrote:
>> ACPI core patches for ARM64 are now upstreamed in 4.1. The PCI support
>> patches for ARM64 ACPI are also in progress. I am sending out this RFC to
>> introduce ACPI support for GICv2m. This would allow MSI to work when
>> booting ACPI.
>>
>> There are some modifications to the irq_domain and acpi/gsi code.
>>
>> Due to a large number of prerequisite patches, I have put together a branch
>> on GitHub for review and testing:
>>
>> 	https://github.com/ssuthiku/linux.git acpi-pci-msi-rfc2
>>
>> This branch has been tested on AMD Seattle Platform. Any feedback and
>> comments are appreciated.
>
> I've had a look at this, and mostly the init_alloc_info method you
> introduce. I have a few issues with the concept:
>
> - The first thing that annoys me a tiny bit is that the bottom irqchip
> (the GIC in your case) is allocating memory on the behalf of all the
> others in the stack, while the actual users are sitting on top. It feels
> really backward. Why can't this allocation be performed at the top of
> the stack? The order of the request goes from top to bottom anyway, so
> what am I missing?

The reason I am allocating struct gic_irq_alloc_info in 
gic_init_irq_alloc_info() is because this structure is specific to GIC, 
and GIC have control of what information it would need to store in this 
structure. The upper levels (ACPI, DT, and GICv2m domains) should not 
need to know about the detail of this structure. They mainly just need 
to keep track of the handle for this structure, and pass it into the 
irq_domain_alloc_irqs().

> - This gic_irq_alloc_info structure is completely GIC specific, and
> contains things that don't make much sense to most domains.

Exactly. Also, I think another benefits is to consolidate the different 
mapping that GIC supports (e.g. SPI, PPI, GSI) used in different places. 
Please refer to patch 2.

> Here, it is
> only useful to the GICv2m driver, but not to the top MSI layer. So why
> should this structure be passed around across domains that don't care?

Actually, this structure is not used just by the GICv2m driver. It's 
also used by the ACPI acpi_register_gsi(), which is also allocating 
interrupts with GSI mapping.

> So I'd like to get back to the intent: why do you need to turn the logic
> around? I understand that of_phandle_args is not ideal for ACPI, and I'm
> happy to find ways around its limitations. But why do we need to reverse
> the allocation logic and make this structure global along the stack,
> rather than keeping it for local interaction at the frontier of two domains?

Please see the explanation above. Let me know if you have other ideas or 
if I am missing your point on the reverse allocation.

Thanks,
Suravee

> Thanks,
>
> 	M.
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFCv2 PATCH 1/8] irqdomain: Introduce irq_domain_ops.init_alloc_info
  2015-07-20 21:28   ` Thomas Gleixner
@ 2015-07-23  6:50     ` Suravee Suthikulpanit
  0 siblings, 0 replies; 13+ messages in thread
From: Suravee Suthikulpanit @ 2015-07-23  6:50 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: marc.zyngier, lorenzo.pieralisi, hanjun.guo, tomasz.nowicki, rjw,
	al.stone, catalin.marinas, will.deacon, msalter, grant.likely,
	leo.duran, sherry.hurwitz, linux-arm-kernel, linux-kernel,
	linux-acpi



On 7/21/15 04:28, Thomas Gleixner wrote:
> On Mon, 13 Jul 2015, Suravee Suthikulpanit wrote:
>
>> Currently, when calling irq_domain_alloc_irqs() on ARM64, it uses
>> struct of_phandle_args to pass irq information. However, this is not
>> appropriate for ACPI since of_phandle_args is specific to DT.
>>
>> Therefore, this patch introduces a new function pointer,
>> irq_domain_ops.init_alloc_info, which can be used by irqchips to provide
>> a way to initialize irqchip-specific data-structure for allocating IRQ.
>>
>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>> ---
>> NOTE:
>> Similarly, x86 is currently using struct irq_alloc_info
>> (see arch/x86/include/asm/hw_irq.h) and each irq_domain has different
>> way of initializing this structure.
>
> And why don't you use the same mechanism on ARM and have a private
> irq_alloc_info implementation which can carry either DT or ACPI
> information?

Let me look further into this. I would like to take a similar approach here.

Suravee

> Thanks,
>
> 	tglx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-07-23  7:06 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-13  9:14 [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Suravee Suthikulpanit
2015-07-13  9:14 ` [RFCv2 PATCH 1/8] irqdomain: Introduce irq_domain_ops.init_alloc_info Suravee Suthikulpanit
2015-07-20 21:28   ` Thomas Gleixner
2015-07-23  6:50     ` Suravee Suthikulpanit
2015-07-13  9:14 ` [RFCv2 PATCH 2/8] gic: Introduce gic_init_irq_alloc_info() Suravee Suthikulpanit
2015-07-13  9:14 ` [RFCv2 PATCH 3/8] gicv2m: Convert to use GIC irq_domain_ops.init_alloc_info Suravee Suthikulpanit
2015-07-13  9:14 ` [RFCv2 PATCH 4/8] acpi: gsi: Adding acpi_init_irq_alloc_info() hook Suravee Suthikulpanit
2015-07-13  9:14 ` [RFCv2 PATCH 5/8] arm64: Adding arch-specific acpi_init_irq_alloc_info Suravee Suthikulpanit
2015-07-13  9:14 ` [RFCv2 PATCH 6/8] gic: acpi: Introduce GIC MSI frame handle and helper functions Suravee Suthikulpanit
2015-07-13  9:14 ` [RFCv2 PATCH 7/8] gicv2m: Introducing gicv2m_acpi_init() Suravee Suthikulpanit
2015-07-13  9:14 ` [RFCv2 PATCH 8/8] pci: acpi: Bind GICv2m MSI frame to PCI host bridge Suravee Suthikulpanit
2015-07-17 15:46 ` [RFCv2 PATCH 0/8] Introducing ACPI support for GICv2m Marc Zyngier
2015-07-23  6:49   ` Suravee Suthikulpanit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).