diff mbox series

[virtio,3/4] pds_vdpa: clean and reset vqs entries

Message ID 20230630003609.28527-4-shannon.nelson@amd.com (mailing list archive)
State Superseded
Headers show
Series pds_vdpa: mac, reset, and irq updates | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Nelson, Shannon June 30, 2023, 12:36 a.m. UTC
Make sure that we initialize the vqs[] entries the same
way both for initial setup and after a vq reset.

Fixes: 151cc834f3dd ("pds_vdpa: add support for vdpa and vdpamgmt interfaces")
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/vdpa/pds/vdpa_dev.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

Comments

Jason Wang July 7, 2023, 7:36 a.m. UTC | #1
On Fri, Jun 30, 2023 at 8:36 AM Shannon Nelson <shannon.nelson@amd.com> wrote:
>
> Make sure that we initialize the vqs[] entries the same
> way both for initial setup and after a vq reset.
>
> Fixes: 151cc834f3dd ("pds_vdpa: add support for vdpa and vdpamgmt interfaces")
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> Reviewed-by: Brett Creeley <brett.creeley@amd.com>
> ---
>  drivers/vdpa/pds/vdpa_dev.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
> index 5e761d625ef3..5e1046c9af3d 100644
> --- a/drivers/vdpa/pds/vdpa_dev.c
> +++ b/drivers/vdpa/pds/vdpa_dev.c
> @@ -429,6 +429,18 @@ static void pds_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
>         }
>  }
>
> +static void pds_vdpa_init_vqs_entry(struct pds_vdpa_device *pdsv, int qid)
> +{
> +       memset(&pdsv->vqs[qid], 0, sizeof(pdsv->vqs[0]));
> +       pdsv->vqs[qid].qid = qid;
> +       pdsv->vqs[qid].pdsv = pdsv;
> +       pdsv->vqs[qid].ready = false;
> +       pdsv->vqs[qid].irq = VIRTIO_MSI_NO_VECTOR;
> +       pdsv->vqs[qid].notify =
> +               vp_modern_map_vq_notify(&pdsv->vdpa_aux->vd_mdev,
> +                                       qid, &pdsv->vqs[qid].notify_pa);

Nit: It looks to me this would not change. So we probably don't need
this during reset?

Thanks

> +}
> +
>  static int pds_vdpa_reset(struct vdpa_device *vdpa_dev)
>  {
>         struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
> @@ -451,8 +463,7 @@ static int pds_vdpa_reset(struct vdpa_device *vdpa_dev)
>                                 dev_err(dev, "%s: reset_vq failed qid %d: %pe\n",
>                                         __func__, i, ERR_PTR(err));
>                         pds_vdpa_release_irq(pdsv, i);
> -                       memset(&pdsv->vqs[i], 0, sizeof(pdsv->vqs[0]));
> -                       pdsv->vqs[i].ready = false;
> +                       pds_vdpa_init_vqs_entry(pdsv, i);
>                 }
>         }
>
> @@ -640,13 +651,8 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>         }
>         pds_vdpa_cmd_set_mac(pdsv, pdsv->mac);
>
> -       for (i = 0; i < pdsv->num_vqs; i++) {
> -               pdsv->vqs[i].qid = i;
> -               pdsv->vqs[i].pdsv = pdsv;
> -               pdsv->vqs[i].irq = VIRTIO_MSI_NO_VECTOR;
> -               pdsv->vqs[i].notify = vp_modern_map_vq_notify(&pdsv->vdpa_aux->vd_mdev,
> -                                                             i, &pdsv->vqs[i].notify_pa);
> -       }
> +       for (i = 0; i < pdsv->num_vqs; i++)
> +               pds_vdpa_init_vqs_entry(pdsv, i);
>
>         pdsv->vdpa_dev.mdev = &vdpa_aux->vdpa_mdev;
>
> --
> 2.17.1
>
Nelson, Shannon July 7, 2023, 8:12 p.m. UTC | #2
On 7/7/23 12:36 AM, Jason Wang wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Fri, Jun 30, 2023 at 8:36 AM Shannon Nelson <shannon.nelson@amd.com> wrote:
>>
>> Make sure that we initialize the vqs[] entries the same
>> way both for initial setup and after a vq reset.
>>
>> Fixes: 151cc834f3dd ("pds_vdpa: add support for vdpa and vdpamgmt interfaces")
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>> Reviewed-by: Brett Creeley <brett.creeley@amd.com>
>> ---
>>   drivers/vdpa/pds/vdpa_dev.c | 24 +++++++++++++++---------
>>   1 file changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
>> index 5e761d625ef3..5e1046c9af3d 100644
>> --- a/drivers/vdpa/pds/vdpa_dev.c
>> +++ b/drivers/vdpa/pds/vdpa_dev.c
>> @@ -429,6 +429,18 @@ static void pds_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
>>          }
>>   }
>>
>> +static void pds_vdpa_init_vqs_entry(struct pds_vdpa_device *pdsv, int qid)
>> +{
>> +       memset(&pdsv->vqs[qid], 0, sizeof(pdsv->vqs[0]));
>> +       pdsv->vqs[qid].qid = qid;
>> +       pdsv->vqs[qid].pdsv = pdsv;
>> +       pdsv->vqs[qid].ready = false;
>> +       pdsv->vqs[qid].irq = VIRTIO_MSI_NO_VECTOR;
>> +       pdsv->vqs[qid].notify =
>> +               vp_modern_map_vq_notify(&pdsv->vdpa_aux->vd_mdev,
>> +                                       qid, &pdsv->vqs[qid].notify_pa);
> 
> Nit: It looks to me this would not change. So we probably don't need
> this during reset?

We set it again here because we used memset to clean the struct and need 
to put it back.  But we could grap the value before the memset then 
restore it, and do the map_vq_notify call just the first time.  I'll fix 
that up for v2.

sln

> 
> Thanks
> 
>> +}
>> +
>>   static int pds_vdpa_reset(struct vdpa_device *vdpa_dev)
>>   {
>>          struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>> @@ -451,8 +463,7 @@ static int pds_vdpa_reset(struct vdpa_device *vdpa_dev)
>>                                  dev_err(dev, "%s: reset_vq failed qid %d: %pe\n",
>>                                          __func__, i, ERR_PTR(err));
>>                          pds_vdpa_release_irq(pdsv, i);
>> -                       memset(&pdsv->vqs[i], 0, sizeof(pdsv->vqs[0]));
>> -                       pdsv->vqs[i].ready = false;
>> +                       pds_vdpa_init_vqs_entry(pdsv, i);
>>                  }
>>          }
>>
>> @@ -640,13 +651,8 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>>          }
>>          pds_vdpa_cmd_set_mac(pdsv, pdsv->mac);
>>
>> -       for (i = 0; i < pdsv->num_vqs; i++) {
>> -               pdsv->vqs[i].qid = i;
>> -               pdsv->vqs[i].pdsv = pdsv;
>> -               pdsv->vqs[i].irq = VIRTIO_MSI_NO_VECTOR;
>> -               pdsv->vqs[i].notify = vp_modern_map_vq_notify(&pdsv->vdpa_aux->vd_mdev,
>> -                                                             i, &pdsv->vqs[i].notify_pa);
>> -       }
>> +       for (i = 0; i < pdsv->num_vqs; i++)
>> +               pds_vdpa_init_vqs_entry(pdsv, i);
>>
>>          pdsv->vdpa_dev.mdev = &vdpa_aux->vdpa_mdev;
>>
>> --
>> 2.17.1
>>
>
diff mbox series

Patch

diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
index 5e761d625ef3..5e1046c9af3d 100644
--- a/drivers/vdpa/pds/vdpa_dev.c
+++ b/drivers/vdpa/pds/vdpa_dev.c
@@ -429,6 +429,18 @@  static void pds_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
 	}
 }
 
+static void pds_vdpa_init_vqs_entry(struct pds_vdpa_device *pdsv, int qid)
+{
+	memset(&pdsv->vqs[qid], 0, sizeof(pdsv->vqs[0]));
+	pdsv->vqs[qid].qid = qid;
+	pdsv->vqs[qid].pdsv = pdsv;
+	pdsv->vqs[qid].ready = false;
+	pdsv->vqs[qid].irq = VIRTIO_MSI_NO_VECTOR;
+	pdsv->vqs[qid].notify =
+		vp_modern_map_vq_notify(&pdsv->vdpa_aux->vd_mdev,
+					qid, &pdsv->vqs[qid].notify_pa);
+}
+
 static int pds_vdpa_reset(struct vdpa_device *vdpa_dev)
 {
 	struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
@@ -451,8 +463,7 @@  static int pds_vdpa_reset(struct vdpa_device *vdpa_dev)
 				dev_err(dev, "%s: reset_vq failed qid %d: %pe\n",
 					__func__, i, ERR_PTR(err));
 			pds_vdpa_release_irq(pdsv, i);
-			memset(&pdsv->vqs[i], 0, sizeof(pdsv->vqs[0]));
-			pdsv->vqs[i].ready = false;
+			pds_vdpa_init_vqs_entry(pdsv, i);
 		}
 	}
 
@@ -640,13 +651,8 @@  static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
 	}
 	pds_vdpa_cmd_set_mac(pdsv, pdsv->mac);
 
-	for (i = 0; i < pdsv->num_vqs; i++) {
-		pdsv->vqs[i].qid = i;
-		pdsv->vqs[i].pdsv = pdsv;
-		pdsv->vqs[i].irq = VIRTIO_MSI_NO_VECTOR;
-		pdsv->vqs[i].notify = vp_modern_map_vq_notify(&pdsv->vdpa_aux->vd_mdev,
-							      i, &pdsv->vqs[i].notify_pa);
-	}
+	for (i = 0; i < pdsv->num_vqs; i++)
+		pds_vdpa_init_vqs_entry(pdsv, i);
 
 	pdsv->vdpa_dev.mdev = &vdpa_aux->vdpa_mdev;