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

[Shannon Nelson <shannon.nelson@amd.com>]

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