RE: [PATCH v2 1/2] net/virtio: fix legacy device IO port map in secondary process

["Xia, Chenbo" <chenbo.xia@intel.com>]

+Nipun

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, July 3, 2023 4:58 PM
> To: Li, Miao <miao.li@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Xia, Chenbo <chenbo.xia@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.li@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?

Thanks,
Chenbo

> 
> 
> --
> David Marchand