Re: [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac

On 7/9/23 8:04 PM, Jason Wang wrote:

On Sat, Jul 8, 2023 at 4:12 AM Shannon Nelson [off-list ref] wrote:

quoted

On 7/7/23 12:33 AM, Jason Wang wrote:

quoted

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 [off-list ref] wrote:

quoted

From: Allen Hubbe <allen.hubbe@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@amd.com>
Signed-off-by: Shannon Nelson <redacted>
Reviewed-by: Brett Creeley <brett.creeley@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.

Yes, but pds_vdpa_set_status() will be called when status is not zero?

Yes, but the set_mac is only done with status==0, whether called by 
reset or called when some other thing calls set_status with status==0.

sln

Thanks

quoted

sln

quoted

The rest looks good.

Thanks

quoted

          }

          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