+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? Thanks, Chenbo > > > -- > David Marchand