From: Jason Wang <jasowang@redhat.com> To: Cindy Lu <lulu@redhat.com> Cc: mst <mst@redhat.com>, virtualization <virtualization@lists.linux-foundation.org>, linux-kernel <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev Date: Thu, 28 Apr 2022 11:07:11 +0800 [thread overview] Message-ID: <CACGkMEuJhrSxNc3v7hijSpBdA4X6aC-gA2Ogwed4oODUT_DJ6w@mail.gmail.com> (raw) In-Reply-To: <CACLfguW+9OMPMUpehp+Zut7JosFtg2gzr7t7gZ6U-AdtV89S3g@mail.gmail.com> 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 > > >>> > > >
WARNING: multiple messages have this Message-ID (diff)
From: Jason Wang <jasowang@redhat.com> To: Cindy Lu <lulu@redhat.com> Cc: virtualization <virtualization@lists.linux-foundation.org>, linux-kernel <linux-kernel@vger.kernel.org>, mst <mst@redhat.com> Subject: Re: [PATCH v2 1/2] vdpa: add the check for id_table in struct vdpa_mgmt_dev Date: Thu, 28 Apr 2022 11:07:11 +0800 [thread overview] Message-ID: <CACGkMEuJhrSxNc3v7hijSpBdA4X6aC-gA2Ogwed4oODUT_DJ6w@mail.gmail.com> (raw) In-Reply-To: <CACLfguW+9OMPMUpehp+Zut7JosFtg2gzr7t7gZ6U-AdtV89S3g@mail.gmail.com> 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
next prev parent reply other threads:[~2022-04-28 3:07 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 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 [this message] 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
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CACGkMEuJhrSxNc3v7hijSpBdA4X6aC-gA2Ogwed4oODUT_DJ6w@mail.gmail.com \ --to=jasowang@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=lulu@redhat.com \ --cc=mst@redhat.com \ --cc=virtualization@lists.linux-foundation.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.