Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev

[Cindy Lu <lulu@redhat.com>]

On Wed, Apr 27, 2022 at 12:04 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2022/4/27 10:01, Cindy Lu 写道:
> > On Mon, Apr 25, 2022 at 5:00 PM Jason Wang <jasowang@redhat.com> wrote:
> >> On Mon, Apr 25, 2022 at 2:27 PM Cindy Lu <lulu@redhat.com> wrote:
> >>> To support the dynamic ids in vp_vdpa, we need to add the check for
> >>> id table. If the id table is NULL, will not set the device type
> >>>
> >>> Signed-off-by: Cindy Lu <lulu@redhat.com>
> >>> ---
> >>>   drivers/vdpa/vdpa.c | 11 +++++++----
> >>>   1 file changed, 7 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> >>> index 1ea525433a5c..09edd92cede0 100644
> >>> --- a/drivers/vdpa/vdpa.c
> >>> +++ b/drivers/vdpa/vdpa.c
> >>> @@ -492,10 +492,13 @@ static int vdpa_mgmtdev_fill(const struct vdpa_mgmt_dev *mdev, struct sk_buff *m
> >>>          if (err)
> >>>                  goto msg_err;
> >>>
> >>> -       while (mdev->id_table[i].device) {
> >>> -               if (mdev->id_table[i].device <= 63)
> >>> -                       supported_classes |= BIT_ULL(mdev->id_table[i].device);
> >>> -               i++;
> >>> +       if (mdev->id_table != NULL) {
> >>> +               while (mdev->id_table[i].device) {
> >>> +                       if (mdev->id_table[i].device <= 63)
> >>> +                               supported_classes |=
> >>> +                                       BIT_ULL(mdev->id_table[i].device);
> >>> +                       i++;
> >>> +               }
> >>>          }
> >> This will cause 0 to be advertised as the supported classes.
> >>
> >> I wonder if we can simply use VIRTIO_DEV_ANY_ID here (and need to
> >> export it to via uAPI probably).
> >>
> >> Thanks
> >>
> > like the below one? not sure if this ok to use like this?
> > static struct virtio_device_id vp_vdpa_id_table[] = {
> > { VIRTIO_DEV_ANY_ID, VIRTIO_DEV_ANY_ID },
> > { 0 },
> > };
>
>
> Something like this.
>
> Thanks
>
>
I have checked the code, this maybe can not work, because the
#define VIRTIO_DEV_ANY_ID 0xffffffff
 it want't work in
                supported_classes |= BIT_ULL(mdev->id_table[i].device);
if we chane to
            supported_classes |= VIRTIO_DEV_ANY_ID;
the vdpa dev show will be
pci/0000:00:04.0:
  supported_classes net block < unknown class > < unknown class > <
unknown class > < unknown class > < unknown class > < unknow>
  max_supported_vqs 3
  dev_features CSUM GUEST_CSUM CTRL_GUEST_OFFLOADS MAC GUEST_TSO4
GUEST_TSO6 GUEST_ECN GUEST_UFO HOST_TSO4 HOST_TSO6 HOST_
 I think we can use
static struct virtio_device_id id_table[] = {
{ VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
{ 0 },
};
 if we need to add another type of device, we can add the device id at that type

Thanks
cindy



> >
> >
> >>>          if (nla_put_u64_64bit(msg, VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES,
> >>> --
> >>> 2.34.1
> >>>
>