On 7/3/2023 3:01 PM, Xia, Chenbo wrote:
+Nipun
-----Original Message-----
From: David Marchand <david.march...@redhat.com>
Sent: Monday, July 3, 2023 4:58 PM
To: Li, Miao <miao...@intel.com>
Cc: dev@dpdk.org; sta...@dpdk.org; Maxime Coquelin
<maxime.coque...@redhat.com>; Xia, Chenbo <chenbo....@intel.com>
Subject: Re: [PATCH v2 1/2] net/virtio: fix legacy device IO port map in
secondary process
On Mon, Jul 3, 2023 at 10:54 AM Li, Miao <miao...@intel.com> wrote:
When doing IO port map for legacy device in secondary process,
vfio_cfg setup for legacy device like vfio_group_fd and vfio_dev_fd
is
missing. So, in secondary process, rte_pci_map_device is added for
legacy device to setup vfio_cfg and fill in region info like in
primary process.
I think, in legacy mode, there is no PCI mappable memory.
So there should be no need for this call to rte_pci_map_device.
What is missing is a vfio setup, is this correct?
I'd rather see this issue be fixed in the pci_vfio_ioport_map()
function.
If adding vfio setup in the pci_vfio_ioport_map() function, vfio will be
setup twice in primary process because rte_pci_map_device will be called
for legacy device in primary process.
I add IO port region check to skip region map in the next patch.
Well, something must be done so that it is not mapped twice, I did not
look into the details.
This current patch looks wrong to me and I understand this is not a
virtio only issue.
I think we could have some way to improve this:
1. Make rte_pci_map_device map either PIO or MMIO (Based on my knowledge, it's
doable
for vfio. For UIO, I am no expert and not sure). For ioport, it's only about
setting
up the ioport offset and size.
2. rte_pci_ioport_map may not be needed anymore.
3. struct rte_pci_ioport may not be needed anymore as the info could be saved in
struct rte_pci_device_internal.
4. ioport device uses bar #, len, offset to RW specific BAR.
Then for virtio device, either primary or secondary process only calls
rte_pci_map_device
once.
Any comments?
Wouldn't a call to API rte_vfio_setup_device() to setup vfio_cfg,
vfio_group_fd, vfio_dev_fd under a secondary process check suffice to
handle IO port map for legacy device in secondary process?
I do not have much info on legacy Virtio device, and I am not clear on
why and how for these devices rte_pci_map_device() would be called in
case of primary process, but not in case of secondary process as
mentioned by Miao Li?
Steps you have mentioned looks fine but note that this would cause an
ABI breakage and as you mentioned may need changes in UIO (even I am not
an expert in UIO).
Thanks,
Nipun
Thanks,
Chenbo
--
David Marchand
