Virtualization - Jul 2023 - [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac

On Fri, Jun 30, 2023 at 8:36?AM Shannon Nelson <shannon.nelson at amd.com>
wrote:
>
> From: Allen Hubbe <allen.hubbe at amd.com>
>
> When the vdpa device is reset, also reinitialize it with the mac address
> that was assigned when the device was added.
>
> Fixes: 151cc834f3dd ("pds_vdpa: add support for vdpa and vdpamgmt
interfaces")
> Signed-off-by: Allen Hubbe <allen.hubbe at amd.com>
> Signed-off-by: Shannon Nelson <shannon.nelson at amd.com>
> Reviewed-by: Brett Creeley <brett.creeley at amd.com>
> ---
>  drivers/vdpa/pds/vdpa_dev.c | 16 ++++++++--------
>  drivers/vdpa/pds/vdpa_dev.h |  1 +
>  2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
> index 5071a4d58f8d..e2e99bb0be2b 100644
> --- a/drivers/vdpa/pds/vdpa_dev.c
> +++ b/drivers/vdpa/pds/vdpa_dev.c
> @@ -409,6 +409,8 @@ static void pds_vdpa_set_status(struct vdpa_device
*vdpa_dev, u8 status)
>                         pdsv->vqs[i].avail_idx = 0;
>                         pdsv->vqs[i].used_idx = 0;
>                 }
> +
> +               pds_vdpa_cmd_set_mac(pdsv, pdsv->mac);
So this is not necessarily called during reset. So I think we need to
move it to pds_vdpa_reset()?

The rest looks good.

Thanks
>         }
>
>         if (status & ~old_status & VIRTIO_CONFIG_S_FEATURES_OK) {
> @@ -532,7 +534,6 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev,
const char *name,
>         struct device *dma_dev;
>         struct pci_dev *pdev;
>         struct device *dev;
> -       u8 mac[ETH_ALEN];
>         int err;
>         int i;
>
> @@ -617,19 +618,18 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev
*mdev, const char *name,
>          * or set a random mac if default is 00:..:00
>          */
>         if (add_config->mask &
BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
> -               ether_addr_copy(mac, add_config->net.mac);
> -               pds_vdpa_cmd_set_mac(pdsv, mac);
> +               ether_addr_copy(pdsv->mac, add_config->net.mac);
>         } else {
>                 struct virtio_net_config __iomem *vc;
>
>                 vc = pdsv->vdpa_aux->vd_mdev.device;
> -               memcpy_fromio(mac, vc->mac, sizeof(mac));
> -               if (is_zero_ether_addr(mac)) {
> -                       eth_random_addr(mac);
> -                       dev_info(dev, "setting random mac %pM\n",
mac);
> -                       pds_vdpa_cmd_set_mac(pdsv, mac);
> +               memcpy_fromio(pdsv->mac, vc->mac,
sizeof(pdsv->mac));
> +               if (is_zero_ether_addr(pdsv->mac)) {
> +                       eth_random_addr(pdsv->mac);
> +                       dev_info(dev, "setting random mac %pM\n",
pdsv->mac);
>                 }
>         }
> +       pds_vdpa_cmd_set_mac(pdsv, pdsv->mac);
>
>         for (i = 0; i < pdsv->num_vqs; i++) {
>                 pdsv->vqs[i].qid = i;
> diff --git a/drivers/vdpa/pds/vdpa_dev.h b/drivers/vdpa/pds/vdpa_dev.h
> index a1bc37de9537..cf02df287fc4 100644
> --- a/drivers/vdpa/pds/vdpa_dev.h
> +++ b/drivers/vdpa/pds/vdpa_dev.h
> @@ -39,6 +39,7 @@ struct pds_vdpa_device {
>         u64 req_features;               /* features requested by vdpa */
>         u8 vdpa_index;                  /* rsvd for future subdevice use */
>         u8 num_vqs;                     /* num vqs in use */
> +       u8 mac[ETH_ALEN];               /* mac selected when the device was
added */
>         struct vdpa_callback config_cb;
>         struct notifier_block nb;
>  };
> --
> 2.17.1
>

On 7/7/23 12:33 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 at
amd.com> wrote:
>>
>> From: Allen Hubbe <allen.hubbe at amd.com>
>>
>> When the vdpa device is reset, also reinitialize it with the mac
address
>> that was assigned when the device was added.
>>
>> Fixes: 151cc834f3dd ("pds_vdpa: add support for vdpa and vdpamgmt
interfaces")
>> Signed-off-by: Allen Hubbe <allen.hubbe at amd.com>
>> Signed-off-by: Shannon Nelson <shannon.nelson at amd.com>
>> Reviewed-by: Brett Creeley <brett.creeley at amd.com>
>> ---
>>   drivers/vdpa/pds/vdpa_dev.c | 16 ++++++++--------
>>   drivers/vdpa/pds/vdpa_dev.h |  1 +
>>   2 files changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
>> index 5071a4d58f8d..e2e99bb0be2b 100644
>> --- a/drivers/vdpa/pds/vdpa_dev.c
>> +++ b/drivers/vdpa/pds/vdpa_dev.c
>> @@ -409,6 +409,8 @@ static void pds_vdpa_set_status(struct vdpa_device
*vdpa_dev, u8 status)
>>                          pdsv->vqs[i].avail_idx = 0;
>>                          pdsv->vqs[i].used_idx = 0;
>>                  }
>> +
>> +               pds_vdpa_cmd_set_mac(pdsv, pdsv->mac);
> 
> So this is not necessarily called during reset. So I think we need to
> move it to pds_vdpa_reset()?
pds_vdpa_reset() calls pds_vdpa_set_status() with a status=0, so this is 
already covered.

sln
> 
> The rest looks good.
> 
> Thanks
> 
>>          }
>>
>>          if (status & ~old_status &
VIRTIO_CONFIG_S_FEATURES_OK) {
>> @@ -532,7 +534,6 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev
*mdev, const char *name,
>>          struct device *dma_dev;
>>          struct pci_dev *pdev;
>>          struct device *dev;
>> -       u8 mac[ETH_ALEN];
>>          int err;
>>          int i;
>>
>> @@ -617,19 +618,18 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev
*mdev, const char *name,
>>           * or set a random mac if default is 00:..:00
>>           */
>>          if (add_config->mask &
BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
>> -               ether_addr_copy(mac, add_config->net.mac);
>> -               pds_vdpa_cmd_set_mac(pdsv, mac);
>> +               ether_addr_copy(pdsv->mac, add_config->net.mac);
>>          } else {
>>                  struct virtio_net_config __iomem *vc;
>>
>>                  vc = pdsv->vdpa_aux->vd_mdev.device;
>> -               memcpy_fromio(mac, vc->mac, sizeof(mac));
>> -               if (is_zero_ether_addr(mac)) {
>> -                       eth_random_addr(mac);
>> -                       dev_info(dev, "setting random mac
%pM\n", mac);
>> -                       pds_vdpa_cmd_set_mac(pdsv, mac);
>> +               memcpy_fromio(pdsv->mac, vc->mac,
sizeof(pdsv->mac));
>> +               if (is_zero_ether_addr(pdsv->mac)) {
>> +                       eth_random_addr(pdsv->mac);
>> +                       dev_info(dev, "setting random mac
%pM\n", pdsv->mac);
>>                  }
>>          }
>> +       pds_vdpa_cmd_set_mac(pdsv, pdsv->mac);
>>
>>          for (i = 0; i < pdsv->num_vqs; i++) {
>>                  pdsv->vqs[i].qid = i;
>> diff --git a/drivers/vdpa/pds/vdpa_dev.h b/drivers/vdpa/pds/vdpa_dev.h
>> index a1bc37de9537..cf02df287fc4 100644
>> --- a/drivers/vdpa/pds/vdpa_dev.h
>> +++ b/drivers/vdpa/pds/vdpa_dev.h
>> @@ -39,6 +39,7 @@ struct pds_vdpa_device {
>>          u64 req_features;               /* features requested by vdpa
*/
>>          u8 vdpa_index;                  /* rsvd for future subdevice
use */
>>          u8 num_vqs;                     /* num vqs in use */
>> +       u8 mac[ETH_ALEN];               /* mac selected when the device
was added */
>>          struct vdpa_callback config_cb;
>>          struct notifier_block nb;
>>   };
>> --
>> 2.17.1
>>
>