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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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...
>>> +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
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...
>>> 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
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.
>
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 <[email protected]>
> ---
> 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;
>
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...
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.
>
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...
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.
>
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...
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.
>