All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Introducing per-device MSI domain
@ 2014-12-08 20:12 ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	Yijing Wang, Suravee Suthikulpanit

MSI-like interrupts are starting to creep out of the PCI world, and
can now be seen into a number of "platform"-type busses. The MSI
domain patches recognise that fact, and start providing a way to
implement this.

Another problem we have to solve is to identify which MSI domain a
device is "connected" to. Currently, PCI gets away with a mixture of
arch-specific callbacks, and a msi_controller structure that can
optionally carry a pointer to an MSI domain. As we add new bus types
and start dealing with topologies that do not map to what PCI does,
this doesn't scale anymore.

This patch series tries to address some of it by providing a basic
link between 'struct device' and an MSI domain. It also adds (yet
another) way for PCI to propagate the domain pointer through the PCI
device hierarchy, provides a method for OF to kickstart the
propagation process, and finally allows the PCI/MSI layer to use that
information. Hopefully this can serve as a model to implement support
for different but types.

Additionally, the last two patches use all the above to remove any
trace of the msi_controller structure from the two GIC interrupt
controllers we use on arm64, so that they solely rely on the above
infrastructure.

This has been tested on arm64 with GICv2m (AMD Seattle) and GICv3 ITS
(FVP model).

Patches are on top of 3.18-rc7 + tip/irq/irq-domain-arm, and available at:

git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/msi_domain

As always, comments most welcome.

	M.

Marc Zyngier (6):
  device core: Introduce per-device MSI domain pointer
  PCI/MSI: add hooks to populate the msi_domain field
  PCI/MSI: of: add support for OF-provided msi_domain
  PCI/MSI: Let pci_msi_get_domain use struct device's msi_domain
  irqchip: GICv2m: Get rid of struct msi_controller
  irqchip: gicv3-its: Get rid of struct msi_controller

 drivers/irqchip/irq-gic-v2m.c    | 26 +++++++++-----------------
 drivers/irqchip/irq-gic-v3-its.c | 29 ++++++++++++-----------------
 drivers/pci/msi.c                |  3 ++-
 drivers/pci/of.c                 | 15 +++++++++++++++
 drivers/pci/probe.c              | 25 +++++++++++++++++++++++++
 include/linux/device.h           | 20 ++++++++++++++++++++
 include/linux/pci.h              |  3 +++
 7 files changed, 86 insertions(+), 35 deletions(-)

-- 
2.1.3


^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 0/6] Introducing per-device MSI domain
@ 2014-12-08 20:12 ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: linux-arm-kernel

MSI-like interrupts are starting to creep out of the PCI world, and
can now be seen into a number of "platform"-type busses. The MSI
domain patches recognise that fact, and start providing a way to
implement this.

Another problem we have to solve is to identify which MSI domain a
device is "connected" to. Currently, PCI gets away with a mixture of
arch-specific callbacks, and a msi_controller structure that can
optionally carry a pointer to an MSI domain. As we add new bus types
and start dealing with topologies that do not map to what PCI does,
this doesn't scale anymore.

This patch series tries to address some of it by providing a basic
link between 'struct device' and an MSI domain. It also adds (yet
another) way for PCI to propagate the domain pointer through the PCI
device hierarchy, provides a method for OF to kickstart the
propagation process, and finally allows the PCI/MSI layer to use that
information. Hopefully this can serve as a model to implement support
for different but types.

Additionally, the last two patches use all the above to remove any
trace of the msi_controller structure from the two GIC interrupt
controllers we use on arm64, so that they solely rely on the above
infrastructure.

This has been tested on arm64 with GICv2m (AMD Seattle) and GICv3 ITS
(FVP model).

Patches are on top of 3.18-rc7 + tip/irq/irq-domain-arm, and available at:

git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/msi_domain

As always, comments most welcome.

	M.

Marc Zyngier (6):
  device core: Introduce per-device MSI domain pointer
  PCI/MSI: add hooks to populate the msi_domain field
  PCI/MSI: of: add support for OF-provided msi_domain
  PCI/MSI: Let pci_msi_get_domain use struct device's msi_domain
  irqchip: GICv2m: Get rid of struct msi_controller
  irqchip: gicv3-its: Get rid of struct msi_controller

 drivers/irqchip/irq-gic-v2m.c    | 26 +++++++++-----------------
 drivers/irqchip/irq-gic-v3-its.c | 29 ++++++++++++-----------------
 drivers/pci/msi.c                |  3 ++-
 drivers/pci/of.c                 | 15 +++++++++++++++
 drivers/pci/probe.c              | 25 +++++++++++++++++++++++++
 include/linux/device.h           | 20 ++++++++++++++++++++
 include/linux/pci.h              |  3 +++
 7 files changed, 86 insertions(+), 35 deletions(-)

-- 
2.1.3

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 1/6] device core: Introduce per-device MSI domain pointer
  2014-12-08 20:12 ` Marc Zyngier
@ 2014-12-08 20:12   ` Marc Zyngier
  -1 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	Yijing Wang, Suravee Suthikulpanit

As MSI-type features are creeping into non-PCI devices, it is
starting to make sense to give our struct device some form of
support for this, by allowing a pointer to an MSI irq domain to
be set/retrieved.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/linux/device.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index ce1f2160..f3aaaf6 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -690,6 +690,7 @@ struct acpi_dev_node {
  * 		along with subsystem-level and driver-level callbacks.
  * @pins:	For device pin management.
  *		See Documentation/pinctrl.txt for details.
+ * @msi_domain: The generic MSI domain this device is using.
  * @numa_node:	NUMA node this device is close to.
  * @dma_mask:	Dma mask (if dma'ble device).
  * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all
@@ -750,6 +751,9 @@ struct device {
 	struct dev_pm_info	power;
 	struct dev_pm_domain	*pm_domain;
 
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	struct irq_domain	*msi_domain; /* MSI domain device uses */
+#endif
 #ifdef CONFIG_PINCTRL
 	struct dev_pin_info	*pins;
 #endif
@@ -837,6 +841,22 @@ static inline void set_dev_node(struct device *dev, int node)
 }
 #endif
 
+static inline struct irq_domain *dev_get_msi_domain(const struct device *dev)
+{
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	return dev->msi_domain;
+#else
+	return NULL;
+#endif
+}
+
+static inline void dev_set_msi_domain(struct device *dev, struct irq_domain *d)
+{
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	dev->msi_domain = d;
+#endif
+}
+
 static inline void *dev_get_drvdata(const struct device *dev)
 {
 	return dev->driver_data;
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 1/6] device core: Introduce per-device MSI domain pointer
@ 2014-12-08 20:12   ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: linux-arm-kernel

As MSI-type features are creeping into non-PCI devices, it is
starting to make sense to give our struct device some form of
support for this, by allowing a pointer to an MSI irq domain to
be set/retrieved.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/linux/device.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index ce1f2160..f3aaaf6 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -690,6 +690,7 @@ struct acpi_dev_node {
  * 		along with subsystem-level and driver-level callbacks.
  * @pins:	For device pin management.
  *		See Documentation/pinctrl.txt for details.
+ * @msi_domain: The generic MSI domain this device is using.
  * @numa_node:	NUMA node this device is close to.
  * @dma_mask:	Dma mask (if dma'ble device).
  * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all
@@ -750,6 +751,9 @@ struct device {
 	struct dev_pm_info	power;
 	struct dev_pm_domain	*pm_domain;
 
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	struct irq_domain	*msi_domain; /* MSI domain device uses */
+#endif
 #ifdef CONFIG_PINCTRL
 	struct dev_pin_info	*pins;
 #endif
@@ -837,6 +841,22 @@ static inline void set_dev_node(struct device *dev, int node)
 }
 #endif
 
+static inline struct irq_domain *dev_get_msi_domain(const struct device *dev)
+{
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	return dev->msi_domain;
+#else
+	return NULL;
+#endif
+}
+
+static inline void dev_set_msi_domain(struct device *dev, struct irq_domain *d)
+{
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	dev->msi_domain = d;
+#endif
+}
+
 static inline void *dev_get_drvdata(const struct device *dev)
 {
 	return dev->driver_data;
-- 
2.1.3

^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-08 20:12 ` Marc Zyngier
@ 2014-12-08 20:12   ` Marc Zyngier
  -1 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	Yijing Wang, Suravee Suthikulpanit

In order to be able to populate the device msi_domain field,
add the necesary hooks to propagate the PHB msi_domain across
secondary busses to devices.

So far, nobody populates the initial msi_domain.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/pci/probe.c | 24 ++++++++++++++++++++++++
 include/linux/pci.h |  1 +
 2 files changed, 25 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c8ca98c..d1009a2 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
 	}
 }
 
+void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
+{
+}
+
+static void pci_set_bus_msi_domain(struct pci_bus *bus)
+{
+	struct pci_dev *bridge = bus->self;
+
+	if (!bridge)
+		pcibios_set_phb_msi_domain(bus);
+	else
+		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
+}
+
 static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 					   struct pci_dev *bridge, int busnr)
 {
@@ -723,6 +737,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 	bridge->subordinate = child;
 
 add_dev:
+	pci_set_bus_msi_domain(child);
 	ret = device_register(&child->dev);
 	WARN_ON(ret < 0);
 
@@ -1517,6 +1532,11 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_enable_acs(dev);
 }
 
+static void pci_set_msi_domain(struct pci_dev *dev)
+{
+	dev_set_msi_domain(&dev->dev, dev_get_msi_domain(&dev->bus->dev));
+}
+
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 {
 	int ret;
@@ -1546,6 +1566,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	/* Initialize various capabilities */
 	pci_init_capabilities(dev);
 
+	/* Setup MSI irq domain */
+	pci_set_msi_domain(dev);
+
 	/*
 	 * Add the device to our list of discovered devices
 	 * and the bus list for fixup functions, etc.
@@ -1947,6 +1970,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	b->bridge = get_device(&bridge->dev);
 	device_enable_async_suspend(b->bridge);
 	pci_set_bus_of_node(b);
+	pci_set_bus_msi_domain(b);
 
 	if (!parent)
 		set_dev_node(b->bridge, pcibus_to_node(b));
diff --git a/include/linux/pci.h b/include/linux/pci.h
index a523cee..3266764 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1637,6 +1637,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
 int pcibios_add_device(struct pci_dev *dev);
 void pcibios_release_device(struct pci_dev *dev);
 void pcibios_penalize_isa_irq(int irq, int active);
+void pcibios_set_phb_msi_domain(struct pci_bus *bus);
 
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 extern struct dev_pm_ops pcibios_pm_ops;
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-08 20:12   ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: linux-arm-kernel

In order to be able to populate the device msi_domain field,
add the necesary hooks to propagate the PHB msi_domain across
secondary busses to devices.

So far, nobody populates the initial msi_domain.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/pci/probe.c | 24 ++++++++++++++++++++++++
 include/linux/pci.h |  1 +
 2 files changed, 25 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c8ca98c..d1009a2 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
 	}
 }
 
+void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
+{
+}
+
+static void pci_set_bus_msi_domain(struct pci_bus *bus)
+{
+	struct pci_dev *bridge = bus->self;
+
+	if (!bridge)
+		pcibios_set_phb_msi_domain(bus);
+	else
+		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
+}
+
 static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 					   struct pci_dev *bridge, int busnr)
 {
@@ -723,6 +737,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 	bridge->subordinate = child;
 
 add_dev:
+	pci_set_bus_msi_domain(child);
 	ret = device_register(&child->dev);
 	WARN_ON(ret < 0);
 
@@ -1517,6 +1532,11 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_enable_acs(dev);
 }
 
+static void pci_set_msi_domain(struct pci_dev *dev)
+{
+	dev_set_msi_domain(&dev->dev, dev_get_msi_domain(&dev->bus->dev));
+}
+
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 {
 	int ret;
@@ -1546,6 +1566,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	/* Initialize various capabilities */
 	pci_init_capabilities(dev);
 
+	/* Setup MSI irq domain */
+	pci_set_msi_domain(dev);
+
 	/*
 	 * Add the device to our list of discovered devices
 	 * and the bus list for fixup functions, etc.
@@ -1947,6 +1970,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	b->bridge = get_device(&bridge->dev);
 	device_enable_async_suspend(b->bridge);
 	pci_set_bus_of_node(b);
+	pci_set_bus_msi_domain(b);
 
 	if (!parent)
 		set_dev_node(b->bridge, pcibus_to_node(b));
diff --git a/include/linux/pci.h b/include/linux/pci.h
index a523cee..3266764 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1637,6 +1637,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
 int pcibios_add_device(struct pci_dev *dev);
 void pcibios_release_device(struct pci_dev *dev);
 void pcibios_penalize_isa_irq(int irq, int active);
+void pcibios_set_phb_msi_domain(struct pci_bus *bus);
 
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 extern struct dev_pm_ops pcibios_pm_ops;
-- 
2.1.3

^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 3/6] PCI/MSI: of: add support for OF-provided msi_domain
  2014-12-08 20:12 ` Marc Zyngier
@ 2014-12-08 20:12   ` Marc Zyngier
  -1 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	Yijing Wang, Suravee Suthikulpanit

In order to populate the PHB msi_domain, use the "msi-parent"
attribute to lookup a corresponding irq domain. If found,
this is our MSI domain.

This gets plugged into the core PCI code.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/pci/of.c    | 15 +++++++++++++++
 drivers/pci/probe.c |  1 +
 include/linux/pci.h |  2 ++
 3 files changed, 18 insertions(+)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index f092993..d8d1274 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -9,6 +9,7 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/pci.h>
 #include <linux/of.h>
@@ -59,3 +60,17 @@ struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
 		return of_node_get(bus->bridge->parent->of_node);
 	return NULL;
 }
+
+void pci_set_phb_of_msi_domain(struct pci_bus *bus)
+{
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	struct device_node *np;
+
+	if (!bus->dev.of_node)
+		return;
+	np = of_parse_phandle(bus->dev.of_node, "msi-parent", 0);
+	if (!np)
+		return;
+	dev_set_msi_domain(&bus->dev, irq_find_host(np));
+#endif
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index d1009a2..65600e5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -672,6 +672,7 @@ static void pci_set_bus_speed(struct pci_bus *bus)
 
 void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
 {
+	pci_set_phb_of_msi_domain(bus);
 }
 
 static void pci_set_bus_msi_domain(struct pci_bus *bus)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 3266764..6a9713f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1825,6 +1825,7 @@ void pci_set_of_node(struct pci_dev *dev);
 void pci_release_of_node(struct pci_dev *dev);
 void pci_set_bus_of_node(struct pci_bus *bus);
 void pci_release_bus_of_node(struct pci_bus *bus);
+void pci_set_phb_of_msi_domain(struct pci_bus *bus);
 
 /* Arch may override this (weak) */
 struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus);
@@ -1845,6 +1846,7 @@ static inline void pci_set_of_node(struct pci_dev *dev) { }
 static inline void pci_release_of_node(struct pci_dev *dev) { }
 static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
 static inline void pci_release_bus_of_node(struct pci_bus *bus) { }
+static inline void pci_set_phb_of_msi_domain(struct pci_bus *bus) {}
 #endif  /* CONFIG_OF */
 
 #ifdef CONFIG_EEH
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 3/6] PCI/MSI: of: add support for OF-provided msi_domain
@ 2014-12-08 20:12   ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: linux-arm-kernel

In order to populate the PHB msi_domain, use the "msi-parent"
attribute to lookup a corresponding irq domain. If found,
this is our MSI domain.

This gets plugged into the core PCI code.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/pci/of.c    | 15 +++++++++++++++
 drivers/pci/probe.c |  1 +
 include/linux/pci.h |  2 ++
 3 files changed, 18 insertions(+)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index f092993..d8d1274 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -9,6 +9,7 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/pci.h>
 #include <linux/of.h>
@@ -59,3 +60,17 @@ struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
 		return of_node_get(bus->bridge->parent->of_node);
 	return NULL;
 }
+
+void pci_set_phb_of_msi_domain(struct pci_bus *bus)
+{
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	struct device_node *np;
+
+	if (!bus->dev.of_node)
+		return;
+	np = of_parse_phandle(bus->dev.of_node, "msi-parent", 0);
+	if (!np)
+		return;
+	dev_set_msi_domain(&bus->dev, irq_find_host(np));
+#endif
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index d1009a2..65600e5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -672,6 +672,7 @@ static void pci_set_bus_speed(struct pci_bus *bus)
 
 void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
 {
+	pci_set_phb_of_msi_domain(bus);
 }
 
 static void pci_set_bus_msi_domain(struct pci_bus *bus)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 3266764..6a9713f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1825,6 +1825,7 @@ void pci_set_of_node(struct pci_dev *dev);
 void pci_release_of_node(struct pci_dev *dev);
 void pci_set_bus_of_node(struct pci_bus *bus);
 void pci_release_bus_of_node(struct pci_bus *bus);
+void pci_set_phb_of_msi_domain(struct pci_bus *bus);
 
 /* Arch may override this (weak) */
 struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus);
@@ -1845,6 +1846,7 @@ static inline void pci_set_of_node(struct pci_dev *dev) { }
 static inline void pci_release_of_node(struct pci_dev *dev) { }
 static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
 static inline void pci_release_bus_of_node(struct pci_bus *bus) { }
+static inline void pci_set_phb_of_msi_domain(struct pci_bus *bus) {}
 #endif  /* CONFIG_OF */
 
 #ifdef CONFIG_EEH
-- 
2.1.3

^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 4/6] PCI/MSI: Let pci_msi_get_domain use struct device's msi_domain
  2014-12-08 20:12 ` Marc Zyngier
@ 2014-12-08 20:12   ` Marc Zyngier
  -1 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	Yijing Wang, Suravee Suthikulpanit

Now that we can easily find which MSI domain a PCI device is
using, use dev_get_msi_domain as a way to retrieve the information.

The original code is still used as a fallback.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/pci/msi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index fd60806..003040b 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -41,7 +41,8 @@ static struct irq_domain *pci_msi_get_domain(struct pci_dev *dev)
 {
 	struct irq_domain *domain = NULL;
 
-	if (dev->bus->msi)
+	domain = dev_get_msi_domain(&dev->dev);
+	if (!domain && dev->bus->msi)
 		domain = dev->bus->msi->domain;
 	if (!domain)
 		domain = arch_get_pci_msi_domain(dev);
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 4/6] PCI/MSI: Let pci_msi_get_domain use struct device's msi_domain
@ 2014-12-08 20:12   ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: linux-arm-kernel

Now that we can easily find which MSI domain a PCI device is
using, use dev_get_msi_domain as a way to retrieve the information.

The original code is still used as a fallback.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/pci/msi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index fd60806..003040b 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -41,7 +41,8 @@ static struct irq_domain *pci_msi_get_domain(struct pci_dev *dev)
 {
 	struct irq_domain *domain = NULL;
 
-	if (dev->bus->msi)
+	domain = dev_get_msi_domain(&dev->dev);
+	if (!domain && dev->bus->msi)
 		domain = dev->bus->msi->domain;
 	if (!domain)
 		domain = arch_get_pci_msi_domain(dev);
-- 
2.1.3

^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 5/6] irqchip: GICv2m: Get rid of struct msi_controller
  2014-12-08 20:12 ` Marc Zyngier
@ 2014-12-08 20:12   ` Marc Zyngier
  -1 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	Yijing Wang, Suravee Suthikulpanit

GICv2m only uses the msi_controller structure as a way to match
the PHB with its MSI HW, and thus the msi_domain. But now that
we can directly associate an msi_domain with a device, there is
no use keeping this msi_controller around.

Just remove all traces of msi_controller from the driver.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v2m.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index fdf7065..a76b802 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -45,7 +45,6 @@
 
 struct v2m_data {
 	spinlock_t msi_cnt_lock;
-	struct msi_controller mchip;
 	struct resource res;	/* GICv2m resource */
 	void __iomem *base;	/* GICv2m virt address */
 	u32 spi_start;		/* The SPI number that MSIs start */
@@ -218,6 +217,7 @@ static int __init gicv2m_init_one(struct device_node *node,
 {
 	int ret;
 	struct v2m_data *v2m;
+	struct irq_domain *inner_domain;
 
 	v2m = kzalloc(sizeof(struct v2m_data), GFP_KERNEL);
 	if (!v2m) {
@@ -261,19 +261,17 @@ static int __init gicv2m_init_one(struct device_node *node,
 		goto err_iounmap;
 	}
 
-	v2m->domain = irq_domain_add_tree(NULL, &gicv2m_domain_ops, v2m);
-	if (!v2m->domain) {
+	inner_domain = irq_domain_add_tree(NULL, &gicv2m_domain_ops, v2m);
+	if (!inner_domain) {
 		pr_err("Failed to create GICv2m domain\n");
 		ret = -ENOMEM;
 		goto err_free_bm;
 	}
 
-	v2m->domain->parent = parent;
-	v2m->mchip.of_node = node;
-	v2m->mchip.domain = pci_msi_create_irq_domain(node,
-						      &gicv2m_msi_domain_info,
-						      v2m->domain);
-	if (!v2m->mchip.domain) {
+	inner_domain->parent = parent;
+	v2m->domain = pci_msi_create_irq_domain(node, &gicv2m_msi_domain_info,
+						inner_domain);
+	if (!v2m->domain) {
 		pr_err("Failed to create MSI domain\n");
 		ret = -ENOMEM;
 		goto err_free_domains;
@@ -281,12 +279,6 @@ static int __init gicv2m_init_one(struct device_node *node,
 
 	spin_lock_init(&v2m->msi_cnt_lock);
 
-	ret = of_pci_msi_chip_add(&v2m->mchip);
-	if (ret) {
-		pr_err("Failed to add msi_chip.\n");
-		goto err_free_domains;
-	}
-
 	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));
@@ -294,10 +286,10 @@ static int __init gicv2m_init_one(struct device_node *node,
 	return 0;
 
 err_free_domains:
-	if (v2m->mchip.domain)
-		irq_domain_remove(v2m->mchip.domain);
 	if (v2m->domain)
 		irq_domain_remove(v2m->domain);
+	if (inner_domain)
+		irq_domain_remove(inner_domain);
 err_free_bm:
 	kfree(v2m->bm);
 err_iounmap:
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 5/6] irqchip: GICv2m: Get rid of struct msi_controller
@ 2014-12-08 20:12   ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: linux-arm-kernel

GICv2m only uses the msi_controller structure as a way to match
the PHB with its MSI HW, and thus the msi_domain. But now that
we can directly associate an msi_domain with a device, there is
no use keeping this msi_controller around.

Just remove all traces of msi_controller from the driver.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v2m.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index fdf7065..a76b802 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -45,7 +45,6 @@
 
 struct v2m_data {
 	spinlock_t msi_cnt_lock;
-	struct msi_controller mchip;
 	struct resource res;	/* GICv2m resource */
 	void __iomem *base;	/* GICv2m virt address */
 	u32 spi_start;		/* The SPI number that MSIs start */
@@ -218,6 +217,7 @@ static int __init gicv2m_init_one(struct device_node *node,
 {
 	int ret;
 	struct v2m_data *v2m;
+	struct irq_domain *inner_domain;
 
 	v2m = kzalloc(sizeof(struct v2m_data), GFP_KERNEL);
 	if (!v2m) {
@@ -261,19 +261,17 @@ static int __init gicv2m_init_one(struct device_node *node,
 		goto err_iounmap;
 	}
 
-	v2m->domain = irq_domain_add_tree(NULL, &gicv2m_domain_ops, v2m);
-	if (!v2m->domain) {
+	inner_domain = irq_domain_add_tree(NULL, &gicv2m_domain_ops, v2m);
+	if (!inner_domain) {
 		pr_err("Failed to create GICv2m domain\n");
 		ret = -ENOMEM;
 		goto err_free_bm;
 	}
 
-	v2m->domain->parent = parent;
-	v2m->mchip.of_node = node;
-	v2m->mchip.domain = pci_msi_create_irq_domain(node,
-						      &gicv2m_msi_domain_info,
-						      v2m->domain);
-	if (!v2m->mchip.domain) {
+	inner_domain->parent = parent;
+	v2m->domain = pci_msi_create_irq_domain(node, &gicv2m_msi_domain_info,
+						inner_domain);
+	if (!v2m->domain) {
 		pr_err("Failed to create MSI domain\n");
 		ret = -ENOMEM;
 		goto err_free_domains;
@@ -281,12 +279,6 @@ static int __init gicv2m_init_one(struct device_node *node,
 
 	spin_lock_init(&v2m->msi_cnt_lock);
 
-	ret = of_pci_msi_chip_add(&v2m->mchip);
-	if (ret) {
-		pr_err("Failed to add msi_chip.\n");
-		goto err_free_domains;
-	}
-
 	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));
@@ -294,10 +286,10 @@ static int __init gicv2m_init_one(struct device_node *node,
 	return 0;
 
 err_free_domains:
-	if (v2m->mchip.domain)
-		irq_domain_remove(v2m->mchip.domain);
 	if (v2m->domain)
 		irq_domain_remove(v2m->domain);
+	if (inner_domain)
+		irq_domain_remove(inner_domain);
 err_free_bm:
 	kfree(v2m->bm);
 err_iounmap:
-- 
2.1.3

^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 6/6] irqchip: gicv3-its: Get rid of struct msi_controller
  2014-12-08 20:12 ` Marc Zyngier
@ 2014-12-08 20:12   ` Marc Zyngier
  -1 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	Yijing Wang, Suravee Suthikulpanit

The GICv3 ITS only uses the msi_controller structure as a way
to match the PHB with its MSI HW, and thus the msi_domain.
But now that we can directly associate an msi_domain with a device,
there is no use keeping this msi_controller around.

Just remove all traces of msi_controller from the driver.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e9d1615..5523241 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -60,7 +60,6 @@ struct its_collection {
 struct its_node {
 	raw_spinlock_t		lock;
 	struct list_head	entry;
-	struct msi_controller	msi_chip;
 	struct irq_domain	*domain;
 	void __iomem		*base;
 	unsigned long		phys_base;
@@ -875,7 +874,7 @@ retry_baser:
 
 		if (val != tmp) {
 			pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n",
-			       its->msi_chip.of_node->full_name, i,
+			       its->domain->of_node->full_name, i,
 			       (unsigned long) val, (unsigned long) tmp);
 			err = -ENXIO;
 			goto out_free;
@@ -1237,6 +1236,7 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
 	struct resource res;
 	struct its_node *its;
 	void __iomem *its_base;
+	struct irq_domain *inner_domain = NULL;
 	u32 val;
 	u64 baser, tmp;
 	int err;
@@ -1273,7 +1273,6 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
 	INIT_LIST_HEAD(&its->its_device_list);
 	its->base = its_base;
 	its->phys_base = res.start;
-	its->msi_chip.of_node = node;
 	its->ite_size = ((readl_relaxed(its_base + GITS_TYPER) >> 4) & 0xf) + 1;
 
 	its->cmd_base = kzalloc(ITS_CMD_QUEUE_SZ, GFP_KERNEL);
@@ -1307,26 +1306,22 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
 		its->flags |= ITS_FLAGS_CMDQ_NEEDS_FLUSHING;
 	}
 
-	if (of_property_read_bool(its->msi_chip.of_node, "msi-controller")) {
-		its->domain = irq_domain_add_tree(NULL, &its_domain_ops, its);
-		if (!its->domain) {
+	if (of_property_read_bool(node, "msi-controller")) {
+		inner_domain = irq_domain_add_tree(NULL, &its_domain_ops, its);
+		if (!inner_domain) {
 			err = -ENOMEM;
 			goto out_free_tables;
 		}
 
-		its->domain->parent = parent;
+		inner_domain->parent = parent;
 
-		its->msi_chip.domain = pci_msi_create_irq_domain(node,
-								 &its_pci_msi_domain_info,
-								 its->domain);
-		if (!its->msi_chip.domain) {
+		its->domain = pci_msi_create_irq_domain(node,
+							&its_pci_msi_domain_info,
+							inner_domain);
+		if (!its->domain) {
 			err = -ENOMEM;
 			goto out_free_domains;
 		}
-
-		err = of_pci_msi_chip_add(&its->msi_chip);
-		if (err)
-			goto out_free_domains;
 	}
 
 	spin_lock(&its_lock);
@@ -1336,10 +1331,10 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
 	return 0;
 
 out_free_domains:
-	if (its->msi_chip.domain)
-		irq_domain_remove(its->msi_chip.domain);
 	if (its->domain)
 		irq_domain_remove(its->domain);
+	if (inner_domain)
+		irq_domain_remove(inner_domain);
 out_free_tables:
 	its_free_tables(its);
 out_free_cmd:
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 6/6] irqchip: gicv3-its: Get rid of struct msi_controller
@ 2014-12-08 20:12   ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-08 20:12 UTC (permalink / raw)
  To: linux-arm-kernel

The GICv3 ITS only uses the msi_controller structure as a way
to match the PHB with its MSI HW, and thus the msi_domain.
But now that we can directly associate an msi_domain with a device,
there is no use keeping this msi_controller around.

Just remove all traces of msi_controller from the driver.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e9d1615..5523241 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -60,7 +60,6 @@ struct its_collection {
 struct its_node {
 	raw_spinlock_t		lock;
 	struct list_head	entry;
-	struct msi_controller	msi_chip;
 	struct irq_domain	*domain;
 	void __iomem		*base;
 	unsigned long		phys_base;
@@ -875,7 +874,7 @@ retry_baser:
 
 		if (val != tmp) {
 			pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n",
-			       its->msi_chip.of_node->full_name, i,
+			       its->domain->of_node->full_name, i,
 			       (unsigned long) val, (unsigned long) tmp);
 			err = -ENXIO;
 			goto out_free;
@@ -1237,6 +1236,7 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
 	struct resource res;
 	struct its_node *its;
 	void __iomem *its_base;
+	struct irq_domain *inner_domain = NULL;
 	u32 val;
 	u64 baser, tmp;
 	int err;
@@ -1273,7 +1273,6 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
 	INIT_LIST_HEAD(&its->its_device_list);
 	its->base = its_base;
 	its->phys_base = res.start;
-	its->msi_chip.of_node = node;
 	its->ite_size = ((readl_relaxed(its_base + GITS_TYPER) >> 4) & 0xf) + 1;
 
 	its->cmd_base = kzalloc(ITS_CMD_QUEUE_SZ, GFP_KERNEL);
@@ -1307,26 +1306,22 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
 		its->flags |= ITS_FLAGS_CMDQ_NEEDS_FLUSHING;
 	}
 
-	if (of_property_read_bool(its->msi_chip.of_node, "msi-controller")) {
-		its->domain = irq_domain_add_tree(NULL, &its_domain_ops, its);
-		if (!its->domain) {
+	if (of_property_read_bool(node, "msi-controller")) {
+		inner_domain = irq_domain_add_tree(NULL, &its_domain_ops, its);
+		if (!inner_domain) {
 			err = -ENOMEM;
 			goto out_free_tables;
 		}
 
-		its->domain->parent = parent;
+		inner_domain->parent = parent;
 
-		its->msi_chip.domain = pci_msi_create_irq_domain(node,
-								 &its_pci_msi_domain_info,
-								 its->domain);
-		if (!its->msi_chip.domain) {
+		its->domain = pci_msi_create_irq_domain(node,
+							&its_pci_msi_domain_info,
+							inner_domain);
+		if (!its->domain) {
 			err = -ENOMEM;
 			goto out_free_domains;
 		}
-
-		err = of_pci_msi_chip_add(&its->msi_chip);
-		if (err)
-			goto out_free_domains;
 	}
 
 	spin_lock(&its_lock);
@@ -1336,10 +1331,10 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
 	return 0;
 
 out_free_domains:
-	if (its->msi_chip.domain)
-		irq_domain_remove(its->msi_chip.domain);
 	if (its->domain)
 		irq_domain_remove(its->domain);
+	if (inner_domain)
+		irq_domain_remove(inner_domain);
 out_free_tables:
 	its_free_tables(its);
 out_free_cmd:
-- 
2.1.3

^ permalink raw reply related	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-08 20:12   ` Marc Zyngier
@ 2014-12-09  2:03     ` Yijing Wang
  -1 siblings, 0 replies; 57+ messages in thread
From: Yijing Wang @ 2014-12-09  2:03 UTC (permalink / raw)
  To: Marc Zyngier, Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	Suravee Suthikulpanit

On 2014/12/9 4:12, Marc Zyngier wrote:
> In order to be able to populate the device msi_domain field,
> add the necesary hooks to propagate the PHB msi_domain across
> secondary busses to devices.
> 
> So far, nobody populates the initial msi_domain.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/pci/probe.c | 24 ++++++++++++++++++++++++
>  include/linux/pci.h |  1 +
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index c8ca98c..d1009a2 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>  	}
>  }
>  
> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
> +{
> +}
> +
> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
> +{
> +	struct pci_dev *bridge = bus->self;
> +
> +	if (!bridge)
> +		pcibios_set_phb_msi_domain(bus);
> +	else
> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
> +}


Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
now in x86, pci devices under the same phb may associate different msi irq domain.

> +
>  static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  					   struct pci_dev *bridge, int busnr)
>  {
> @@ -723,6 +737,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  	bridge->subordinate = child;
>  
>  add_dev:
> +	pci_set_bus_msi_domain(child);
>  	ret = device_register(&child->dev);
>  	WARN_ON(ret < 0);
>  
> @@ -1517,6 +1532,11 @@ static void pci_init_capabilities(struct pci_dev *dev)
>  	pci_enable_acs(dev);
>  }
>  
> +static void pci_set_msi_domain(struct pci_dev *dev)
> +{
> +	dev_set_msi_domain(&dev->dev, dev_get_msi_domain(&dev->bus->dev));
> +}
> +
>  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  {
>  	int ret;
> @@ -1546,6 +1566,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	/* Initialize various capabilities */
>  	pci_init_capabilities(dev);
>  
> +	/* Setup MSI irq domain */
> +	pci_set_msi_domain(dev);
> +
>  	/*
>  	 * Add the device to our list of discovered devices
>  	 * and the bus list for fixup functions, etc.
> @@ -1947,6 +1970,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  	b->bridge = get_device(&bridge->dev);
>  	device_enable_async_suspend(b->bridge);
>  	pci_set_bus_of_node(b);
> +	pci_set_bus_msi_domain(b);
>  
>  	if (!parent)
>  		set_dev_node(b->bridge, pcibus_to_node(b));
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index a523cee..3266764 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1637,6 +1637,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
>  int pcibios_add_device(struct pci_dev *dev);
>  void pcibios_release_device(struct pci_dev *dev);
>  void pcibios_penalize_isa_irq(int irq, int active);
> +void pcibios_set_phb_msi_domain(struct pci_bus *bus);
>  
>  #ifdef CONFIG_HIBERNATE_CALLBACKS
>  extern struct dev_pm_ops pcibios_pm_ops;
> 


-- 
Thanks!
Yijing


^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09  2:03     ` Yijing Wang
  0 siblings, 0 replies; 57+ messages in thread
From: Yijing Wang @ 2014-12-09  2:03 UTC (permalink / raw)
  To: linux-arm-kernel

On 2014/12/9 4:12, Marc Zyngier wrote:
> In order to be able to populate the device msi_domain field,
> add the necesary hooks to propagate the PHB msi_domain across
> secondary busses to devices.
> 
> So far, nobody populates the initial msi_domain.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/pci/probe.c | 24 ++++++++++++++++++++++++
>  include/linux/pci.h |  1 +
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index c8ca98c..d1009a2 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>  	}
>  }
>  
> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
> +{
> +}
> +
> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
> +{
> +	struct pci_dev *bridge = bus->self;
> +
> +	if (!bridge)
> +		pcibios_set_phb_msi_domain(bus);
> +	else
> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
> +}


Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
now in x86, pci devices under the same phb may associate different msi irq domain.

> +
>  static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  					   struct pci_dev *bridge, int busnr)
>  {
> @@ -723,6 +737,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  	bridge->subordinate = child;
>  
>  add_dev:
> +	pci_set_bus_msi_domain(child);
>  	ret = device_register(&child->dev);
>  	WARN_ON(ret < 0);
>  
> @@ -1517,6 +1532,11 @@ static void pci_init_capabilities(struct pci_dev *dev)
>  	pci_enable_acs(dev);
>  }
>  
> +static void pci_set_msi_domain(struct pci_dev *dev)
> +{
> +	dev_set_msi_domain(&dev->dev, dev_get_msi_domain(&dev->bus->dev));
> +}
> +
>  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  {
>  	int ret;
> @@ -1546,6 +1566,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	/* Initialize various capabilities */
>  	pci_init_capabilities(dev);
>  
> +	/* Setup MSI irq domain */
> +	pci_set_msi_domain(dev);
> +
>  	/*
>  	 * Add the device to our list of discovered devices
>  	 * and the bus list for fixup functions, etc.
> @@ -1947,6 +1970,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  	b->bridge = get_device(&bridge->dev);
>  	device_enable_async_suspend(b->bridge);
>  	pci_set_bus_of_node(b);
> +	pci_set_bus_msi_domain(b);
>  
>  	if (!parent)
>  		set_dev_node(b->bridge, pcibus_to_node(b));
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index a523cee..3266764 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1637,6 +1637,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
>  int pcibios_add_device(struct pci_dev *dev);
>  void pcibios_release_device(struct pci_dev *dev);
>  void pcibios_penalize_isa_irq(int irq, int active);
> +void pcibios_set_phb_msi_domain(struct pci_bus *bus);
>  
>  #ifdef CONFIG_HIBERNATE_CALLBACKS
>  extern struct dev_pm_ops pcibios_pm_ops;
> 


-- 
Thanks!
Yijing

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-09  2:03     ` Yijing Wang
  (?)
@ 2014-12-09 10:02       ` Marc Zyngier
  -1 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 10:02 UTC (permalink / raw)
  To: Yijing Wang, Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

Hi Yijing,

On 09/12/14 02:03, Yijing Wang wrote:
> On 2014/12/9 4:12, Marc Zyngier wrote:
>> In order to be able to populate the device msi_domain field,
>> add the necesary hooks to propagate the PHB msi_domain across
>> secondary busses to devices.
>>
>> So far, nobody populates the initial msi_domain.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/pci/probe.c | 24 ++++++++++++++++++++++++
>>  include/linux/pci.h |  1 +
>>  2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index c8ca98c..d1009a2 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>>  	}
>>  }
>>  
>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>> +{
>> +}
>> +
>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>> +{
>> +	struct pci_dev *bridge = bus->self;
>> +
>> +	if (!bridge)
>> +		pcibios_set_phb_msi_domain(bus);
>> +	else
>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>> +}
> 
> 
> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
> now in x86, pci devices under the same phb may associate different msi irq domain.

Well, this is not supposed to be a perfect solution yet, but instead a
basis for discussion. What I'd like to find out is:

- What is the minimum granularity for associating a device with its MSI
domain in existing platforms?
- What topology data structures do you use to find out what MSI
controller a device should be matched with?
- What in-tree platform already has this requirements?

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 10:02       ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 10:02 UTC (permalink / raw)
  To: Yijing Wang, Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

Hi Yijing,

On 09/12/14 02:03, Yijing Wang wrote:
> On 2014/12/9 4:12, Marc Zyngier wrote:
>> In order to be able to populate the device msi_domain field,
>> add the necesary hooks to propagate the PHB msi_domain across
>> secondary busses to devices.
>>
>> So far, nobody populates the initial msi_domain.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/pci/probe.c | 24 ++++++++++++++++++++++++
>>  include/linux/pci.h |  1 +
>>  2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index c8ca98c..d1009a2 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>>  	}
>>  }
>>  
>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>> +{
>> +}
>> +
>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>> +{
>> +	struct pci_dev *bridge = bus->self;
>> +
>> +	if (!bridge)
>> +		pcibios_set_phb_msi_domain(bus);
>> +	else
>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>> +}
> 
> 
> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
> now in x86, pci devices under the same phb may associate different msi irq domain.

Well, this is not supposed to be a perfect solution yet, but instead a
basis for discussion. What I'd like to find out is:

- What is the minimum granularity for associating a device with its MSI
domain in existing platforms?
- What topology data structures do you use to find out what MSI
controller a device should be matched with?
- What in-tree platform already has this requirements?

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 10:02       ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 10:02 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Yijing,

On 09/12/14 02:03, Yijing Wang wrote:
> On 2014/12/9 4:12, Marc Zyngier wrote:
>> In order to be able to populate the device msi_domain field,
>> add the necesary hooks to propagate the PHB msi_domain across
>> secondary busses to devices.
>>
>> So far, nobody populates the initial msi_domain.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/pci/probe.c | 24 ++++++++++++++++++++++++
>>  include/linux/pci.h |  1 +
>>  2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index c8ca98c..d1009a2 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>>  	}
>>  }
>>  
>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>> +{
>> +}
>> +
>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>> +{
>> +	struct pci_dev *bridge = bus->self;
>> +
>> +	if (!bridge)
>> +		pcibios_set_phb_msi_domain(bus);
>> +	else
>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>> +}
> 
> 
> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
> now in x86, pci devices under the same phb may associate different msi irq domain.

Well, this is not supposed to be a perfect solution yet, but instead a
basis for discussion. What I'd like to find out is:

- What is the minimum granularity for associating a device with its MSI
domain in existing platforms?
- What topology data structures do you use to find out what MSI
controller a device should be matched with?
- What in-tree platform already has this requirements?

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-09 10:02       ` Marc Zyngier
  (?)
@ 2014-12-09 11:57         ` Yijing Wang
  -1 siblings, 0 replies; 57+ messages in thread
From: Yijing Wang @ 2014-12-09 11:57 UTC (permalink / raw)
  To: Marc Zyngier, Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>> +{
>>> +}
>>> +
>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>> +{
>>> +	struct pci_dev *bridge = bus->self;
>>> +
>>> +	if (!bridge)
>>> +		pcibios_set_phb_msi_domain(bus);
>>> +	else
>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>> +}
>>
>>
>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>> now in x86, pci devices under the same phb may associate different msi irq domain.

Hi Marc,

> 
> Well, this is not supposed to be a perfect solution yet, but instead a
> basis for discussion. What I'd like to find out is:
> 
> - What is the minimum granularity for associating a device with its MSI
> domain in existing platforms?

PCI device, after Gerry's msi irq domain patchset which now in linux-next,
in x86, we will find msi irq domain by pci_dev.

I generally agree your first patch which associate basic device with msi irq domain.

> - What topology data structures do you use to find out what MSI
> controller a device should be matched with?

Now only arm and arm64 use msi controller to setup/teardown msi irqs,
in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
to be saved in pci_bus. For a more common method to find msi controller/irq domain,
I prefer pci_dev/device.

> - What in-tree platform already has this requirements?

As mentioned above, x86 does.


Thanks!
Yijing.




> 
> Thanks,
> 
> 	M.
> 


-- 
Thanks!
Yijing


^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 11:57         ` Yijing Wang
  0 siblings, 0 replies; 57+ messages in thread
From: Yijing Wang @ 2014-12-09 11:57 UTC (permalink / raw)
  To: Marc Zyngier, Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>> +{
>>> +}
>>> +
>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>> +{
>>> +	struct pci_dev *bridge = bus->self;
>>> +
>>> +	if (!bridge)
>>> +		pcibios_set_phb_msi_domain(bus);
>>> +	else
>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>> +}
>>
>>
>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>> now in x86, pci devices under the same phb may associate different msi irq domain.

Hi Marc,

> 
> Well, this is not supposed to be a perfect solution yet, but instead a
> basis for discussion. What I'd like to find out is:
> 
> - What is the minimum granularity for associating a device with its MSI
> domain in existing platforms?

PCI device, after Gerry's msi irq domain patchset which now in linux-next,
in x86, we will find msi irq domain by pci_dev.

I generally agree your first patch which associate basic device with msi irq domain.

> - What topology data structures do you use to find out what MSI
> controller a device should be matched with?

Now only arm and arm64 use msi controller to setup/teardown msi irqs,
in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
to be saved in pci_bus. For a more common method to find msi controller/irq domain,
I prefer pci_dev/device.

> - What in-tree platform already has this requirements?

As mentioned above, x86 does.


Thanks!
Yijing.




> 
> Thanks,
> 
> 	M.
> 


-- 
Thanks!
Yijing


^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 11:57         ` Yijing Wang
  0 siblings, 0 replies; 57+ messages in thread
From: Yijing Wang @ 2014-12-09 11:57 UTC (permalink / raw)
  To: linux-arm-kernel

>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>> +{
>>> +}
>>> +
>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>> +{
>>> +	struct pci_dev *bridge = bus->self;
>>> +
>>> +	if (!bridge)
>>> +		pcibios_set_phb_msi_domain(bus);
>>> +	else
>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>> +}
>>
>>
>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>> now in x86, pci devices under the same phb may associate different msi irq domain.

Hi Marc,

> 
> Well, this is not supposed to be a perfect solution yet, but instead a
> basis for discussion. What I'd like to find out is:
> 
> - What is the minimum granularity for associating a device with its MSI
> domain in existing platforms?

PCI device, after Gerry's msi irq domain patchset which now in linux-next,
in x86, we will find msi irq domain by pci_dev.

I generally agree your first patch which associate basic device with msi irq domain.

> - What topology data structures do you use to find out what MSI
> controller a device should be matched with?

Now only arm and arm64 use msi controller to setup/teardown msi irqs,
in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
to be saved in pci_bus. For a more common method to find msi controller/irq domain,
I prefer pci_dev/device.

> - What in-tree platform already has this requirements?

As mentioned above, x86 does.


Thanks!
Yijing.




> 
> Thanks,
> 
> 	M.
> 


-- 
Thanks!
Yijing

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-09 11:57         ` Yijing Wang
  (?)
@ 2014-12-09 12:12           ` Marc Zyngier
  -1 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 12:12 UTC (permalink / raw)
  To: Yijing Wang, Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

Yijing,

On 09/12/14 11:57, Yijing Wang wrote:
>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>> +{
>>>> +}
>>>> +
>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>> +{
>>>> +	struct pci_dev *bridge = bus->self;
>>>> +
>>>> +	if (!bridge)
>>>> +		pcibios_set_phb_msi_domain(bus);
>>>> +	else
>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>> +}
>>>
>>>
>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>> now in x86, pci devices under the same phb may associate different msi irq domain.
> 
> Hi Marc,
> 
>>
>> Well, this is not supposed to be a perfect solution yet, but instead a
>> basis for discussion. What I'd like to find out is:
>>
>> - What is the minimum granularity for associating a device with its MSI
>> domain in existing platforms?
> 
> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
> in x86, we will find msi irq domain by pci_dev.

Are you *really* associating the MSI domain on a per pci-device basis?
That is, you have devices on the same PCI bus talking to different MSI hw?

> I generally agree your first patch which associate basic device with msi irq domain.
> 
>> - What topology data structures do you use to find out what MSI
>> controller a device should be matched with?
> 
> Now only arm and arm64 use msi controller to setup/teardown msi irqs,
> in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
> to be saved in pci_bus. For a more common method to find msi controller/irq domain,
> I prefer pci_dev/device.

Forget about msi_controller, the whole goal of this series is to make it
obsolete. On your x86 platform, what how do you identify which MSI
domain should be associated with a given PCI device? Surely you must
have a set of data structures or ACPI tables which give you that
information.

>> - What in-tree platform already has this requirements?
> 
> As mentioned above, x86 does.

Let me rephrase that in a non-ambiguous manner: can you point me to a
file implementing this in mainline?

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 12:12           ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 12:12 UTC (permalink / raw)
  To: Yijing Wang, Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

Yijing,

On 09/12/14 11:57, Yijing Wang wrote:
>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>> +{
>>>> +}
>>>> +
>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>> +{
>>>> +	struct pci_dev *bridge = bus->self;
>>>> +
>>>> +	if (!bridge)
>>>> +		pcibios_set_phb_msi_domain(bus);
>>>> +	else
>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>> +}
>>>
>>>
>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>> now in x86, pci devices under the same phb may associate different msi irq domain.
> 
> Hi Marc,
> 
>>
>> Well, this is not supposed to be a perfect solution yet, but instead a
>> basis for discussion. What I'd like to find out is:
>>
>> - What is the minimum granularity for associating a device with its MSI
>> domain in existing platforms?
> 
> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
> in x86, we will find msi irq domain by pci_dev.

Are you *really* associating the MSI domain on a per pci-device basis?
That is, you have devices on the same PCI bus talking to different MSI hw?

> I generally agree your first patch which associate basic device with msi irq domain.
> 
>> - What topology data structures do you use to find out what MSI
>> controller a device should be matched with?
> 
> Now only arm and arm64 use msi controller to setup/teardown msi irqs,
> in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
> to be saved in pci_bus. For a more common method to find msi controller/irq domain,
> I prefer pci_dev/device.

Forget about msi_controller, the whole goal of this series is to make it
obsolete. On your x86 platform, what how do you identify which MSI
domain should be associated with a given PCI device? Surely you must
have a set of data structures or ACPI tables which give you that
information.

>> - What in-tree platform already has this requirements?
> 
> As mentioned above, x86 does.

Let me rephrase that in a non-ambiguous manner: can you point me to a
file implementing this in mainline?

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 12:12           ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 12:12 UTC (permalink / raw)
  To: linux-arm-kernel

Yijing,

On 09/12/14 11:57, Yijing Wang wrote:
>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>> +{
>>>> +}
>>>> +
>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>> +{
>>>> +	struct pci_dev *bridge = bus->self;
>>>> +
>>>> +	if (!bridge)
>>>> +		pcibios_set_phb_msi_domain(bus);
>>>> +	else
>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>> +}
>>>
>>>
>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>> now in x86, pci devices under the same phb may associate different msi irq domain.
> 
> Hi Marc,
> 
>>
>> Well, this is not supposed to be a perfect solution yet, but instead a
>> basis for discussion. What I'd like to find out is:
>>
>> - What is the minimum granularity for associating a device with its MSI
>> domain in existing platforms?
> 
> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
> in x86, we will find msi irq domain by pci_dev.

Are you *really* associating the MSI domain on a per pci-device basis?
That is, you have devices on the same PCI bus talking to different MSI hw?

> I generally agree your first patch which associate basic device with msi irq domain.
> 
>> - What topology data structures do you use to find out what MSI
>> controller a device should be matched with?
> 
> Now only arm and arm64 use msi controller to setup/teardown msi irqs,
> in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
> to be saved in pci_bus. For a more common method to find msi controller/irq domain,
> I prefer pci_dev/device.

Forget about msi_controller, the whole goal of this series is to make it
obsolete. On your x86 platform, what how do you identify which MSI
domain should be associated with a given PCI device? Surely you must
have a set of data structures or ACPI tables which give you that
information.

>> - What in-tree platform already has this requirements?
> 
> As mentioned above, x86 does.

Let me rephrase that in a non-ambiguous manner: can you point me to a
file implementing this in mainline?

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-09 12:12           ` Marc Zyngier
  (?)
@ 2014-12-09 12:24             ` Yijing Wang
  -1 siblings, 0 replies; 57+ messages in thread
From: Yijing Wang @ 2014-12-09 12:24 UTC (permalink / raw)
  To: Marc Zyngier, Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>> basis for discussion. What I'd like to find out is:
>>>
>>> - What is the minimum granularity for associating a device with its MSI
>>> domain in existing platforms?
>>
>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>> in x86, we will find msi irq domain by pci_dev.
> 
> Are you *really* associating the MSI domain on a per pci-device basis?
> That is, you have devices on the same PCI bus talking to different MSI hw?

Yes.

> 
>> I generally agree your first patch which associate basic device with msi irq domain.
>>
>>> - What topology data structures do you use to find out what MSI
>>> controller a device should be matched with?
>>
>> Now only arm and arm64 use msi controller to setup/teardown msi irqs,
>> in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
>> to be saved in pci_bus. For a more common method to find msi controller/irq domain,
>> I prefer pci_dev/device.
> 
> Forget about msi_controller, the whole goal of this series is to make it
> obsolete. On your x86 platform, what how do you identify which MSI
> domain should be associated with a given PCI device? Surely you must
> have a set of data structures or ACPI tables which give you that
> information.

Yes, by ACPI DMAR table.

> 
>>> - What in-tree platform already has this requirements?
>>
>> As mentioned above, x86 does.
> 
> Let me rephrase that in a non-ambiguous manner: can you point me to a
> file implementing this in mainline?

Please refer to arch/x86/kernel/apic/msi.c  native_setup_msi_irqs()  in linux-next tree.

Thanks!
Yijing.

> 
> Thanks,
> 
> 	M.
> 


-- 
Thanks!
Yijing


^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 12:24             ` Yijing Wang
  0 siblings, 0 replies; 57+ messages in thread
From: Yijing Wang @ 2014-12-09 12:24 UTC (permalink / raw)
  To: Marc Zyngier, Bjorn Helgaas, Thomas Gleixner, Jiang Liu
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>> basis for discussion. What I'd like to find out is:
>>>
>>> - What is the minimum granularity for associating a device with its MSI
>>> domain in existing platforms?
>>
>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>> in x86, we will find msi irq domain by pci_dev.
> 
> Are you *really* associating the MSI domain on a per pci-device basis?
> That is, you have devices on the same PCI bus talking to different MSI hw?

Yes.

> 
>> I generally agree your first patch which associate basic device with msi irq domain.
>>
>>> - What topology data structures do you use to find out what MSI
>>> controller a device should be matched with?
>>
>> Now only arm and arm64 use msi controller to setup/teardown msi irqs,
>> in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
>> to be saved in pci_bus. For a more common method to find msi controller/irq domain,
>> I prefer pci_dev/device.
> 
> Forget about msi_controller, the whole goal of this series is to make it
> obsolete. On your x86 platform, what how do you identify which MSI
> domain should be associated with a given PCI device? Surely you must
> have a set of data structures or ACPI tables which give you that
> information.

Yes, by ACPI DMAR table.

> 
>>> - What in-tree platform already has this requirements?
>>
>> As mentioned above, x86 does.
> 
> Let me rephrase that in a non-ambiguous manner: can you point me to a
> file implementing this in mainline?

Please refer to arch/x86/kernel/apic/msi.c  native_setup_msi_irqs()  in linux-next tree.

Thanks!
Yijing.

> 
> Thanks,
> 
> 	M.
> 


-- 
Thanks!
Yijing


^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 12:24             ` Yijing Wang
  0 siblings, 0 replies; 57+ messages in thread
From: Yijing Wang @ 2014-12-09 12:24 UTC (permalink / raw)
  To: linux-arm-kernel

>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>> basis for discussion. What I'd like to find out is:
>>>
>>> - What is the minimum granularity for associating a device with its MSI
>>> domain in existing platforms?
>>
>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>> in x86, we will find msi irq domain by pci_dev.
> 
> Are you *really* associating the MSI domain on a per pci-device basis?
> That is, you have devices on the same PCI bus talking to different MSI hw?

Yes.

> 
>> I generally agree your first patch which associate basic device with msi irq domain.
>>
>>> - What topology data structures do you use to find out what MSI
>>> controller a device should be matched with?
>>
>> Now only arm and arm64 use msi controller to setup/teardown msi irqs,
>> in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
>> to be saved in pci_bus. For a more common method to find msi controller/irq domain,
>> I prefer pci_dev/device.
> 
> Forget about msi_controller, the whole goal of this series is to make it
> obsolete. On your x86 platform, what how do you identify which MSI
> domain should be associated with a given PCI device? Surely you must
> have a set of data structures or ACPI tables which give you that
> information.

Yes, by ACPI DMAR table.

> 
>>> - What in-tree platform already has this requirements?
>>
>> As mentioned above, x86 does.
> 
> Let me rephrase that in a non-ambiguous manner: can you point me to a
> file implementing this in mainline?

Please refer to arch/x86/kernel/apic/msi.c  native_setup_msi_irqs()  in linux-next tree.

Thanks!
Yijing.

> 
> Thanks,
> 
> 	M.
> 


-- 
Thanks!
Yijing

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-09 12:12           ` Marc Zyngier
  (?)
@ 2014-12-09 12:47             ` Jiang Liu
  -1 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 12:47 UTC (permalink / raw)
  To: Marc Zyngier, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

On 2014/12/9 20:12, Marc Zyngier wrote:
> Yijing,
> 
> On 09/12/14 11:57, Yijing Wang wrote:
>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>> +{
>>>>> +}
>>>>> +
>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>> +{
>>>>> +	struct pci_dev *bridge = bus->self;
>>>>> +
>>>>> +	if (!bridge)
>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>> +	else
>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>> +}
>>>>
>>>>
>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>
>> Hi Marc,
>>
>>>
>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>> basis for discussion. What I'd like to find out is:
>>>
>>> - What is the minimum granularity for associating a device with its MSI
>>> domain in existing platforms?
>>
>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>> in x86, we will find msi irq domain by pci_dev.
> 
> Are you *really* associating the MSI domain on a per pci-device basis?
> That is, you have devices on the same PCI bus talking to different MSI hw?
Hi Marc,
	This is a little wild:(
	On x86 platform with Intel VT-d(not the case for AMD-v),
interrupt remapping is tight to DMA remapping (IOMMU) unit.
For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
But it may also manage a specific PCI device. This is typically used to
provide QoS for audio device by using dedicated IOMMU unit to avoid
resource contention on DMA remapping tables. BIOS uses ACPI table to
report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
I have no really experience with such a hardware platform yet, just for
theoretical analysis)
	On the other hand, we now support hierarchy irqdomain. So to
support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
device level.
	This piece of code from your [4/6] is flexible enough, which
retrieves msi_domain from PCI device, then fallback to PCI bus,
then fallback to platform specific method.
	domain = dev_get_msi_domain(&dev->dev);
	if (!domain && dev->bus->msi)
 		domain = dev->bus->msi->domain;
 	if (!domain)
 		domain = arch_get_pci_msi_domain(dev);
Thanks!
Gerry
> 
>> I generally agree your first patch which associate basic device with msi irq domain.
>>
>>> - What topology data structures do you use to find out what MSI
>>> controller a device should be matched with?
>>
>> Now only arm and arm64 use msi controller to setup/teardown msi irqs,
>> in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
>> to be saved in pci_bus. For a more common method to find msi controller/irq domain,
>> I prefer pci_dev/device.
> 
> Forget about msi_controller, the whole goal of this series is to make it
> obsolete. On your x86 platform, what how do you identify which MSI
> domain should be associated with a given PCI device? Surely you must
> have a set of data structures or ACPI tables which give you that
> information.
> 
>>> - What in-tree platform already has this requirements?
>>
>> As mentioned above, x86 does.
> 
> Let me rephrase that in a non-ambiguous manner: can you point me to a
> file implementing this in mainline?
> 
> Thanks,
> 
> 	M.
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 12:47             ` Jiang Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 12:47 UTC (permalink / raw)
  To: Marc Zyngier, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

On 2014/12/9 20:12, Marc Zyngier wrote:
> Yijing,
> 
> On 09/12/14 11:57, Yijing Wang wrote:
>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>> +{
>>>>> +}
>>>>> +
>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>> +{
>>>>> +	struct pci_dev *bridge = bus->self;
>>>>> +
>>>>> +	if (!bridge)
>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>> +	else
>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>> +}
>>>>
>>>>
>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>
>> Hi Marc,
>>
>>>
>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>> basis for discussion. What I'd like to find out is:
>>>
>>> - What is the minimum granularity for associating a device with its MSI
>>> domain in existing platforms?
>>
>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>> in x86, we will find msi irq domain by pci_dev.
> 
> Are you *really* associating the MSI domain on a per pci-device basis?
> That is, you have devices on the same PCI bus talking to different MSI hw?
Hi Marc,
	This is a little wild:(
	On x86 platform with Intel VT-d(not the case for AMD-v),
interrupt remapping is tight to DMA remapping (IOMMU) unit.
For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
But it may also manage a specific PCI device. This is typically used to
provide QoS for audio device by using dedicated IOMMU unit to avoid
resource contention on DMA remapping tables. BIOS uses ACPI table to
report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
I have no really experience with such a hardware platform yet, just for
theoretical analysis)
	On the other hand, we now support hierarchy irqdomain. So to
support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
device level.
	This piece of code from your [4/6] is flexible enough, which
retrieves msi_domain from PCI device, then fallback to PCI bus,
then fallback to platform specific method.
	domain = dev_get_msi_domain(&dev->dev);
	if (!domain && dev->bus->msi)
 		domain = dev->bus->msi->domain;
 	if (!domain)
 		domain = arch_get_pci_msi_domain(dev);
Thanks!
Gerry
> 
>> I generally agree your first patch which associate basic device with msi irq domain.
>>
>>> - What topology data structures do you use to find out what MSI
>>> controller a device should be matched with?
>>
>> Now only arm and arm64 use msi controller to setup/teardown msi irqs,
>> in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
>> to be saved in pci_bus. For a more common method to find msi controller/irq domain,
>> I prefer pci_dev/device.
> 
> Forget about msi_controller, the whole goal of this series is to make it
> obsolete. On your x86 platform, what how do you identify which MSI
> domain should be associated with a given PCI device? Surely you must
> have a set of data structures or ACPI tables which give you that
> information.
> 
>>> - What in-tree platform already has this requirements?
>>
>> As mentioned above, x86 does.
> 
> Let me rephrase that in a non-ambiguous manner: can you point me to a
> file implementing this in mainline?
> 
> Thanks,
> 
> 	M.
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 12:47             ` Jiang Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 12:47 UTC (permalink / raw)
  To: linux-arm-kernel

On 2014/12/9 20:12, Marc Zyngier wrote:
> Yijing,
> 
> On 09/12/14 11:57, Yijing Wang wrote:
>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>> +{
>>>>> +}
>>>>> +
>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>> +{
>>>>> +	struct pci_dev *bridge = bus->self;
>>>>> +
>>>>> +	if (!bridge)
>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>> +	else
>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>> +}
>>>>
>>>>
>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>
>> Hi Marc,
>>
>>>
>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>> basis for discussion. What I'd like to find out is:
>>>
>>> - What is the minimum granularity for associating a device with its MSI
>>> domain in existing platforms?
>>
>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>> in x86, we will find msi irq domain by pci_dev.
> 
> Are you *really* associating the MSI domain on a per pci-device basis?
> That is, you have devices on the same PCI bus talking to different MSI hw?
Hi Marc,
	This is a little wild:(
	On x86 platform with Intel VT-d(not the case for AMD-v),
interrupt remapping is tight to DMA remapping (IOMMU) unit.
For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
But it may also manage a specific PCI device. This is typically used to
provide QoS for audio device by using dedicated IOMMU unit to avoid
resource contention on DMA remapping tables. BIOS uses ACPI table to
report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
I have no really experience with such a hardware platform yet, just for
theoretical analysis)
	On the other hand, we now support hierarchy irqdomain. So to
support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
device level.
	This piece of code from your [4/6] is flexible enough, which
retrieves msi_domain from PCI device, then fallback to PCI bus,
then fallback to platform specific method.
	domain = dev_get_msi_domain(&dev->dev);
	if (!domain && dev->bus->msi)
 		domain = dev->bus->msi->domain;
 	if (!domain)
 		domain = arch_get_pci_msi_domain(dev);
Thanks!
Gerry
> 
>> I generally agree your first patch which associate basic device with msi irq domain.
>>
>>> - What topology data structures do you use to find out what MSI
>>> controller a device should be matched with?
>>
>> Now only arm and arm64 use msi controller to setup/teardown msi irqs,
>> in arm, now msi controller saved in pci_sys_data, and for arm64, it seems
>> to be saved in pci_bus. For a more common method to find msi controller/irq domain,
>> I prefer pci_dev/device.
> 
> Forget about msi_controller, the whole goal of this series is to make it
> obsolete. On your x86 platform, what how do you identify which MSI
> domain should be associated with a given PCI device? Surely you must
> have a set of data structures or ACPI tables which give you that
> information.
> 
>>> - What in-tree platform already has this requirements?
>>
>> As mentioned above, x86 does.
> 
> Let me rephrase that in a non-ambiguous manner: can you point me to a
> file implementing this in mainline?
> 
> Thanks,
> 
> 	M.
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-08 20:12   ` Marc Zyngier
@ 2014-12-09 12:57     ` Jiang Liu
  -1 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 12:57 UTC (permalink / raw)
  To: Marc Zyngier, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	Yijing Wang, Suravee Suthikulpanit



On 2014/12/9 4:12, Marc Zyngier wrote:
> In order to be able to populate the device msi_domain field,
> add the necesary hooks to propagate the PHB msi_domain across
> secondary busses to devices.
> 
> So far, nobody populates the initial msi_domain.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/pci/probe.c | 24 ++++++++++++++++++++++++
>  include/linux/pci.h |  1 +
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index c8ca98c..d1009a2 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>  	}
>  }
>  
> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
> +{
> +}
> +
> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
> +{
> +	struct pci_dev *bridge = bus->self;
> +
> +	if (!bridge)
> +		pcibios_set_phb_msi_domain(bus);
> +	else
> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
> +}
> +
>  static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  					   struct pci_dev *bridge, int busnr)
>  {
> @@ -723,6 +737,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  	bridge->subordinate = child;
>  
>  add_dev:
> +	pci_set_bus_msi_domain(child);
>  	ret = device_register(&child->dev);
>  	WARN_ON(ret < 0);
>  
> @@ -1517,6 +1532,11 @@ static void pci_init_capabilities(struct pci_dev *dev)
>  	pci_enable_acs(dev);
>  }
>  
> +static void pci_set_msi_domain(struct pci_dev *dev)
> +{
> +	dev_set_msi_domain(&dev->dev, dev_get_msi_domain(&dev->bus->dev));
> +}
> +
Hi Marc,
	The default implementation of pci_set_msi_domain() conflicts
with interrupt mapping based on x86 Intel VT-d:(
	If pci_set_msi_domain() is weak, thing gets perfect.
Regards!
Gerry

>  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  {
>  	int ret;
> @@ -1546,6 +1566,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	/* Initialize various capabilities */
>  	pci_init_capabilities(dev);
>  
> +	/* Setup MSI irq domain */
> +	pci_set_msi_domain(dev);
> +
>  	/*
>  	 * Add the device to our list of discovered devices
>  	 * and the bus list for fixup functions, etc.
> @@ -1947,6 +1970,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  	b->bridge = get_device(&bridge->dev);
>  	device_enable_async_suspend(b->bridge);
>  	pci_set_bus_of_node(b);
> +	pci_set_bus_msi_domain(b);
>  
>  	if (!parent)
>  		set_dev_node(b->bridge, pcibus_to_node(b));
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index a523cee..3266764 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1637,6 +1637,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
>  int pcibios_add_device(struct pci_dev *dev);
>  void pcibios_release_device(struct pci_dev *dev);
>  void pcibios_penalize_isa_irq(int irq, int active);
> +void pcibios_set_phb_msi_domain(struct pci_bus *bus);
>  
>  #ifdef CONFIG_HIBERNATE_CALLBACKS
>  extern struct dev_pm_ops pcibios_pm_ops;
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 12:57     ` Jiang Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 12:57 UTC (permalink / raw)
  To: linux-arm-kernel



On 2014/12/9 4:12, Marc Zyngier wrote:
> In order to be able to populate the device msi_domain field,
> add the necesary hooks to propagate the PHB msi_domain across
> secondary busses to devices.
> 
> So far, nobody populates the initial msi_domain.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/pci/probe.c | 24 ++++++++++++++++++++++++
>  include/linux/pci.h |  1 +
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index c8ca98c..d1009a2 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>  	}
>  }
>  
> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
> +{
> +}
> +
> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
> +{
> +	struct pci_dev *bridge = bus->self;
> +
> +	if (!bridge)
> +		pcibios_set_phb_msi_domain(bus);
> +	else
> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
> +}
> +
>  static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  					   struct pci_dev *bridge, int busnr)
>  {
> @@ -723,6 +737,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  	bridge->subordinate = child;
>  
>  add_dev:
> +	pci_set_bus_msi_domain(child);
>  	ret = device_register(&child->dev);
>  	WARN_ON(ret < 0);
>  
> @@ -1517,6 +1532,11 @@ static void pci_init_capabilities(struct pci_dev *dev)
>  	pci_enable_acs(dev);
>  }
>  
> +static void pci_set_msi_domain(struct pci_dev *dev)
> +{
> +	dev_set_msi_domain(&dev->dev, dev_get_msi_domain(&dev->bus->dev));
> +}
> +
Hi Marc,
	The default implementation of pci_set_msi_domain() conflicts
with interrupt mapping based on x86 Intel VT-d:(
	If pci_set_msi_domain() is weak, thing gets perfect.
Regards!
Gerry

>  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  {
>  	int ret;
> @@ -1546,6 +1566,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	/* Initialize various capabilities */
>  	pci_init_capabilities(dev);
>  
> +	/* Setup MSI irq domain */
> +	pci_set_msi_domain(dev);
> +
>  	/*
>  	 * Add the device to our list of discovered devices
>  	 * and the bus list for fixup functions, etc.
> @@ -1947,6 +1970,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  	b->bridge = get_device(&bridge->dev);
>  	device_enable_async_suspend(b->bridge);
>  	pci_set_bus_of_node(b);
> +	pci_set_bus_msi_domain(b);
>  
>  	if (!parent)
>  		set_dev_node(b->bridge, pcibus_to_node(b));
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index a523cee..3266764 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1637,6 +1637,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
>  int pcibios_add_device(struct pci_dev *dev);
>  void pcibios_release_device(struct pci_dev *dev);
>  void pcibios_penalize_isa_irq(int irq, int active);
> +void pcibios_set_phb_msi_domain(struct pci_bus *bus);
>  
>  #ifdef CONFIG_HIBERNATE_CALLBACKS
>  extern struct dev_pm_ops pcibios_pm_ops;
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-09 12:47             ` Jiang Liu
  (?)
@ 2014-12-09 14:03               ` Marc Zyngier
  -1 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 14:03 UTC (permalink / raw)
  To: Jiang Liu, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

Hi Gerry,

On 09/12/14 12:47, Jiang Liu wrote:
> On 2014/12/9 20:12, Marc Zyngier wrote:
>> Yijing,
>>
>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>> +{
>>>>>> +}
>>>>>> +
>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>> +{
>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>> +
>>>>>> +	if (!bridge)
>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>> +	else
>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>> +}
>>>>>
>>>>>
>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>
>>> Hi Marc,
>>>
>>>>
>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>> basis for discussion. What I'd like to find out is:
>>>>
>>>> - What is the minimum granularity for associating a device with its MSI
>>>> domain in existing platforms?
>>>
>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>> in x86, we will find msi irq domain by pci_dev.
>>
>> Are you *really* associating the MSI domain on a per pci-device basis?
>> That is, you have devices on the same PCI bus talking to different MSI hw?
> Hi Marc,
> 	This is a little wild:(
> 	On x86 platform with Intel VT-d(not the case for AMD-v),
> interrupt remapping is tight to DMA remapping (IOMMU) unit.
> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
> But it may also manage a specific PCI device. This is typically used to
> provide QoS for audio device by using dedicated IOMMU unit to avoid
> resource contention on DMA remapping tables. BIOS uses ACPI table to
> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
> I have no really experience with such a hardware platform yet, just for
> theoretical analysis)
> 	On the other hand, we now support hierarchy irqdomain. So to
> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
> device level.
> 	This piece of code from your [4/6] is flexible enough, which
> retrieves msi_domain from PCI device, then fallback to PCI bus,
> then fallback to platform specific method.
> 	domain = dev_get_msi_domain(&dev->dev);
> 	if (!domain && dev->bus->msi)
>  		domain = dev->bus->msi->domain;
>  	if (!domain)
>  		domain = arch_get_pci_msi_domain(dev);

OK. But what I'd really like to see is a way to setup the
device<->domain binding as early as possible, without having to use more
conditional code in pci_msi_get_domain.

IOW, can we do something similar to what pci_set_bus_msi_domain and
pci_set_msi_domain do in this patch?

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 14:03               ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 14:03 UTC (permalink / raw)
  To: Jiang Liu, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

Hi Gerry,

On 09/12/14 12:47, Jiang Liu wrote:
> On 2014/12/9 20:12, Marc Zyngier wrote:
>> Yijing,
>>
>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>> +{
>>>>>> +}
>>>>>> +
>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>> +{
>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>> +
>>>>>> +	if (!bridge)
>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>> +	else
>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>> +}
>>>>>
>>>>>
>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>
>>> Hi Marc,
>>>
>>>>
>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>> basis for discussion. What I'd like to find out is:
>>>>
>>>> - What is the minimum granularity for associating a device with its MSI
>>>> domain in existing platforms?
>>>
>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>> in x86, we will find msi irq domain by pci_dev.
>>
>> Are you *really* associating the MSI domain on a per pci-device basis?
>> That is, you have devices on the same PCI bus talking to different MSI hw?
> Hi Marc,
> 	This is a little wild:(
> 	On x86 platform with Intel VT-d(not the case for AMD-v),
> interrupt remapping is tight to DMA remapping (IOMMU) unit.
> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
> But it may also manage a specific PCI device. This is typically used to
> provide QoS for audio device by using dedicated IOMMU unit to avoid
> resource contention on DMA remapping tables. BIOS uses ACPI table to
> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
> I have no really experience with such a hardware platform yet, just for
> theoretical analysis)
> 	On the other hand, we now support hierarchy irqdomain. So to
> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
> device level.
> 	This piece of code from your [4/6] is flexible enough, which
> retrieves msi_domain from PCI device, then fallback to PCI bus,
> then fallback to platform specific method.
> 	domain = dev_get_msi_domain(&dev->dev);
> 	if (!domain && dev->bus->msi)
>  		domain = dev->bus->msi->domain;
>  	if (!domain)
>  		domain = arch_get_pci_msi_domain(dev);

OK. But what I'd really like to see is a way to setup the
device<->domain binding as early as possible, without having to use more
conditional code in pci_msi_get_domain.

IOW, can we do something similar to what pci_set_bus_msi_domain and
pci_set_msi_domain do in this patch?

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 14:03               ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 14:03 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Gerry,

On 09/12/14 12:47, Jiang Liu wrote:
> On 2014/12/9 20:12, Marc Zyngier wrote:
>> Yijing,
>>
>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>> +{
>>>>>> +}
>>>>>> +
>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>> +{
>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>> +
>>>>>> +	if (!bridge)
>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>> +	else
>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>> +}
>>>>>
>>>>>
>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>
>>> Hi Marc,
>>>
>>>>
>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>> basis for discussion. What I'd like to find out is:
>>>>
>>>> - What is the minimum granularity for associating a device with its MSI
>>>> domain in existing platforms?
>>>
>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>> in x86, we will find msi irq domain by pci_dev.
>>
>> Are you *really* associating the MSI domain on a per pci-device basis?
>> That is, you have devices on the same PCI bus talking to different MSI hw?
> Hi Marc,
> 	This is a little wild:(
> 	On x86 platform with Intel VT-d(not the case for AMD-v),
> interrupt remapping is tight to DMA remapping (IOMMU) unit.
> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
> But it may also manage a specific PCI device. This is typically used to
> provide QoS for audio device by using dedicated IOMMU unit to avoid
> resource contention on DMA remapping tables. BIOS uses ACPI table to
> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
> I have no really experience with such a hardware platform yet, just for
> theoretical analysis)
> 	On the other hand, we now support hierarchy irqdomain. So to
> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
> device level.
> 	This piece of code from your [4/6] is flexible enough, which
> retrieves msi_domain from PCI device, then fallback to PCI bus,
> then fallback to platform specific method.
> 	domain = dev_get_msi_domain(&dev->dev);
> 	if (!domain && dev->bus->msi)
>  		domain = dev->bus->msi->domain;
>  	if (!domain)
>  		domain = arch_get_pci_msi_domain(dev);

OK. But what I'd really like to see is a way to setup the
device<->domain binding as early as possible, without having to use more
conditional code in pci_msi_get_domain.

IOW, can we do something similar to what pci_set_bus_msi_domain and
pci_set_msi_domain do in this patch?

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-09 14:03               ` Marc Zyngier
  (?)
@ 2014-12-09 14:11                 ` Jiang Liu
  -1 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 14:11 UTC (permalink / raw)
  To: Marc Zyngier, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

On 2014/12/9 22:03, Marc Zyngier wrote:
> Hi Gerry,
> 
> On 09/12/14 12:47, Jiang Liu wrote:
>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>> Yijing,
>>>
>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>> +{
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>> +{
>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>> +
>>>>>>> +	if (!bridge)
>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>> +	else
>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>> +}
>>>>>>
>>>>>>
>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>
>>>> Hi Marc,
>>>>
>>>>>
>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>> basis for discussion. What I'd like to find out is:
>>>>>
>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>> domain in existing platforms?
>>>>
>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>> in x86, we will find msi irq domain by pci_dev.
>>>
>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>> Hi Marc,
>> 	This is a little wild:(
>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>> But it may also manage a specific PCI device. This is typically used to
>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>> I have no really experience with such a hardware platform yet, just for
>> theoretical analysis)
>> 	On the other hand, we now support hierarchy irqdomain. So to
>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>> device level.
>> 	This piece of code from your [4/6] is flexible enough, which
>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>> then fallback to platform specific method.
>> 	domain = dev_get_msi_domain(&dev->dev);
>> 	if (!domain && dev->bus->msi)
>>  		domain = dev->bus->msi->domain;
>>  	if (!domain)
>>  		domain = arch_get_pci_msi_domain(dev);
> 
> OK. But what I'd really like to see is a way to setup the
> device<->domain binding as early as possible, without having to use more
> conditional code in pci_msi_get_domain.
> 
> IOW, can we do something similar to what pci_set_bus_msi_domain and
> pci_set_msi_domain do in this patch?
Hi Marc,
	I have checked x86 code, we could set pci_dev->msi_domain
when creating PCI devices, just need to find some hook points
into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
PCI MSI core may provide a default way to set pci_dev->msi_domain.
This may make the implementation simpler, I guess:)
Thanks!
Gerry

> 
> Thanks,
> 
> 	M.
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 14:11                 ` Jiang Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 14:11 UTC (permalink / raw)
  To: Marc Zyngier, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

On 2014/12/9 22:03, Marc Zyngier wrote:
> Hi Gerry,
> 
> On 09/12/14 12:47, Jiang Liu wrote:
>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>> Yijing,
>>>
>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>> +{
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>> +{
>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>> +
>>>>>>> +	if (!bridge)
>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>> +	else
>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>> +}
>>>>>>
>>>>>>
>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>
>>>> Hi Marc,
>>>>
>>>>>
>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>> basis for discussion. What I'd like to find out is:
>>>>>
>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>> domain in existing platforms?
>>>>
>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>> in x86, we will find msi irq domain by pci_dev.
>>>
>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>> Hi Marc,
>> 	This is a little wild:(
>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>> But it may also manage a specific PCI device. This is typically used to
>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>> I have no really experience with such a hardware platform yet, just for
>> theoretical analysis)
>> 	On the other hand, we now support hierarchy irqdomain. So to
>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>> device level.
>> 	This piece of code from your [4/6] is flexible enough, which
>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>> then fallback to platform specific method.
>> 	domain = dev_get_msi_domain(&dev->dev);
>> 	if (!domain && dev->bus->msi)
>>  		domain = dev->bus->msi->domain;
>>  	if (!domain)
>>  		domain = arch_get_pci_msi_domain(dev);
> 
> OK. But what I'd really like to see is a way to setup the
> device<->domain binding as early as possible, without having to use more
> conditional code in pci_msi_get_domain.
> 
> IOW, can we do something similar to what pci_set_bus_msi_domain and
> pci_set_msi_domain do in this patch?
Hi Marc,
	I have checked x86 code, we could set pci_dev->msi_domain
when creating PCI devices, just need to find some hook points
into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
PCI MSI core may provide a default way to set pci_dev->msi_domain.
This may make the implementation simpler, I guess:)
Thanks!
Gerry

> 
> Thanks,
> 
> 	M.
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 14:11                 ` Jiang Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

On 2014/12/9 22:03, Marc Zyngier wrote:
> Hi Gerry,
> 
> On 09/12/14 12:47, Jiang Liu wrote:
>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>> Yijing,
>>>
>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>> +{
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>> +{
>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>> +
>>>>>>> +	if (!bridge)
>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>> +	else
>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>> +}
>>>>>>
>>>>>>
>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>
>>>> Hi Marc,
>>>>
>>>>>
>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>> basis for discussion. What I'd like to find out is:
>>>>>
>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>> domain in existing platforms?
>>>>
>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>> in x86, we will find msi irq domain by pci_dev.
>>>
>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>> Hi Marc,
>> 	This is a little wild:(
>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>> But it may also manage a specific PCI device. This is typically used to
>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>> I have no really experience with such a hardware platform yet, just for
>> theoretical analysis)
>> 	On the other hand, we now support hierarchy irqdomain. So to
>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>> device level.
>> 	This piece of code from your [4/6] is flexible enough, which
>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>> then fallback to platform specific method.
>> 	domain = dev_get_msi_domain(&dev->dev);
>> 	if (!domain && dev->bus->msi)
>>  		domain = dev->bus->msi->domain;
>>  	if (!domain)
>>  		domain = arch_get_pci_msi_domain(dev);
> 
> OK. But what I'd really like to see is a way to setup the
> device<->domain binding as early as possible, without having to use more
> conditional code in pci_msi_get_domain.
> 
> IOW, can we do something similar to what pci_set_bus_msi_domain and
> pci_set_msi_domain do in this patch?
Hi Marc,
	I have checked x86 code, we could set pci_dev->msi_domain
when creating PCI devices, just need to find some hook points
into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
PCI MSI core may provide a default way to set pci_dev->msi_domain.
This may make the implementation simpler, I guess:)
Thanks!
Gerry

> 
> Thanks,
> 
> 	M.
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-09 14:11                 ` Jiang Liu
  (?)
@ 2014-12-09 14:27                   ` Marc Zyngier
  -1 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 14:27 UTC (permalink / raw)
  To: Jiang Liu, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

On 09/12/14 14:11, Jiang Liu wrote:
> On 2014/12/9 22:03, Marc Zyngier wrote:
>> Hi Gerry,
>>
>> On 09/12/14 12:47, Jiang Liu wrote:
>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>> Yijing,
>>>>
>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>> +{
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>> +{
>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>> +
>>>>>>>> +	if (!bridge)
>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>> +	else
>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>> +}
>>>>>>>
>>>>>>>
>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>
>>>>> Hi Marc,
>>>>>
>>>>>>
>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>
>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>> domain in existing platforms?
>>>>>
>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>
>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>> Hi Marc,
>>> 	This is a little wild:(
>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>> But it may also manage a specific PCI device. This is typically used to
>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>> I have no really experience with such a hardware platform yet, just for
>>> theoretical analysis)
>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>> device level.
>>> 	This piece of code from your [4/6] is flexible enough, which
>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>> then fallback to platform specific method.
>>> 	domain = dev_get_msi_domain(&dev->dev);
>>> 	if (!domain && dev->bus->msi)
>>>  		domain = dev->bus->msi->domain;
>>>  	if (!domain)
>>>  		domain = arch_get_pci_msi_domain(dev);
>>
>> OK. But what I'd really like to see is a way to setup the
>> device<->domain binding as early as possible, without having to use more
>> conditional code in pci_msi_get_domain.
>>
>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>> pci_set_msi_domain do in this patch?
> Hi Marc,
> 	I have checked x86 code, we could set pci_dev->msi_domain
> when creating PCI devices, just need to find some hook points
> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
> PCI MSI core may provide a default way to set pci_dev->msi_domain.
> This may make the implementation simpler, I guess:)

Right. So following your earlier suggestion, I could make
pci_set_msi_domain a weak symbol and let arch code override this.

My preference would have been to have arch code to create a set of
arch-independent data structures describing the topology, and use that
for everything, but maybe that's a bit ambitious for a start.

I'll rework the series to make the symbols weak.

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 14:27                   ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 14:27 UTC (permalink / raw)
  To: Jiang Liu, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

On 09/12/14 14:11, Jiang Liu wrote:
> On 2014/12/9 22:03, Marc Zyngier wrote:
>> Hi Gerry,
>>
>> On 09/12/14 12:47, Jiang Liu wrote:
>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>> Yijing,
>>>>
>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>> +{
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>> +{
>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>> +
>>>>>>>> +	if (!bridge)
>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>> +	else
>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>> +}
>>>>>>>
>>>>>>>
>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>
>>>>> Hi Marc,
>>>>>
>>>>>>
>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>
>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>> domain in existing platforms?
>>>>>
>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>
>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>> Hi Marc,
>>> 	This is a little wild:(
>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>> But it may also manage a specific PCI device. This is typically used to
>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>> I have no really experience with such a hardware platform yet, just for
>>> theoretical analysis)
>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>> device level.
>>> 	This piece of code from your [4/6] is flexible enough, which
>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>> then fallback to platform specific method.
>>> 	domain = dev_get_msi_domain(&dev->dev);
>>> 	if (!domain && dev->bus->msi)
>>>  		domain = dev->bus->msi->domain;
>>>  	if (!domain)
>>>  		domain = arch_get_pci_msi_domain(dev);
>>
>> OK. But what I'd really like to see is a way to setup the
>> device<->domain binding as early as possible, without having to use more
>> conditional code in pci_msi_get_domain.
>>
>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>> pci_set_msi_domain do in this patch?
> Hi Marc,
> 	I have checked x86 code, we could set pci_dev->msi_domain
> when creating PCI devices, just need to find some hook points
> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
> PCI MSI core may provide a default way to set pci_dev->msi_domain.
> This may make the implementation simpler, I guess:)

Right. So following your earlier suggestion, I could make
pci_set_msi_domain a weak symbol and let arch code override this.

My preference would have been to have arch code to create a set of
arch-independent data structures describing the topology, and use that
for everything, but maybe that's a bit ambitious for a start.

I'll rework the series to make the symbols weak.

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 14:27                   ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 14:27 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/12/14 14:11, Jiang Liu wrote:
> On 2014/12/9 22:03, Marc Zyngier wrote:
>> Hi Gerry,
>>
>> On 09/12/14 12:47, Jiang Liu wrote:
>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>> Yijing,
>>>>
>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>> +{
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>> +{
>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>> +
>>>>>>>> +	if (!bridge)
>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>> +	else
>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>> +}
>>>>>>>
>>>>>>>
>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>
>>>>> Hi Marc,
>>>>>
>>>>>>
>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>
>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>> domain in existing platforms?
>>>>>
>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>
>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>> Hi Marc,
>>> 	This is a little wild:(
>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>> But it may also manage a specific PCI device. This is typically used to
>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>> I have no really experience with such a hardware platform yet, just for
>>> theoretical analysis)
>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>> device level.
>>> 	This piece of code from your [4/6] is flexible enough, which
>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>> then fallback to platform specific method.
>>> 	domain = dev_get_msi_domain(&dev->dev);
>>> 	if (!domain && dev->bus->msi)
>>>  		domain = dev->bus->msi->domain;
>>>  	if (!domain)
>>>  		domain = arch_get_pci_msi_domain(dev);
>>
>> OK. But what I'd really like to see is a way to setup the
>> device<->domain binding as early as possible, without having to use more
>> conditional code in pci_msi_get_domain.
>>
>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>> pci_set_msi_domain do in this patch?
> Hi Marc,
> 	I have checked x86 code, we could set pci_dev->msi_domain
> when creating PCI devices, just need to find some hook points
> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
> PCI MSI core may provide a default way to set pci_dev->msi_domain.
> This may make the implementation simpler, I guess:)

Right. So following your earlier suggestion, I could make
pci_set_msi_domain a weak symbol and let arch code override this.

My preference would have been to have arch code to create a set of
arch-independent data structures describing the topology, and use that
for everything, but maybe that's a bit ambitious for a start.

I'll rework the series to make the symbols weak.

Thanks,

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-09 14:27                   ` Marc Zyngier
  (?)
@ 2014-12-09 14:35                     ` Jiang Liu
  -1 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 14:35 UTC (permalink / raw)
  To: Marc Zyngier, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

On 2014/12/9 22:27, Marc Zyngier wrote:
> On 09/12/14 14:11, Jiang Liu wrote:
>> On 2014/12/9 22:03, Marc Zyngier wrote:
>>> Hi Gerry,
>>>
>>> On 09/12/14 12:47, Jiang Liu wrote:
>>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>>> Yijing,
>>>>>
>>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>>> +{
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>>> +{
>>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>>> +
>>>>>>>>> +	if (!bridge)
>>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>>> +	else
>>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>>> +}
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>>
>>>>>> Hi Marc,
>>>>>>
>>>>>>>
>>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>>
>>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>>> domain in existing platforms?
>>>>>>
>>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>>
>>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>>> Hi Marc,
>>>> 	This is a little wild:(
>>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>>> But it may also manage a specific PCI device. This is typically used to
>>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>>> I have no really experience with such a hardware platform yet, just for
>>>> theoretical analysis)
>>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>>> device level.
>>>> 	This piece of code from your [4/6] is flexible enough, which
>>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>>> then fallback to platform specific method.
>>>> 	domain = dev_get_msi_domain(&dev->dev);
>>>> 	if (!domain && dev->bus->msi)
>>>>  		domain = dev->bus->msi->domain;
>>>>  	if (!domain)
>>>>  		domain = arch_get_pci_msi_domain(dev);
>>>
>>> OK. But what I'd really like to see is a way to setup the
>>> device<->domain binding as early as possible, without having to use more
>>> conditional code in pci_msi_get_domain.
>>>
>>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>>> pci_set_msi_domain do in this patch?
>> Hi Marc,
>> 	I have checked x86 code, we could set pci_dev->msi_domain
>> when creating PCI devices, just need to find some hook points
>> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
>> PCI MSI core may provide a default way to set pci_dev->msi_domain.
>> This may make the implementation simpler, I guess:)
> 
> Right. So following your earlier suggestion, I could make
> pci_set_msi_domain a weak symbol and let arch code override this.
> 
> My preference would have been to have arch code to create a set of
> arch-independent data structures describing the topology, and use that
> for everything, but maybe that's a bit ambitious for a start.
> 
> I'll rework the series to make the symbols weak.
Hi Marc,
	I think we may not need the weak symbol at all. With following
draft patch, the PCI MSI core may simply do:
	if (pci_dev->dev.msi_domain == NULL)
		dev_set_msi_domain(&dev->dev,
			 dev_get_msi_domain(&dev->bus->dev));
-----------------------------------------------------------------------
Note: the patch won't pass compilation, just to show the key idea:)

diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
index da163da5fdee..8147d25d4349 100644
--- a/arch/x86/kernel/apic/msi.c
+++ b/arch/x86/kernel/apic/msi.c
@@ -67,6 +67,23 @@ static struct irq_chip pci_msi_controller = {
        .flags                  = IRQCHIP_SKIP_SET_WAKE,
 };

+struct irq_domain *x86_get_pci_msi_domain(struct pci_dev *dev)
+{
+       struct irq_domain *domain;
+       struct irq_alloc_info info;
+
+       init_irq_alloc_info(&info, NULL);
+       info.type = X86_IRQ_ALLOC_TYPE_MSI;
+       info.msi_dev = dev;
+       domain = irq_remapping_get_irq_domain(&info);
+       if (domain == NULL)
+               domain = msi_default_domain;
+       if (domain == NULL)
+               domain = ERR_PTR(-ENOSYS);
+
+       return domain;
+}
+
 int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 {
        struct irq_domain *domain;
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 7b20bccf3648..a26f30a8bb8f 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -652,6 +652,9 @@ int pcibios_add_device(struct pci_dev *dev)
                pa_data = data->next;
                iounmap(data);
        }
+
+       dev->dev.msi_domain = x86_get_pci_msi_domain(dev);
+
        return 0;
 }


> 
> Thanks,
> 
> 	M.
> 

^ permalink raw reply related	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 14:35                     ` Jiang Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 14:35 UTC (permalink / raw)
  To: Marc Zyngier, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

On 2014/12/9 22:27, Marc Zyngier wrote:
> On 09/12/14 14:11, Jiang Liu wrote:
>> On 2014/12/9 22:03, Marc Zyngier wrote:
>>> Hi Gerry,
>>>
>>> On 09/12/14 12:47, Jiang Liu wrote:
>>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>>> Yijing,
>>>>>
>>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>>> +{
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>>> +{
>>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>>> +
>>>>>>>>> +	if (!bridge)
>>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>>> +	else
>>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>>> +}
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>>
>>>>>> Hi Marc,
>>>>>>
>>>>>>>
>>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>>
>>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>>> domain in existing platforms?
>>>>>>
>>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>>
>>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>>> Hi Marc,
>>>> 	This is a little wild:(
>>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>>> But it may also manage a specific PCI device. This is typically used to
>>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>>> I have no really experience with such a hardware platform yet, just for
>>>> theoretical analysis)
>>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>>> device level.
>>>> 	This piece of code from your [4/6] is flexible enough, which
>>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>>> then fallback to platform specific method.
>>>> 	domain = dev_get_msi_domain(&dev->dev);
>>>> 	if (!domain && dev->bus->msi)
>>>>  		domain = dev->bus->msi->domain;
>>>>  	if (!domain)
>>>>  		domain = arch_get_pci_msi_domain(dev);
>>>
>>> OK. But what I'd really like to see is a way to setup the
>>> device<->domain binding as early as possible, without having to use more
>>> conditional code in pci_msi_get_domain.
>>>
>>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>>> pci_set_msi_domain do in this patch?
>> Hi Marc,
>> 	I have checked x86 code, we could set pci_dev->msi_domain
>> when creating PCI devices, just need to find some hook points
>> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
>> PCI MSI core may provide a default way to set pci_dev->msi_domain.
>> This may make the implementation simpler, I guess:)
> 
> Right. So following your earlier suggestion, I could make
> pci_set_msi_domain a weak symbol and let arch code override this.
> 
> My preference would have been to have arch code to create a set of
> arch-independent data structures describing the topology, and use that
> for everything, but maybe that's a bit ambitious for a start.
> 
> I'll rework the series to make the symbols weak.
Hi Marc,
	I think we may not need the weak symbol at all. With following
draft patch, the PCI MSI core may simply do:
	if (pci_dev->dev.msi_domain == NULL)
		dev_set_msi_domain(&dev->dev,
			 dev_get_msi_domain(&dev->bus->dev));
-----------------------------------------------------------------------
Note: the patch won't pass compilation, just to show the key idea:)

diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
index da163da5fdee..8147d25d4349 100644
--- a/arch/x86/kernel/apic/msi.c
+++ b/arch/x86/kernel/apic/msi.c
@@ -67,6 +67,23 @@ static struct irq_chip pci_msi_controller = {
        .flags                  = IRQCHIP_SKIP_SET_WAKE,
 };

+struct irq_domain *x86_get_pci_msi_domain(struct pci_dev *dev)
+{
+       struct irq_domain *domain;
+       struct irq_alloc_info info;
+
+       init_irq_alloc_info(&info, NULL);
+       info.type = X86_IRQ_ALLOC_TYPE_MSI;
+       info.msi_dev = dev;
+       domain = irq_remapping_get_irq_domain(&info);
+       if (domain == NULL)
+               domain = msi_default_domain;
+       if (domain == NULL)
+               domain = ERR_PTR(-ENOSYS);
+
+       return domain;
+}
+
 int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 {
        struct irq_domain *domain;
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 7b20bccf3648..a26f30a8bb8f 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -652,6 +652,9 @@ int pcibios_add_device(struct pci_dev *dev)
                pa_data = data->next;
                iounmap(data);
        }
+
+       dev->dev.msi_domain = x86_get_pci_msi_domain(dev);
+
        return 0;
 }


> 
> Thanks,
> 
> 	M.
> 

^ permalink raw reply related	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 14:35                     ` Jiang Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 14:35 UTC (permalink / raw)
  To: linux-arm-kernel

On 2014/12/9 22:27, Marc Zyngier wrote:
> On 09/12/14 14:11, Jiang Liu wrote:
>> On 2014/12/9 22:03, Marc Zyngier wrote:
>>> Hi Gerry,
>>>
>>> On 09/12/14 12:47, Jiang Liu wrote:
>>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>>> Yijing,
>>>>>
>>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>>> +{
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>>> +{
>>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>>> +
>>>>>>>>> +	if (!bridge)
>>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>>> +	else
>>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>>> +}
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>>
>>>>>> Hi Marc,
>>>>>>
>>>>>>>
>>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>>
>>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>>> domain in existing platforms?
>>>>>>
>>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>>
>>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>>> Hi Marc,
>>>> 	This is a little wild:(
>>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>>> But it may also manage a specific PCI device. This is typically used to
>>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>>> I have no really experience with such a hardware platform yet, just for
>>>> theoretical analysis)
>>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>>> device level.
>>>> 	This piece of code from your [4/6] is flexible enough, which
>>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>>> then fallback to platform specific method.
>>>> 	domain = dev_get_msi_domain(&dev->dev);
>>>> 	if (!domain && dev->bus->msi)
>>>>  		domain = dev->bus->msi->domain;
>>>>  	if (!domain)
>>>>  		domain = arch_get_pci_msi_domain(dev);
>>>
>>> OK. But what I'd really like to see is a way to setup the
>>> device<->domain binding as early as possible, without having to use more
>>> conditional code in pci_msi_get_domain.
>>>
>>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>>> pci_set_msi_domain do in this patch?
>> Hi Marc,
>> 	I have checked x86 code, we could set pci_dev->msi_domain
>> when creating PCI devices, just need to find some hook points
>> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
>> PCI MSI core may provide a default way to set pci_dev->msi_domain.
>> This may make the implementation simpler, I guess:)
> 
> Right. So following your earlier suggestion, I could make
> pci_set_msi_domain a weak symbol and let arch code override this.
> 
> My preference would have been to have arch code to create a set of
> arch-independent data structures describing the topology, and use that
> for everything, but maybe that's a bit ambitious for a start.
> 
> I'll rework the series to make the symbols weak.
Hi Marc,
	I think we may not need the weak symbol at all. With following
draft patch, the PCI MSI core may simply do:
	if (pci_dev->dev.msi_domain == NULL)
		dev_set_msi_domain(&dev->dev,
			 dev_get_msi_domain(&dev->bus->dev));
-----------------------------------------------------------------------
Note: the patch won't pass compilation, just to show the key idea:)

diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
index da163da5fdee..8147d25d4349 100644
--- a/arch/x86/kernel/apic/msi.c
+++ b/arch/x86/kernel/apic/msi.c
@@ -67,6 +67,23 @@ static struct irq_chip pci_msi_controller = {
        .flags                  = IRQCHIP_SKIP_SET_WAKE,
 };

+struct irq_domain *x86_get_pci_msi_domain(struct pci_dev *dev)
+{
+       struct irq_domain *domain;
+       struct irq_alloc_info info;
+
+       init_irq_alloc_info(&info, NULL);
+       info.type = X86_IRQ_ALLOC_TYPE_MSI;
+       info.msi_dev = dev;
+       domain = irq_remapping_get_irq_domain(&info);
+       if (domain == NULL)
+               domain = msi_default_domain;
+       if (domain == NULL)
+               domain = ERR_PTR(-ENOSYS);
+
+       return domain;
+}
+
 int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 {
        struct irq_domain *domain;
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 7b20bccf3648..a26f30a8bb8f 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -652,6 +652,9 @@ int pcibios_add_device(struct pci_dev *dev)
                pa_data = data->next;
                iounmap(data);
        }
+
+       dev->dev.msi_domain = x86_get_pci_msi_domain(dev);
+
        return 0;
 }


> 
> Thanks,
> 
> 	M.
> 

^ permalink raw reply related	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-09 14:35                     ` Jiang Liu
  (?)
@ 2014-12-09 14:59                       ` Marc Zyngier
  -1 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 14:59 UTC (permalink / raw)
  To: Jiang Liu, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

On 09/12/14 14:35, Jiang Liu wrote:
> On 2014/12/9 22:27, Marc Zyngier wrote:
>> On 09/12/14 14:11, Jiang Liu wrote:
>>> On 2014/12/9 22:03, Marc Zyngier wrote:
>>>> Hi Gerry,
>>>>
>>>> On 09/12/14 12:47, Jiang Liu wrote:
>>>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>>>> Yijing,
>>>>>>
>>>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>>>> +{
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>>>> +{
>>>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>>>> +
>>>>>>>>>> +	if (!bridge)
>>>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>>>> +	else
>>>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>>>> +}
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>>>
>>>>>>> Hi Marc,
>>>>>>>
>>>>>>>>
>>>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>>>
>>>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>>>> domain in existing platforms?
>>>>>>>
>>>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>>>
>>>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>>>> Hi Marc,
>>>>> 	This is a little wild:(
>>>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>>>> But it may also manage a specific PCI device. This is typically used to
>>>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>>>> I have no really experience with such a hardware platform yet, just for
>>>>> theoretical analysis)
>>>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>>>> device level.
>>>>> 	This piece of code from your [4/6] is flexible enough, which
>>>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>>>> then fallback to platform specific method.
>>>>> 	domain = dev_get_msi_domain(&dev->dev);
>>>>> 	if (!domain && dev->bus->msi)
>>>>>  		domain = dev->bus->msi->domain;
>>>>>  	if (!domain)
>>>>>  		domain = arch_get_pci_msi_domain(dev);
>>>>
>>>> OK. But what I'd really like to see is a way to setup the
>>>> device<->domain binding as early as possible, without having to use more
>>>> conditional code in pci_msi_get_domain.
>>>>
>>>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>>>> pci_set_msi_domain do in this patch?
>>> Hi Marc,
>>> 	I have checked x86 code, we could set pci_dev->msi_domain
>>> when creating PCI devices, just need to find some hook points
>>> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
>>> PCI MSI core may provide a default way to set pci_dev->msi_domain.
>>> This may make the implementation simpler, I guess:)
>>
>> Right. So following your earlier suggestion, I could make
>> pci_set_msi_domain a weak symbol and let arch code override this.
>>
>> My preference would have been to have arch code to create a set of
>> arch-independent data structures describing the topology, and use that
>> for everything, but maybe that's a bit ambitious for a start.
>>
>> I'll rework the series to make the symbols weak.
> Hi Marc,
> 	I think we may not need the weak symbol at all. With following
> draft patch, the PCI MSI core may simply do:
> 	if (pci_dev->dev.msi_domain == NULL)
> 		dev_set_msi_domain(&dev->dev,
> 			 dev_get_msi_domain(&dev->bus->dev));
> -----------------------------------------------------------------------
> Note: the patch won't pass compilation, just to show the key idea:)
> 
> diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
> index da163da5fdee..8147d25d4349 100644
> --- a/arch/x86/kernel/apic/msi.c
> +++ b/arch/x86/kernel/apic/msi.c
> @@ -67,6 +67,23 @@ static struct irq_chip pci_msi_controller = {
>         .flags                  = IRQCHIP_SKIP_SET_WAKE,
>  };
> 
> +struct irq_domain *x86_get_pci_msi_domain(struct pci_dev *dev)
> +{
> +       struct irq_domain *domain;
> +       struct irq_alloc_info info;
> +
> +       init_irq_alloc_info(&info, NULL);
> +       info.type = X86_IRQ_ALLOC_TYPE_MSI;
> +       info.msi_dev = dev;
> +       domain = irq_remapping_get_irq_domain(&info);
> +       if (domain == NULL)
> +               domain = msi_default_domain;
> +       if (domain == NULL)
> +               domain = ERR_PTR(-ENOSYS);
> +
> +       return domain;
> +}
> +
>  int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>  {
>         struct irq_domain *domain;
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 7b20bccf3648..a26f30a8bb8f 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -652,6 +652,9 @@ int pcibios_add_device(struct pci_dev *dev)
>                 pa_data = data->next;
>                 iounmap(data);
>         }
> +
> +       dev->dev.msi_domain = x86_get_pci_msi_domain(dev);
> +
>         return 0;
>  }

Right. So you set the msi_domain using the pcibios_add_device callback.
That will require some minimal surgery (the call to pci_set_msi_domain
happens before the pcibios call, so it needs to be relocated after), but
that seems like a sensible solution to me.

Thanks!

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 14:59                       ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 14:59 UTC (permalink / raw)
  To: Jiang Liu, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit

On 09/12/14 14:35, Jiang Liu wrote:
> On 2014/12/9 22:27, Marc Zyngier wrote:
>> On 09/12/14 14:11, Jiang Liu wrote:
>>> On 2014/12/9 22:03, Marc Zyngier wrote:
>>>> Hi Gerry,
>>>>
>>>> On 09/12/14 12:47, Jiang Liu wrote:
>>>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>>>> Yijing,
>>>>>>
>>>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>>>> +{
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>>>> +{
>>>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>>>> +
>>>>>>>>>> +	if (!bridge)
>>>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>>>> +	else
>>>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>>>> +}
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>>>
>>>>>>> Hi Marc,
>>>>>>>
>>>>>>>>
>>>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>>>
>>>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>>>> domain in existing platforms?
>>>>>>>
>>>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>>>
>>>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>>>> Hi Marc,
>>>>> 	This is a little wild:(
>>>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>>>> But it may also manage a specific PCI device. This is typically used to
>>>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>>>> I have no really experience with such a hardware platform yet, just for
>>>>> theoretical analysis)
>>>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>>>> device level.
>>>>> 	This piece of code from your [4/6] is flexible enough, which
>>>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>>>> then fallback to platform specific method.
>>>>> 	domain = dev_get_msi_domain(&dev->dev);
>>>>> 	if (!domain && dev->bus->msi)
>>>>>  		domain = dev->bus->msi->domain;
>>>>>  	if (!domain)
>>>>>  		domain = arch_get_pci_msi_domain(dev);
>>>>
>>>> OK. But what I'd really like to see is a way to setup the
>>>> device<->domain binding as early as possible, without having to use more
>>>> conditional code in pci_msi_get_domain.
>>>>
>>>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>>>> pci_set_msi_domain do in this patch?
>>> Hi Marc,
>>> 	I have checked x86 code, we could set pci_dev->msi_domain
>>> when creating PCI devices, just need to find some hook points
>>> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
>>> PCI MSI core may provide a default way to set pci_dev->msi_domain.
>>> This may make the implementation simpler, I guess:)
>>
>> Right. So following your earlier suggestion, I could make
>> pci_set_msi_domain a weak symbol and let arch code override this.
>>
>> My preference would have been to have arch code to create a set of
>> arch-independent data structures describing the topology, and use that
>> for everything, but maybe that's a bit ambitious for a start.
>>
>> I'll rework the series to make the symbols weak.
> Hi Marc,
> 	I think we may not need the weak symbol at all. With following
> draft patch, the PCI MSI core may simply do:
> 	if (pci_dev->dev.msi_domain == NULL)
> 		dev_set_msi_domain(&dev->dev,
> 			 dev_get_msi_domain(&dev->bus->dev));
> -----------------------------------------------------------------------
> Note: the patch won't pass compilation, just to show the key idea:)
> 
> diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
> index da163da5fdee..8147d25d4349 100644
> --- a/arch/x86/kernel/apic/msi.c
> +++ b/arch/x86/kernel/apic/msi.c
> @@ -67,6 +67,23 @@ static struct irq_chip pci_msi_controller = {
>         .flags                  = IRQCHIP_SKIP_SET_WAKE,
>  };
> 
> +struct irq_domain *x86_get_pci_msi_domain(struct pci_dev *dev)
> +{
> +       struct irq_domain *domain;
> +       struct irq_alloc_info info;
> +
> +       init_irq_alloc_info(&info, NULL);
> +       info.type = X86_IRQ_ALLOC_TYPE_MSI;
> +       info.msi_dev = dev;
> +       domain = irq_remapping_get_irq_domain(&info);
> +       if (domain == NULL)
> +               domain = msi_default_domain;
> +       if (domain == NULL)
> +               domain = ERR_PTR(-ENOSYS);
> +
> +       return domain;
> +}
> +
>  int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>  {
>         struct irq_domain *domain;
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 7b20bccf3648..a26f30a8bb8f 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -652,6 +652,9 @@ int pcibios_add_device(struct pci_dev *dev)
>                 pa_data = data->next;
>                 iounmap(data);
>         }
> +
> +       dev->dev.msi_domain = x86_get_pci_msi_domain(dev);
> +
>         return 0;
>  }

Right. So you set the msi_domain using the pcibios_add_device callback.
That will require some minimal surgery (the call to pci_set_msi_domain
happens before the pcibios call, so it needs to be relocated after), but
that seems like a sensible solution to me.

Thanks!

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 14:59                       ` Marc Zyngier
  0 siblings, 0 replies; 57+ messages in thread
From: Marc Zyngier @ 2014-12-09 14:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/12/14 14:35, Jiang Liu wrote:
> On 2014/12/9 22:27, Marc Zyngier wrote:
>> On 09/12/14 14:11, Jiang Liu wrote:
>>> On 2014/12/9 22:03, Marc Zyngier wrote:
>>>> Hi Gerry,
>>>>
>>>> On 09/12/14 12:47, Jiang Liu wrote:
>>>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>>>> Yijing,
>>>>>>
>>>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>>>> +{
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>>>> +{
>>>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>>>> +
>>>>>>>>>> +	if (!bridge)
>>>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>>>> +	else
>>>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>>>> +}
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>>>
>>>>>>> Hi Marc,
>>>>>>>
>>>>>>>>
>>>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>>>
>>>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>>>> domain in existing platforms?
>>>>>>>
>>>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>>>
>>>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>>>> Hi Marc,
>>>>> 	This is a little wild:(
>>>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>>>> But it may also manage a specific PCI device. This is typically used to
>>>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>>>> I have no really experience with such a hardware platform yet, just for
>>>>> theoretical analysis)
>>>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>>>> device level.
>>>>> 	This piece of code from your [4/6] is flexible enough, which
>>>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>>>> then fallback to platform specific method.
>>>>> 	domain = dev_get_msi_domain(&dev->dev);
>>>>> 	if (!domain && dev->bus->msi)
>>>>>  		domain = dev->bus->msi->domain;
>>>>>  	if (!domain)
>>>>>  		domain = arch_get_pci_msi_domain(dev);
>>>>
>>>> OK. But what I'd really like to see is a way to setup the
>>>> device<->domain binding as early as possible, without having to use more
>>>> conditional code in pci_msi_get_domain.
>>>>
>>>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>>>> pci_set_msi_domain do in this patch?
>>> Hi Marc,
>>> 	I have checked x86 code, we could set pci_dev->msi_domain
>>> when creating PCI devices, just need to find some hook points
>>> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
>>> PCI MSI core may provide a default way to set pci_dev->msi_domain.
>>> This may make the implementation simpler, I guess:)
>>
>> Right. So following your earlier suggestion, I could make
>> pci_set_msi_domain a weak symbol and let arch code override this.
>>
>> My preference would have been to have arch code to create a set of
>> arch-independent data structures describing the topology, and use that
>> for everything, but maybe that's a bit ambitious for a start.
>>
>> I'll rework the series to make the symbols weak.
> Hi Marc,
> 	I think we may not need the weak symbol at all. With following
> draft patch, the PCI MSI core may simply do:
> 	if (pci_dev->dev.msi_domain == NULL)
> 		dev_set_msi_domain(&dev->dev,
> 			 dev_get_msi_domain(&dev->bus->dev));
> -----------------------------------------------------------------------
> Note: the patch won't pass compilation, just to show the key idea:)
> 
> diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
> index da163da5fdee..8147d25d4349 100644
> --- a/arch/x86/kernel/apic/msi.c
> +++ b/arch/x86/kernel/apic/msi.c
> @@ -67,6 +67,23 @@ static struct irq_chip pci_msi_controller = {
>         .flags                  = IRQCHIP_SKIP_SET_WAKE,
>  };
> 
> +struct irq_domain *x86_get_pci_msi_domain(struct pci_dev *dev)
> +{
> +       struct irq_domain *domain;
> +       struct irq_alloc_info info;
> +
> +       init_irq_alloc_info(&info, NULL);
> +       info.type = X86_IRQ_ALLOC_TYPE_MSI;
> +       info.msi_dev = dev;
> +       domain = irq_remapping_get_irq_domain(&info);
> +       if (domain == NULL)
> +               domain = msi_default_domain;
> +       if (domain == NULL)
> +               domain = ERR_PTR(-ENOSYS);
> +
> +       return domain;
> +}
> +
>  int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>  {
>         struct irq_domain *domain;
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 7b20bccf3648..a26f30a8bb8f 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -652,6 +652,9 @@ int pcibios_add_device(struct pci_dev *dev)
>                 pa_data = data->next;
>                 iounmap(data);
>         }
> +
> +       dev->dev.msi_domain = x86_get_pci_msi_domain(dev);
> +
>         return 0;
>  }

Right. So you set the msi_domain using the pcibios_add_device callback.
That will require some minimal surgery (the call to pci_set_msi_domain
happens before the pcibios call, so it needs to be relocated after), but
that seems like a sensible solution to me.

Thanks!

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

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-09 14:59                       ` Marc Zyngier
  (?)
@ 2014-12-09 15:42                         ` Jiang Liu
  -1 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 15:42 UTC (permalink / raw)
  To: Marc Zyngier, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit



On 2014/12/9 22:59, Marc Zyngier wrote:
> On 09/12/14 14:35, Jiang Liu wrote:
>> On 2014/12/9 22:27, Marc Zyngier wrote:
>>> On 09/12/14 14:11, Jiang Liu wrote:
>>>> On 2014/12/9 22:03, Marc Zyngier wrote:
>>>>> Hi Gerry,
>>>>>
>>>>> On 09/12/14 12:47, Jiang Liu wrote:
>>>>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>>>>> Yijing,
>>>>>>>
>>>>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>>>>> +{
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>>>>> +{
>>>>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>>>>> +
>>>>>>>>>>> +	if (!bridge)
>>>>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>>>>> +	else
>>>>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>>>>> +}
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>>>>
>>>>>>>> Hi Marc,
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>>>>
>>>>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>>>>> domain in existing platforms?
>>>>>>>>
>>>>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>>>>
>>>>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>>>>> Hi Marc,
>>>>>> 	This is a little wild:(
>>>>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>>>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>>>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>>>>> But it may also manage a specific PCI device. This is typically used to
>>>>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>>>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>>>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>>>>> I have no really experience with such a hardware platform yet, just for
>>>>>> theoretical analysis)
>>>>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>>>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>>>>> device level.
>>>>>> 	This piece of code from your [4/6] is flexible enough, which
>>>>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>>>>> then fallback to platform specific method.
>>>>>> 	domain = dev_get_msi_domain(&dev->dev);
>>>>>> 	if (!domain && dev->bus->msi)
>>>>>>  		domain = dev->bus->msi->domain;
>>>>>>  	if (!domain)
>>>>>>  		domain = arch_get_pci_msi_domain(dev);
>>>>>
>>>>> OK. But what I'd really like to see is a way to setup the
>>>>> device<->domain binding as early as possible, without having to use more
>>>>> conditional code in pci_msi_get_domain.
>>>>>
>>>>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>>>>> pci_set_msi_domain do in this patch?
>>>> Hi Marc,
>>>> 	I have checked x86 code, we could set pci_dev->msi_domain
>>>> when creating PCI devices, just need to find some hook points
>>>> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
>>>> PCI MSI core may provide a default way to set pci_dev->msi_domain.
>>>> This may make the implementation simpler, I guess:)
>>>
>>> Right. So following your earlier suggestion, I could make
>>> pci_set_msi_domain a weak symbol and let arch code override this.
>>>
>>> My preference would have been to have arch code to create a set of
>>> arch-independent data structures describing the topology, and use that
>>> for everything, but maybe that's a bit ambitious for a start.
>>>
>>> I'll rework the series to make the symbols weak.
>> Hi Marc,
>> 	I think we may not need the weak symbol at all. With following
>> draft patch, the PCI MSI core may simply do:
>> 	if (pci_dev->dev.msi_domain == NULL)
>> 		dev_set_msi_domain(&dev->dev,
>> 			 dev_get_msi_domain(&dev->bus->dev));
>> -----------------------------------------------------------------------
>> Note: the patch won't pass compilation, just to show the key idea:)
>>
>> diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
>> index da163da5fdee..8147d25d4349 100644
>> --- a/arch/x86/kernel/apic/msi.c
>> +++ b/arch/x86/kernel/apic/msi.c
>> @@ -67,6 +67,23 @@ static struct irq_chip pci_msi_controller = {
>>         .flags                  = IRQCHIP_SKIP_SET_WAKE,
>>  };
>>
>> +struct irq_domain *x86_get_pci_msi_domain(struct pci_dev *dev)
>> +{
>> +       struct irq_domain *domain;
>> +       struct irq_alloc_info info;
>> +
>> +       init_irq_alloc_info(&info, NULL);
>> +       info.type = X86_IRQ_ALLOC_TYPE_MSI;
>> +       info.msi_dev = dev;
>> +       domain = irq_remapping_get_irq_domain(&info);
>> +       if (domain == NULL)
>> +               domain = msi_default_domain;
>> +       if (domain == NULL)
>> +               domain = ERR_PTR(-ENOSYS);
>> +
>> +       return domain;
>> +}
>> +
>>  int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>>  {
>>         struct irq_domain *domain;
>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>> index 7b20bccf3648..a26f30a8bb8f 100644
>> --- a/arch/x86/pci/common.c
>> +++ b/arch/x86/pci/common.c
>> @@ -652,6 +652,9 @@ int pcibios_add_device(struct pci_dev *dev)
>>                 pa_data = data->next;
>>                 iounmap(data);
>>         }
>> +
>> +       dev->dev.msi_domain = x86_get_pci_msi_domain(dev);
>> +
>>         return 0;
>>  }
> 
> Right. So you set the msi_domain using the pcibios_add_device callback.
> That will require some minimal surgery (the call to pci_set_msi_domain
> happens before the pcibios call, so it needs to be relocated after), but
> that seems like a sensible solution to me.
So the key point is clear now:
The PCI MSI core will try to set a default value for
pci_dev->dev.msi_domain if the arch code doesn't do that.

Seems like a solution:)

> 
> Thanks!
> 
> 	M.
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 15:42                         ` Jiang Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 15:42 UTC (permalink / raw)
  To: Marc Zyngier, Yijing Wang, Bjorn Helgaas, Thomas Gleixner
  Cc: linux-arm-kernel, linux-pci, linux-kernel, Will Deacon,
	suravee.suthikulpanit



On 2014/12/9 22:59, Marc Zyngier wrote:
> On 09/12/14 14:35, Jiang Liu wrote:
>> On 2014/12/9 22:27, Marc Zyngier wrote:
>>> On 09/12/14 14:11, Jiang Liu wrote:
>>>> On 2014/12/9 22:03, Marc Zyngier wrote:
>>>>> Hi Gerry,
>>>>>
>>>>> On 09/12/14 12:47, Jiang Liu wrote:
>>>>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>>>>> Yijing,
>>>>>>>
>>>>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>>>>> +{
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>>>>> +{
>>>>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>>>>> +
>>>>>>>>>>> +	if (!bridge)
>>>>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>>>>> +	else
>>>>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>>>>> +}
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>>>>
>>>>>>>> Hi Marc,
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>>>>
>>>>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>>>>> domain in existing platforms?
>>>>>>>>
>>>>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>>>>
>>>>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>>>>> Hi Marc,
>>>>>> 	This is a little wild:(
>>>>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>>>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>>>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>>>>> But it may also manage a specific PCI device. This is typically used to
>>>>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>>>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>>>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>>>>> I have no really experience with such a hardware platform yet, just for
>>>>>> theoretical analysis)
>>>>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>>>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>>>>> device level.
>>>>>> 	This piece of code from your [4/6] is flexible enough, which
>>>>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>>>>> then fallback to platform specific method.
>>>>>> 	domain = dev_get_msi_domain(&dev->dev);
>>>>>> 	if (!domain && dev->bus->msi)
>>>>>>  		domain = dev->bus->msi->domain;
>>>>>>  	if (!domain)
>>>>>>  		domain = arch_get_pci_msi_domain(dev);
>>>>>
>>>>> OK. But what I'd really like to see is a way to setup the
>>>>> device<->domain binding as early as possible, without having to use more
>>>>> conditional code in pci_msi_get_domain.
>>>>>
>>>>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>>>>> pci_set_msi_domain do in this patch?
>>>> Hi Marc,
>>>> 	I have checked x86 code, we could set pci_dev->msi_domain
>>>> when creating PCI devices, just need to find some hook points
>>>> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
>>>> PCI MSI core may provide a default way to set pci_dev->msi_domain.
>>>> This may make the implementation simpler, I guess:)
>>>
>>> Right. So following your earlier suggestion, I could make
>>> pci_set_msi_domain a weak symbol and let arch code override this.
>>>
>>> My preference would have been to have arch code to create a set of
>>> arch-independent data structures describing the topology, and use that
>>> for everything, but maybe that's a bit ambitious for a start.
>>>
>>> I'll rework the series to make the symbols weak.
>> Hi Marc,
>> 	I think we may not need the weak symbol at all. With following
>> draft patch, the PCI MSI core may simply do:
>> 	if (pci_dev->dev.msi_domain == NULL)
>> 		dev_set_msi_domain(&dev->dev,
>> 			 dev_get_msi_domain(&dev->bus->dev));
>> -----------------------------------------------------------------------
>> Note: the patch won't pass compilation, just to show the key idea:)
>>
>> diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
>> index da163da5fdee..8147d25d4349 100644
>> --- a/arch/x86/kernel/apic/msi.c
>> +++ b/arch/x86/kernel/apic/msi.c
>> @@ -67,6 +67,23 @@ static struct irq_chip pci_msi_controller = {
>>         .flags                  = IRQCHIP_SKIP_SET_WAKE,
>>  };
>>
>> +struct irq_domain *x86_get_pci_msi_domain(struct pci_dev *dev)
>> +{
>> +       struct irq_domain *domain;
>> +       struct irq_alloc_info info;
>> +
>> +       init_irq_alloc_info(&info, NULL);
>> +       info.type = X86_IRQ_ALLOC_TYPE_MSI;
>> +       info.msi_dev = dev;
>> +       domain = irq_remapping_get_irq_domain(&info);
>> +       if (domain == NULL)
>> +               domain = msi_default_domain;
>> +       if (domain == NULL)
>> +               domain = ERR_PTR(-ENOSYS);
>> +
>> +       return domain;
>> +}
>> +
>>  int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>>  {
>>         struct irq_domain *domain;
>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>> index 7b20bccf3648..a26f30a8bb8f 100644
>> --- a/arch/x86/pci/common.c
>> +++ b/arch/x86/pci/common.c
>> @@ -652,6 +652,9 @@ int pcibios_add_device(struct pci_dev *dev)
>>                 pa_data = data->next;
>>                 iounmap(data);
>>         }
>> +
>> +       dev->dev.msi_domain = x86_get_pci_msi_domain(dev);
>> +
>>         return 0;
>>  }
> 
> Right. So you set the msi_domain using the pcibios_add_device callback.
> That will require some minimal surgery (the call to pci_set_msi_domain
> happens before the pcibios call, so it needs to be relocated after), but
> that seems like a sensible solution to me.
So the key point is clear now:
The PCI MSI core will try to set a default value for
pci_dev->dev.msi_domain if the arch code doesn't do that.

Seems like a solution:)

> 
> Thanks!
> 
> 	M.
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2014-12-09 15:42                         ` Jiang Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Jiang Liu @ 2014-12-09 15:42 UTC (permalink / raw)
  To: linux-arm-kernel



On 2014/12/9 22:59, Marc Zyngier wrote:
> On 09/12/14 14:35, Jiang Liu wrote:
>> On 2014/12/9 22:27, Marc Zyngier wrote:
>>> On 09/12/14 14:11, Jiang Liu wrote:
>>>> On 2014/12/9 22:03, Marc Zyngier wrote:
>>>>> Hi Gerry,
>>>>>
>>>>> On 09/12/14 12:47, Jiang Liu wrote:
>>>>>> On 2014/12/9 20:12, Marc Zyngier wrote:
>>>>>>> Yijing,
>>>>>>>
>>>>>>> On 09/12/14 11:57, Yijing Wang wrote:
>>>>>>>>>>> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>>>>>>>>>>> +{
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
>>>>>>>>>>> +{
>>>>>>>>>>> +	struct pci_dev *bridge = bus->self;
>>>>>>>>>>> +
>>>>>>>>>>> +	if (!bridge)
>>>>>>>>>>> +		pcibios_set_phb_msi_domain(bus);
>>>>>>>>>>> +	else
>>>>>>>>>>> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
>>>>>>>>>>> +}
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi Marc, we can not assume pci devices under same phb share the same msi irq domain,
>>>>>>>>>> now in x86, pci devices under the same phb may associate different msi irq domain.
>>>>>>>>
>>>>>>>> Hi Marc,
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Well, this is not supposed to be a perfect solution yet, but instead a
>>>>>>>>> basis for discussion. What I'd like to find out is:
>>>>>>>>>
>>>>>>>>> - What is the minimum granularity for associating a device with its MSI
>>>>>>>>> domain in existing platforms?
>>>>>>>>
>>>>>>>> PCI device, after Gerry's msi irq domain patchset which now in linux-next,
>>>>>>>> in x86, we will find msi irq domain by pci_dev.
>>>>>>>
>>>>>>> Are you *really* associating the MSI domain on a per pci-device basis?
>>>>>>> That is, you have devices on the same PCI bus talking to different MSI hw?
>>>>>> Hi Marc,
>>>>>> 	This is a little wild:(
>>>>>> 	On x86 platform with Intel VT-d(not the case for AMD-v),
>>>>>> interrupt remapping is tight to DMA remapping (IOMMU) unit.
>>>>>> For most common cases, IOMMU unit manages PCI bus and its sub-hierarchy.
>>>>>> But it may also manage a specific PCI device. This is typically used to
>>>>>> provide QoS for audio device by using dedicated IOMMU unit to avoid
>>>>>> resource contention on DMA remapping tables. BIOS uses ACPI table to
>>>>>> report PCI bus/device to IOMMU unit mapping relationship. (To be honest,
>>>>>> I have no really experience with such a hardware platform yet, just for
>>>>>> theoretical analysis)
>>>>>> 	On the other hand, we now support hierarchy irqdomain. So to
>>>>>> support per-PCI IOMMU unit case, we need maintain irqdomain at PCI
>>>>>> device level.
>>>>>> 	This piece of code from your [4/6] is flexible enough, which
>>>>>> retrieves msi_domain from PCI device, then fallback to PCI bus,
>>>>>> then fallback to platform specific method.
>>>>>> 	domain = dev_get_msi_domain(&dev->dev);
>>>>>> 	if (!domain && dev->bus->msi)
>>>>>>  		domain = dev->bus->msi->domain;
>>>>>>  	if (!domain)
>>>>>>  		domain = arch_get_pci_msi_domain(dev);
>>>>>
>>>>> OK. But what I'd really like to see is a way to setup the
>>>>> device<->domain binding as early as possible, without having to use more
>>>>> conditional code in pci_msi_get_domain.
>>>>>
>>>>> IOW, can we do something similar to what pci_set_bus_msi_domain and
>>>>> pci_set_msi_domain do in this patch?
>>>> Hi Marc,
>>>> 	I have checked x86 code, we could set pci_dev->msi_domain
>>>> when creating PCI devices, just need to find some hook points
>>>> into PCI core next step. If arch code doesn't set pci_dev->msi_domain,
>>>> PCI MSI core may provide a default way to set pci_dev->msi_domain.
>>>> This may make the implementation simpler, I guess:)
>>>
>>> Right. So following your earlier suggestion, I could make
>>> pci_set_msi_domain a weak symbol and let arch code override this.
>>>
>>> My preference would have been to have arch code to create a set of
>>> arch-independent data structures describing the topology, and use that
>>> for everything, but maybe that's a bit ambitious for a start.
>>>
>>> I'll rework the series to make the symbols weak.
>> Hi Marc,
>> 	I think we may not need the weak symbol at all. With following
>> draft patch, the PCI MSI core may simply do:
>> 	if (pci_dev->dev.msi_domain == NULL)
>> 		dev_set_msi_domain(&dev->dev,
>> 			 dev_get_msi_domain(&dev->bus->dev));
>> -----------------------------------------------------------------------
>> Note: the patch won't pass compilation, just to show the key idea:)
>>
>> diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
>> index da163da5fdee..8147d25d4349 100644
>> --- a/arch/x86/kernel/apic/msi.c
>> +++ b/arch/x86/kernel/apic/msi.c
>> @@ -67,6 +67,23 @@ static struct irq_chip pci_msi_controller = {
>>         .flags                  = IRQCHIP_SKIP_SET_WAKE,
>>  };
>>
>> +struct irq_domain *x86_get_pci_msi_domain(struct pci_dev *dev)
>> +{
>> +       struct irq_domain *domain;
>> +       struct irq_alloc_info info;
>> +
>> +       init_irq_alloc_info(&info, NULL);
>> +       info.type = X86_IRQ_ALLOC_TYPE_MSI;
>> +       info.msi_dev = dev;
>> +       domain = irq_remapping_get_irq_domain(&info);
>> +       if (domain == NULL)
>> +               domain = msi_default_domain;
>> +       if (domain == NULL)
>> +               domain = ERR_PTR(-ENOSYS);
>> +
>> +       return domain;
>> +}
>> +
>>  int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>>  {
>>         struct irq_domain *domain;
>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>> index 7b20bccf3648..a26f30a8bb8f 100644
>> --- a/arch/x86/pci/common.c
>> +++ b/arch/x86/pci/common.c
>> @@ -652,6 +652,9 @@ int pcibios_add_device(struct pci_dev *dev)
>>                 pa_data = data->next;
>>                 iounmap(data);
>>         }
>> +
>> +       dev->dev.msi_domain = x86_get_pci_msi_domain(dev);
>> +
>>         return 0;
>>  }
> 
> Right. So you set the msi_domain using the pcibios_add_device callback.
> That will require some minimal surgery (the call to pci_set_msi_domain
> happens before the pcibios call, so it needs to be relocated after), but
> that seems like a sensible solution to me.
So the key point is clear now:
The PCI MSI core will try to set a default value for
pci_dev->dev.msi_domain if the arch code doesn't do that.

Seems like a solution:)

> 
> Thanks!
> 
> 	M.
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2014-12-08 20:12   ` Marc Zyngier
@ 2015-01-27  0:40     ` Bjorn Helgaas
  -1 siblings, 0 replies; 57+ messages in thread
From: Bjorn Helgaas @ 2015-01-27  0:40 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Thomas Gleixner, Jiang Liu, linux-arm-kernel, linux-pci,
	linux-kernel, Will Deacon, Yijing Wang, Suravee Suthikulpanit

On Mon, Dec 08, 2014 at 08:12:19PM +0000, Marc Zyngier wrote:
> In order to be able to populate the device msi_domain field,
> add the necesary hooks to propagate the PHB msi_domain across
> secondary busses to devices.
> 
> So far, nobody populates the initial msi_domain.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/pci/probe.c | 24 ++++++++++++++++++++++++
>  include/linux/pci.h |  1 +
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index c8ca98c..d1009a2 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>  	}
>  }
>  
> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)

We haven't previously used the "phb" abbreviation in the PCI core, so I'd
rather not start now.  I'm not really sure how relevant it is anyway --
maybe the name doesn't need to include a reference to the host bridge at
all?

> +{
> +}
> +
> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
> +{
> +	struct pci_dev *bridge = bus->self;
> +
> +	if (!bridge)
> +		pcibios_set_phb_msi_domain(bus);
> +	else
> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
> +}
> +
>  static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  					   struct pci_dev *bridge, int busnr)
>  {
> @@ -723,6 +737,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  	bridge->subordinate = child;
>  
>  add_dev:
> +	pci_set_bus_msi_domain(child);
>  	ret = device_register(&child->dev);
>  	WARN_ON(ret < 0);
>  
> @@ -1517,6 +1532,11 @@ static void pci_init_capabilities(struct pci_dev *dev)
>  	pci_enable_acs(dev);
>  }
>  
> +static void pci_set_msi_domain(struct pci_dev *dev)
> +{
> +	dev_set_msi_domain(&dev->dev, dev_get_msi_domain(&dev->bus->dev));
> +}
> +
>  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  {
>  	int ret;
> @@ -1546,6 +1566,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	/* Initialize various capabilities */
>  	pci_init_capabilities(dev);
>  
> +	/* Setup MSI irq domain */

This is *really* a nitpick, but "setup" suggests that we're doing more than
just setting a pointer, i.e., it suggests that we're doing something more
involved to get it all ready for use.  But here, I think we really are just
setting a pointer, so I think the comment is pointless since the function
name already says that.

> +	pci_set_msi_domain(dev);
> +
>  	/*
>  	 * Add the device to our list of discovered devices
>  	 * and the bus list for fixup functions, etc.
> @@ -1947,6 +1970,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  	b->bridge = get_device(&bridge->dev);
>  	device_enable_async_suspend(b->bridge);
>  	pci_set_bus_of_node(b);
> +	pci_set_bus_msi_domain(b);
>  
>  	if (!parent)
>  		set_dev_node(b->bridge, pcibus_to_node(b));
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index a523cee..3266764 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1637,6 +1637,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
>  int pcibios_add_device(struct pci_dev *dev);
>  void pcibios_release_device(struct pci_dev *dev);
>  void pcibios_penalize_isa_irq(int irq, int active);
> +void pcibios_set_phb_msi_domain(struct pci_bus *bus);
>  
>  #ifdef CONFIG_HIBERNATE_CALLBACKS
>  extern struct dev_pm_ops pcibios_pm_ops;
> -- 
> 2.1.3
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2015-01-27  0:40     ` Bjorn Helgaas
  0 siblings, 0 replies; 57+ messages in thread
From: Bjorn Helgaas @ 2015-01-27  0:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Dec 08, 2014 at 08:12:19PM +0000, Marc Zyngier wrote:
> In order to be able to populate the device msi_domain field,
> add the necesary hooks to propagate the PHB msi_domain across
> secondary busses to devices.
> 
> So far, nobody populates the initial msi_domain.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/pci/probe.c | 24 ++++++++++++++++++++++++
>  include/linux/pci.h |  1 +
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index c8ca98c..d1009a2 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>  	}
>  }
>  
> +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)

We haven't previously used the "phb" abbreviation in the PCI core, so I'd
rather not start now.  I'm not really sure how relevant it is anyway --
maybe the name doesn't need to include a reference to the host bridge at
all?

> +{
> +}
> +
> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
> +{
> +	struct pci_dev *bridge = bus->self;
> +
> +	if (!bridge)
> +		pcibios_set_phb_msi_domain(bus);
> +	else
> +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
> +}
> +
>  static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  					   struct pci_dev *bridge, int busnr)
>  {
> @@ -723,6 +737,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  	bridge->subordinate = child;
>  
>  add_dev:
> +	pci_set_bus_msi_domain(child);
>  	ret = device_register(&child->dev);
>  	WARN_ON(ret < 0);
>  
> @@ -1517,6 +1532,11 @@ static void pci_init_capabilities(struct pci_dev *dev)
>  	pci_enable_acs(dev);
>  }
>  
> +static void pci_set_msi_domain(struct pci_dev *dev)
> +{
> +	dev_set_msi_domain(&dev->dev, dev_get_msi_domain(&dev->bus->dev));
> +}
> +
>  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  {
>  	int ret;
> @@ -1546,6 +1566,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	/* Initialize various capabilities */
>  	pci_init_capabilities(dev);
>  
> +	/* Setup MSI irq domain */

This is *really* a nitpick, but "setup" suggests that we're doing more than
just setting a pointer, i.e., it suggests that we're doing something more
involved to get it all ready for use.  But here, I think we really are just
setting a pointer, so I think the comment is pointless since the function
name already says that.

> +	pci_set_msi_domain(dev);
> +
>  	/*
>  	 * Add the device to our list of discovered devices
>  	 * and the bus list for fixup functions, etc.
> @@ -1947,6 +1970,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  	b->bridge = get_device(&bridge->dev);
>  	device_enable_async_suspend(b->bridge);
>  	pci_set_bus_of_node(b);
> +	pci_set_bus_msi_domain(b);
>  
>  	if (!parent)
>  		set_dev_node(b->bridge, pcibus_to_node(b));
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index a523cee..3266764 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1637,6 +1637,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
>  int pcibios_add_device(struct pci_dev *dev);
>  void pcibios_release_device(struct pci_dev *dev);
>  void pcibios_penalize_isa_irq(int irq, int active);
> +void pcibios_set_phb_msi_domain(struct pci_bus *bus);
>  
>  #ifdef CONFIG_HIBERNATE_CALLBACKS
>  extern struct dev_pm_ops pcibios_pm_ops;
> -- 
> 2.1.3
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 3/6] PCI/MSI: of: add support for OF-provided msi_domain
  2014-12-08 20:12   ` Marc Zyngier
@ 2015-01-27  0:41     ` Bjorn Helgaas
  -1 siblings, 0 replies; 57+ messages in thread
From: Bjorn Helgaas @ 2015-01-27  0:41 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Thomas Gleixner, Jiang Liu, linux-arm-kernel, linux-pci,
	linux-kernel, Will Deacon, Yijing Wang, Suravee Suthikulpanit

On Mon, Dec 08, 2014 at 08:12:20PM +0000, Marc Zyngier wrote:
> In order to populate the PHB msi_domain, use the "msi-parent"
> attribute to lookup a corresponding irq domain. If found,
> this is our MSI domain.
> 
> This gets plugged into the core PCI code.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/pci/of.c    | 15 +++++++++++++++
>  drivers/pci/probe.c |  1 +
>  include/linux/pci.h |  2 ++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index f092993..d8d1274 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -9,6 +9,7 @@
>   * 2 of the License, or (at your option) any later version.
>   */
>  
> +#include <linux/irqdomain.h>
>  #include <linux/kernel.h>
>  #include <linux/pci.h>
>  #include <linux/of.h>
> @@ -59,3 +60,17 @@ struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
>  		return of_node_get(bus->bridge->parent->of_node);
>  	return NULL;
>  }
> +
> +void pci_set_phb_of_msi_domain(struct pci_bus *bus)

Same "phb" comment as before (also in the changelog above).

> +{
> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
> +	struct device_node *np;
> +
> +	if (!bus->dev.of_node)
> +		return;
> +	np = of_parse_phandle(bus->dev.of_node, "msi-parent", 0);
> +	if (!np)
> +		return;
> +	dev_set_msi_domain(&bus->dev, irq_find_host(np));
> +#endif
> +}
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index d1009a2..65600e5 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -672,6 +672,7 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>  
>  void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>  {
> +	pci_set_phb_of_msi_domain(bus);
>  }
>  
>  static void pci_set_bus_msi_domain(struct pci_bus *bus)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 3266764..6a9713f 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1825,6 +1825,7 @@ void pci_set_of_node(struct pci_dev *dev);
>  void pci_release_of_node(struct pci_dev *dev);
>  void pci_set_bus_of_node(struct pci_bus *bus);
>  void pci_release_bus_of_node(struct pci_bus *bus);
> +void pci_set_phb_of_msi_domain(struct pci_bus *bus);
>  
>  /* Arch may override this (weak) */
>  struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus);
> @@ -1845,6 +1846,7 @@ static inline void pci_set_of_node(struct pci_dev *dev) { }
>  static inline void pci_release_of_node(struct pci_dev *dev) { }
>  static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
>  static inline void pci_release_bus_of_node(struct pci_bus *bus) { }
> +static inline void pci_set_phb_of_msi_domain(struct pci_bus *bus) {}

Add a space between the braces to match the others.

>  #endif  /* CONFIG_OF */
>  
>  #ifdef CONFIG_EEH
> -- 
> 2.1.3
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 3/6] PCI/MSI: of: add support for OF-provided msi_domain
@ 2015-01-27  0:41     ` Bjorn Helgaas
  0 siblings, 0 replies; 57+ messages in thread
From: Bjorn Helgaas @ 2015-01-27  0:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Dec 08, 2014 at 08:12:20PM +0000, Marc Zyngier wrote:
> In order to populate the PHB msi_domain, use the "msi-parent"
> attribute to lookup a corresponding irq domain. If found,
> this is our MSI domain.
> 
> This gets plugged into the core PCI code.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/pci/of.c    | 15 +++++++++++++++
>  drivers/pci/probe.c |  1 +
>  include/linux/pci.h |  2 ++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index f092993..d8d1274 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -9,6 +9,7 @@
>   * 2 of the License, or (at your option) any later version.
>   */
>  
> +#include <linux/irqdomain.h>
>  #include <linux/kernel.h>
>  #include <linux/pci.h>
>  #include <linux/of.h>
> @@ -59,3 +60,17 @@ struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
>  		return of_node_get(bus->bridge->parent->of_node);
>  	return NULL;
>  }
> +
> +void pci_set_phb_of_msi_domain(struct pci_bus *bus)

Same "phb" comment as before (also in the changelog above).

> +{
> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
> +	struct device_node *np;
> +
> +	if (!bus->dev.of_node)
> +		return;
> +	np = of_parse_phandle(bus->dev.of_node, "msi-parent", 0);
> +	if (!np)
> +		return;
> +	dev_set_msi_domain(&bus->dev, irq_find_host(np));
> +#endif
> +}
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index d1009a2..65600e5 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -672,6 +672,7 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>  
>  void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
>  {
> +	pci_set_phb_of_msi_domain(bus);
>  }
>  
>  static void pci_set_bus_msi_domain(struct pci_bus *bus)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 3266764..6a9713f 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1825,6 +1825,7 @@ void pci_set_of_node(struct pci_dev *dev);
>  void pci_release_of_node(struct pci_dev *dev);
>  void pci_set_bus_of_node(struct pci_bus *bus);
>  void pci_release_bus_of_node(struct pci_bus *bus);
> +void pci_set_phb_of_msi_domain(struct pci_bus *bus);
>  
>  /* Arch may override this (weak) */
>  struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus);
> @@ -1845,6 +1846,7 @@ static inline void pci_set_of_node(struct pci_dev *dev) { }
>  static inline void pci_release_of_node(struct pci_dev *dev) { }
>  static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
>  static inline void pci_release_bus_of_node(struct pci_bus *bus) { }
> +static inline void pci_set_phb_of_msi_domain(struct pci_bus *bus) {}

Add a space between the braces to match the others.

>  #endif  /* CONFIG_OF */
>  
>  #ifdef CONFIG_EEH
> -- 
> 2.1.3
> 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* Re: [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
  2015-01-27  0:40     ` Bjorn Helgaas
@ 2015-01-27  0:43       ` Bjorn Helgaas
  -1 siblings, 0 replies; 57+ messages in thread
From: Bjorn Helgaas @ 2015-01-27  0:43 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Thomas Gleixner, Jiang Liu, linux-arm-kernel, linux-pci,
	linux-kernel, Will Deacon, Yijing Wang, Suravee Suthikulpanit

On Mon, Jan 26, 2015 at 06:40:05PM -0600, Bjorn Helgaas wrote:
> On Mon, Dec 08, 2014 at 08:12:19PM +0000, Marc Zyngier wrote:
> > In order to be able to populate the device msi_domain field,
> > add the necesary hooks to propagate the PHB msi_domain across
> > secondary busses to devices.
> > 
> > So far, nobody populates the initial msi_domain.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  drivers/pci/probe.c | 24 ++++++++++++++++++++++++
> >  include/linux/pci.h |  1 +
> >  2 files changed, 25 insertions(+)
> > 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index c8ca98c..d1009a2 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
> >  	}
> >  }
> >  
> > +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
> 
> We haven't previously used the "phb" abbreviation in the PCI core, so I'd
> rather not start now.  I'm not really sure how relevant it is anyway --
> maybe the name doesn't need to include a reference to the host bridge at
> all?

Sorry, I meant to reply to the v2 patch, not the old one.  But I think the
comment still applies there.

> > +{
> > +}
> > +
> > +static void pci_set_bus_msi_domain(struct pci_bus *bus)
> > +{
> > +	struct pci_dev *bridge = bus->self;
> > +
> > +	if (!bridge)
> > +		pcibios_set_phb_msi_domain(bus);
> > +	else
> > +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
> > +}
> > +
> >  static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
> >  					   struct pci_dev *bridge, int busnr)
> >  {
> > @@ -723,6 +737,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
> >  	bridge->subordinate = child;
> >  
> >  add_dev:
> > +	pci_set_bus_msi_domain(child);
> >  	ret = device_register(&child->dev);
> >  	WARN_ON(ret < 0);
> >  
> > @@ -1517,6 +1532,11 @@ static void pci_init_capabilities(struct pci_dev *dev)
> >  	pci_enable_acs(dev);
> >  }
> >  
> > +static void pci_set_msi_domain(struct pci_dev *dev)
> > +{
> > +	dev_set_msi_domain(&dev->dev, dev_get_msi_domain(&dev->bus->dev));
> > +}
> > +
> >  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> >  {
> >  	int ret;
> > @@ -1546,6 +1566,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> >  	/* Initialize various capabilities */
> >  	pci_init_capabilities(dev);
> >  
> > +	/* Setup MSI irq domain */
> 
> This is *really* a nitpick, but "setup" suggests that we're doing more than
> just setting a pointer, i.e., it suggests that we're doing something more
> involved to get it all ready for use.  But here, I think we really are just
> setting a pointer, so I think the comment is pointless since the function
> name already says that.
> 
> > +	pci_set_msi_domain(dev);
> > +
> >  	/*
> >  	 * Add the device to our list of discovered devices
> >  	 * and the bus list for fixup functions, etc.
> > @@ -1947,6 +1970,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> >  	b->bridge = get_device(&bridge->dev);
> >  	device_enable_async_suspend(b->bridge);
> >  	pci_set_bus_of_node(b);
> > +	pci_set_bus_msi_domain(b);
> >  
> >  	if (!parent)
> >  		set_dev_node(b->bridge, pcibus_to_node(b));
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index a523cee..3266764 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1637,6 +1637,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
> >  int pcibios_add_device(struct pci_dev *dev);
> >  void pcibios_release_device(struct pci_dev *dev);
> >  void pcibios_penalize_isa_irq(int irq, int active);
> > +void pcibios_set_phb_msi_domain(struct pci_bus *bus);
> >  
> >  #ifdef CONFIG_HIBERNATE_CALLBACKS
> >  extern struct dev_pm_ops pcibios_pm_ops;
> > -- 
> > 2.1.3
> > 

^ permalink raw reply	[flat|nested] 57+ messages in thread
* [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field
@ 2015-01-27  0:43       ` Bjorn Helgaas
  0 siblings, 0 replies; 57+ messages in thread
From: Bjorn Helgaas @ 2015-01-27  0:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 26, 2015 at 06:40:05PM -0600, Bjorn Helgaas wrote:
> On Mon, Dec 08, 2014 at 08:12:19PM +0000, Marc Zyngier wrote:
> > In order to be able to populate the device msi_domain field,
> > add the necesary hooks to propagate the PHB msi_domain across
> > secondary busses to devices.
> > 
> > So far, nobody populates the initial msi_domain.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  drivers/pci/probe.c | 24 ++++++++++++++++++++++++
> >  include/linux/pci.h |  1 +
> >  2 files changed, 25 insertions(+)
> > 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index c8ca98c..d1009a2 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -670,6 +670,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
> >  	}
> >  }
> >  
> > +void __weak pcibios_set_phb_msi_domain(struct pci_bus *bus)
> 
> We haven't previously used the "phb" abbreviation in the PCI core, so I'd
> rather not start now.  I'm not really sure how relevant it is anyway --
> maybe the name doesn't need to include a reference to the host bridge at
> all?

Sorry, I meant to reply to the v2 patch, not the old one.  But I think the
comment still applies there.

> > +{
> > +}
> > +
> > +static void pci_set_bus_msi_domain(struct pci_bus *bus)
> > +{
> > +	struct pci_dev *bridge = bus->self;
> > +
> > +	if (!bridge)
> > +		pcibios_set_phb_msi_domain(bus);
> > +	else
> > +		dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
> > +}
> > +
> >  static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
> >  					   struct pci_dev *bridge, int busnr)
> >  {
> > @@ -723,6 +737,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
> >  	bridge->subordinate = child;
> >  
> >  add_dev:
> > +	pci_set_bus_msi_domain(child);
> >  	ret = device_register(&child->dev);
> >  	WARN_ON(ret < 0);
> >  
> > @@ -1517,6 +1532,11 @@ static void pci_init_capabilities(struct pci_dev *dev)
> >  	pci_enable_acs(dev);
> >  }
> >  
> > +static void pci_set_msi_domain(struct pci_dev *dev)
> > +{
> > +	dev_set_msi_domain(&dev->dev, dev_get_msi_domain(&dev->bus->dev));
> > +}
> > +
> >  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> >  {
> >  	int ret;
> > @@ -1546,6 +1566,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> >  	/* Initialize various capabilities */
> >  	pci_init_capabilities(dev);
> >  
> > +	/* Setup MSI irq domain */
> 
> This is *really* a nitpick, but "setup" suggests that we're doing more than
> just setting a pointer, i.e., it suggests that we're doing something more
> involved to get it all ready for use.  But here, I think we really are just
> setting a pointer, so I think the comment is pointless since the function
> name already says that.
> 
> > +	pci_set_msi_domain(dev);
> > +
> >  	/*
> >  	 * Add the device to our list of discovered devices
> >  	 * and the bus list for fixup functions, etc.
> > @@ -1947,6 +1970,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> >  	b->bridge = get_device(&bridge->dev);
> >  	device_enable_async_suspend(b->bridge);
> >  	pci_set_bus_of_node(b);
> > +	pci_set_bus_msi_domain(b);
> >  
> >  	if (!parent)
> >  		set_dev_node(b->bridge, pcibus_to_node(b));
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index a523cee..3266764 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1637,6 +1637,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
> >  int pcibios_add_device(struct pci_dev *dev);
> >  void pcibios_release_device(struct pci_dev *dev);
> >  void pcibios_penalize_isa_irq(int irq, int active);
> > +void pcibios_set_phb_msi_domain(struct pci_bus *bus);
> >  
> >  #ifdef CONFIG_HIBERNATE_CALLBACKS
> >  extern struct dev_pm_ops pcibios_pm_ops;
> > -- 
> > 2.1.3
> > 

^ permalink raw reply	[flat|nested] 57+ messages in thread
end of thread, other threads:[~2015-01-27  0:43 UTC | newest]

Thread overview: 57+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-08 20:12 [PATCH 0/6] Introducing per-device MSI domain Marc Zyngier
2014-12-08 20:12 ` Marc Zyngier
2014-12-08 20:12 ` [PATCH 1/6] device core: Introduce per-device MSI domain pointer Marc Zyngier
2014-12-08 20:12   ` Marc Zyngier
2014-12-08 20:12 ` [PATCH 2/6] PCI/MSI: add hooks to populate the msi_domain field Marc Zyngier
2014-12-08 20:12   ` Marc Zyngier
2014-12-09  2:03   ` Yijing Wang
2014-12-09  2:03     ` Yijing Wang
2014-12-09 10:02     ` Marc Zyngier
2014-12-09 10:02       ` Marc Zyngier
2014-12-09 10:02       ` Marc Zyngier
2014-12-09 11:57       ` Yijing Wang
2014-12-09 11:57         ` Yijing Wang
2014-12-09 11:57         ` Yijing Wang
2014-12-09 12:12         ` Marc Zyngier
2014-12-09 12:12           ` Marc Zyngier
2014-12-09 12:12           ` Marc Zyngier
2014-12-09 12:24           ` Yijing Wang
2014-12-09 12:24             ` Yijing Wang
2014-12-09 12:24             ` Yijing Wang
2014-12-09 12:47           ` Jiang Liu
2014-12-09 12:47             ` Jiang Liu
2014-12-09 12:47             ` Jiang Liu
2014-12-09 14:03             ` Marc Zyngier
2014-12-09 14:03               ` Marc Zyngier
2014-12-09 14:03               ` Marc Zyngier
2014-12-09 14:11               ` Jiang Liu
2014-12-09 14:11                 ` Jiang Liu
2014-12-09 14:11                 ` Jiang Liu
2014-12-09 14:27                 ` Marc Zyngier
2014-12-09 14:27                   ` Marc Zyngier
2014-12-09 14:27                   ` Marc Zyngier
2014-12-09 14:35                   ` Jiang Liu
2014-12-09 14:35                     ` Jiang Liu
2014-12-09 14:35                     ` Jiang Liu
2014-12-09 14:59                     ` Marc Zyngier
2014-12-09 14:59                       ` Marc Zyngier
2014-12-09 14:59                       ` Marc Zyngier
2014-12-09 15:42                       ` Jiang Liu
2014-12-09 15:42                         ` Jiang Liu
2014-12-09 15:42                         ` Jiang Liu
2014-12-09 12:57   ` Jiang Liu
2014-12-09 12:57     ` Jiang Liu
2015-01-27  0:40   ` Bjorn Helgaas
2015-01-27  0:40     ` Bjorn Helgaas
2015-01-27  0:43     ` Bjorn Helgaas
2015-01-27  0:43       ` Bjorn Helgaas
2014-12-08 20:12 ` [PATCH 3/6] PCI/MSI: of: add support for OF-provided msi_domain Marc Zyngier
2014-12-08 20:12   ` Marc Zyngier
2015-01-27  0:41   ` Bjorn Helgaas
2015-01-27  0:41     ` Bjorn Helgaas
2014-12-08 20:12 ` [PATCH 4/6] PCI/MSI: Let pci_msi_get_domain use struct device's msi_domain Marc Zyngier
2014-12-08 20:12   ` Marc Zyngier
2014-12-08 20:12 ` [PATCH 5/6] irqchip: GICv2m: Get rid of struct msi_controller Marc Zyngier
2014-12-08 20:12   ` Marc Zyngier
2014-12-08 20:12 ` [PATCH 6/6] irqchip: gicv3-its: " Marc Zyngier
2014-12-08 20:12   ` Marc Zyngier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.