Re: [dpdk-stable] [PATCH] bus/pci: fix Windows kernel driver categories

[Thomas Monjalon <thomas@monjalon.net>]

18/03/2021 00:17, Ranjit Menon:
> Hi Thomas,
> 
> On 3/16/2021 4:11 PM, Thomas Monjalon wrote:
> > In Windows probing, the value RTE_PCI_KDRV_NONE was used
> > instead of RTE_PCI_KDRV_UNKNOWN (mlx case),
> > and RTE_PCI_KDRV_NIC_UIO (FreeBSD) was re-used
> > instead of having a new RTE_PCI_KDRV_NET_UIO for Windows NetUIO.
> Shouldn't the mlx case actually remain RTE_PCI_KDRV_NONE?
> 
> mlx does not require a UIO-like kernel driver...No? And NONE implies that no kernel driver is used/required.
> Not sure what is correct here.

No this is a bifurcated model, meaning kernel and userland
work together. The PCI device is bound to the kernel driver,
but the driver is not listed because no special treatment is required.

> > While adding the new value RTE_PCI_KDRV_NET_UIO,
> > the enum of kernel driver categories is annotated.
> >
> > Fixes: b762221ac24f ("bus/pci: support Windows with bifurcated drivers")
> > Fixes: c76ec01b4591 ("bus/pci: support netuio on Windows")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >   drivers/bus/pci/rte_bus_pci.h | 13 +++++++------
> >   drivers/bus/pci/windows/pci.c | 14 +++++++-------
> >   2 files changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h
> > index fdda046515..3d009cc74b 100644
> > --- a/drivers/bus/pci/rte_bus_pci.h
> > +++ b/drivers/bus/pci/rte_bus_pci.h
> > @@ -52,12 +52,13 @@ TAILQ_HEAD(rte_pci_driver_list, rte_pci_driver);
> >   struct rte_devargs;
> >   
> >   enum rte_pci_kernel_driver {
> > -	RTE_PCI_KDRV_UNKNOWN = 0,
> > -	RTE_PCI_KDRV_IGB_UIO,
> > -	RTE_PCI_KDRV_VFIO,
> > -	RTE_PCI_KDRV_UIO_GENERIC,
> > -	RTE_PCI_KDRV_NIC_UIO,
> > -	RTE_PCI_KDRV_NONE,
> > +	RTE_PCI_KDRV_UNKNOWN = 0,  /* not listed - may be a bifurcated driver */
> > +	RTE_PCI_KDRV_IGB_UIO,      /* igb_uio for Linux */
> > +	RTE_PCI_KDRV_VFIO,         /* VFIO for Linux */
> > +	RTE_PCI_KDRV_UIO_GENERIC,  /* uio_generic for Linux */
> > +	RTE_PCI_KDRV_NIC_UIO,      /* nic_uio for FreeBSD */
> > +	RTE_PCI_KDRV_NONE,         /* error */
> > +	RTE_PCI_KDRV_NET_UIO,      /* NetUIO for Windows */
> >   };
> >   
> 
> Any chance we can re-order the enums, so that _NONE and _UNKNOWN are at 
> the top?

No, it would break the ABI.

> This will change the value, and break code where this value was 
> hard-coded. But how likely is that...?

The problem is when loading the new PCI bus driver with an old device driver.