Re: [PATCH V2 2/4] vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA

[Zhu, Lingshan]

On 3/9/2021 10:42 AM, Jason Wang wrote:
> On 2021/3/9 10:28 上午, Zhu, Lingshan wrote:
> > On 3/9/2021 10:23 AM, Jason Wang wrote:
> > > On 2021/3/8 4:35 下午, Zhu Lingshan wrote:
> > > > This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-net
> > > > for vDPA
> > > >
> > > > Signed-off-by: Zhu Lingshan <lingshan.zhu@xxxxxxxxx>
> > > > ---
> > > >   drivers/vdpa/ifcvf/ifcvf_base.h | 5 +++++
> > > >   drivers/vdpa/ifcvf/ifcvf_main.c | 5 +++++
> > > >   2 files changed, 10 insertions(+)
> > > >
> > > > diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h 
> > > > b/drivers/vdpa/ifcvf/ifcvf_base.h
> > > > index 64696d63fe07..75d9a8052039 100644
> > > > --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> > > > +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> > > > @@ -23,6 +23,11 @@
> > > >   #define IFCVF_SUBSYS_VENDOR_ID    0x8086
> > > >   #define IFCVF_SUBSYS_DEVICE_ID    0x001A
> > > >   +#define C5000X_PL_VENDOR_ID        0x1AF4
> > > > +#define C5000X_PL_DEVICE_ID        0x1000
> > > > +#define C5000X_PL_SUBSYS_VENDOR_ID    0x8086
> > > > +#define C5000X_PL_SUBSYS_DEVICE_ID    0x0001
> > >
> > >
> > > I just notice that the device is a transtitional one. Any reason for 
> > > doing this?
> > >
> > > Note that IFCVF is a moden device anyhow (0x1041). Supporting legacy 
> > > drive may bring many issues (e.g the definition is non-nomartive). 
> > > One example is the support of VIRTIO_F_IOMMU_PLATFORM, legacy driver 
> > > may assume the device can bypass IOMMU.
> > >
> > > Thanks
> > Hi Jason,
> >
> > This device will support virtio1.0 by default, so has 
> > VIRTIO_F_IOMMU_PLATFORM by default.
>
>
> If you device want to force VIRTIO_F_IOMMU_PLATFORM you probably need 
> to do what has been done by mlx5 (verify_min_features).
>
> According to the spec, if VIRTIO_F_IOMMU_PLATFORM is not mandatory, 
> when it's not negotiated, device needs to disable or bypass IOMMU:
>
>
> "
>
> If this feature bit is set to 0, then the device has same access to 
> memory addresses supplied to it as the driver has. In particular, the 
> device will always use physical addresses matching addresses used by 
> the driver (typically meaning physical addresses used by the CPU) and 
> not translated further, and can access any address supplied to it by 
> the driver.
>
> "
sure, I can implement code to check the feature bits.
> > Transitional device gives the software a chance to fall back to 
> > virtio 0.95.
>
>
> This only applies if you want to passthrough the card to guest 
> directly without the help of vDPA.
>
> If we go with vDPA, it doesn't hlep. For virtio-vdpa, we know it will 
> negotiated IOMMU_PLATFORM. For vhost-vdpa, Qemu can provide a legacy 
> or transitional device on top of a modern vDPA device.
>
> Thanks
For some cases, users may run quite out of date OS does not have vDPA 
nor virtio 1.0 support, transitional characters give them a chance to 
use the devices.

Thanks
Zhu Lingshan
> > ifcvf drives this device in virtio 1.0 mode, set features 
> > VIRTIO_F_IOMMU_PLATFORM successfully.
> >
> > Thanks,
> > Zhu Lingshan
> > > > +
> > > >   #define IFCVF_SUPPORTED_FEATURES \
> > > >           ((1ULL << VIRTIO_NET_F_MAC)            | \
> > > >            (1ULL << VIRTIO_F_ANY_LAYOUT) | \
> > > > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 
> > > > b/drivers/vdpa/ifcvf/ifcvf_main.c
> > > > index e501ee07de17..26a2dab7ca66 100644
> > > > --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> > > > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> > > > @@ -484,6 +484,11 @@ static struct pci_device_id ifcvf_pci_ids[] = {
> > > >           IFCVF_DEVICE_ID,
> > > >           IFCVF_SUBSYS_VENDOR_ID,
> > > >           IFCVF_SUBSYS_DEVICE_ID) },
> > > > +    { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID,
> > > > +             C5000X_PL_DEVICE_ID,
> > > > +             C5000X_PL_SUBSYS_VENDOR_ID,
> > > > +             C5000X_PL_SUBSYS_DEVICE_ID) },
> > > > +
> > > >       { 0 },
> > > >   };
> > > >   MODULE_DEVICE_TABLE(pci, ifcvf_pci_ids);