Messages in this thread | | | Date | Wed, 15 Mar 2023 13:11:23 +0800 | Subject | Re: [PATCH net-next v2 01/14] sfc: add function personality support for EF100 devices | From | Jason Wang <> |
| |
在 2023/3/13 19:50, Martin Habets 写道: > On Fri, Mar 10, 2023 at 01:04:14PM +0800, Jason Wang wrote: >> On Tue, Mar 7, 2023 at 7:36 PM Gautam Dawar <gautam.dawar@amd.com> wrote: >>> A function personality defines the location and semantics of >>> registers in the BAR. EF100 NICs allow different personalities >>> of a PCIe function and changing it at run-time. A total of three >>> function personalities are defined as of now: EF100, vDPA and >>> None with EF100 being the default. >>> For now, vDPA net devices can be created on a EF100 virtual >>> function and the VF personality will be changed to vDPA in the >>> process. >>> >>> Co-developed-by: Martin Habets <habetsm.xilinx@gmail.com> >>> Signed-off-by: Martin Habets <habetsm.xilinx@gmail.com> >>> Signed-off-by: Gautam Dawar <gautam.dawar@amd.com> >>> --- >>> drivers/net/ethernet/sfc/ef100.c | 6 +- >>> drivers/net/ethernet/sfc/ef100_nic.c | 98 +++++++++++++++++++++++++++- >>> drivers/net/ethernet/sfc/ef100_nic.h | 11 ++++ >>> 3 files changed, 111 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/sfc/ef100.c b/drivers/net/ethernet/sfc/ef100.c >>> index 71aab3d0480f..c1c69783db7b 100644 >>> --- a/drivers/net/ethernet/sfc/ef100.c >>> +++ b/drivers/net/ethernet/sfc/ef100.c >>> @@ -429,8 +429,7 @@ static void ef100_pci_remove(struct pci_dev *pci_dev) >>> if (!efx) >>> return; >>> >>> - probe_data = container_of(efx, struct efx_probe_data, efx); >>> - ef100_remove_netdev(probe_data); >>> + efx_ef100_set_bar_config(efx, EF100_BAR_CONFIG_NONE); >>> #ifdef CONFIG_SFC_SRIOV >>> efx_fini_struct_tc(efx); >>> #endif >>> @@ -443,6 +442,7 @@ static void ef100_pci_remove(struct pci_dev *pci_dev) >>> pci_disable_pcie_error_reporting(pci_dev); >>> >>> pci_set_drvdata(pci_dev, NULL); >>> + probe_data = container_of(efx, struct efx_probe_data, efx); >>> efx_fini_struct(efx); >>> kfree(probe_data); >>> }; >>> @@ -508,7 +508,7 @@ static int ef100_pci_probe(struct pci_dev *pci_dev, >>> goto fail; >>> >>> efx->state = STATE_PROBED; >>> - rc = ef100_probe_netdev(probe_data); >>> + rc = efx_ef100_set_bar_config(efx, EF100_BAR_CONFIG_EF100); >>> if (rc) >>> goto fail; >>> >>> diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c >>> index 4dc643b0d2db..8cbe5e0f4bdf 100644 >>> --- a/drivers/net/ethernet/sfc/ef100_nic.c >>> +++ b/drivers/net/ethernet/sfc/ef100_nic.c >>> @@ -772,6 +772,99 @@ static int efx_ef100_get_base_mport(struct efx_nic *efx) >>> return 0; >>> } >>> >>> +/* BAR configuration. >>> + * To change BAR configuration, tear down the current configuration (which >>> + * leaves the hardware in the PROBED state), and then initialise the new >>> + * BAR state. >>> + */ >>> +struct ef100_bar_config_ops { >>> + int (*init)(struct efx_probe_data *probe_data); >>> + void (*fini)(struct efx_probe_data *probe_data); >>> +}; >>> + >>> +static const struct ef100_bar_config_ops bar_config_ops[] = { >>> + [EF100_BAR_CONFIG_EF100] = { >>> + .init = ef100_probe_netdev, >>> + .fini = ef100_remove_netdev >>> + }, >>> +#ifdef CONFIG_SFC_VDPA >>> + [EF100_BAR_CONFIG_VDPA] = { >>> + .init = NULL, >>> + .fini = NULL >>> + }, >>> +#endif >>> + [EF100_BAR_CONFIG_NONE] = { >>> + .init = NULL, >>> + .fini = NULL >>> + }, >>> +}; >> This looks more like a mini bus implementation. I wonder if we can >> reuse an auxiliary bus here which is more user friendly for management >> tools. > When we were in the design phase of vDPA for EF100 it was still called > virtbus, and the virtbus discussion was in full swing at that time. > We could not afford to add risk to the project by depending on it, as > it might not have been merged at all.
Right.
> If we were doing the same design now I would definitely consider using > the auxiliary bus. > > Martin
But it's not late to do the change now. Auxiliary bus has been used by a lot of devices (even with vDPA device). The change looks not too complicated.
This looks more scalable and convenient for management layer.
Thanks
| |