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 <[email protected]>
---
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
On Mon, Apr 25, 2022 at 2:27 PM Cindy Lu <[email protected]> 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 <[email protected]>
> ---
> 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
>
在 2022/4/27 10:01, Cindy Lu 写道:
> On Mon, Apr 25, 2022 at 5:00 PM Jason Wang <[email protected]> wrote:
>> On Mon, Apr 25, 2022 at 2:27 PM Cindy Lu <[email protected]> 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 <[email protected]>
>>> ---
>>> 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
>>>
On Mon, Apr 25, 2022 at 5:00 PM Jason Wang <[email protected]> wrote:
>
> On Mon, Apr 25, 2022 at 2:27 PM Cindy Lu <[email protected]> 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 <[email protected]>
> > ---
> > 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
> >
>
On Thu, Apr 28, 2022 at 11:07 AM Jason Wang <[email protected]> wrote:
>
> On Thu, Apr 28, 2022 at 9:56 AM Cindy Lu <[email protected]> wrote:
> >
> > On Wed, Apr 27, 2022 at 12:04 PM Jason Wang <[email protected]> wrote:
> > >
> > >
> > > 在 2022/4/27 10:01, Cindy Lu 写道:
> > > > On Mon, Apr 25, 2022 at 5:00 PM Jason Wang <[email protected]> wrote:
> > > >> On Mon, Apr 25, 2022 at 2:27 PM Cindy Lu <[email protected]> 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 <[email protected]>
> > > >>> ---
> > > >>> 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
> > > >>>
> > >
> >
>
On Thu, Apr 28, 2022 at 9:56 AM Cindy Lu <[email protected]> wrote:
>
> On Wed, Apr 27, 2022 at 12:04 PM Jason Wang <[email protected]> wrote:
> >
> >
> > 在 2022/4/27 10:01, Cindy Lu 写道:
> > > On Mon, Apr 25, 2022 at 5:00 PM Jason Wang <[email protected]> wrote:
> > >> On Mon, Apr 25, 2022 at 2:27 PM Cindy Lu <[email protected]> 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 <[email protected]>
> > >>> ---
> > >>> 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
> > >>>
> >
>
On Wed, Apr 27, 2022 at 12:04 PM Jason Wang <[email protected]> wrote:
>
>
> 在 2022/4/27 10:01, Cindy Lu 写道:
> > On Mon, Apr 25, 2022 at 5:00 PM Jason Wang <[email protected]> wrote:
> >> On Mon, Apr 25, 2022 at 2:27 PM Cindy Lu <[email protected]> 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 <[email protected]>
> >>> ---
> >>> 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
> >>>
>
On Thu, Apr 28, 2022 at 11:46 AM Jason Wang <[email protected]> wrote:
>
> On Thu, Apr 28, 2022 at 11:21 AM Cindy Lu <[email protected]> wrote:
> >
> > On Thu, Apr 28, 2022 at 11:07 AM Jason Wang <[email protected]> wrote:
> > >
> > > On Thu, Apr 28, 2022 at 9:56 AM Cindy Lu <[email protected]> wrote:
> > > >
> > > > On Wed, Apr 27, 2022 at 12:04 PM Jason Wang <[email protected]> wrote:
> > > > >
> > > > >
> > > > > 在 2022/4/27 10:01, Cindy Lu 写道:
> > > > > > On Mon, Apr 25, 2022 at 5:00 PM Jason Wang <[email protected]> wrote:
> > > > > >> On Mon, Apr 25, 2022 at 2:27 PM Cindy Lu <[email protected]> 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 <[email protected]>
> > > > > >>> ---
> > > > > >>> 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
> > > > > >>>
> > > > >
> > > >
> > >
> >
>
On Thu, Apr 28, 2022 at 11:21 AM Cindy Lu <[email protected]> wrote:
>
> On Thu, Apr 28, 2022 at 11:07 AM Jason Wang <[email protected]> wrote:
> >
> > On Thu, Apr 28, 2022 at 9:56 AM Cindy Lu <[email protected]> wrote:
> > >
> > > On Wed, Apr 27, 2022 at 12:04 PM Jason Wang <[email protected]> wrote:
> > > >
> > > >
> > > > 在 2022/4/27 10:01, Cindy Lu 写道:
> > > > > On Mon, Apr 25, 2022 at 5:00 PM Jason Wang <[email protected]> wrote:
> > > > >> On Mon, Apr 25, 2022 at 2:27 PM Cindy Lu <[email protected]> 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 <[email protected]>
> > > > >>> ---
> > > > >>> 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
> > > > >>>
> > > >
> > >
> >
>