| Message ID | 20230630003609.28527-2-shannon.nelson@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pds_vdpa: mac, reset, and irq updates | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Fri, Jun 30, 2023 at 8:36 AM Shannon Nelson <shannon.nelson@amd.com> wrote: > > 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 <shannon.nelson@amd.com> > 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()? 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@amd.com> wrote: >> >> 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 <shannon.nelson@amd.com> >> 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. 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@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@amd.com> wrote: > >> > >> 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 <shannon.nelson@amd.com> > >> 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? 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 > >> > > >
On 7/9/23 8:04 PM, Jason Wang wrote: > > On Sat, Jul 8, 2023 at 4:12 AM Shannon Nelson <shannon.nelson@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@amd.com> wrote: >>>> >>>> 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 <shannon.nelson@amd.com> >>>> 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 > >> >> 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 >>>> >>> >> >
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); } 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; };