| Message ID | 20221207145428.31544-6-gautam.dawar@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | sfc: add vDPA support for EF100 devices | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 1 this patch: 1 |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/build_clang | success | Errors and warnings before: 2 this patch: 2 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 197 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Wed, Dec 7, 2022 at 10:56 PM Gautam Dawar <gautam.dawar@amd.com> wrote: > > vDPA config operations can be broadly categorized in to either > virtqueue operations, device operations or DMA operations. > This patch implements most of the device level config operations. > > SN1022 supports VIRTIO_F_IN_ORDER which is supported by the DPDK > virtio driver but not the kernel virtio driver. Due to a bug in > QEMU (https://gitlab.com/qemu-project/qemu/-/issues/331#), with > vhost-vdpa, this feature bit is returned with guest kernel virtio > driver in set_features config operation. The fix for this bug > (qemu_commit c33f23a419f95da16ab4faaf08be635c89b96ff0) is available > in QEMU versions 6.1.0 and later. Hence, that's the oldest QEMU > version required for testing with the vhost-vdpa driver. > > With older QEMU releases, VIRTIO_F_IN_ORDER is negotiated but > not honored causing Firmware exception. > > Signed-off-by: Gautam Dawar <gautam.dawar@amd.com> > --- > drivers/net/ethernet/sfc/ef100_vdpa.h | 14 ++ > drivers/net/ethernet/sfc/ef100_vdpa_ops.c | 148 ++++++++++++++++++++++ > 2 files changed, 162 insertions(+) > > diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.h b/drivers/net/ethernet/sfc/ef100_vdpa.h > index 83f6d819f6a5..be7650c3166a 100644 > --- a/drivers/net/ethernet/sfc/ef100_vdpa.h > +++ b/drivers/net/ethernet/sfc/ef100_vdpa.h > @@ -21,6 +21,18 @@ > /* Max queue pairs currently supported */ > #define EF100_VDPA_MAX_QUEUES_PAIRS 1 > > +/* Device ID of a virtio net device */ > +#define EF100_VDPA_VIRTIO_NET_DEVICE_ID VIRTIO_ID_NET > + > +/* Vendor ID of Xilinx vDPA NIC */ > +#define EF100_VDPA_VENDOR_ID PCI_VENDOR_ID_XILINX > + > +/* Max number of Buffers supported in the virtqueue */ > +#define EF100_VDPA_VQ_NUM_MAX_SIZE 512 > + > +/* Alignment requirement of the Virtqueue */ > +#define EF100_VDPA_VQ_ALIGN 4096 > + > /** > * enum ef100_vdpa_nic_state - possible states for a vDPA NIC > * > @@ -61,6 +73,7 @@ enum ef100_vdpa_vq_type { > * @net_config: virtio_net_config data > * @mac_address: mac address of interface associated with this vdpa device > * @mac_configured: true after MAC address is configured > + * @cfg_cb: callback for config change > */ > struct ef100_vdpa_nic { > struct vdpa_device vdpa_dev; > @@ -76,6 +89,7 @@ struct ef100_vdpa_nic { > struct virtio_net_config net_config; > u8 *mac_address; > bool mac_configured; > + struct vdpa_callback cfg_cb; > }; > > int ef100_vdpa_init(struct efx_probe_data *probe_data); > diff --git a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c > index 31952931c198..87899baa1c52 100644 > --- a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c > +++ b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c > @@ -10,12 +10,148 @@ > > #include <linux/vdpa.h> > #include "ef100_vdpa.h" > +#include "mcdi_vdpa.h" > > static struct ef100_vdpa_nic *get_vdpa_nic(struct vdpa_device *vdev) > { > return container_of(vdev, struct ef100_vdpa_nic, vdpa_dev); > } > > +static u32 ef100_vdpa_get_vq_align(struct vdpa_device *vdev) > +{ > + return EF100_VDPA_VQ_ALIGN; > +} > + > +static u64 ef100_vdpa_get_device_features(struct vdpa_device *vdev) > +{ > + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); > + u64 features; > + int rc; > + > + rc = efx_vdpa_get_features(vdpa_nic->efx, > + EF100_VDPA_DEVICE_TYPE_NET, &features); > + if (rc) { > + dev_err(&vdev->dev, "%s: MCDI get features error:%d\n", > + __func__, rc); > + /* Returning 0 as value of features will lead to failure > + * of feature negotiation. > + */ > + return 0; > + } > + > + /* SN1022 supports VIRTIO_F_IN_ORDER which is supported by the DPDK > + * virtio driver but not the kernel virtio driver. Due to a bug in > + * QEMU (https://gitlab.com/qemu-project/qemu/-/issues/331#), with > + * vhost-vdpa, this feature bit is returned with guest kernel virtio > + * driver in set_features config operation. The fix for this bug > + * (commit c33f23a419f95da16ab4faaf08be635c89b96ff0) is available > + * in QEMU versions 6.1.0 and later. Hence, that's the oldest QEMU > + * version required for testing with the vhost-vdpa driver. > + */ I don't see why this comment is placed here? > + features |= BIT_ULL(VIRTIO_NET_F_MAC); > + > + return features; > +} > + > +static int ef100_vdpa_set_driver_features(struct vdpa_device *vdev, > + u64 features) > +{ > + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); > + u64 verify_features; > + int rc; > + > + mutex_lock(&vdpa_nic->lock); > + if (vdpa_nic->vdpa_state != EF100_VDPA_STATE_INITIALIZED) { Under which case could we reach this condition? The vdpa_device_register() should be called after switching the state to EF100_VDPA_STATE_INITIALIZED. > + dev_err(&vdev->dev, "%s: Invalid state %u\n", > + __func__, vdpa_nic->vdpa_state); > + rc = -EINVAL; > + goto err; > + } > + verify_features = features & ~BIT_ULL(VIRTIO_NET_F_MAC); > + rc = efx_vdpa_verify_features(vdpa_nic->efx, > + EF100_VDPA_DEVICE_TYPE_NET, > + verify_features); It looks to me this will use MC_CMD_VIRTIO_TEST_FEATURES command, I wonder if it's better to use MC_CMD_VIRTIO_SET_FEATURES to align with the virtio spec and maybe change efx_vdpa_verify_features to efx_vdpa_set_features()? > + > + if (rc) { > + dev_err(&vdev->dev, "%s: MCDI verify features error:%d\n", > + __func__, rc); > + goto err; > + } > + > + vdpa_nic->features = features; > +err: > + mutex_unlock(&vdpa_nic->lock); > + return rc; > +} > + > +static u64 ef100_vdpa_get_driver_features(struct vdpa_device *vdev) > +{ > + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); > + > + return vdpa_nic->features; > +} > + > +static void ef100_vdpa_set_config_cb(struct vdpa_device *vdev, > + struct vdpa_callback *cb) > +{ > + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); > + > + if (cb) > + vdpa_nic->cfg_cb = *cb; > +} > + > +static u16 ef100_vdpa_get_vq_num_max(struct vdpa_device *vdev) > +{ > + return EF100_VDPA_VQ_NUM_MAX_SIZE; > +} > + > +static u32 ef100_vdpa_get_device_id(struct vdpa_device *vdev) > +{ > + return EF100_VDPA_VIRTIO_NET_DEVICE_ID; > +} > + > +static u32 ef100_vdpa_get_vendor_id(struct vdpa_device *vdev) > +{ > + return EF100_VDPA_VENDOR_ID; > +} > + > +static size_t ef100_vdpa_get_config_size(struct vdpa_device *vdev) > +{ > + return sizeof(struct virtio_net_config); > +} > + > +static void ef100_vdpa_get_config(struct vdpa_device *vdev, > + unsigned int offset, > + void *buf, unsigned int len) > +{ > + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); > + > + /* Avoid the possibility of wrap-up after the sum exceeds U32_MAX */ > + if (WARN_ON(((u64)offset + len) > sizeof(struct virtio_net_config))) { > + dev_err(&vdev->dev, > + "%s: Offset + len exceeds config size\n", __func__); > + return; I wonder if we need similar checks in the vdpa core. > + } > + memcpy(buf, (u8 *)&vdpa_nic->net_config + offset, len); > +} > + > +static void ef100_vdpa_set_config(struct vdpa_device *vdev, unsigned int offset, > + const void *buf, unsigned int len) > +{ > + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); > + > + /* Avoid the possibility of wrap-up after the sum exceeds U32_MAX */ > + if (WARN_ON(((u64)offset + len) > sizeof(vdpa_nic->net_config))) { > + dev_err(&vdev->dev, > + "%s: Offset + len exceeds config size\n", __func__); > + return; > + } > + > + memcpy((u8 *)&vdpa_nic->net_config + offset, buf, len); > + if (is_valid_ether_addr(vdpa_nic->mac_address)) > + vdpa_nic->mac_configured = true; Do we need to update hardware filters? Thanks > +} > + > static void ef100_vdpa_free(struct vdpa_device *vdev) > { > struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); > @@ -24,5 +160,17 @@ static void ef100_vdpa_free(struct vdpa_device *vdev) > } > > const struct vdpa_config_ops ef100_vdpa_config_ops = { > + .get_vq_align = ef100_vdpa_get_vq_align, > + .get_device_features = ef100_vdpa_get_device_features, > + .set_driver_features = ef100_vdpa_set_driver_features, > + .get_driver_features = ef100_vdpa_get_driver_features, > + .set_config_cb = ef100_vdpa_set_config_cb, > + .get_vq_num_max = ef100_vdpa_get_vq_num_max, > + .get_device_id = ef100_vdpa_get_device_id, > + .get_vendor_id = ef100_vdpa_get_vendor_id, > + .get_config_size = ef100_vdpa_get_config_size, > + .get_config = ef100_vdpa_get_config, > + .set_config = ef100_vdpa_set_config, > + .get_generation = NULL, > .free = ef100_vdpa_free, > }; > -- > 2.30.1 >
On 12/14/22 12:14, Jason Wang wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On Wed, Dec 7, 2022 at 10:56 PM Gautam Dawar <gautam.dawar@amd.com> wrote: >> vDPA config operations can be broadly categorized in to either >> virtqueue operations, device operations or DMA operations. >> This patch implements most of the device level config operations. >> >> SN1022 supports VIRTIO_F_IN_ORDER which is supported by the DPDK >> virtio driver but not the kernel virtio driver. Due to a bug in >> QEMU (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fqemu-project%2Fqemu%2F-%2Fissues%2F331%23&data=05%7C01%7Cgautam.dawar%40amd.com%7C58787eb502484eeaa6f508dadd9ea016%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638065970623127805%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=cVomPr2aFJBHW7kwzeZJrNHuU6oTYOTV14eOS%2BpeNVc%3D&reserved=0), with >> vhost-vdpa, this feature bit is returned with guest kernel virtio >> driver in set_features config operation. The fix for this bug >> (qemu_commit c33f23a419f95da16ab4faaf08be635c89b96ff0) is available >> in QEMU versions 6.1.0 and later. Hence, that's the oldest QEMU >> version required for testing with the vhost-vdpa driver. >> >> With older QEMU releases, VIRTIO_F_IN_ORDER is negotiated but >> not honored causing Firmware exception. >> >> Signed-off-by: Gautam Dawar <gautam.dawar@amd.com> >> --- >> drivers/net/ethernet/sfc/ef100_vdpa.h | 14 ++ >> drivers/net/ethernet/sfc/ef100_vdpa_ops.c | 148 ++++++++++++++++++++++ >> 2 files changed, 162 insertions(+) >> >> diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.h b/drivers/net/ethernet/sfc/ef100_vdpa.h >> index 83f6d819f6a5..be7650c3166a 100644 >> --- a/drivers/net/ethernet/sfc/ef100_vdpa.h >> +++ b/drivers/net/ethernet/sfc/ef100_vdpa.h >> @@ -21,6 +21,18 @@ >> /* Max queue pairs currently supported */ >> #define EF100_VDPA_MAX_QUEUES_PAIRS 1 >> >> +/* Device ID of a virtio net device */ >> +#define EF100_VDPA_VIRTIO_NET_DEVICE_ID VIRTIO_ID_NET >> + >> +/* Vendor ID of Xilinx vDPA NIC */ >> +#define EF100_VDPA_VENDOR_ID PCI_VENDOR_ID_XILINX >> + >> +/* Max number of Buffers supported in the virtqueue */ >> +#define EF100_VDPA_VQ_NUM_MAX_SIZE 512 >> + >> +/* Alignment requirement of the Virtqueue */ >> +#define EF100_VDPA_VQ_ALIGN 4096 >> + >> /** >> * enum ef100_vdpa_nic_state - possible states for a vDPA NIC >> * >> @@ -61,6 +73,7 @@ enum ef100_vdpa_vq_type { >> * @net_config: virtio_net_config data >> * @mac_address: mac address of interface associated with this vdpa device >> * @mac_configured: true after MAC address is configured >> + * @cfg_cb: callback for config change >> */ >> struct ef100_vdpa_nic { >> struct vdpa_device vdpa_dev; >> @@ -76,6 +89,7 @@ struct ef100_vdpa_nic { >> struct virtio_net_config net_config; >> u8 *mac_address; >> bool mac_configured; >> + struct vdpa_callback cfg_cb; >> }; >> >> int ef100_vdpa_init(struct efx_probe_data *probe_data); >> diff --git a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c >> index 31952931c198..87899baa1c52 100644 >> --- a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c >> +++ b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c >> @@ -10,12 +10,148 @@ >> >> #include <linux/vdpa.h> >> #include "ef100_vdpa.h" >> +#include "mcdi_vdpa.h" >> >> static struct ef100_vdpa_nic *get_vdpa_nic(struct vdpa_device *vdev) >> { >> return container_of(vdev, struct ef100_vdpa_nic, vdpa_dev); >> } >> >> +static u32 ef100_vdpa_get_vq_align(struct vdpa_device *vdev) >> +{ >> + return EF100_VDPA_VQ_ALIGN; >> +} >> + >> +static u64 ef100_vdpa_get_device_features(struct vdpa_device *vdev) >> +{ >> + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); >> + u64 features; >> + int rc; >> + >> + rc = efx_vdpa_get_features(vdpa_nic->efx, >> + EF100_VDPA_DEVICE_TYPE_NET, &features); >> + if (rc) { >> + dev_err(&vdev->dev, "%s: MCDI get features error:%d\n", >> + __func__, rc); >> + /* Returning 0 as value of features will lead to failure >> + * of feature negotiation. >> + */ >> + return 0; >> + } >> + >> + /* SN1022 supports VIRTIO_F_IN_ORDER which is supported by the DPDK >> + * virtio driver but not the kernel virtio driver. Due to a bug in >> + * QEMU (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fqemu-project%2Fqemu%2F-%2Fissues%2F331%23&data=05%7C01%7Cgautam.dawar%40amd.com%7C58787eb502484eeaa6f508dadd9ea016%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638065970623127805%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=cVomPr2aFJBHW7kwzeZJrNHuU6oTYOTV14eOS%2BpeNVc%3D&reserved=0), with >> + * vhost-vdpa, this feature bit is returned with guest kernel virtio >> + * driver in set_features config operation. The fix for this bug >> + * (commit c33f23a419f95da16ab4faaf08be635c89b96ff0) is available >> + * in QEMU versions 6.1.0 and later. Hence, that's the oldest QEMU >> + * version required for testing with the vhost-vdpa driver. >> + */ > I don't see why this comment is placed here? As the comment was related to VIRTIO_F_IN_ORDER, I thought of adding it in config operation related to virtio features handling. But I think since it is already added in the commit description, this code comment can be removed. > >> + features |= BIT_ULL(VIRTIO_NET_F_MAC); >> + >> + return features; >> +} >> + >> +static int ef100_vdpa_set_driver_features(struct vdpa_device *vdev, >> + u64 features) >> +{ >> + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); >> + u64 verify_features; >> + int rc; >> + >> + mutex_lock(&vdpa_nic->lock); >> + if (vdpa_nic->vdpa_state != EF100_VDPA_STATE_INITIALIZED) { > Under which case could we reach this condition? The > vdpa_device_register() should be called after switching the state to > EF100_VDPA_STATE_INITIALIZED. You're right. I'll remove this check as state is set to EF100_VDPA_STATE_INITIALIZED soon after vdpa_alloc_device but before _vdpa_register_device() > >> + dev_err(&vdev->dev, "%s: Invalid state %u\n", >> + __func__, vdpa_nic->vdpa_state); >> + rc = -EINVAL; >> + goto err; >> + } >> + verify_features = features & ~BIT_ULL(VIRTIO_NET_F_MAC); >> + rc = efx_vdpa_verify_features(vdpa_nic->efx, >> + EF100_VDPA_DEVICE_TYPE_NET, >> + verify_features); > It looks to me this will use MC_CMD_VIRTIO_TEST_FEATURES command, I > wonder if it's better to use > > MC_CMD_VIRTIO_SET_FEATURES to align with the virtio spec and maybe > change efx_vdpa_verify_features to efx_vdpa_set_features()? Yes, it makes more sense to match the function names with the operations in virtio spec. However, MC_CMD_VIRTIO_TEST_FEATURES queries if the passed set of features is supported and it fails in case either driver requests an unsupported feature or misses a feature that device requires. Hence, it's more of a verification of feature set than applying/setting them. >> + >> + if (rc) { >> + dev_err(&vdev->dev, "%s: MCDI verify features error:%d\n", >> + __func__, rc); >> + goto err; >> + } >> + >> + vdpa_nic->features = features; >> +err: >> + mutex_unlock(&vdpa_nic->lock); >> + return rc; >> +} >> + >> +static u64 ef100_vdpa_get_driver_features(struct vdpa_device *vdev) >> +{ >> + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); >> + >> + return vdpa_nic->features; >> +} >> + >> +static void ef100_vdpa_set_config_cb(struct vdpa_device *vdev, >> + struct vdpa_callback *cb) >> +{ >> + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); >> + >> + if (cb) >> + vdpa_nic->cfg_cb = *cb; >> +} >> + >> +static u16 ef100_vdpa_get_vq_num_max(struct vdpa_device *vdev) >> +{ >> + return EF100_VDPA_VQ_NUM_MAX_SIZE; >> +} >> + >> +static u32 ef100_vdpa_get_device_id(struct vdpa_device *vdev) >> +{ >> + return EF100_VDPA_VIRTIO_NET_DEVICE_ID; >> +} >> + >> +static u32 ef100_vdpa_get_vendor_id(struct vdpa_device *vdev) >> +{ >> + return EF100_VDPA_VENDOR_ID; >> +} >> + >> +static size_t ef100_vdpa_get_config_size(struct vdpa_device *vdev) >> +{ >> + return sizeof(struct virtio_net_config); >> +} >> + >> +static void ef100_vdpa_get_config(struct vdpa_device *vdev, >> + unsigned int offset, >> + void *buf, unsigned int len) >> +{ >> + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); >> + >> + /* Avoid the possibility of wrap-up after the sum exceeds U32_MAX */ >> + if (WARN_ON(((u64)offset + len) > sizeof(struct virtio_net_config))) { >> + dev_err(&vdev->dev, >> + "%s: Offset + len exceeds config size\n", __func__); >> + return; > I wonder if we need similar checks in the vdpa core. Yes, it would be better to have this validation at one place (vdpa framework) instead of individual vendor drivers. Although I am not sure if the framework can choose the correct config size based on the device class. > >> + } >> + memcpy(buf, (u8 *)&vdpa_nic->net_config + offset, len); >> +} >> + >> +static void ef100_vdpa_set_config(struct vdpa_device *vdev, unsigned int offset, >> + const void *buf, unsigned int len) >> +{ >> + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); >> + >> + /* Avoid the possibility of wrap-up after the sum exceeds U32_MAX */ >> + if (WARN_ON(((u64)offset + len) > sizeof(vdpa_nic->net_config))) { >> + dev_err(&vdev->dev, >> + "%s: Offset + len exceeds config size\n", __func__); >> + return; >> + } >> + >> + memcpy((u8 *)&vdpa_nic->net_config + offset, buf, len); >> + if (is_valid_ether_addr(vdpa_nic->mac_address)) >> + vdpa_nic->mac_configured = true; > Do we need to update hardware filters? Yes, it's done in a later patch where filters support is added (sfc: implement filters for receiving traffic). > > Thanks > >> +} >> + >> static void ef100_vdpa_free(struct vdpa_device *vdev) >> { >> struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); >> @@ -24,5 +160,17 @@ static void ef100_vdpa_free(struct vdpa_device *vdev) >> } >> >> const struct vdpa_config_ops ef100_vdpa_config_ops = { >> + .get_vq_align = ef100_vdpa_get_vq_align, >> + .get_device_features = ef100_vdpa_get_device_features, >> + .set_driver_features = ef100_vdpa_set_driver_features, >> + .get_driver_features = ef100_vdpa_get_driver_features, >> + .set_config_cb = ef100_vdpa_set_config_cb, >> + .get_vq_num_max = ef100_vdpa_get_vq_num_max, >> + .get_device_id = ef100_vdpa_get_device_id, >> + .get_vendor_id = ef100_vdpa_get_vendor_id, >> + .get_config_size = ef100_vdpa_get_config_size, >> + .get_config = ef100_vdpa_get_config, >> + .set_config = ef100_vdpa_set_config, >> + .get_generation = NULL, >> .free = ef100_vdpa_free, >> }; >> -- >> 2.30.1 >>
diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.h b/drivers/net/ethernet/sfc/ef100_vdpa.h index 83f6d819f6a5..be7650c3166a 100644 --- a/drivers/net/ethernet/sfc/ef100_vdpa.h +++ b/drivers/net/ethernet/sfc/ef100_vdpa.h @@ -21,6 +21,18 @@ /* Max queue pairs currently supported */ #define EF100_VDPA_MAX_QUEUES_PAIRS 1 +/* Device ID of a virtio net device */ +#define EF100_VDPA_VIRTIO_NET_DEVICE_ID VIRTIO_ID_NET + +/* Vendor ID of Xilinx vDPA NIC */ +#define EF100_VDPA_VENDOR_ID PCI_VENDOR_ID_XILINX + +/* Max number of Buffers supported in the virtqueue */ +#define EF100_VDPA_VQ_NUM_MAX_SIZE 512 + +/* Alignment requirement of the Virtqueue */ +#define EF100_VDPA_VQ_ALIGN 4096 + /** * enum ef100_vdpa_nic_state - possible states for a vDPA NIC * @@ -61,6 +73,7 @@ enum ef100_vdpa_vq_type { * @net_config: virtio_net_config data * @mac_address: mac address of interface associated with this vdpa device * @mac_configured: true after MAC address is configured + * @cfg_cb: callback for config change */ struct ef100_vdpa_nic { struct vdpa_device vdpa_dev; @@ -76,6 +89,7 @@ struct ef100_vdpa_nic { struct virtio_net_config net_config; u8 *mac_address; bool mac_configured; + struct vdpa_callback cfg_cb; }; int ef100_vdpa_init(struct efx_probe_data *probe_data); diff --git a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c index 31952931c198..87899baa1c52 100644 --- a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c +++ b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c @@ -10,12 +10,148 @@ #include <linux/vdpa.h> #include "ef100_vdpa.h" +#include "mcdi_vdpa.h" static struct ef100_vdpa_nic *get_vdpa_nic(struct vdpa_device *vdev) { return container_of(vdev, struct ef100_vdpa_nic, vdpa_dev); } +static u32 ef100_vdpa_get_vq_align(struct vdpa_device *vdev) +{ + return EF100_VDPA_VQ_ALIGN; +} + +static u64 ef100_vdpa_get_device_features(struct vdpa_device *vdev) +{ + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); + u64 features; + int rc; + + rc = efx_vdpa_get_features(vdpa_nic->efx, + EF100_VDPA_DEVICE_TYPE_NET, &features); + if (rc) { + dev_err(&vdev->dev, "%s: MCDI get features error:%d\n", + __func__, rc); + /* Returning 0 as value of features will lead to failure + * of feature negotiation. + */ + return 0; + } + + /* SN1022 supports VIRTIO_F_IN_ORDER which is supported by the DPDK + * virtio driver but not the kernel virtio driver. Due to a bug in + * QEMU (https://gitlab.com/qemu-project/qemu/-/issues/331#), with + * vhost-vdpa, this feature bit is returned with guest kernel virtio + * driver in set_features config operation. The fix for this bug + * (commit c33f23a419f95da16ab4faaf08be635c89b96ff0) is available + * in QEMU versions 6.1.0 and later. Hence, that's the oldest QEMU + * version required for testing with the vhost-vdpa driver. + */ + features |= BIT_ULL(VIRTIO_NET_F_MAC); + + return features; +} + +static int ef100_vdpa_set_driver_features(struct vdpa_device *vdev, + u64 features) +{ + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); + u64 verify_features; + int rc; + + mutex_lock(&vdpa_nic->lock); + if (vdpa_nic->vdpa_state != EF100_VDPA_STATE_INITIALIZED) { + dev_err(&vdev->dev, "%s: Invalid state %u\n", + __func__, vdpa_nic->vdpa_state); + rc = -EINVAL; + goto err; + } + verify_features = features & ~BIT_ULL(VIRTIO_NET_F_MAC); + rc = efx_vdpa_verify_features(vdpa_nic->efx, + EF100_VDPA_DEVICE_TYPE_NET, + verify_features); + + if (rc) { + dev_err(&vdev->dev, "%s: MCDI verify features error:%d\n", + __func__, rc); + goto err; + } + + vdpa_nic->features = features; +err: + mutex_unlock(&vdpa_nic->lock); + return rc; +} + +static u64 ef100_vdpa_get_driver_features(struct vdpa_device *vdev) +{ + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); + + return vdpa_nic->features; +} + +static void ef100_vdpa_set_config_cb(struct vdpa_device *vdev, + struct vdpa_callback *cb) +{ + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); + + if (cb) + vdpa_nic->cfg_cb = *cb; +} + +static u16 ef100_vdpa_get_vq_num_max(struct vdpa_device *vdev) +{ + return EF100_VDPA_VQ_NUM_MAX_SIZE; +} + +static u32 ef100_vdpa_get_device_id(struct vdpa_device *vdev) +{ + return EF100_VDPA_VIRTIO_NET_DEVICE_ID; +} + +static u32 ef100_vdpa_get_vendor_id(struct vdpa_device *vdev) +{ + return EF100_VDPA_VENDOR_ID; +} + +static size_t ef100_vdpa_get_config_size(struct vdpa_device *vdev) +{ + return sizeof(struct virtio_net_config); +} + +static void ef100_vdpa_get_config(struct vdpa_device *vdev, + unsigned int offset, + void *buf, unsigned int len) +{ + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); + + /* Avoid the possibility of wrap-up after the sum exceeds U32_MAX */ + if (WARN_ON(((u64)offset + len) > sizeof(struct virtio_net_config))) { + dev_err(&vdev->dev, + "%s: Offset + len exceeds config size\n", __func__); + return; + } + memcpy(buf, (u8 *)&vdpa_nic->net_config + offset, len); +} + +static void ef100_vdpa_set_config(struct vdpa_device *vdev, unsigned int offset, + const void *buf, unsigned int len) +{ + struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); + + /* Avoid the possibility of wrap-up after the sum exceeds U32_MAX */ + if (WARN_ON(((u64)offset + len) > sizeof(vdpa_nic->net_config))) { + dev_err(&vdev->dev, + "%s: Offset + len exceeds config size\n", __func__); + return; + } + + memcpy((u8 *)&vdpa_nic->net_config + offset, buf, len); + if (is_valid_ether_addr(vdpa_nic->mac_address)) + vdpa_nic->mac_configured = true; +} + static void ef100_vdpa_free(struct vdpa_device *vdev) { struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); @@ -24,5 +160,17 @@ static void ef100_vdpa_free(struct vdpa_device *vdev) } const struct vdpa_config_ops ef100_vdpa_config_ops = { + .get_vq_align = ef100_vdpa_get_vq_align, + .get_device_features = ef100_vdpa_get_device_features, + .set_driver_features = ef100_vdpa_set_driver_features, + .get_driver_features = ef100_vdpa_get_driver_features, + .set_config_cb = ef100_vdpa_set_config_cb, + .get_vq_num_max = ef100_vdpa_get_vq_num_max, + .get_device_id = ef100_vdpa_get_device_id, + .get_vendor_id = ef100_vdpa_get_vendor_id, + .get_config_size = ef100_vdpa_get_config_size, + .get_config = ef100_vdpa_get_config, + .set_config = ef100_vdpa_set_config, + .get_generation = NULL, .free = ef100_vdpa_free, };
vDPA config operations can be broadly categorized in to either virtqueue operations, device operations or DMA operations. This patch implements most of the device level config operations. SN1022 supports VIRTIO_F_IN_ORDER which is supported by the DPDK virtio driver but not the kernel virtio driver. Due to a bug in QEMU (https://gitlab.com/qemu-project/qemu/-/issues/331#), with vhost-vdpa, this feature bit is returned with guest kernel virtio driver in set_features config operation. The fix for this bug (qemu_commit c33f23a419f95da16ab4faaf08be635c89b96ff0) is available in QEMU versions 6.1.0 and later. Hence, that's the oldest QEMU version required for testing with the vhost-vdpa driver. With older QEMU releases, VIRTIO_F_IN_ORDER is negotiated but not honored causing Firmware exception. Signed-off-by: Gautam Dawar <gautam.dawar@amd.com> --- drivers/net/ethernet/sfc/ef100_vdpa.h | 14 ++ drivers/net/ethernet/sfc/ef100_vdpa_ops.c | 148 ++++++++++++++++++++++ 2 files changed, 162 insertions(+)