Re: [PATCH kernel v4 4/6] iommu: Set PCI_BUS_FLAGS_MSI_REMAP on iommu driver initialization

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 19/07/17 20:02, Alexey Kardashevskiy wrote:
> On 11/07/17 05:23, Bjorn Helgaas 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.
> 
> 
> It does look like a wart, however it did not cause immediate rejection
> after first couple of revisions of this before I took it over...
> 
> So. Here is the problem - deliver an MSIX isolation flag from IOMMU to
> VFIO-PCI driver. The options are:
> 
> 1. PCI_BUS_FLAGS_MSI_REMAP on a PCI bus - MSIX isolation is not really a
> PCI bus property and it is "like a wart".
> 
> 2. Introduce "flags" in iommu_group and define IOMMU_GROUP_MSIX_ISOLATED
> and set it when an IOMMU group is created; VFIO-PCI has ways to get to the
> group and read the flag.
> 
> 3. Create IOMMU_DOMAIN_UNMANAGED IOMMU domains for PPC64/powernv IOMMU
> groups and only define capable() hook to report IOMMU_CAP_INTR_REMAP;
> others already use these IOMMU domains. VFIO-PCI's mmap() hook could then
> check the capability via iommu_capable(bus). The problems is as Robin said:
> "iommu_capable() is a fundamentally broken and unworkable interface
> anyway"; however it is still not clear to me why it is unworkable in this
> particular case of isolation checking.
> 
> I am missing knowledge about ARM, is there a good overview of these MSIX
> domains vs. IOMMU on ARM (as MSIX remapping seems not to be an IOMMU property)?
> 
> 
> Which one to pick? Thanks.



Anyone, please?



> 
> 
> 
>>
>>>         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
>>>
> 
> 


-- 
Alexey



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux