[dpdk-dev] [PATCH v5 3/3] PCI: don't use vfio ioctl call to access PIO resource

Maxime Coquelin maxime.coquelin at redhat.com
Thu Jan 21 09:29:54 CET 2021



On 1/20/21 3:54 PM, 谢华伟(此时此刻) wrote:
> 
> On 2021/1/13 0:58, Maxime Coquelin wrote:
>>
>> On 1/12/21 10:37 AM, Maxime Coquelin wrote:
>>> bus/pci: ...
>>>
>>> On 10/22/20 5:51 PM, 谢华伟(此时此刻) wrote:
>>>> From: "huawei.xhw" <huawei.xhw at alibaba-inc.com>
>>>>
>>>> VFIO should use the same way to map/read/write PORT IO as UIO, for
>>>> virtio PMD.
>>> Please provide more details in the commit message on why the way VFIO
>>> works today is wrong (The cover letter is lost once applied).
> ok
>>>
>>>> Signed-off-by: huawei.xhw <huawei.xhw at alibaba-inc.com>
>>> Same comment about name format as on previous patches.
>>>
>>>> ---
>>>>   drivers/bus/pci/linux/pci.c     | 8 ++++----
>>>>   drivers/bus/pci/linux/pci_uio.c | 4 +++-
>>>>   2 files changed, 7 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
>>>> index 0dc99e9..2ed9f2b 100644
>>>> --- a/drivers/bus/pci/linux/pci.c
>>>> +++ b/drivers/bus/pci/linux/pci.c
>>>> @@ -687,7 +687,7 @@ int rte_pci_write_config(const struct
>>>> rte_pci_device *device,
>>>>   #ifdef VFIO_PRESENT
>>>>       case RTE_PCI_KDRV_VFIO:
>>>>           if (pci_vfio_is_enabled())
>>>> -            ret = pci_vfio_ioport_map(dev, bar, p);
>>>> +            ret = pci_uio_ioport_map(dev, bar, p);
>>> Doesn't it create a regression with regards to needed capabilities?
>>> My understanding is that before this patch we don't need to call iopl(),
>>> whereas once applied it is required, correct?
>> I did some testing today, and think it is not a regression with para-
>> virtualized Virtio devices.
>>
>> Indeed, I thought it would be a regression with Legacy devices when
>> IOMMU is enabled and the program is run as non-root (IOMMU enabled
>> just to suport IOVA as VA mode). But it turns out para-virtualized
>> Virtio legacy device and vIOMMU enabled is not a supported configuration
>> by QEMU.
>>
>> Note that when noiommu mode is enabled, the app needs cap_sys_rawio, so
>> same as iopl(). No regression in this case too.
>>
>> That said, with real (non para-virtualized) Virtio device using PIO like
>> yours, doesn't your patch introduce a restriction for your device that
>> it will require cap_sys_rawio whereas it would not be needed?
> 
> I don't catch the regression issue.
> 
> With real virtio device(hardware implemented), if it is using MMIO, no
> cap_sys_rawio is required.
> 
> If it is using PIO, iopl is required always.

My understanding of the Kernel VFIO driver is that cap_sys_rawio is only
necessary in noiommu mode, i.e. when VFIO is loaded with
enable_unsafe_noiommu parameter set. The doc for this parameters seems
to validate my understanding of the code:
"
MODULE_PARM_DESC(enable_unsafe_noiommu_mode, "Enable UNSAFE, no-IOMMU
mode.  This mode provides no device isolation, no DMA translation, no
host kernel protection, cannot be used for device assignment to virtual
machines, requires RAWIO permissions, and will taint the kernel.  If you
do not know what this is for, step away. (default: false)");
"

I think that using inb/outb in the case of VFIO with IOMMU enabled won't
work without cap_sys_rawio, and using it in the case of VFIO with IOMMU
disabled just bypasses VFIO and so is not correct.

In my opinion, what we should do is to add something like this in the
DPDK documentation:

 - MMIO BAR: VFIO with IOMMU enabled recommended. Equivalent performance
as with IGB UIO or VFIO with NOIOMMU. VFIO with IOMMU is recommended for
security reasons.
 - PIO BAR: VFIO with IOMMU enabled is recommended for security reasons,
providing proper isolation and not requiring cap_sys_rawio. However, use
of IOMMU is not always possible in some cases (e.g. para-virtualized
Virtio-net legacy device). Also, performance of using VFIO for PIO BARs
accesses has an impact on performance as it uses pread/pwrite syscalls,
whereas UIO drivers use inb/outb. If security is not a concern or IOMMU
is not available, one might consider using UIO driver in this case for
performance reasons.

What do you think?

Thanks,
Maxime

> 
>> Thanks,
>> Maxime
>>
>>> Regards,
>>> Maxime
>>>
> 



More information about the dev mailing list