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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4345f212-c919-d2fe-b47f-919629ef69b0@amd.com>
Date: Fri, 7 Jul 2023 13:12:54 -0700
From: Shannon Nelson <shannon.nelson@....com>
To: Jason Wang <jasowang@...hat.com>
Cc: mst@...hat.com, virtualization@...ts.linux-foundation.org,
 brett.creeley@....com, netdev@...r.kernel.org, drivers@...sando.io
Subject: Re: [PATCH virtio 3/4] pds_vdpa: clean and reset vqs entries



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@....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@....com>
>> Reviewed-by: Brett Creeley <brett.creeley@....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
>>
>