* [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac
2023-06-30 0:36 [PATCH virtio 0/4] pds_vdpa: mac, reset, and irq updates Shannon Nelson
@ 2023-06-30 0:36 ` Shannon Nelson
2023-07-07 7:33 ` Jason Wang
2023-06-30 0:36 ` [PATCH virtio 2/4] pds_vdpa: always allow offering VIRTIO_NET_F_MAC Shannon Nelson
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Shannon Nelson @ 2023-06-30 0:36 UTC (permalink / raw)
To: jasowang, mst, virtualization, shannon.nelson, brett.creeley,
netdev
Cc: drivers, Allen Hubbe
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);
}
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac
2023-06-30 0:36 ` [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac Shannon Nelson
@ 2023-07-07 7:33 ` Jason Wang
2023-07-07 20:12 ` Shannon Nelson
0 siblings, 1 reply; 14+ messages in thread
From: Jason Wang @ 2023-07-07 7:33 UTC (permalink / raw)
To: Shannon Nelson
Cc: mst, virtualization, brett.creeley, netdev, drivers, Allen Hubbe
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac
2023-07-07 7:33 ` Jason Wang
@ 2023-07-07 20:12 ` Shannon Nelson
2023-07-10 3:04 ` Jason Wang
0 siblings, 1 reply; 14+ messages in thread
From: Shannon Nelson @ 2023-07-07 20:12 UTC (permalink / raw)
To: Jason Wang
Cc: mst, virtualization, brett.creeley, netdev, drivers, Allen Hubbe
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
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac
2023-07-07 20:12 ` Shannon Nelson
@ 2023-07-10 3:04 ` Jason Wang
2023-07-10 15:42 ` Shannon Nelson
0 siblings, 1 reply; 14+ messages in thread
From: Jason Wang @ 2023-07-10 3:04 UTC (permalink / raw)
To: Shannon Nelson
Cc: mst, virtualization, brett.creeley, netdev, drivers, Allen Hubbe
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
> >>
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac
2023-07-10 3:04 ` Jason Wang
@ 2023-07-10 15:42 ` Shannon Nelson
0 siblings, 0 replies; 14+ messages in thread
From: Shannon Nelson @ 2023-07-10 15:42 UTC (permalink / raw)
To: Jason Wang
Cc: mst, virtualization, brett.creeley, netdev, drivers, Allen Hubbe
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
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH virtio 2/4] pds_vdpa: always allow offering VIRTIO_NET_F_MAC
2023-06-30 0:36 [PATCH virtio 0/4] pds_vdpa: mac, reset, and irq updates Shannon Nelson
2023-06-30 0:36 ` [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac Shannon Nelson
@ 2023-06-30 0:36 ` Shannon Nelson
2023-07-07 7:44 ` Jason Wang
2023-06-30 0:36 ` [PATCH virtio 3/4] pds_vdpa: clean and reset vqs entries Shannon Nelson
2023-06-30 0:36 ` [PATCH virtio 4/4] pds_vdpa: alloc irq vectors on DRIVER_OK Shannon Nelson
3 siblings, 1 reply; 14+ messages in thread
From: Shannon Nelson @ 2023-06-30 0:36 UTC (permalink / raw)
To: jasowang, mst, virtualization, shannon.nelson, brett.creeley,
netdev
Cc: drivers
Our driver sets a mac if the HW is 00:..:00 so we need to be sure to
advertise VIRTIO_NET_F_MAC even if the HW doesn't. We also need to be
sure that virtio_net sees the VIRTIO_NET_F_MAC and doesn't rewrite the
mac address that a user may have set with the vdpa utility.
After reading the hw_feature bits, add the VIRTIO_NET_F_MAC to the driver's
supported_features and use that for reporting what is available. If the
HW is not advertising it, be sure to strip the VIRTIO_NET_F_MAC before
finishing the feature negotiation. If the user specifies a device_features
bitpattern in the vdpa utility without the VIRTIO_NET_F_MAC set, then
don't set the mac.
Fixes: 151cc834f3dd ("pds_vdpa: add support for vdpa and vdpamgmt interfaces")
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
drivers/vdpa/pds/vdpa_dev.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
index e2e99bb0be2b..5e761d625ef3 100644
--- a/drivers/vdpa/pds/vdpa_dev.c
+++ b/drivers/vdpa/pds/vdpa_dev.c
@@ -316,6 +316,7 @@ static int pds_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 featur
{
struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
struct device *dev = &pdsv->vdpa_dev.dev;
+ u64 requested_features;
u64 driver_features;
u64 nego_features;
u64 missing;
@@ -325,18 +326,24 @@ static int pds_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 featur
return -EOPNOTSUPP;
}
+ /* save original request for debugfs */
pdsv->req_features = features;
+ requested_features = features;
+
+ /* if we're faking the F_MAC, strip it from features to be negotiated */
+ driver_features = pds_vdpa_get_driver_features(vdpa_dev);
+ if (!(driver_features & BIT_ULL(VIRTIO_NET_F_MAC)))
+ requested_features &= ~BIT_ULL(VIRTIO_NET_F_MAC);
/* Check for valid feature bits */
- nego_features = features & le64_to_cpu(pdsv->vdpa_aux->ident.hw_features);
- missing = pdsv->req_features & ~nego_features;
+ nego_features = requested_features & le64_to_cpu(pdsv->vdpa_aux->ident.hw_features);
+ missing = requested_features & ~nego_features;
if (missing) {
dev_err(dev, "Can't support all requested features in %#llx, missing %#llx features\n",
pdsv->req_features, missing);
return -EOPNOTSUPP;
}
- driver_features = pds_vdpa_get_driver_features(vdpa_dev);
dev_dbg(dev, "%s: %#llx => %#llx\n",
__func__, driver_features, nego_features);
@@ -564,7 +571,7 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
u64 unsupp_features =
- add_config->device_features & ~mgmt->supported_features;
+ add_config->device_features & ~pdsv->supported_features;
if (unsupp_features) {
dev_err(dev, "Unsupported features: %#llx\n", unsupp_features);
@@ -615,7 +622,8 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
}
/* Set a mac, either from the user config if provided
- * or set a random mac if default is 00:..:00
+ * or use the device's mac if not 00:..:00
+ * or set a random mac
*/
if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
ether_addr_copy(pdsv->mac, add_config->net.mac);
@@ -624,7 +632,8 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
vc = pdsv->vdpa_aux->vd_mdev.device;
memcpy_fromio(pdsv->mac, vc->mac, sizeof(pdsv->mac));
- if (is_zero_ether_addr(pdsv->mac)) {
+ if (is_zero_ether_addr(pdsv->mac) &&
+ (pdsv->supported_features & BIT_ULL(VIRTIO_NET_F_MAC))) {
eth_random_addr(pdsv->mac);
dev_info(dev, "setting random mac %pM\n", pdsv->mac);
}
@@ -752,6 +761,10 @@ int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux)
mgmt->id_table = pds_vdpa_id_table;
mgmt->device = dev;
mgmt->supported_features = le64_to_cpu(vdpa_aux->ident.hw_features);
+
+ /* advertise F_MAC even if the device doesn't */
+ mgmt->supported_features |= BIT_ULL(VIRTIO_NET_F_MAC);
+
mgmt->config_attr_mask = BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR);
mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP);
mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_FEATURES);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH virtio 2/4] pds_vdpa: always allow offering VIRTIO_NET_F_MAC
2023-06-30 0:36 ` [PATCH virtio 2/4] pds_vdpa: always allow offering VIRTIO_NET_F_MAC Shannon Nelson
@ 2023-07-07 7:44 ` Jason Wang
2023-07-07 20:12 ` Shannon Nelson
0 siblings, 1 reply; 14+ messages in thread
From: Jason Wang @ 2023-07-07 7:44 UTC (permalink / raw)
To: Shannon Nelson; +Cc: mst, virtualization, brett.creeley, netdev, drivers
On Fri, Jun 30, 2023 at 8:36 AM Shannon Nelson <shannon.nelson@amd.com> wrote:
>
> Our driver sets a mac if the HW is 00:..:00 so we need to be sure to
> advertise VIRTIO_NET_F_MAC even if the HW doesn't. We also need to be
> sure that virtio_net sees the VIRTIO_NET_F_MAC and doesn't rewrite the
> mac address that a user may have set with the vdpa utility.
>
> After reading the hw_feature bits, add the VIRTIO_NET_F_MAC to the driver's
> supported_features and use that for reporting what is available. If the
> HW is not advertising it, be sure to strip the VIRTIO_NET_F_MAC before
> finishing the feature negotiation. If the user specifies a device_features
> bitpattern in the vdpa utility without the VIRTIO_NET_F_MAC set, then
> don't set the mac.
>
> Fixes: 151cc834f3dd ("pds_vdpa: add support for vdpa and vdpamgmt interfaces")
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> Reviewed-by: Brett Creeley <brett.creeley@amd.com>
> ---
> drivers/vdpa/pds/vdpa_dev.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
> index e2e99bb0be2b..5e761d625ef3 100644
> --- a/drivers/vdpa/pds/vdpa_dev.c
> +++ b/drivers/vdpa/pds/vdpa_dev.c
> @@ -316,6 +316,7 @@ static int pds_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 featur
> {
> struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
> struct device *dev = &pdsv->vdpa_dev.dev;
> + u64 requested_features;
> u64 driver_features;
> u64 nego_features;
> u64 missing;
> @@ -325,18 +326,24 @@ static int pds_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 featur
> return -EOPNOTSUPP;
> }
>
> + /* save original request for debugfs */
> pdsv->req_features = features;
> + requested_features = features;
> +
> + /* if we're faking the F_MAC, strip it from features to be negotiated */
> + driver_features = pds_vdpa_get_driver_features(vdpa_dev);
> + if (!(driver_features & BIT_ULL(VIRTIO_NET_F_MAC)))
> + requested_features &= ~BIT_ULL(VIRTIO_NET_F_MAC);
I'm not sure I understand here, assuming we are doing feature
negotiation here. In this case driver_features we read should be zero?
Or did you actually mean device_features here?
Thanks
>
> /* Check for valid feature bits */
> - nego_features = features & le64_to_cpu(pdsv->vdpa_aux->ident.hw_features);
> - missing = pdsv->req_features & ~nego_features;
> + nego_features = requested_features & le64_to_cpu(pdsv->vdpa_aux->ident.hw_features);
> + missing = requested_features & ~nego_features;
> if (missing) {
> dev_err(dev, "Can't support all requested features in %#llx, missing %#llx features\n",
> pdsv->req_features, missing);
> return -EOPNOTSUPP;
> }
>
> - driver_features = pds_vdpa_get_driver_features(vdpa_dev);
> dev_dbg(dev, "%s: %#llx => %#llx\n",
> __func__, driver_features, nego_features);
>
> @@ -564,7 +571,7 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>
> if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> u64 unsupp_features =
> - add_config->device_features & ~mgmt->supported_features;
> + add_config->device_features & ~pdsv->supported_features;
>
> if (unsupp_features) {
> dev_err(dev, "Unsupported features: %#llx\n", unsupp_features);
> @@ -615,7 +622,8 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
> }
>
> /* Set a mac, either from the user config if provided
> - * or set a random mac if default is 00:..:00
> + * or use the device's mac if not 00:..:00
> + * or set a random mac
> */
> if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
> ether_addr_copy(pdsv->mac, add_config->net.mac);
> @@ -624,7 +632,8 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>
> vc = pdsv->vdpa_aux->vd_mdev.device;
> memcpy_fromio(pdsv->mac, vc->mac, sizeof(pdsv->mac));
> - if (is_zero_ether_addr(pdsv->mac)) {
> + if (is_zero_ether_addr(pdsv->mac) &&
> + (pdsv->supported_features & BIT_ULL(VIRTIO_NET_F_MAC))) {
> eth_random_addr(pdsv->mac);
> dev_info(dev, "setting random mac %pM\n", pdsv->mac);
> }
> @@ -752,6 +761,10 @@ int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux)
> mgmt->id_table = pds_vdpa_id_table;
> mgmt->device = dev;
> mgmt->supported_features = le64_to_cpu(vdpa_aux->ident.hw_features);
> +
> + /* advertise F_MAC even if the device doesn't */
> + mgmt->supported_features |= BIT_ULL(VIRTIO_NET_F_MAC);
> +
> mgmt->config_attr_mask = BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR);
> mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP);
> mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_FEATURES);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH virtio 2/4] pds_vdpa: always allow offering VIRTIO_NET_F_MAC
2023-07-07 7:44 ` Jason Wang
@ 2023-07-07 20:12 ` Shannon Nelson
0 siblings, 0 replies; 14+ messages in thread
From: Shannon Nelson @ 2023-07-07 20:12 UTC (permalink / raw)
To: Jason Wang; +Cc: mst, virtualization, brett.creeley, netdev, drivers
On 7/7/23 12:44 AM, Jason Wang wrote:
>
> On Fri, Jun 30, 2023 at 8:36 AM Shannon Nelson <shannon.nelson@amd.com> wrote:
>>
>> Our driver sets a mac if the HW is 00:..:00 so we need to be sure to
>> advertise VIRTIO_NET_F_MAC even if the HW doesn't. We also need to be
>> sure that virtio_net sees the VIRTIO_NET_F_MAC and doesn't rewrite the
>> mac address that a user may have set with the vdpa utility.
>>
>> After reading the hw_feature bits, add the VIRTIO_NET_F_MAC to the driver's
>> supported_features and use that for reporting what is available. If the
>> HW is not advertising it, be sure to strip the VIRTIO_NET_F_MAC before
>> finishing the feature negotiation. If the user specifies a device_features
>> bitpattern in the vdpa utility without the VIRTIO_NET_F_MAC set, then
>> don't set the mac.
>>
>> Fixes: 151cc834f3dd ("pds_vdpa: add support for vdpa and vdpamgmt interfaces")
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>> Reviewed-by: Brett Creeley <brett.creeley@amd.com>
>> ---
>> drivers/vdpa/pds/vdpa_dev.c | 25 +++++++++++++++++++------
>> 1 file changed, 19 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
>> index e2e99bb0be2b..5e761d625ef3 100644
>> --- a/drivers/vdpa/pds/vdpa_dev.c
>> +++ b/drivers/vdpa/pds/vdpa_dev.c
>> @@ -316,6 +316,7 @@ static int pds_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 featur
>> {
>> struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>> struct device *dev = &pdsv->vdpa_dev.dev;
>> + u64 requested_features;
>> u64 driver_features;
>> u64 nego_features;
>> u64 missing;
>> @@ -325,18 +326,24 @@ static int pds_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 featur
>> return -EOPNOTSUPP;
>> }
>>
>> + /* save original request for debugfs */
>> pdsv->req_features = features;
>> + requested_features = features;
>> +
>> + /* if we're faking the F_MAC, strip it from features to be negotiated */
>> + driver_features = pds_vdpa_get_driver_features(vdpa_dev);
>> + if (!(driver_features & BIT_ULL(VIRTIO_NET_F_MAC)))
>> + requested_features &= ~BIT_ULL(VIRTIO_NET_F_MAC);
>
> I'm not sure I understand here, assuming we are doing feature
> negotiation here. In this case driver_features we read should be zero?
> Or did you actually mean device_features here?
Yes, this needs to be cleared up. I'll address it in v2.
sln
>
> Thanks
>
>>
>> /* Check for valid feature bits */
>> - nego_features = features & le64_to_cpu(pdsv->vdpa_aux->ident.hw_features);
>> - missing = pdsv->req_features & ~nego_features;
>> + nego_features = requested_features & le64_to_cpu(pdsv->vdpa_aux->ident.hw_features);
>> + missing = requested_features & ~nego_features;
>> if (missing) {
>> dev_err(dev, "Can't support all requested features in %#llx, missing %#llx features\n",
>> pdsv->req_features, missing);
>> return -EOPNOTSUPP;
>> }
>>
>> - driver_features = pds_vdpa_get_driver_features(vdpa_dev);
>> dev_dbg(dev, "%s: %#llx => %#llx\n",
>> __func__, driver_features, nego_features);
>>
>> @@ -564,7 +571,7 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>>
>> if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
>> u64 unsupp_features =
>> - add_config->device_features & ~mgmt->supported_features;
>> + add_config->device_features & ~pdsv->supported_features;
>>
>> if (unsupp_features) {
>> dev_err(dev, "Unsupported features: %#llx\n", unsupp_features);
>> @@ -615,7 +622,8 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>> }
>>
>> /* Set a mac, either from the user config if provided
>> - * or set a random mac if default is 00:..:00
>> + * or use the device's mac if not 00:..:00
>> + * or set a random mac
>> */
>> if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
>> ether_addr_copy(pdsv->mac, add_config->net.mac);
>> @@ -624,7 +632,8 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>>
>> vc = pdsv->vdpa_aux->vd_mdev.device;
>> memcpy_fromio(pdsv->mac, vc->mac, sizeof(pdsv->mac));
>> - if (is_zero_ether_addr(pdsv->mac)) {
>> + if (is_zero_ether_addr(pdsv->mac) &&
>> + (pdsv->supported_features & BIT_ULL(VIRTIO_NET_F_MAC))) {
>> eth_random_addr(pdsv->mac);
>> dev_info(dev, "setting random mac %pM\n", pdsv->mac);
>> }
>> @@ -752,6 +761,10 @@ int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux)
>> mgmt->id_table = pds_vdpa_id_table;
>> mgmt->device = dev;
>> mgmt->supported_features = le64_to_cpu(vdpa_aux->ident.hw_features);
>> +
>> + /* advertise F_MAC even if the device doesn't */
>> + mgmt->supported_features |= BIT_ULL(VIRTIO_NET_F_MAC);
>> +
>> mgmt->config_attr_mask = BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR);
>> mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP);
>> mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_FEATURES);
>> --
>> 2.17.1
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH virtio 3/4] pds_vdpa: clean and reset vqs entries
2023-06-30 0:36 [PATCH virtio 0/4] pds_vdpa: mac, reset, and irq updates Shannon Nelson
2023-06-30 0:36 ` [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac Shannon Nelson
2023-06-30 0:36 ` [PATCH virtio 2/4] pds_vdpa: always allow offering VIRTIO_NET_F_MAC Shannon Nelson
@ 2023-06-30 0:36 ` Shannon Nelson
2023-07-07 7:36 ` Jason Wang
2023-06-30 0:36 ` [PATCH virtio 4/4] pds_vdpa: alloc irq vectors on DRIVER_OK Shannon Nelson
3 siblings, 1 reply; 14+ messages in thread
From: Shannon Nelson @ 2023-06-30 0:36 UTC (permalink / raw)
To: jasowang, mst, virtualization, shannon.nelson, brett.creeley,
netdev
Cc: drivers
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@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.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);
+}
+
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH virtio 3/4] pds_vdpa: clean and reset vqs entries
2023-06-30 0:36 ` [PATCH virtio 3/4] pds_vdpa: clean and reset vqs entries Shannon Nelson
@ 2023-07-07 7:36 ` Jason Wang
2023-07-07 20:12 ` Shannon Nelson
0 siblings, 1 reply; 14+ messages in thread
From: Jason Wang @ 2023-07-07 7:36 UTC (permalink / raw)
To: Shannon Nelson; +Cc: mst, virtualization, brett.creeley, netdev, drivers
On Fri, Jun 30, 2023 at 8:36 AM Shannon Nelson <shannon.nelson@amd.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@amd.com>
> Reviewed-by: Brett Creeley <brett.creeley@amd.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?
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH virtio 3/4] pds_vdpa: clean and reset vqs entries
2023-07-07 7:36 ` Jason Wang
@ 2023-07-07 20:12 ` Shannon Nelson
0 siblings, 0 replies; 14+ messages in thread
From: Shannon Nelson @ 2023-07-07 20:12 UTC (permalink / raw)
To: Jason Wang; +Cc: mst, virtualization, brett.creeley, netdev, drivers
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@amd.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@amd.com>
>> Reviewed-by: Brett Creeley <brett.creeley@amd.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
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH virtio 4/4] pds_vdpa: alloc irq vectors on DRIVER_OK
2023-06-30 0:36 [PATCH virtio 0/4] pds_vdpa: mac, reset, and irq updates Shannon Nelson
` (2 preceding siblings ...)
2023-06-30 0:36 ` [PATCH virtio 3/4] pds_vdpa: clean and reset vqs entries Shannon Nelson
@ 2023-06-30 0:36 ` Shannon Nelson
2023-07-07 7:38 ` Jason Wang
3 siblings, 1 reply; 14+ messages in thread
From: Shannon Nelson @ 2023-06-30 0:36 UTC (permalink / raw)
To: jasowang, mst, virtualization, shannon.nelson, brett.creeley,
netdev
Cc: drivers, Allen Hubbe
From: Allen Hubbe <allen.hubbe@amd.com>
We were allocating irq vectors at the time the aux dev was probed,
but that is before the PCI VF is assigned to a separate iommu domain
by vhost_vdpa. Because vhost_vdpa later changes the iommu domain the
interrupts do not work.
Instead, we can allocate the irq vectors later when we see DRIVER_OK and
know that the reassignment of the PCI VF to an iommu domain has already
happened.
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 | 110 ++++++++++++++++++++++++++----------
1 file changed, 81 insertions(+), 29 deletions(-)
diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
index 5e1046c9af3d..6c337f7a0f06 100644
--- a/drivers/vdpa/pds/vdpa_dev.c
+++ b/drivers/vdpa/pds/vdpa_dev.c
@@ -126,11 +126,9 @@ static void pds_vdpa_release_irq(struct pds_vdpa_device *pdsv, int qid)
static void pds_vdpa_set_vq_ready(struct vdpa_device *vdpa_dev, u16 qid, bool ready)
{
struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
- struct pci_dev *pdev = pdsv->vdpa_aux->padev->vf_pdev;
struct device *dev = &pdsv->vdpa_dev.dev;
u64 driver_features;
u16 invert_idx = 0;
- int irq;
int err;
dev_dbg(dev, "%s: qid %d ready %d => %d\n",
@@ -143,19 +141,6 @@ static void pds_vdpa_set_vq_ready(struct vdpa_device *vdpa_dev, u16 qid, bool re
invert_idx = PDS_VDPA_PACKED_INVERT_IDX;
if (ready) {
- irq = pci_irq_vector(pdev, qid);
- snprintf(pdsv->vqs[qid].irq_name, sizeof(pdsv->vqs[qid].irq_name),
- "vdpa-%s-%d", dev_name(dev), qid);
-
- err = request_irq(irq, pds_vdpa_isr, 0,
- pdsv->vqs[qid].irq_name, &pdsv->vqs[qid]);
- if (err) {
- dev_err(dev, "%s: no irq for qid %d: %pe\n",
- __func__, qid, ERR_PTR(err));
- return;
- }
- pdsv->vqs[qid].irq = irq;
-
/* Pass vq setup info to DSC using adminq to gather up and
* send all info at once so FW can do its full set up in
* one easy operation
@@ -164,7 +149,6 @@ static void pds_vdpa_set_vq_ready(struct vdpa_device *vdpa_dev, u16 qid, bool re
if (err) {
dev_err(dev, "Failed to init vq %d: %pe\n",
qid, ERR_PTR(err));
- pds_vdpa_release_irq(pdsv, qid);
ready = false;
}
} else {
@@ -172,7 +156,6 @@ static void pds_vdpa_set_vq_ready(struct vdpa_device *vdpa_dev, u16 qid, bool re
if (err)
dev_err(dev, "%s: reset_vq failed qid %d: %pe\n",
__func__, qid, ERR_PTR(err));
- pds_vdpa_release_irq(pdsv, qid);
}
pdsv->vqs[qid].ready = ready;
@@ -396,6 +379,72 @@ static u8 pds_vdpa_get_status(struct vdpa_device *vdpa_dev)
return vp_modern_get_status(&pdsv->vdpa_aux->vd_mdev);
}
+static int pds_vdpa_request_irqs(struct pds_vdpa_device *pdsv)
+{
+ struct pci_dev *pdev = pdsv->vdpa_aux->padev->vf_pdev;
+ struct pds_vdpa_aux *vdpa_aux = pdsv->vdpa_aux;
+ struct device *dev = &pdsv->vdpa_dev.dev;
+ int max_vq, nintrs, qid, err;
+
+ max_vq = vdpa_aux->vdpa_mdev.max_supported_vqs;
+
+ nintrs = pci_alloc_irq_vectors(pdev, max_vq, max_vq, PCI_IRQ_MSIX);
+ if (nintrs < 0) {
+ dev_err(dev, "Couldn't get %d msix vectors: %pe\n",
+ max_vq, ERR_PTR(nintrs));
+ return nintrs;
+ }
+
+ for (qid = 0; qid < pdsv->num_vqs; ++qid) {
+ int irq = pci_irq_vector(pdev, qid);
+
+ snprintf(pdsv->vqs[qid].irq_name, sizeof(pdsv->vqs[qid].irq_name),
+ "vdpa-%s-%d", dev_name(dev), qid);
+
+ err = request_irq(irq, pds_vdpa_isr, 0,
+ pdsv->vqs[qid].irq_name,
+ &pdsv->vqs[qid]);
+ if (err) {
+ dev_err(dev, "%s: no irq for qid %d: %pe\n",
+ __func__, qid, ERR_PTR(err));
+ goto err_release;
+ }
+
+ pdsv->vqs[qid].irq = irq;
+ }
+
+ vdpa_aux->nintrs = nintrs;
+
+ return 0;
+
+err_release:
+ while (qid--)
+ pds_vdpa_release_irq(pdsv, qid);
+
+ pci_free_irq_vectors(pdev);
+
+ vdpa_aux->nintrs = 0;
+
+ return err;
+}
+
+static void pds_vdpa_release_irqs(struct pds_vdpa_device *pdsv)
+{
+ struct pci_dev *pdev = pdsv->vdpa_aux->padev->vf_pdev;
+ struct pds_vdpa_aux *vdpa_aux = pdsv->vdpa_aux;
+ int qid;
+
+ if (!vdpa_aux->nintrs)
+ return;
+
+ for (qid = 0; qid < pdsv->num_vqs; qid++)
+ pds_vdpa_release_irq(pdsv, qid);
+
+ pci_free_irq_vectors(pdev);
+
+ vdpa_aux->nintrs = 0;
+}
+
static void pds_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
{
struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
@@ -406,6 +455,11 @@ static void pds_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
old_status = pds_vdpa_get_status(vdpa_dev);
dev_dbg(dev, "%s: old %#x new %#x\n", __func__, old_status, status);
+ if (status & ~old_status & VIRTIO_CONFIG_S_DRIVER_OK) {
+ if (pds_vdpa_request_irqs(pdsv))
+ status = old_status | VIRTIO_CONFIG_S_FAILED;
+ }
+
pds_vdpa_cmd_set_status(pdsv, status);
/* Note: still working with FW on the need for this reset cmd */
@@ -427,6 +481,9 @@ static void pds_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
i, &pdsv->vqs[i].notify_pa);
}
}
+
+ if (old_status & ~status & VIRTIO_CONFIG_S_DRIVER_OK)
+ pds_vdpa_release_irqs(pdsv);
}
static void pds_vdpa_init_vqs_entry(struct pds_vdpa_device *pdsv, int qid)
@@ -462,13 +519,17 @@ static int pds_vdpa_reset(struct vdpa_device *vdpa_dev)
if (err)
dev_err(dev, "%s: reset_vq failed qid %d: %pe\n",
__func__, i, ERR_PTR(err));
- pds_vdpa_release_irq(pdsv, i);
- pds_vdpa_init_vqs_entry(pdsv, i);
}
}
pds_vdpa_set_status(vdpa_dev, 0);
+ if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
+ /* Reset the vq info */
+ for (i = 0; i < pdsv->num_vqs && !err; i++)
+ pds_vdpa_init_vqs_entry(pdsv, i);
+ }
+
return 0;
}
@@ -761,7 +822,7 @@ int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux)
max_vqs = min_t(u16, dev_intrs, max_vqs);
mgmt->max_supported_vqs = min_t(u16, PDS_VDPA_MAX_QUEUES, max_vqs);
- vdpa_aux->nintrs = mgmt->max_supported_vqs;
+ vdpa_aux->nintrs = 0;
mgmt->ops = &pds_vdpa_mgmt_dev_ops;
mgmt->id_table = pds_vdpa_id_table;
@@ -775,14 +836,5 @@ int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux)
mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP);
mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_FEATURES);
- err = pci_alloc_irq_vectors(pdev, vdpa_aux->nintrs, vdpa_aux->nintrs,
- PCI_IRQ_MSIX);
- if (err < 0) {
- dev_err(dev, "Couldn't get %d msix vectors: %pe\n",
- vdpa_aux->nintrs, ERR_PTR(err));
- return err;
- }
- vdpa_aux->nintrs = err;
-
return 0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH virtio 4/4] pds_vdpa: alloc irq vectors on DRIVER_OK
2023-06-30 0:36 ` [PATCH virtio 4/4] pds_vdpa: alloc irq vectors on DRIVER_OK Shannon Nelson
@ 2023-07-07 7:38 ` Jason Wang
0 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2023-07-07 7:38 UTC (permalink / raw)
To: Shannon Nelson
Cc: mst, virtualization, brett.creeley, netdev, drivers, Allen Hubbe
On Fri, Jun 30, 2023 at 8:36 AM Shannon Nelson <shannon.nelson@amd.com> wrote:
>
> From: Allen Hubbe <allen.hubbe@amd.com>
>
> We were allocating irq vectors at the time the aux dev was probed,
> but that is before the PCI VF is assigned to a separate iommu domain
> by vhost_vdpa. Because vhost_vdpa later changes the iommu domain the
> interrupts do not work.
>
> Instead, we can allocate the irq vectors later when we see DRIVER_OK and
> know that the reassignment of the PCI VF to an iommu domain has already
> happened.
>
> 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>
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
> ---
> drivers/vdpa/pds/vdpa_dev.c | 110 ++++++++++++++++++++++++++----------
> 1 file changed, 81 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
> index 5e1046c9af3d..6c337f7a0f06 100644
> --- a/drivers/vdpa/pds/vdpa_dev.c
> +++ b/drivers/vdpa/pds/vdpa_dev.c
> @@ -126,11 +126,9 @@ static void pds_vdpa_release_irq(struct pds_vdpa_device *pdsv, int qid)
> static void pds_vdpa_set_vq_ready(struct vdpa_device *vdpa_dev, u16 qid, bool ready)
> {
> struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
> - struct pci_dev *pdev = pdsv->vdpa_aux->padev->vf_pdev;
> struct device *dev = &pdsv->vdpa_dev.dev;
> u64 driver_features;
> u16 invert_idx = 0;
> - int irq;
> int err;
>
> dev_dbg(dev, "%s: qid %d ready %d => %d\n",
> @@ -143,19 +141,6 @@ static void pds_vdpa_set_vq_ready(struct vdpa_device *vdpa_dev, u16 qid, bool re
> invert_idx = PDS_VDPA_PACKED_INVERT_IDX;
>
> if (ready) {
> - irq = pci_irq_vector(pdev, qid);
> - snprintf(pdsv->vqs[qid].irq_name, sizeof(pdsv->vqs[qid].irq_name),
> - "vdpa-%s-%d", dev_name(dev), qid);
> -
> - err = request_irq(irq, pds_vdpa_isr, 0,
> - pdsv->vqs[qid].irq_name, &pdsv->vqs[qid]);
> - if (err) {
> - dev_err(dev, "%s: no irq for qid %d: %pe\n",
> - __func__, qid, ERR_PTR(err));
> - return;
> - }
> - pdsv->vqs[qid].irq = irq;
> -
> /* Pass vq setup info to DSC using adminq to gather up and
> * send all info at once so FW can do its full set up in
> * one easy operation
> @@ -164,7 +149,6 @@ static void pds_vdpa_set_vq_ready(struct vdpa_device *vdpa_dev, u16 qid, bool re
> if (err) {
> dev_err(dev, "Failed to init vq %d: %pe\n",
> qid, ERR_PTR(err));
> - pds_vdpa_release_irq(pdsv, qid);
> ready = false;
> }
> } else {
> @@ -172,7 +156,6 @@ static void pds_vdpa_set_vq_ready(struct vdpa_device *vdpa_dev, u16 qid, bool re
> if (err)
> dev_err(dev, "%s: reset_vq failed qid %d: %pe\n",
> __func__, qid, ERR_PTR(err));
> - pds_vdpa_release_irq(pdsv, qid);
> }
>
> pdsv->vqs[qid].ready = ready;
> @@ -396,6 +379,72 @@ static u8 pds_vdpa_get_status(struct vdpa_device *vdpa_dev)
> return vp_modern_get_status(&pdsv->vdpa_aux->vd_mdev);
> }
>
> +static int pds_vdpa_request_irqs(struct pds_vdpa_device *pdsv)
> +{
> + struct pci_dev *pdev = pdsv->vdpa_aux->padev->vf_pdev;
> + struct pds_vdpa_aux *vdpa_aux = pdsv->vdpa_aux;
> + struct device *dev = &pdsv->vdpa_dev.dev;
> + int max_vq, nintrs, qid, err;
> +
> + max_vq = vdpa_aux->vdpa_mdev.max_supported_vqs;
> +
> + nintrs = pci_alloc_irq_vectors(pdev, max_vq, max_vq, PCI_IRQ_MSIX);
> + if (nintrs < 0) {
> + dev_err(dev, "Couldn't get %d msix vectors: %pe\n",
> + max_vq, ERR_PTR(nintrs));
> + return nintrs;
> + }
> +
> + for (qid = 0; qid < pdsv->num_vqs; ++qid) {
> + int irq = pci_irq_vector(pdev, qid);
> +
> + snprintf(pdsv->vqs[qid].irq_name, sizeof(pdsv->vqs[qid].irq_name),
> + "vdpa-%s-%d", dev_name(dev), qid);
> +
> + err = request_irq(irq, pds_vdpa_isr, 0,
> + pdsv->vqs[qid].irq_name,
> + &pdsv->vqs[qid]);
> + if (err) {
> + dev_err(dev, "%s: no irq for qid %d: %pe\n",
> + __func__, qid, ERR_PTR(err));
> + goto err_release;
> + }
> +
> + pdsv->vqs[qid].irq = irq;
> + }
> +
> + vdpa_aux->nintrs = nintrs;
> +
> + return 0;
> +
> +err_release:
> + while (qid--)
> + pds_vdpa_release_irq(pdsv, qid);
> +
> + pci_free_irq_vectors(pdev);
> +
> + vdpa_aux->nintrs = 0;
> +
> + return err;
> +}
> +
> +static void pds_vdpa_release_irqs(struct pds_vdpa_device *pdsv)
> +{
> + struct pci_dev *pdev = pdsv->vdpa_aux->padev->vf_pdev;
> + struct pds_vdpa_aux *vdpa_aux = pdsv->vdpa_aux;
> + int qid;
> +
> + if (!vdpa_aux->nintrs)
> + return;
> +
> + for (qid = 0; qid < pdsv->num_vqs; qid++)
> + pds_vdpa_release_irq(pdsv, qid);
> +
> + pci_free_irq_vectors(pdev);
> +
> + vdpa_aux->nintrs = 0;
> +}
> +
> static void pds_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
> {
> struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
> @@ -406,6 +455,11 @@ static void pds_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
> old_status = pds_vdpa_get_status(vdpa_dev);
> dev_dbg(dev, "%s: old %#x new %#x\n", __func__, old_status, status);
>
> + if (status & ~old_status & VIRTIO_CONFIG_S_DRIVER_OK) {
> + if (pds_vdpa_request_irqs(pdsv))
> + status = old_status | VIRTIO_CONFIG_S_FAILED;
> + }
> +
> pds_vdpa_cmd_set_status(pdsv, status);
>
> /* Note: still working with FW on the need for this reset cmd */
> @@ -427,6 +481,9 @@ static void pds_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
> i, &pdsv->vqs[i].notify_pa);
> }
> }
> +
> + if (old_status & ~status & VIRTIO_CONFIG_S_DRIVER_OK)
> + pds_vdpa_release_irqs(pdsv);
> }
>
> static void pds_vdpa_init_vqs_entry(struct pds_vdpa_device *pdsv, int qid)
> @@ -462,13 +519,17 @@ static int pds_vdpa_reset(struct vdpa_device *vdpa_dev)
> if (err)
> dev_err(dev, "%s: reset_vq failed qid %d: %pe\n",
> __func__, i, ERR_PTR(err));
> - pds_vdpa_release_irq(pdsv, i);
> - pds_vdpa_init_vqs_entry(pdsv, i);
> }
> }
>
> pds_vdpa_set_status(vdpa_dev, 0);
>
> + if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
> + /* Reset the vq info */
> + for (i = 0; i < pdsv->num_vqs && !err; i++)
> + pds_vdpa_init_vqs_entry(pdsv, i);
> + }
> +
> return 0;
> }
>
> @@ -761,7 +822,7 @@ int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux)
>
> max_vqs = min_t(u16, dev_intrs, max_vqs);
> mgmt->max_supported_vqs = min_t(u16, PDS_VDPA_MAX_QUEUES, max_vqs);
> - vdpa_aux->nintrs = mgmt->max_supported_vqs;
> + vdpa_aux->nintrs = 0;
>
> mgmt->ops = &pds_vdpa_mgmt_dev_ops;
> mgmt->id_table = pds_vdpa_id_table;
> @@ -775,14 +836,5 @@ int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux)
> mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP);
> mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_FEATURES);
>
> - err = pci_alloc_irq_vectors(pdev, vdpa_aux->nintrs, vdpa_aux->nintrs,
> - PCI_IRQ_MSIX);
> - if (err < 0) {
> - dev_err(dev, "Couldn't get %d msix vectors: %pe\n",
> - vdpa_aux->nintrs, ERR_PTR(err));
> - return err;
> - }
> - vdpa_aux->nintrs = err;
> -
> return 0;
> }
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread