All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
@ 2022-04-25  6:27 Cindy Lu
  2022-04-25  8:59   ` Jason Wang
  0 siblings, 1 reply; 13+ messages in thread
From: Cindy Lu @ 2022-04-25  6:27 UTC (permalink / raw)
  To: mst, jasowang, lulu, virtualization, linux-kernel

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++;
+		}
 	}
 
 	if (nla_put_u64_64bit(msg, VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
  2022-04-25  6:27 [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev Cindy Lu
@ 2022-04-25  8:59   ` Jason Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2022-04-25  8:59 UTC (permalink / raw)
  To: Cindy Lu; +Cc: mst, virtualization, linux-kernel

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

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
@ 2022-04-25  8:59   ` Jason Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2022-04-25  8:59 UTC (permalink / raw)
  To: Cindy Lu; +Cc: virtualization, linux-kernel, mst

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

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

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
  2022-04-25  8:59   ` Jason Wang
  (?)
@ 2022-04-27  2:01   ` Cindy Lu
  2022-04-27  4:03       ` Jason Wang
  -1 siblings, 1 reply; 13+ messages in thread
From: Cindy Lu @ 2022-04-27  2:01 UTC (permalink / raw)
  To: Jason Wang; +Cc: mst, virtualization, linux-kernel

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 },
};


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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
  2022-04-27  2:01   ` Cindy Lu
@ 2022-04-27  4:03       ` Jason Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2022-04-27  4:03 UTC (permalink / raw)
  To: Cindy Lu; +Cc: mst, virtualization, linux-kernel


在 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


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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
@ 2022-04-27  4:03       ` Jason Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2022-04-27  4:03 UTC (permalink / raw)
  To: Cindy Lu; +Cc: virtualization, linux-kernel, mst


在 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


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

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
  2022-04-27  4:03       ` Jason Wang
  (?)
@ 2022-04-28  1:56       ` Cindy Lu
  2022-04-28  3:07           ` Jason Wang
  -1 siblings, 1 reply; 13+ messages in thread
From: Cindy Lu @ 2022-04-28  1:56 UTC (permalink / raw)
  To: Jason Wang; +Cc: mst, virtualization, linux-kernel

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
> >>>
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
  2022-04-28  1:56       ` Cindy Lu
@ 2022-04-28  3:07           ` Jason Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2022-04-28  3:07 UTC (permalink / raw)
  To: Cindy Lu; +Cc: mst, virtualization, linux-kernel

On Thu, Apr 28, 2022 at 9:56 AM Cindy Lu <lulu@redhat.com> wrote:
>
> 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

That's why I suggest exporting the ANY_ID via uAPI and then we can fix
the userspace.

>   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
>

My point is that, we have supported any virtio devices before. But
after this change, we only support virtio-net.

So if we choose to use id arrays, let's just add all possible virtio
devices that are supported by the kernel here.

Thanks

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
@ 2022-04-28  3:07           ` Jason Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2022-04-28  3:07 UTC (permalink / raw)
  To: Cindy Lu; +Cc: virtualization, linux-kernel, mst

On Thu, Apr 28, 2022 at 9:56 AM Cindy Lu <lulu@redhat.com> wrote:
>
> 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

That's why I suggest exporting the ANY_ID via uAPI and then we can fix
the userspace.

>   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
>

My point is that, we have supported any virtio devices before. But
after this change, we only support virtio-net.

So if we choose to use id arrays, let's just add all possible virtio
devices that are supported by the kernel here.

Thanks

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

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
  2022-04-28  3:07           ` Jason Wang
  (?)
@ 2022-04-28  3:21           ` Cindy Lu
  2022-04-28  3:45               ` Jason Wang
  -1 siblings, 1 reply; 13+ messages in thread
From: Cindy Lu @ 2022-04-28  3:21 UTC (permalink / raw)
  To: Jason Wang; +Cc: mst, virtualization, linux-kernel

On Thu, Apr 28, 2022 at 11:07 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Thu, Apr 28, 2022 at 9:56 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > 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
>
> That's why I suggest exporting the ANY_ID via uAPI and then we can fix
> the userspace.
>
sure.But I think maybe we can fix this in another patch, since it
related to userspace

> >   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
> >
>
> My point is that, we have supported any virtio devices before. But
> after this change, we only support virtio-net.
>
> So if we choose to use id arrays, let's just add all possible virtio
> devices that are supported by the kernel here.
>
sorry, I didn't make it clearly,  I mean  we can use the vp_vdpa device id as
 static struct virtio_device_id id_table[] = {
 { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
{ 0 },
since it now only support the net device and This will make the vp_vdpa work.

Thanks
cindy

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
  2022-04-28  3:21           ` Cindy Lu
@ 2022-04-28  3:45               ` Jason Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2022-04-28  3:45 UTC (permalink / raw)
  To: Cindy Lu; +Cc: mst, virtualization, linux-kernel

On Thu, Apr 28, 2022 at 11:21 AM Cindy Lu <lulu@redhat.com> wrote:
>
> On Thu, Apr 28, 2022 at 11:07 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Thu, Apr 28, 2022 at 9:56 AM Cindy Lu <lulu@redhat.com> wrote:
> > >
> > > 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
> >
> > That's why I suggest exporting the ANY_ID via uAPI and then we can fix
> > the userspace.
> >
> sure.But I think maybe we can fix this in another patch, since it
> related to userspace

Yes.

>
> > >   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
> > >
> >
> > My point is that, we have supported any virtio devices before. But
> > after this change, we only support virtio-net.
> >
> > So if we choose to use id arrays, let's just add all possible virtio
> > devices that are supported by the kernel here.
> >
> sorry, I didn't make it clearly,  I mean  we can use the vp_vdpa device id as
>  static struct virtio_device_id id_table[] = {
>  { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
> { 0 },
> since it now only support the net device

This is not what I read from the code:

static int virtio_vdpa_probe(struct vdpa_device *vdpa)
{
...
vd_dev->vdev.id.device = ops->get_device_id(vdpa);
        if (vd_dev->vdev.id.device == 0)
                goto err;

vd_dev->vdev.id.vendor = ops->get_vendor_id(vdpa);
        ret = register_virtio_device(&vd_dev->vdev);
...
}

?

Thanks

>  and This will make the vp_vdpa work.
>
> Thanks
> cindy
>
> > Thanks
> >
> > > Thanks
> > > cindy
> > >
> > >
> > >
> > > > >
> > > > >
> > > > >>>          if (nla_put_u64_64bit(msg, VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES,
> > > > >>> --
> > > > >>> 2.34.1
> > > > >>>
> > > >
> > >
> >
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
@ 2022-04-28  3:45               ` Jason Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2022-04-28  3:45 UTC (permalink / raw)
  To: Cindy Lu; +Cc: virtualization, linux-kernel, mst

On Thu, Apr 28, 2022 at 11:21 AM Cindy Lu <lulu@redhat.com> wrote:
>
> On Thu, Apr 28, 2022 at 11:07 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Thu, Apr 28, 2022 at 9:56 AM Cindy Lu <lulu@redhat.com> wrote:
> > >
> > > 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
> >
> > That's why I suggest exporting the ANY_ID via uAPI and then we can fix
> > the userspace.
> >
> sure.But I think maybe we can fix this in another patch, since it
> related to userspace

Yes.

>
> > >   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
> > >
> >
> > My point is that, we have supported any virtio devices before. But
> > after this change, we only support virtio-net.
> >
> > So if we choose to use id arrays, let's just add all possible virtio
> > devices that are supported by the kernel here.
> >
> sorry, I didn't make it clearly,  I mean  we can use the vp_vdpa device id as
>  static struct virtio_device_id id_table[] = {
>  { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
> { 0 },
> since it now only support the net device

This is not what I read from the code:

static int virtio_vdpa_probe(struct vdpa_device *vdpa)
{
...
vd_dev->vdev.id.device = ops->get_device_id(vdpa);
        if (vd_dev->vdev.id.device == 0)
                goto err;

vd_dev->vdev.id.vendor = ops->get_vendor_id(vdpa);
        ret = register_virtio_device(&vd_dev->vdev);
...
}

?

Thanks

>  and This will make the vp_vdpa work.
>
> Thanks
> cindy
>
> > Thanks
> >
> > > Thanks
> > > cindy
> > >
> > >
> > >
> > > > >
> > > > >
> > > > >>>          if (nla_put_u64_64bit(msg, VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES,
> > > > >>> --
> > > > >>> 2.34.1
> > > > >>>
> > > >
> > >
> >
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev
  2022-04-28  3:45               ` Jason Wang
  (?)
@ 2022-04-28  7:14               ` Cindy Lu
  -1 siblings, 0 replies; 13+ messages in thread
From: Cindy Lu @ 2022-04-28  7:14 UTC (permalink / raw)
  To: Jason Wang; +Cc: mst, virtualization, linux-kernel

On Thu, Apr 28, 2022 at 11:46 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Thu, Apr 28, 2022 at 11:21 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > On Thu, Apr 28, 2022 at 11:07 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Thu, Apr 28, 2022 at 9:56 AM Cindy Lu <lulu@redhat.com> wrote:
> > > >
> > > > 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
> > >
> > > That's why I suggest exporting the ANY_ID via uAPI and then we can fix
> > > the userspace.
> > >
> > sure.But I think maybe we can fix this in another patch, since it
> > related to userspace
>
> Yes.
>
> >
> > > >   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
> > > >
> > >
> > > My point is that, we have supported any virtio devices before. But
> > > after this change, we only support virtio-net.
> > >
> > > So if we choose to use id arrays, let's just add all possible virtio
> > > devices that are supported by the kernel here.
> > >
> > sorry, I didn't make it clearly,  I mean  we can use the vp_vdpa device id as
> >  static struct virtio_device_id id_table[] = {
> >  { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
> > { 0 },
> > since it now only support the net device
>
> This is not what I read from the code:
>
> static int virtio_vdpa_probe(struct vdpa_device *vdpa)
> {
> ...
> vd_dev->vdev.id.device = ops->get_device_id(vdpa);
>         if (vd_dev->vdev.id.device == 0)
>                 goto err;
>
> vd_dev->vdev.id.vendor = ops->get_vendor_id(vdpa);
>         ret = register_virtio_device(&vd_dev->vdev);
> ...
> }
>
> ?
>
sure, Thanks I will post another verson

> Thanks
>
> >  and This will make the vp_vdpa work.
> >
> > Thanks
> > cindy
> >
> > > Thanks
> > >
> > > > Thanks
> > > > cindy
> > > >
> > > >
> > > >
> > > > > >
> > > > > >
> > > > > >>>          if (nla_put_u64_64bit(msg, VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES,
> > > > > >>> --
> > > > > >>> 2.34.1
> > > > > >>>
> > > > >
> > > >
> > >
> >
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-04-28  7:15 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-25  6:27 [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev Cindy Lu
2022-04-25  8:59 ` Jason Wang
2022-04-25  8:59   ` Jason Wang
2022-04-27  2:01   ` Cindy Lu
2022-04-27  4:03     ` Jason Wang
2022-04-27  4:03       ` Jason Wang
2022-04-28  1:56       ` Cindy Lu
2022-04-28  3:07         ` Jason Wang
2022-04-28  3:07           ` Jason Wang
2022-04-28  3:21           ` Cindy Lu
2022-04-28  3:45             ` Jason Wang
2022-04-28  3:45               ` Jason Wang
2022-04-28  7:14               ` Cindy Lu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.