Shannon Nelson
2023-Jul-07 20:12 UTC
[PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac
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 >> >
On Sat, Jul 8, 2023 at 4:12?AM Shannon Nelson <shannon.nelson at amd.com> wrote:> > > > 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.Yes, but pds_vdpa_set_status() will be called when status is not zero? Thanks> > 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 > >> > > >