On 10/07/17 20:23, Bjorn Helgaas via iommu wrote: > [+cc Joerg, iommu] > > On Fri, Jun 30, 2017 at 12:24 AM, Alexey Kardashevskiy <aik@xxxxxxxxx> wrote: >> From: Yongji Xie <elohimes@xxxxxxxxx> >> >> Some iommu drivers would be initialized after PCI device >> enumeration. So PCI_BUS_FLAGS_MSI_REMAP would not be set >> when probing PCI devices although IOMMU enables capability >> of IRQ remapping. This patch tests this capability and >> set the flag when iommu driver is initialized. >> >> Signed-off-by: Yongji Xie <xyjxie@xxxxxxxxxxxxxxxxxx> >> Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx> >> --- >> drivers/iommu/iommu.c | 8 ++++++++ >> drivers/pci/probe.c | 1 + >> 2 files changed, 9 insertions(+) >> >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >> index cf7ca7e70777..0b5881ddca09 100644 >> --- a/drivers/iommu/iommu.c >> +++ b/drivers/iommu/iommu.c >> @@ -1063,6 +1063,14 @@ static int add_iommu_group(struct device *dev, void *data) >> const struct iommu_ops *ops = cb->ops; >> int ret; >> >> + /* >> + * Set PCI_BUS_FLAGS_MSI_REMAP for all PCI buses when IOMMU >> + * have capability of IRQ remapping. >> + */ >> + if (dev_is_pci(dev) && ops->capable && >> + ops->capable(IOMMU_CAP_INTR_REMAP)) >> + to_pci_dev(dev)->bus->bus_flags |= PCI_BUS_FLAGS_MSI_REMAP; > > This isn't my area, but this addition is really ugly. It doesn't look > anything like the rest of the code in add_iommu_group(), so it just > feels like a wart. I have no idea what the context is here, but this flag looks wrong generally. IRQ remapping is a property of the irqchip and has nothing to do with PCI, so pretending it's a property of PCI buses looks like a massive hack around... something. Also, iommu_capable() is a fundamentally broken and unworkable interface anyway. The existing IRQ remapping code really wants updating to advertise IRQ_DOMAIN_FLAG_MSI_REMAP on the relevant MSI domains so that IOMMU_CAP_INTR_REMAP can go away for good. That way, all consumers who actually care about whether IRQ remapping is implemented (see e.g. VFIO) can use irq_domain_check_msi_remap() or check specific devices in a sane and scalable manner. Robin. >> if (!ops->add_device) >> return 0; >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> index f2393b7d7ebf..14aac9df3d17 100644 >> --- a/drivers/pci/probe.c >> +++ b/drivers/pci/probe.c >> @@ -17,6 +17,7 @@ >> #include <linux/acpi.h> >> #include <linux/irqdomain.h> >> #include <linux/pm_runtime.h> >> +#include <linux/iommu.h> > > This obviously belongs in another patch, as the compile error showed. > >> #include "pci.h" >> >> #define CARDBUS_LATENCY_TIMER 176 /* secondary latency timer */ >> -- >> 2.11.0 >> > _______________________________________________ > iommu mailing list > iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx > https://lists.linuxfoundation.org/mailman/listinfo/iommu >
