| Message ID | 20230307113621.64153-13-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/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 21 this patch: 21 |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/build_clang | success | Errors and warnings before: 23 this patch: 23 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| 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: 21 this patch: 21 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 161 lines checked |
| netdev/kdoc | success | Errors and warnings before: 59 this patch: 59 |
| netdev/source_inline | success | Was 0 now: 0 |
On Tue, Mar 7, 2023 at 7:38 PM Gautam Dawar <gautam.dawar@amd.com> wrote: > > To avoid clash of IOVA range of VF's MCDI DMA buffer with the guest > buffer IOVAs, unmap the MCDI buffer when switching to vDPA mode > and use PF's IOMMU domain for running VF's MCDI commands. > > Signed-off-by: Gautam Dawar <gautam.dawar@amd.com> > --- > drivers/net/ethernet/sfc/ef100_nic.c | 1 - > drivers/net/ethernet/sfc/ef100_vdpa.c | 25 ++++++++++++++++ > drivers/net/ethernet/sfc/ef100_vdpa.h | 3 ++ > drivers/net/ethernet/sfc/ef100_vdpa_ops.c | 35 +++++++++++++++++++++++ > drivers/net/ethernet/sfc/mcdi.h | 3 ++ > drivers/net/ethernet/sfc/net_driver.h | 12 ++++++++ > 6 files changed, 78 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c > index cd9f724a9e64..1bffc1994ed8 100644 > --- a/drivers/net/ethernet/sfc/ef100_nic.c > +++ b/drivers/net/ethernet/sfc/ef100_nic.c > @@ -33,7 +33,6 @@ > > #define EF100_MAX_VIS 4096 > #define EF100_NUM_MCDI_BUFFERS 1 > -#define MCDI_BUF_LEN (8 + MCDI_CTL_SDU_LEN_MAX) > > #define EF100_RESET_PORT ((ETH_RESET_MAC | ETH_RESET_PHY) << ETH_RESET_SHARED_SHIFT) > > diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.c b/drivers/net/ethernet/sfc/ef100_vdpa.c > index 5c9f29f881a6..30ca4ab00175 100644 > --- a/drivers/net/ethernet/sfc/ef100_vdpa.c > +++ b/drivers/net/ethernet/sfc/ef100_vdpa.c > @@ -223,10 +223,19 @@ static int vdpa_allocate_vis(struct efx_nic *efx, unsigned int *allocated_vis) > static void ef100_vdpa_delete(struct efx_nic *efx) > { > struct vdpa_device *vdpa_dev; > + int rc; > > if (efx->vdpa_nic) { > vdpa_dev = &efx->vdpa_nic->vdpa_dev; > ef100_vdpa_reset(vdpa_dev); > + if (efx->mcdi_buf_mode == EFX_BUF_MODE_VDPA) { > + rc = ef100_vdpa_map_mcdi_buffer(efx); > + if (rc) { > + pci_err(efx->pci_dev, > + "map_mcdi_buffer failed, err: %d\n", > + rc); > + } > + } > > /* replace with _vdpa_unregister_device later */ > put_device(&vdpa_dev->dev); > @@ -276,6 +285,21 @@ static int get_net_config(struct ef100_vdpa_nic *vdpa_nic) > return 0; > } > > +static void unmap_mcdi_buffer(struct efx_nic *efx) > +{ > + struct ef100_nic_data *nic_data = efx->nic_data; > + struct efx_mcdi_iface *mcdi; > + > + mcdi = efx_mcdi(efx); > + spin_lock_bh(&mcdi->iface_lock); > + /* Save current MCDI mode to be restored later */ > + efx->vdpa_nic->mcdi_mode = mcdi->mode; > + efx->mcdi_buf_mode = EFX_BUF_MODE_VDPA; > + mcdi->mode = MCDI_MODE_FAIL; > + spin_unlock_bh(&mcdi->iface_lock); > + efx_nic_free_buffer(efx, &nic_data->mcdi_buf); > +} > + > static struct ef100_vdpa_nic *ef100_vdpa_create(struct efx_nic *efx, > const char *dev_name, > enum ef100_vdpa_class dev_type, > @@ -342,6 +366,7 @@ static struct ef100_vdpa_nic *ef100_vdpa_create(struct efx_nic *efx, > for (i = 0; i < EF100_VDPA_MAC_FILTER_NTYPES; i++) > vdpa_nic->filters[i].filter_id = EFX_INVALID_FILTER_ID; > > + unmap_mcdi_buffer(efx); > rc = get_net_config(vdpa_nic); > if (rc) > goto err_put_device; > diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.h b/drivers/net/ethernet/sfc/ef100_vdpa.h > index 49fb6be04eb3..0913ac2519cb 100644 > --- a/drivers/net/ethernet/sfc/ef100_vdpa.h > +++ b/drivers/net/ethernet/sfc/ef100_vdpa.h > @@ -147,6 +147,7 @@ struct ef100_vdpa_filter { > * @status: device status as per VIRTIO spec > * @features: negotiated feature bits > * @max_queue_pairs: maximum number of queue pairs supported > + * @mcdi_mode: MCDI mode at the time of unmapping VF mcdi buffer > * @net_config: virtio_net_config data > * @vring: vring information of the vDPA device. > * @mac_address: mac address of interface associated with this vdpa device > @@ -166,6 +167,7 @@ struct ef100_vdpa_nic { > u8 status; > u64 features; > u32 max_queue_pairs; > + enum efx_mcdi_mode mcdi_mode; > struct virtio_net_config net_config; > struct ef100_vdpa_vring_info vring[EF100_VDPA_MAX_QUEUES_PAIRS * 2]; > u8 *mac_address; > @@ -185,6 +187,7 @@ int ef100_vdpa_add_filter(struct ef100_vdpa_nic *vdpa_nic, > int ef100_vdpa_init_vring(struct ef100_vdpa_nic *vdpa_nic, u16 idx); > void ef100_vdpa_irq_vectors_free(void *data); > int ef100_vdpa_reset(struct vdpa_device *vdev); > +int ef100_vdpa_map_mcdi_buffer(struct efx_nic *efx); > > static inline bool efx_vdpa_is_little_endian(struct ef100_vdpa_nic *vdpa_nic) > { > diff --git a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c > index db86c2693950..c6c9458f0e6f 100644 > --- a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c > +++ b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c > @@ -711,12 +711,47 @@ static int ef100_vdpa_suspend(struct vdpa_device *vdev) > mutex_unlock(&vdpa_nic->lock); > return rc; > } > + > +int ef100_vdpa_map_mcdi_buffer(struct efx_nic *efx) > +{ The name of this function is confusing, it's actually map buffer for ef100 netdev mode. Actually, I wonder why not moving this to init/fini of bar config ops or if we use aux bus, it should be done during aux driver probe/remove. Thanks > + struct ef100_nic_data *nic_data = efx->nic_data; > + struct efx_mcdi_iface *mcdi; > + int rc; > + > + /* Update VF's MCDI buffer when switching out of vdpa mode */ > + rc = efx_nic_alloc_buffer(efx, &nic_data->mcdi_buf, > + MCDI_BUF_LEN, GFP_KERNEL); > + if (rc) > + return rc; > + > + mcdi = efx_mcdi(efx); > + spin_lock_bh(&mcdi->iface_lock); > + mcdi->mode = efx->vdpa_nic->mcdi_mode; > + efx->mcdi_buf_mode = EFX_BUF_MODE_EF100; > + spin_unlock_bh(&mcdi->iface_lock); > + > + return 0; > +} > + > static void ef100_vdpa_free(struct vdpa_device *vdev) > { > struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); > + int rc; > int i; > > if (vdpa_nic) { > + if (vdpa_nic->efx->mcdi_buf_mode == EFX_BUF_MODE_VDPA) { > + /* This will only be called via call to put_device() > + * on vdpa device creation failure > + */ > + rc = ef100_vdpa_map_mcdi_buffer(vdpa_nic->efx); > + if (rc) { > + dev_err(&vdev->dev, > + "map_mcdi_buffer failed, err: %d\n", > + rc); > + } > + } > + > for (i = 0; i < (vdpa_nic->max_queue_pairs * 2); i++) { > reset_vring(vdpa_nic, i); > if (vdpa_nic->vring[i].vring_ctx) > diff --git a/drivers/net/ethernet/sfc/mcdi.h b/drivers/net/ethernet/sfc/mcdi.h > index 2c526d2edeb6..bc4de3b4e6f3 100644 > --- a/drivers/net/ethernet/sfc/mcdi.h > +++ b/drivers/net/ethernet/sfc/mcdi.h > @@ -6,6 +6,9 @@ > > #ifndef EFX_MCDI_H > #define EFX_MCDI_H > +#include "mcdi_pcol.h" > + > +#define MCDI_BUF_LEN (8 + MCDI_CTL_SDU_LEN_MAX) > > /** > * enum efx_mcdi_state - MCDI request handling state > diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h > index 948c7a06403a..9cdfeb6ad05a 100644 > --- a/drivers/net/ethernet/sfc/net_driver.h > +++ b/drivers/net/ethernet/sfc/net_driver.h > @@ -848,6 +848,16 @@ enum efx_xdp_tx_queues_mode { > > struct efx_mae; > > +/** > + * enum efx_buf_alloc_mode - buffer allocation mode > + * @EFX_BUF_MODE_EF100: buffer setup in ef100 mode > + * @EFX_BUF_MODE_VDPA: buffer setup in vdpa mode > + */ > +enum efx_buf_alloc_mode { > + EFX_BUF_MODE_EF100, > + EFX_BUF_MODE_VDPA > +}; > + > /** > * struct efx_nic - an Efx NIC > * @name: Device name (net device name or bus id before net device registered) > @@ -877,6 +887,7 @@ struct efx_mae; > * @irq_rx_mod_step_us: Step size for IRQ moderation for RX event queues > * @irq_rx_moderation_us: IRQ moderation time for RX event queues > * @msg_enable: Log message enable flags > + * @mcdi_buf_mode: mcdi buffer allocation mode > * @state: Device state number (%STATE_*). Serialised by the rtnl_lock. > * @reset_pending: Bitmask for pending resets > * @tx_queue: TX DMA queues > @@ -1045,6 +1056,7 @@ struct efx_nic { > u32 msg_enable; > > enum nic_state state; > + enum efx_buf_alloc_mode mcdi_buf_mode; > unsigned long reset_pending; > > struct efx_channel *channel[EFX_MAX_CHANNELS]; > -- > 2.30.1 >
On 3/10/23 10:35, Jason Wang wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On Tue, Mar 7, 2023 at 7:38 PM Gautam Dawar <gautam.dawar@amd.com> wrote: >> To avoid clash of IOVA range of VF's MCDI DMA buffer with the guest >> buffer IOVAs, unmap the MCDI buffer when switching to vDPA mode >> and use PF's IOMMU domain for running VF's MCDI commands. >> >> Signed-off-by: Gautam Dawar <gautam.dawar@amd.com> >> --- >> drivers/net/ethernet/sfc/ef100_nic.c | 1 - >> drivers/net/ethernet/sfc/ef100_vdpa.c | 25 ++++++++++++++++ >> drivers/net/ethernet/sfc/ef100_vdpa.h | 3 ++ >> drivers/net/ethernet/sfc/ef100_vdpa_ops.c | 35 +++++++++++++++++++++++ >> drivers/net/ethernet/sfc/mcdi.h | 3 ++ >> drivers/net/ethernet/sfc/net_driver.h | 12 ++++++++ >> 6 files changed, 78 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c >> index cd9f724a9e64..1bffc1994ed8 100644 >> --- a/drivers/net/ethernet/sfc/ef100_nic.c >> +++ b/drivers/net/ethernet/sfc/ef100_nic.c >> @@ -33,7 +33,6 @@ >> >> #define EF100_MAX_VIS 4096 >> #define EF100_NUM_MCDI_BUFFERS 1 >> -#define MCDI_BUF_LEN (8 + MCDI_CTL_SDU_LEN_MAX) >> >> #define EF100_RESET_PORT ((ETH_RESET_MAC | ETH_RESET_PHY) << ETH_RESET_SHARED_SHIFT) >> >> diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.c b/drivers/net/ethernet/sfc/ef100_vdpa.c >> index 5c9f29f881a6..30ca4ab00175 100644 >> --- a/drivers/net/ethernet/sfc/ef100_vdpa.c >> +++ b/drivers/net/ethernet/sfc/ef100_vdpa.c >> @@ -223,10 +223,19 @@ static int vdpa_allocate_vis(struct efx_nic *efx, unsigned int *allocated_vis) >> static void ef100_vdpa_delete(struct efx_nic *efx) >> { >> struct vdpa_device *vdpa_dev; >> + int rc; >> >> if (efx->vdpa_nic) { >> vdpa_dev = &efx->vdpa_nic->vdpa_dev; >> ef100_vdpa_reset(vdpa_dev); >> + if (efx->mcdi_buf_mode == EFX_BUF_MODE_VDPA) { >> + rc = ef100_vdpa_map_mcdi_buffer(efx); >> + if (rc) { >> + pci_err(efx->pci_dev, >> + "map_mcdi_buffer failed, err: %d\n", >> + rc); >> + } >> + } >> >> /* replace with _vdpa_unregister_device later */ >> put_device(&vdpa_dev->dev); >> @@ -276,6 +285,21 @@ static int get_net_config(struct ef100_vdpa_nic *vdpa_nic) >> return 0; >> } >> >> +static void unmap_mcdi_buffer(struct efx_nic *efx) >> +{ >> + struct ef100_nic_data *nic_data = efx->nic_data; >> + struct efx_mcdi_iface *mcdi; >> + >> + mcdi = efx_mcdi(efx); >> + spin_lock_bh(&mcdi->iface_lock); >> + /* Save current MCDI mode to be restored later */ >> + efx->vdpa_nic->mcdi_mode = mcdi->mode; >> + efx->mcdi_buf_mode = EFX_BUF_MODE_VDPA; >> + mcdi->mode = MCDI_MODE_FAIL; >> + spin_unlock_bh(&mcdi->iface_lock); >> + efx_nic_free_buffer(efx, &nic_data->mcdi_buf); >> +} >> + >> static struct ef100_vdpa_nic *ef100_vdpa_create(struct efx_nic *efx, >> const char *dev_name, >> enum ef100_vdpa_class dev_type, >> @@ -342,6 +366,7 @@ static struct ef100_vdpa_nic *ef100_vdpa_create(struct efx_nic *efx, >> for (i = 0; i < EF100_VDPA_MAC_FILTER_NTYPES; i++) >> vdpa_nic->filters[i].filter_id = EFX_INVALID_FILTER_ID; >> >> + unmap_mcdi_buffer(efx); >> rc = get_net_config(vdpa_nic); >> if (rc) >> goto err_put_device; >> diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.h b/drivers/net/ethernet/sfc/ef100_vdpa.h >> index 49fb6be04eb3..0913ac2519cb 100644 >> --- a/drivers/net/ethernet/sfc/ef100_vdpa.h >> +++ b/drivers/net/ethernet/sfc/ef100_vdpa.h >> @@ -147,6 +147,7 @@ struct ef100_vdpa_filter { >> * @status: device status as per VIRTIO spec >> * @features: negotiated feature bits >> * @max_queue_pairs: maximum number of queue pairs supported >> + * @mcdi_mode: MCDI mode at the time of unmapping VF mcdi buffer >> * @net_config: virtio_net_config data >> * @vring: vring information of the vDPA device. >> * @mac_address: mac address of interface associated with this vdpa device >> @@ -166,6 +167,7 @@ struct ef100_vdpa_nic { >> u8 status; >> u64 features; >> u32 max_queue_pairs; >> + enum efx_mcdi_mode mcdi_mode; >> struct virtio_net_config net_config; >> struct ef100_vdpa_vring_info vring[EF100_VDPA_MAX_QUEUES_PAIRS * 2]; >> u8 *mac_address; >> @@ -185,6 +187,7 @@ int ef100_vdpa_add_filter(struct ef100_vdpa_nic *vdpa_nic, >> int ef100_vdpa_init_vring(struct ef100_vdpa_nic *vdpa_nic, u16 idx); >> void ef100_vdpa_irq_vectors_free(void *data); >> int ef100_vdpa_reset(struct vdpa_device *vdev); >> +int ef100_vdpa_map_mcdi_buffer(struct efx_nic *efx); >> >> static inline bool efx_vdpa_is_little_endian(struct ef100_vdpa_nic *vdpa_nic) >> { >> diff --git a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c >> index db86c2693950..c6c9458f0e6f 100644 >> --- a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c >> +++ b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c >> @@ -711,12 +711,47 @@ static int ef100_vdpa_suspend(struct vdpa_device *vdev) >> mutex_unlock(&vdpa_nic->lock); >> return rc; >> } >> + >> +int ef100_vdpa_map_mcdi_buffer(struct efx_nic *efx) >> +{ > The name of this function is confusing, it's actually map buffer for > ef100 netdev mode. Yeah, I get your point. Actually, I am using the prefix ef100_vdpa_ for extern functions and this function is just mapping the MCDI buffer resulting in the name ef100_vdpa_map_mcdi_buffer(). I think ef100_vdpa_map_vf_mcdi_buffer() would remove this confusion. What do you think? > > Actually, I wonder why not moving this to init/fini of bar config ops > or if we use aux bus, it should be done during aux driver > probe/remove. It makes sense, however we store the last mcdi mode (poll or events) in vdpa_nic->mcdi_mode to restore later, which requires vdpa_nic to be allocated that happens later than switching to vdpa bar_config. > Thanks > > >> + struct ef100_nic_data *nic_data = efx->nic_data; >> + struct efx_mcdi_iface *mcdi; >> + int rc; >> + >> + /* Update VF's MCDI buffer when switching out of vdpa mode */ >> + rc = efx_nic_alloc_buffer(efx, &nic_data->mcdi_buf, >> + MCDI_BUF_LEN, GFP_KERNEL); >> + if (rc) >> + return rc; >> + >> + mcdi = efx_mcdi(efx); >> + spin_lock_bh(&mcdi->iface_lock); >> + mcdi->mode = efx->vdpa_nic->mcdi_mode; >> + efx->mcdi_buf_mode = EFX_BUF_MODE_EF100; >> + spin_unlock_bh(&mcdi->iface_lock); >> + >> + return 0; >> +} >> + >> static void ef100_vdpa_free(struct vdpa_device *vdev) >> { >> struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); >> + int rc; >> int i; >> >> if (vdpa_nic) { >> + if (vdpa_nic->efx->mcdi_buf_mode == EFX_BUF_MODE_VDPA) { >> + /* This will only be called via call to put_device() >> + * on vdpa device creation failure >> + */ >> + rc = ef100_vdpa_map_mcdi_buffer(vdpa_nic->efx); >> + if (rc) { >> + dev_err(&vdev->dev, >> + "map_mcdi_buffer failed, err: %d\n", >> + rc); >> + } >> + } >> + >> for (i = 0; i < (vdpa_nic->max_queue_pairs * 2); i++) { >> reset_vring(vdpa_nic, i); >> if (vdpa_nic->vring[i].vring_ctx) >> diff --git a/drivers/net/ethernet/sfc/mcdi.h b/drivers/net/ethernet/sfc/mcdi.h >> index 2c526d2edeb6..bc4de3b4e6f3 100644 >> --- a/drivers/net/ethernet/sfc/mcdi.h >> +++ b/drivers/net/ethernet/sfc/mcdi.h >> @@ -6,6 +6,9 @@ >> >> #ifndef EFX_MCDI_H >> #define EFX_MCDI_H >> +#include "mcdi_pcol.h" >> + >> +#define MCDI_BUF_LEN (8 + MCDI_CTL_SDU_LEN_MAX) >> >> /** >> * enum efx_mcdi_state - MCDI request handling state >> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h >> index 948c7a06403a..9cdfeb6ad05a 100644 >> --- a/drivers/net/ethernet/sfc/net_driver.h >> +++ b/drivers/net/ethernet/sfc/net_driver.h >> @@ -848,6 +848,16 @@ enum efx_xdp_tx_queues_mode { >> >> struct efx_mae; >> >> +/** >> + * enum efx_buf_alloc_mode - buffer allocation mode >> + * @EFX_BUF_MODE_EF100: buffer setup in ef100 mode >> + * @EFX_BUF_MODE_VDPA: buffer setup in vdpa mode >> + */ >> +enum efx_buf_alloc_mode { >> + EFX_BUF_MODE_EF100, >> + EFX_BUF_MODE_VDPA >> +}; >> + >> /** >> * struct efx_nic - an Efx NIC >> * @name: Device name (net device name or bus id before net device registered) >> @@ -877,6 +887,7 @@ struct efx_mae; >> * @irq_rx_mod_step_us: Step size for IRQ moderation for RX event queues >> * @irq_rx_moderation_us: IRQ moderation time for RX event queues >> * @msg_enable: Log message enable flags >> + * @mcdi_buf_mode: mcdi buffer allocation mode >> * @state: Device state number (%STATE_*). Serialised by the rtnl_lock. >> * @reset_pending: Bitmask for pending resets >> * @tx_queue: TX DMA queues >> @@ -1045,6 +1056,7 @@ struct efx_nic { >> u32 msg_enable; >> >> enum nic_state state; >> + enum efx_buf_alloc_mode mcdi_buf_mode; >> unsigned long reset_pending; >> >> struct efx_channel *channel[EFX_MAX_CHANNELS]; >> -- >> 2.30.1 >>
在 2023/3/13 15:09, Gautam Dawar 写道: > > On 3/10/23 10:35, Jason Wang wrote: >> Caution: This message originated from an External Source. Use proper >> caution when opening attachments, clicking links, or responding. >> >> >> On Tue, Mar 7, 2023 at 7:38 PM Gautam Dawar <gautam.dawar@amd.com> >> wrote: >>> To avoid clash of IOVA range of VF's MCDI DMA buffer with the guest >>> buffer IOVAs, unmap the MCDI buffer when switching to vDPA mode >>> and use PF's IOMMU domain for running VF's MCDI commands. >>> >>> Signed-off-by: Gautam Dawar <gautam.dawar@amd.com> >>> --- >>> drivers/net/ethernet/sfc/ef100_nic.c | 1 - >>> drivers/net/ethernet/sfc/ef100_vdpa.c | 25 ++++++++++++++++ >>> drivers/net/ethernet/sfc/ef100_vdpa.h | 3 ++ >>> drivers/net/ethernet/sfc/ef100_vdpa_ops.c | 35 >>> +++++++++++++++++++++++ >>> drivers/net/ethernet/sfc/mcdi.h | 3 ++ >>> drivers/net/ethernet/sfc/net_driver.h | 12 ++++++++ >>> 6 files changed, 78 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/sfc/ef100_nic.c >>> b/drivers/net/ethernet/sfc/ef100_nic.c >>> index cd9f724a9e64..1bffc1994ed8 100644 >>> --- a/drivers/net/ethernet/sfc/ef100_nic.c >>> +++ b/drivers/net/ethernet/sfc/ef100_nic.c >>> @@ -33,7 +33,6 @@ >>> >>> #define EF100_MAX_VIS 4096 >>> #define EF100_NUM_MCDI_BUFFERS 1 >>> -#define MCDI_BUF_LEN (8 + MCDI_CTL_SDU_LEN_MAX) >>> >>> #define EF100_RESET_PORT ((ETH_RESET_MAC | ETH_RESET_PHY) << >>> ETH_RESET_SHARED_SHIFT) >>> >>> diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.c >>> b/drivers/net/ethernet/sfc/ef100_vdpa.c >>> index 5c9f29f881a6..30ca4ab00175 100644 >>> --- a/drivers/net/ethernet/sfc/ef100_vdpa.c >>> +++ b/drivers/net/ethernet/sfc/ef100_vdpa.c >>> @@ -223,10 +223,19 @@ static int vdpa_allocate_vis(struct efx_nic >>> *efx, unsigned int *allocated_vis) >>> static void ef100_vdpa_delete(struct efx_nic *efx) >>> { >>> struct vdpa_device *vdpa_dev; >>> + int rc; >>> >>> if (efx->vdpa_nic) { >>> vdpa_dev = &efx->vdpa_nic->vdpa_dev; >>> ef100_vdpa_reset(vdpa_dev); >>> + if (efx->mcdi_buf_mode == EFX_BUF_MODE_VDPA) { >>> + rc = ef100_vdpa_map_mcdi_buffer(efx); >>> + if (rc) { >>> + pci_err(efx->pci_dev, >>> + "map_mcdi_buffer failed, >>> err: %d\n", >>> + rc); >>> + } >>> + } >>> >>> /* replace with _vdpa_unregister_device later */ >>> put_device(&vdpa_dev->dev); >>> @@ -276,6 +285,21 @@ static int get_net_config(struct ef100_vdpa_nic >>> *vdpa_nic) >>> return 0; >>> } >>> >>> +static void unmap_mcdi_buffer(struct efx_nic *efx) >>> +{ >>> + struct ef100_nic_data *nic_data = efx->nic_data; >>> + struct efx_mcdi_iface *mcdi; >>> + >>> + mcdi = efx_mcdi(efx); >>> + spin_lock_bh(&mcdi->iface_lock); >>> + /* Save current MCDI mode to be restored later */ >>> + efx->vdpa_nic->mcdi_mode = mcdi->mode; >>> + efx->mcdi_buf_mode = EFX_BUF_MODE_VDPA; >>> + mcdi->mode = MCDI_MODE_FAIL; >>> + spin_unlock_bh(&mcdi->iface_lock); >>> + efx_nic_free_buffer(efx, &nic_data->mcdi_buf); >>> +} >>> + >>> static struct ef100_vdpa_nic *ef100_vdpa_create(struct efx_nic *efx, >>> const char *dev_name, >>> enum >>> ef100_vdpa_class dev_type, >>> @@ -342,6 +366,7 @@ static struct ef100_vdpa_nic >>> *ef100_vdpa_create(struct efx_nic *efx, >>> for (i = 0; i < EF100_VDPA_MAC_FILTER_NTYPES; i++) >>> vdpa_nic->filters[i].filter_id = >>> EFX_INVALID_FILTER_ID; >>> >>> + unmap_mcdi_buffer(efx); >>> rc = get_net_config(vdpa_nic); >>> if (rc) >>> goto err_put_device; >>> diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.h >>> b/drivers/net/ethernet/sfc/ef100_vdpa.h >>> index 49fb6be04eb3..0913ac2519cb 100644 >>> --- a/drivers/net/ethernet/sfc/ef100_vdpa.h >>> +++ b/drivers/net/ethernet/sfc/ef100_vdpa.h >>> @@ -147,6 +147,7 @@ struct ef100_vdpa_filter { >>> * @status: device status as per VIRTIO spec >>> * @features: negotiated feature bits >>> * @max_queue_pairs: maximum number of queue pairs supported >>> + * @mcdi_mode: MCDI mode at the time of unmapping VF mcdi buffer >>> * @net_config: virtio_net_config data >>> * @vring: vring information of the vDPA device. >>> * @mac_address: mac address of interface associated with this >>> vdpa device >>> @@ -166,6 +167,7 @@ struct ef100_vdpa_nic { >>> u8 status; >>> u64 features; >>> u32 max_queue_pairs; >>> + enum efx_mcdi_mode mcdi_mode; >>> struct virtio_net_config net_config; >>> struct ef100_vdpa_vring_info >>> vring[EF100_VDPA_MAX_QUEUES_PAIRS * 2]; >>> u8 *mac_address; >>> @@ -185,6 +187,7 @@ int ef100_vdpa_add_filter(struct ef100_vdpa_nic >>> *vdpa_nic, >>> int ef100_vdpa_init_vring(struct ef100_vdpa_nic *vdpa_nic, u16 idx); >>> void ef100_vdpa_irq_vectors_free(void *data); >>> int ef100_vdpa_reset(struct vdpa_device *vdev); >>> +int ef100_vdpa_map_mcdi_buffer(struct efx_nic *efx); >>> >>> static inline bool efx_vdpa_is_little_endian(struct ef100_vdpa_nic >>> *vdpa_nic) >>> { >>> diff --git a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c >>> b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c >>> index db86c2693950..c6c9458f0e6f 100644 >>> --- a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c >>> +++ b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c >>> @@ -711,12 +711,47 @@ static int ef100_vdpa_suspend(struct >>> vdpa_device *vdev) >>> mutex_unlock(&vdpa_nic->lock); >>> return rc; >>> } >>> + >>> +int ef100_vdpa_map_mcdi_buffer(struct efx_nic *efx) >>> +{ >> The name of this function is confusing, it's actually map buffer for >> ef100 netdev mode. > > Yeah, I get your point. Actually, I am using the prefix ef100_vdpa_ > for extern functions and this function is just mapping the MCDI > buffer resulting in the name ef100_vdpa_map_mcdi_buffer(). > > I think ef100_vdpa_map_vf_mcdi_buffer() would remove this confusion. > What do you think? Not a native speaker, but for me, it would be better to remove "vdpa" from the name since the buffer is not used by vDPA (if I understand the code correctly). > >> >> Actually, I wonder why not moving this to init/fini of bar config ops >> or if we use aux bus, it should be done during aux driver >> probe/remove. > It makes sense, however we store the last mcdi mode (poll or events) > in vdpa_nic->mcdi_mode to restore later, which requires vdpa_nic to be > allocated that happens later than switching to vdpa bar_config. Ok. Thanks >> Thanks >> >> >>> + struct ef100_nic_data *nic_data = efx->nic_data; >>> + struct efx_mcdi_iface *mcdi; >>> + int rc; >>> + >>> + /* Update VF's MCDI buffer when switching out of vdpa mode */ >>> + rc = efx_nic_alloc_buffer(efx, &nic_data->mcdi_buf, >>> + MCDI_BUF_LEN, GFP_KERNEL); >>> + if (rc) >>> + return rc; >>> + >>> + mcdi = efx_mcdi(efx); >>> + spin_lock_bh(&mcdi->iface_lock); >>> + mcdi->mode = efx->vdpa_nic->mcdi_mode; >>> + efx->mcdi_buf_mode = EFX_BUF_MODE_EF100; >>> + spin_unlock_bh(&mcdi->iface_lock); >>> + >>> + return 0; >>> +} >>> + >>> static void ef100_vdpa_free(struct vdpa_device *vdev) >>> { >>> struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); >>> + int rc; >>> int i; >>> >>> if (vdpa_nic) { >>> + if (vdpa_nic->efx->mcdi_buf_mode == >>> EFX_BUF_MODE_VDPA) { >>> + /* This will only be called via call to >>> put_device() >>> + * on vdpa device creation failure >>> + */ >>> + rc = ef100_vdpa_map_mcdi_buffer(vdpa_nic->efx); >>> + if (rc) { >>> + dev_err(&vdev->dev, >>> + "map_mcdi_buffer failed, >>> err: %d\n", >>> + rc); >>> + } >>> + } >>> + >>> for (i = 0; i < (vdpa_nic->max_queue_pairs * 2); >>> i++) { >>> reset_vring(vdpa_nic, i); >>> if (vdpa_nic->vring[i].vring_ctx) >>> diff --git a/drivers/net/ethernet/sfc/mcdi.h >>> b/drivers/net/ethernet/sfc/mcdi.h >>> index 2c526d2edeb6..bc4de3b4e6f3 100644 >>> --- a/drivers/net/ethernet/sfc/mcdi.h >>> +++ b/drivers/net/ethernet/sfc/mcdi.h >>> @@ -6,6 +6,9 @@ >>> >>> #ifndef EFX_MCDI_H >>> #define EFX_MCDI_H >>> +#include "mcdi_pcol.h" >>> + >>> +#define MCDI_BUF_LEN (8 + MCDI_CTL_SDU_LEN_MAX) >>> >>> /** >>> * enum efx_mcdi_state - MCDI request handling state >>> diff --git a/drivers/net/ethernet/sfc/net_driver.h >>> b/drivers/net/ethernet/sfc/net_driver.h >>> index 948c7a06403a..9cdfeb6ad05a 100644 >>> --- a/drivers/net/ethernet/sfc/net_driver.h >>> +++ b/drivers/net/ethernet/sfc/net_driver.h >>> @@ -848,6 +848,16 @@ enum efx_xdp_tx_queues_mode { >>> >>> struct efx_mae; >>> >>> +/** >>> + * enum efx_buf_alloc_mode - buffer allocation mode >>> + * @EFX_BUF_MODE_EF100: buffer setup in ef100 mode >>> + * @EFX_BUF_MODE_VDPA: buffer setup in vdpa mode >>> + */ >>> +enum efx_buf_alloc_mode { >>> + EFX_BUF_MODE_EF100, >>> + EFX_BUF_MODE_VDPA >>> +}; >>> + >>> /** >>> * struct efx_nic - an Efx NIC >>> * @name: Device name (net device name or bus id before net device >>> registered) >>> @@ -877,6 +887,7 @@ struct efx_mae; >>> * @irq_rx_mod_step_us: Step size for IRQ moderation for RX event >>> queues >>> * @irq_rx_moderation_us: IRQ moderation time for RX event queues >>> * @msg_enable: Log message enable flags >>> + * @mcdi_buf_mode: mcdi buffer allocation mode >>> * @state: Device state number (%STATE_*). Serialised by the >>> rtnl_lock. >>> * @reset_pending: Bitmask for pending resets >>> * @tx_queue: TX DMA queues >>> @@ -1045,6 +1056,7 @@ struct efx_nic { >>> u32 msg_enable; >>> >>> enum nic_state state; >>> + enum efx_buf_alloc_mode mcdi_buf_mode; >>> unsigned long reset_pending; >>> >>> struct efx_channel *channel[EFX_MAX_CHANNELS]; >>> -- >>> 2.30.1 >>> >
diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c index cd9f724a9e64..1bffc1994ed8 100644 --- a/drivers/net/ethernet/sfc/ef100_nic.c +++ b/drivers/net/ethernet/sfc/ef100_nic.c @@ -33,7 +33,6 @@ #define EF100_MAX_VIS 4096 #define EF100_NUM_MCDI_BUFFERS 1 -#define MCDI_BUF_LEN (8 + MCDI_CTL_SDU_LEN_MAX) #define EF100_RESET_PORT ((ETH_RESET_MAC | ETH_RESET_PHY) << ETH_RESET_SHARED_SHIFT) diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.c b/drivers/net/ethernet/sfc/ef100_vdpa.c index 5c9f29f881a6..30ca4ab00175 100644 --- a/drivers/net/ethernet/sfc/ef100_vdpa.c +++ b/drivers/net/ethernet/sfc/ef100_vdpa.c @@ -223,10 +223,19 @@ static int vdpa_allocate_vis(struct efx_nic *efx, unsigned int *allocated_vis) static void ef100_vdpa_delete(struct efx_nic *efx) { struct vdpa_device *vdpa_dev; + int rc; if (efx->vdpa_nic) { vdpa_dev = &efx->vdpa_nic->vdpa_dev; ef100_vdpa_reset(vdpa_dev); + if (efx->mcdi_buf_mode == EFX_BUF_MODE_VDPA) { + rc = ef100_vdpa_map_mcdi_buffer(efx); + if (rc) { + pci_err(efx->pci_dev, + "map_mcdi_buffer failed, err: %d\n", + rc); + } + } /* replace with _vdpa_unregister_device later */ put_device(&vdpa_dev->dev); @@ -276,6 +285,21 @@ static int get_net_config(struct ef100_vdpa_nic *vdpa_nic) return 0; } +static void unmap_mcdi_buffer(struct efx_nic *efx) +{ + struct ef100_nic_data *nic_data = efx->nic_data; + struct efx_mcdi_iface *mcdi; + + mcdi = efx_mcdi(efx); + spin_lock_bh(&mcdi->iface_lock); + /* Save current MCDI mode to be restored later */ + efx->vdpa_nic->mcdi_mode = mcdi->mode; + efx->mcdi_buf_mode = EFX_BUF_MODE_VDPA; + mcdi->mode = MCDI_MODE_FAIL; + spin_unlock_bh(&mcdi->iface_lock); + efx_nic_free_buffer(efx, &nic_data->mcdi_buf); +} + static struct ef100_vdpa_nic *ef100_vdpa_create(struct efx_nic *efx, const char *dev_name, enum ef100_vdpa_class dev_type, @@ -342,6 +366,7 @@ static struct ef100_vdpa_nic *ef100_vdpa_create(struct efx_nic *efx, for (i = 0; i < EF100_VDPA_MAC_FILTER_NTYPES; i++) vdpa_nic->filters[i].filter_id = EFX_INVALID_FILTER_ID; + unmap_mcdi_buffer(efx); rc = get_net_config(vdpa_nic); if (rc) goto err_put_device; diff --git a/drivers/net/ethernet/sfc/ef100_vdpa.h b/drivers/net/ethernet/sfc/ef100_vdpa.h index 49fb6be04eb3..0913ac2519cb 100644 --- a/drivers/net/ethernet/sfc/ef100_vdpa.h +++ b/drivers/net/ethernet/sfc/ef100_vdpa.h @@ -147,6 +147,7 @@ struct ef100_vdpa_filter { * @status: device status as per VIRTIO spec * @features: negotiated feature bits * @max_queue_pairs: maximum number of queue pairs supported + * @mcdi_mode: MCDI mode at the time of unmapping VF mcdi buffer * @net_config: virtio_net_config data * @vring: vring information of the vDPA device. * @mac_address: mac address of interface associated with this vdpa device @@ -166,6 +167,7 @@ struct ef100_vdpa_nic { u8 status; u64 features; u32 max_queue_pairs; + enum efx_mcdi_mode mcdi_mode; struct virtio_net_config net_config; struct ef100_vdpa_vring_info vring[EF100_VDPA_MAX_QUEUES_PAIRS * 2]; u8 *mac_address; @@ -185,6 +187,7 @@ int ef100_vdpa_add_filter(struct ef100_vdpa_nic *vdpa_nic, int ef100_vdpa_init_vring(struct ef100_vdpa_nic *vdpa_nic, u16 idx); void ef100_vdpa_irq_vectors_free(void *data); int ef100_vdpa_reset(struct vdpa_device *vdev); +int ef100_vdpa_map_mcdi_buffer(struct efx_nic *efx); static inline bool efx_vdpa_is_little_endian(struct ef100_vdpa_nic *vdpa_nic) { diff --git a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c index db86c2693950..c6c9458f0e6f 100644 --- a/drivers/net/ethernet/sfc/ef100_vdpa_ops.c +++ b/drivers/net/ethernet/sfc/ef100_vdpa_ops.c @@ -711,12 +711,47 @@ static int ef100_vdpa_suspend(struct vdpa_device *vdev) mutex_unlock(&vdpa_nic->lock); return rc; } + +int ef100_vdpa_map_mcdi_buffer(struct efx_nic *efx) +{ + struct ef100_nic_data *nic_data = efx->nic_data; + struct efx_mcdi_iface *mcdi; + int rc; + + /* Update VF's MCDI buffer when switching out of vdpa mode */ + rc = efx_nic_alloc_buffer(efx, &nic_data->mcdi_buf, + MCDI_BUF_LEN, GFP_KERNEL); + if (rc) + return rc; + + mcdi = efx_mcdi(efx); + spin_lock_bh(&mcdi->iface_lock); + mcdi->mode = efx->vdpa_nic->mcdi_mode; + efx->mcdi_buf_mode = EFX_BUF_MODE_EF100; + spin_unlock_bh(&mcdi->iface_lock); + + return 0; +} + static void ef100_vdpa_free(struct vdpa_device *vdev) { struct ef100_vdpa_nic *vdpa_nic = get_vdpa_nic(vdev); + int rc; int i; if (vdpa_nic) { + if (vdpa_nic->efx->mcdi_buf_mode == EFX_BUF_MODE_VDPA) { + /* This will only be called via call to put_device() + * on vdpa device creation failure + */ + rc = ef100_vdpa_map_mcdi_buffer(vdpa_nic->efx); + if (rc) { + dev_err(&vdev->dev, + "map_mcdi_buffer failed, err: %d\n", + rc); + } + } + for (i = 0; i < (vdpa_nic->max_queue_pairs * 2); i++) { reset_vring(vdpa_nic, i); if (vdpa_nic->vring[i].vring_ctx) diff --git a/drivers/net/ethernet/sfc/mcdi.h b/drivers/net/ethernet/sfc/mcdi.h index 2c526d2edeb6..bc4de3b4e6f3 100644 --- a/drivers/net/ethernet/sfc/mcdi.h +++ b/drivers/net/ethernet/sfc/mcdi.h @@ -6,6 +6,9 @@ #ifndef EFX_MCDI_H #define EFX_MCDI_H +#include "mcdi_pcol.h" + +#define MCDI_BUF_LEN (8 + MCDI_CTL_SDU_LEN_MAX) /** * enum efx_mcdi_state - MCDI request handling state diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h index 948c7a06403a..9cdfeb6ad05a 100644 --- a/drivers/net/ethernet/sfc/net_driver.h +++ b/drivers/net/ethernet/sfc/net_driver.h @@ -848,6 +848,16 @@ enum efx_xdp_tx_queues_mode { struct efx_mae; +/** + * enum efx_buf_alloc_mode - buffer allocation mode + * @EFX_BUF_MODE_EF100: buffer setup in ef100 mode + * @EFX_BUF_MODE_VDPA: buffer setup in vdpa mode + */ +enum efx_buf_alloc_mode { + EFX_BUF_MODE_EF100, + EFX_BUF_MODE_VDPA +}; + /** * struct efx_nic - an Efx NIC * @name: Device name (net device name or bus id before net device registered) @@ -877,6 +887,7 @@ struct efx_mae; * @irq_rx_mod_step_us: Step size for IRQ moderation for RX event queues * @irq_rx_moderation_us: IRQ moderation time for RX event queues * @msg_enable: Log message enable flags + * @mcdi_buf_mode: mcdi buffer allocation mode * @state: Device state number (%STATE_*). Serialised by the rtnl_lock. * @reset_pending: Bitmask for pending resets * @tx_queue: TX DMA queues @@ -1045,6 +1056,7 @@ struct efx_nic { u32 msg_enable; enum nic_state state; + enum efx_buf_alloc_mode mcdi_buf_mode; unsigned long reset_pending; struct efx_channel *channel[EFX_MAX_CHANNELS];
To avoid clash of IOVA range of VF's MCDI DMA buffer with the guest buffer IOVAs, unmap the MCDI buffer when switching to vDPA mode and use PF's IOMMU domain for running VF's MCDI commands. Signed-off-by: Gautam Dawar <gautam.dawar@amd.com> --- drivers/net/ethernet/sfc/ef100_nic.c | 1 - drivers/net/ethernet/sfc/ef100_vdpa.c | 25 ++++++++++++++++ drivers/net/ethernet/sfc/ef100_vdpa.h | 3 ++ drivers/net/ethernet/sfc/ef100_vdpa_ops.c | 35 +++++++++++++++++++++++ drivers/net/ethernet/sfc/mcdi.h | 3 ++ drivers/net/ethernet/sfc/net_driver.h | 12 ++++++++ 6 files changed, 78 insertions(+), 1 deletion(-)