[PATCH v4] bus/pci: fix legacy device IO port map in secondary process
Gupta, Nipun
nipun.gupta at amd.com
Wed Oct 18 14:38:18 CEST 2023
On 10/18/2023 3:35 PM, David Marchand wrote:
> On Mon, Oct 9, 2023 at 5:06 AM Ma, WenwuX <wenwux.ma at intel.com> wrote:
>>> From a pci bus API pov, nothing prevents a driver from mixing memory
>>> mapped with vfio and ioport resources (iow, calls to
>>> rte_pci_map_resource() and rte_pci_ioport_map()).
>>> IOW, it may not be the case with the net/virtio driver but, in theory,
>>> rte_pci_ioport_map()/pci_vfio_ioport_map() may be called after a
>>> rte_pci_map_resource() call.
>>>
>>> In a similar manner, from the API pov,
>>> rte_pci_ioport_map()/pci_vfio_ioport_map() may be called for multiple bars.
>>>
>>> In summary, nothing in this patch checks that vfio has been configured already
>>> and I think we need a refcount to handle those situations.
>>>
>> We call rte_vfio_setup_device just to get device info, we can call rte_vfio_release_device as soon as pci_vfio_fill_regions is done.
>> This avoids reference counting operations, do you think it works?
>
> Afaics, rte_vfio_setup_device should not be called if a call to
> rte_pci_map_device for this device was successful (rte_pci_map_device
> itself calls rte_vfio_setup_device).
> And as a consequence, calling rte_vfio_release_device cannot be done
> unconditionnally neither.
Hi David,
AFAIU, rte_vfio_setup_device() is written as re-entrant and does not
create the DMA mapping again if it is already done for the iommu group.
When this API is called again either for a device within the same group
or from the device for which it is already called, it mainly only does
the work for device info get. Though not the best thing to use like
this, but if this is called multiple times it should not have any
negative impact.
As Wenmu mention that they need only device info from VFIO, a separate
API to get device info can be added in eal_vfio.c/h. The device info
portion of rte_vfio_setup_device() can be moved out to a new API, and
rte_vfio_setup_device() can call this new API too?
Thanks,
Nipun
More information about the stable
mailing list