linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
@ 2022-07-07 13:07 Martin Habets
  2022-07-07 13:07 ` [PATCH net-next v2 1/2] " Martin Habets
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Martin Habets @ 2022-07-07 13:07 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet
  Cc: netdev, ecree.xilinx, linux-pci, virtualization

The EF100 NICs allow for different register layouts of a PCI memory BAR.
This series provides the framework to switch this layout at runtime.

Subsequent patch series will use this to add support for vDPA.

v2: Include PCI and virtio maintainers.
---

Martin Habets (2):
      sfc: Add EF100 BAR config support
      sfc: Implement change of BAR configuration


 drivers/net/ethernet/sfc/ef100_nic.c |   80 ++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/ef100_nic.h |    6 +++
 2 files changed, 86 insertions(+)

--
Martin Habets <habetsm.xilinx@gmail.com>



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH net-next v2 1/2] sfc: Add EF100 BAR config support
  2022-07-07 13:07 [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support Martin Habets
@ 2022-07-07 13:07 ` Martin Habets
  2022-07-07 13:07 ` [PATCH net-next v2 2/2] sfc: Implement change of BAR configuration Martin Habets
  2022-07-07 15:55 ` [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support Bjorn Helgaas
  2 siblings, 0 replies; 14+ messages in thread
From: Martin Habets @ 2022-07-07 13:07 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet
  Cc: netdev, ecree.xilinx, linux-pci, virtualization

Provide a "bar_config" file in the sysfs directory of the PCI device.
This can be used to switch the PCI BAR layout to/from vDPA mode.

Signed-off-by: Martin Habets <habetsm.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/ef100_nic.c |   45 ++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/ef100_nic.h |    6 +++++
 2 files changed, 51 insertions(+)

diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
index f89e695cf8ac..218db3cb31eb 100644
--- a/drivers/net/ethernet/sfc/ef100_nic.c
+++ b/drivers/net/ethernet/sfc/ef100_nic.c
@@ -704,6 +704,49 @@ static unsigned int efx_ef100_recycle_ring_size(const struct efx_nic *efx)
 	return 10 * EFX_RECYCLE_RING_SIZE_10G;
 }
 
+/* BAR configuration */
+static ssize_t bar_config_show(struct device *dev,
+			       struct device_attribute *attr, char *buf_out)
+{
+	struct efx_nic *efx = pci_get_drvdata(to_pci_dev(dev));
+	struct ef100_nic_data *nic_data = efx->nic_data;
+
+	switch (nic_data->bar_config) {
+	case EF100_BAR_CONFIG_EF100:
+		sprintf(buf_out, "EF100\n");
+		break;
+	case EF100_BAR_CONFIG_VDPA:
+		sprintf(buf_out, "vDPA\n");
+		break;
+	default: /* this should not happen */
+		WARN_ON_ONCE(1);
+		return 0;
+	}
+
+	return strlen(buf_out);
+}
+
+static ssize_t bar_config_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct efx_nic *efx = pci_get_drvdata(to_pci_dev(dev));
+	struct ef100_nic_data *nic_data = efx->nic_data;
+	enum ef100_bar_config new_config;
+
+	if (!strncasecmp(buf, "ef100", min_t(size_t, count, 5)))
+		new_config = EF100_BAR_CONFIG_EF100;
+	else if (!strncasecmp(buf, "vdpa", min_t(size_t, count, 4)))
+		new_config = EF100_BAR_CONFIG_VDPA;
+	else
+		return -EIO;
+
+	nic_data->bar_config = new_config;
+	return count;
+}
+
+static DEVICE_ATTR_RW(bar_config);
+
 static int compare_versions(const char *a, const char *b)
 {
 	int a_major, a_minor, a_point, a_patch;
@@ -1039,6 +1082,7 @@ static int ef100_probe_main(struct efx_nic *efx)
 		goto fail;
 	}
 
+	device_create_file(&efx->pci_dev->dev, &dev_attr_bar_config);
 	return 0;
 fail:
 	return rc;
@@ -1072,6 +1116,7 @@ void ef100_remove(struct efx_nic *efx)
 {
 	struct ef100_nic_data *nic_data = efx->nic_data;
 
+	device_remove_file(&efx->pci_dev->dev, &dev_attr_bar_config);
 	efx_mcdi_detach(efx);
 	efx_mcdi_fini(efx);
 	if (nic_data)
diff --git a/drivers/net/ethernet/sfc/ef100_nic.h b/drivers/net/ethernet/sfc/ef100_nic.h
index 744dbbdb4adc..64b82cae6b54 100644
--- a/drivers/net/ethernet/sfc/ef100_nic.h
+++ b/drivers/net/ethernet/sfc/ef100_nic.h
@@ -61,6 +61,11 @@ enum {
 	EF100_STAT_COUNT
 };
 
+enum ef100_bar_config {
+	EF100_BAR_CONFIG_EF100,
+	EF100_BAR_CONFIG_VDPA,
+};
+
 struct ef100_nic_data {
 	struct efx_nic *efx;
 	struct efx_buffer mcdi_buf;
@@ -70,6 +75,7 @@ struct ef100_nic_data {
 	unsigned int pf_index;
 	u16 warm_boot_count;
 	u8 port_id[ETH_ALEN];
+	enum ef100_bar_config bar_config;
 	DECLARE_BITMAP(evq_phases, EFX_MAX_CHANNELS);
 	u64 stats[EF100_STAT_COUNT];
 	u16 tso_max_hdr_len;



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v2 2/2] sfc: Implement change of BAR configuration
  2022-07-07 13:07 [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support Martin Habets
  2022-07-07 13:07 ` [PATCH net-next v2 1/2] " Martin Habets
@ 2022-07-07 13:07 ` Martin Habets
  2022-07-07 15:55 ` [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support Bjorn Helgaas
  2 siblings, 0 replies; 14+ messages in thread
From: Martin Habets @ 2022-07-07 13:07 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet
  Cc: netdev, ecree.xilinx, linux-pci, virtualization

Placeholders are added for vDPA. These will be assigned with
a later patch.

Signed-off-by: Martin Habets <habetsm.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/ef100_nic.c |   39 ++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
index 218db3cb31eb..ce4b7b4e705e 100644
--- a/drivers/net/ethernet/sfc/ef100_nic.c
+++ b/drivers/net/ethernet/sfc/ef100_nic.c
@@ -704,7 +704,25 @@ static unsigned int efx_ef100_recycle_ring_size(const struct efx_nic *efx)
 	return 10 * EFX_RECYCLE_RING_SIZE_10G;
 }
 
-/* BAR configuration */
+/* BAR configuration.
+ * To change BAR configuration we tear down the current configuration (which
+ * leaves the hardware in the PROBED state), and then initialise the new
+ * BAR state.
+ */
+static struct {
+	int (*init)(struct efx_probe_data *probe_data);
+	void (*fini)(struct efx_probe_data *probe_data);
+} bar_config_std[] = {
+	[EF100_BAR_CONFIG_EF100] = {
+		.init = ef100_probe_netdev,
+		.fini = ef100_remove_netdev
+	},
+	[EF100_BAR_CONFIG_VDPA] = {
+		.init = NULL,	/* TODO: assign these */
+		.fini = NULL
+	},
+};
+
 static ssize_t bar_config_show(struct device *dev,
 			       struct device_attribute *attr, char *buf_out)
 {
@@ -732,7 +750,9 @@ static ssize_t bar_config_store(struct device *dev,
 {
 	struct efx_nic *efx = pci_get_drvdata(to_pci_dev(dev));
 	struct ef100_nic_data *nic_data = efx->nic_data;
-	enum ef100_bar_config new_config;
+	enum ef100_bar_config new_config, old_config;
+	struct efx_probe_data *probe_data;
+	int rc;
 
 	if (!strncasecmp(buf, "ef100", min_t(size_t, count, 5)))
 		new_config = EF100_BAR_CONFIG_EF100;
@@ -741,7 +761,22 @@ static ssize_t bar_config_store(struct device *dev,
 	else
 		return -EIO;
 
+	old_config = nic_data->bar_config;
+	if (new_config == old_config)
+		return count;
+
+	probe_data = container_of(efx, struct efx_probe_data, efx);
+	if (bar_config_std[old_config].fini)
+		bar_config_std[old_config].fini(probe_data);
+
 	nic_data->bar_config = new_config;
+	if (bar_config_std[new_config].init) {
+		rc = bar_config_std[new_config].init(probe_data);
+		if (rc)
+			return rc;
+	}
+
+	pci_info(efx->pci_dev, "BAR configuration changed to %s", buf);
 	return count;
 }
 



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
  2022-07-07 13:07 [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support Martin Habets
  2022-07-07 13:07 ` [PATCH net-next v2 1/2] " Martin Habets
  2022-07-07 13:07 ` [PATCH net-next v2 2/2] sfc: Implement change of BAR configuration Martin Habets
@ 2022-07-07 15:55 ` Bjorn Helgaas
  2022-07-11 13:38   ` Martin Habets
  2 siblings, 1 reply; 14+ messages in thread
From: Bjorn Helgaas @ 2022-07-07 15:55 UTC (permalink / raw)
  To: Martin Habets
  Cc: davem, kuba, pabeni, edumazet, netdev, ecree.xilinx, linux-pci,
	virtualization

On Thu, Jul 07, 2022 at 02:07:07PM +0100, Martin Habets wrote:
> The EF100 NICs allow for different register layouts of a PCI memory BAR.
> This series provides the framework to switch this layout at runtime.
> 
> Subsequent patch series will use this to add support for vDPA.

Normally drivers rely on the PCI Vendor and Device ID to learn the
number of BARs and their layouts.  I guess this series implies that
doesn't work on this device?  And the user needs to manually specify
what kind of device this is?

I'm confused about how this is supposed to work.  What if the driver
is built-in and claims a device before the user can specify the
register layout?  What if the user specifies the wrong layout and the
driver writes to the wrong registers?

> ---
> 
> Martin Habets (2):
>       sfc: Add EF100 BAR config support
>       sfc: Implement change of BAR configuration
> 
> 
>  drivers/net/ethernet/sfc/ef100_nic.c |   80 ++++++++++++++++++++++++++++++++++
>  drivers/net/ethernet/sfc/ef100_nic.h |    6 +++
>  2 files changed, 86 insertions(+)
> 
> --
> Martin Habets <habetsm.xilinx@gmail.com>
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
  2022-07-07 15:55 ` [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support Bjorn Helgaas
@ 2022-07-11 13:38   ` Martin Habets
  2022-07-11 18:48     ` Jakub Kicinski
  2022-07-11 22:00     ` Bjorn Helgaas
  0 siblings, 2 replies; 14+ messages in thread
From: Martin Habets @ 2022-07-11 13:38 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: davem, kuba, pabeni, edumazet, netdev, ecree.xilinx, linux-pci,
	virtualization

On Thu, Jul 07, 2022 at 10:55:00AM -0500, Bjorn Helgaas wrote:
> On Thu, Jul 07, 2022 at 02:07:07PM +0100, Martin Habets wrote:
> > The EF100 NICs allow for different register layouts of a PCI memory BAR.
> > This series provides the framework to switch this layout at runtime.
> > 
> > Subsequent patch series will use this to add support for vDPA.
> 
> Normally drivers rely on the PCI Vendor and Device ID to learn the
> number of BARs and their layouts.  I guess this series implies that
> doesn't work on this device?  And the user needs to manually specify
> what kind of device this is?

When a new PCI device is added (like a VF) it always starts of with
the register layout for an EF100 network device. This is hardcoded,
i.e. it cannot be customised.
The layout can be changed after bootup, and only after the sfc driver has
bound to the device.
The PCI Vendor and Device ID do not change when the layout is changed.

For vDPA specifically we return the Xilinx PCI Vendor and our device ID
to the vDPA framework via struct vdpa_config_opts.

> I'm confused about how this is supposed to work.  What if the driver
> is built-in and claims a device before the user can specify the
> register layout?

The bar_config file will only exist once the sfc driver has bound to
the device. So in fact we count on that driver getting loaded.
When a new value is written to bar_config it is the sfc driver that
instructs the NIC to change the register layout.

> What if the user specifies the wrong layout and the
> driver writes to the wrong registers?

We have specific hardware and driver requirements for this sort of
situation. For example, the register layouts must have some common
registers (to ensure some compatibility).
A layout that is too different will require a separate device ID.
A driver that writes to the wrong register is a bug.

Maybe the name "bar_config" is causing most of the confusion here.
Internally we also talk about "function profiles" or "personalities",
but we thought such a name would be too vague.

Martin

> > ---
> > 
> > Martin Habets (2):
> >       sfc: Add EF100 BAR config support
> >       sfc: Implement change of BAR configuration
> > 
> > 
> >  drivers/net/ethernet/sfc/ef100_nic.c |   80 ++++++++++++++++++++++++++++++++++
> >  drivers/net/ethernet/sfc/ef100_nic.h |    6 +++
> >  2 files changed, 86 insertions(+)
> > 
> > --
> > Martin Habets <habetsm.xilinx@gmail.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
  2022-07-11 13:38   ` Martin Habets
@ 2022-07-11 18:48     ` Jakub Kicinski
  2022-07-13  8:40       ` Martin Habets
  2022-07-11 22:00     ` Bjorn Helgaas
  1 sibling, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2022-07-11 18:48 UTC (permalink / raw)
  To: Martin Habets
  Cc: Bjorn Helgaas, davem, pabeni, edumazet, netdev, ecree.xilinx,
	linux-pci, virtualization

On Mon, 11 Jul 2022 14:38:54 +0100 Martin Habets wrote:
> > Normally drivers rely on the PCI Vendor and Device ID to learn the
> > number of BARs and their layouts.  I guess this series implies that
> > doesn't work on this device?  And the user needs to manually specify
> > what kind of device this is?  
> 
> When a new PCI device is added (like a VF) it always starts of with
> the register layout for an EF100 network device. This is hardcoded,
> i.e. it cannot be customised.
> The layout can be changed after bootup, and only after the sfc driver has
> bound to the device.
> The PCI Vendor and Device ID do not change when the layout is changed.
> 
> For vDPA specifically we return the Xilinx PCI Vendor and our device ID
> to the vDPA framework via struct vdpa_config_opts.

So it's switching between ethernet and vdpa? Isn't there a general
problem for configuring vdpa capabilities (net vs storage etc) and
shouldn't we seek to solve your BAR format switch in a similar fashion
rather than adding PCI device attrs, which I believe is not done for
anything vDPA-related?

> > I'm confused about how this is supposed to work.  What if the driver
> > is built-in and claims a device before the user can specify the
> > register layout?  
> 
> The bar_config file will only exist once the sfc driver has bound to
> the device. So in fact we count on that driver getting loaded.
> When a new value is written to bar_config it is the sfc driver that
> instructs the NIC to change the register layout.

When you say "driver bound" you mean the VF driver, right?

> > What if the user specifies the wrong layout and the
> > driver writes to the wrong registers?  
> 
> We have specific hardware and driver requirements for this sort of
> situation. For example, the register layouts must have some common
> registers (to ensure some compatibility).
> A layout that is too different will require a separate device ID.
> A driver that writes to the wrong register is a bug.
> 
> Maybe the name "bar_config" is causing most of the confusion here.
> Internally we also talk about "function profiles" or "personalities",
> but we thought such a name would be too vague.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
  2022-07-11 13:38   ` Martin Habets
  2022-07-11 18:48     ` Jakub Kicinski
@ 2022-07-11 22:00     ` Bjorn Helgaas
  1 sibling, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2022-07-11 22:00 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet, netdev, ecree.xilinx, linux-pci,
	virtualization

On Mon, Jul 11, 2022 at 02:38:54PM +0100, Martin Habets wrote:
> On Thu, Jul 07, 2022 at 10:55:00AM -0500, Bjorn Helgaas wrote:
> > On Thu, Jul 07, 2022 at 02:07:07PM +0100, Martin Habets wrote:
> > > The EF100 NICs allow for different register layouts of a PCI memory BAR.
> > > This series provides the framework to switch this layout at runtime.
> > > 
> > > Subsequent patch series will use this to add support for vDPA.
> > 
> > Normally drivers rely on the PCI Vendor and Device ID to learn the
> > number of BARs and their layouts.  I guess this series implies that
> > doesn't work on this device?  And the user needs to manually specify
> > what kind of device this is?
> 
> When a new PCI device is added (like a VF) it always starts of with
> the register layout for an EF100 network device. This is hardcoded,
> i.e. it cannot be customised.
> The layout can be changed after bootup, and only after the sfc driver has
> bound to the device.
> The PCI Vendor and Device ID do not change when the layout is changed.
> 
> For vDPA specifically we return the Xilinx PCI Vendor and our device ID
> to the vDPA framework via struct vdpa_config_opts.
> 
> > I'm confused about how this is supposed to work.  What if the driver
> > is built-in and claims a device before the user can specify the
> > register layout?
> 
> The bar_config file will only exist once the sfc driver has bound to
> the device. So in fact we count on that driver getting loaded.
> When a new value is written to bar_config it is the sfc driver that
> instructs the NIC to change the register layout.
>
> > What if the user specifies the wrong layout and the
> > driver writes to the wrong registers?
> 
> We have specific hardware and driver requirements for this sort of
> situation. For example, the register layouts must have some common
> registers (to ensure some compatibility).

Obviously we have to deal with the hardware as it exists, but it seems
like a hardware design problem that you can change the register
layout but the change is not detectable via those common registers.  

Anyway, it seems weird to me, but doesn't affect the PCI core and I
won't stand in your way ;)

> A layout that is too different will require a separate device ID.
> A driver that writes to the wrong register is a bug.
> 
> Maybe the name "bar_config" is causing most of the confusion here.
> Internally we also talk about "function profiles" or "personalities",
> but we thought such a name would be too vague.
> 
> Martin
> 
> > > ---
> > > 
> > > Martin Habets (2):
> > >       sfc: Add EF100 BAR config support
> > >       sfc: Implement change of BAR configuration
> > > 
> > > 
> > >  drivers/net/ethernet/sfc/ef100_nic.c |   80 ++++++++++++++++++++++++++++++++++
> > >  drivers/net/ethernet/sfc/ef100_nic.h |    6 +++
> > >  2 files changed, 86 insertions(+)
> > > 
> > > --
> > > Martin Habets <habetsm.xilinx@gmail.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
  2022-07-11 18:48     ` Jakub Kicinski
@ 2022-07-13  8:40       ` Martin Habets
  2022-07-13 18:48         ` Jakub Kicinski
  0 siblings, 1 reply; 14+ messages in thread
From: Martin Habets @ 2022-07-13  8:40 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Bjorn Helgaas, davem, pabeni, edumazet, netdev, ecree.xilinx,
	linux-pci, virtualization

On Mon, Jul 11, 2022 at 11:48:06AM -0700, Jakub Kicinski wrote:
> On Mon, 11 Jul 2022 14:38:54 +0100 Martin Habets wrote:
> > > Normally drivers rely on the PCI Vendor and Device ID to learn the
> > > number of BARs and their layouts.  I guess this series implies that
> > > doesn't work on this device?  And the user needs to manually specify
> > > what kind of device this is?  
> > 
> > When a new PCI device is added (like a VF) it always starts of with
> > the register layout for an EF100 network device. This is hardcoded,
> > i.e. it cannot be customised.
> > The layout can be changed after bootup, and only after the sfc driver has
> > bound to the device.
> > The PCI Vendor and Device ID do not change when the layout is changed.
> > 
> > For vDPA specifically we return the Xilinx PCI Vendor and our device ID
> > to the vDPA framework via struct vdpa_config_opts.
> 
> So it's switching between ethernet and vdpa? Isn't there a general
> problem for configuring vdpa capabilities (net vs storage etc) and
> shouldn't we seek to solve your BAR format switch in a similar fashion
> rather than adding PCI device attrs, which I believe is not done for
> anything vDPA-related?

The initial support will be for vdpa net. vdpa block and RDMA will follow
later, and we also need to consider FPGA management.

When it comes to vDPA there is a "vdpa" tool that we intend to support.
This comes into play after we've switched a device into vdpa mode (using
this new file).
For a network device there is also "devlink" to consider. That could be used
to switch a device into vdpa mode, but it cannot be used to switch it
back (there is no netdev to operate on).
My current understanding is that we won't have this issue for RDMA.
For FPGA management there is no general configuration tool, just what
fpga_mgr exposes (drivers/fpga). We intend to remove the special PF
devices we have for this (PCI space is valuable), and use the normal
network device in stead. I can give more details on this if you want.
Worst case a special BAR config would be needed for this, but if needed I
expect we can restrict this to the NIC provisioning stage.

So there is a general problem I think. The solution here is something at
lower level, which is PCI in this case.
Another solution would be a proprietary tool, something we are off course
keen to avoid.

> > > I'm confused about how this is supposed to work.  What if the driver
> > > is built-in and claims a device before the user can specify the
> > > register layout?  
> > 
> > The bar_config file will only exist once the sfc driver has bound to
> > the device. So in fact we count on that driver getting loaded.
> > When a new value is written to bar_config it is the sfc driver that
> > instructs the NIC to change the register layout.
> 
> When you say "driver bound" you mean the VF driver, right?

For a VF device yes it's the VF driver.
For a PF device it would be the PF driver.

Martin

> > > What if the user specifies the wrong layout and the
> > > driver writes to the wrong registers?  
> > 
> > We have specific hardware and driver requirements for this sort of
> > situation. For example, the register layouts must have some common
> > registers (to ensure some compatibility).
> > A layout that is too different will require a separate device ID.
> > A driver that writes to the wrong register is a bug.
> > 
> > Maybe the name "bar_config" is causing most of the confusion here.
> > Internally we also talk about "function profiles" or "personalities",
> > but we thought such a name would be too vague.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
  2022-07-13  8:40       ` Martin Habets
@ 2022-07-13 18:48         ` Jakub Kicinski
  2022-07-14 11:32           ` Martin Habets
  0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2022-07-13 18:48 UTC (permalink / raw)
  To: Martin Habets
  Cc: Bjorn Helgaas, davem, pabeni, edumazet, netdev, ecree.xilinx,
	linux-pci, virtualization

On Wed, 13 Jul 2022 09:40:01 +0100 Martin Habets wrote:
> > So it's switching between ethernet and vdpa? Isn't there a general
> > problem for configuring vdpa capabilities (net vs storage etc) and
> > shouldn't we seek to solve your BAR format switch in a similar fashion
> > rather than adding PCI device attrs, which I believe is not done for
> > anything vDPA-related?  
> 
> The initial support will be for vdpa net. vdpa block and RDMA will follow
> later, and we also need to consider FPGA management.
> 
> When it comes to vDPA there is a "vdpa" tool that we intend to support.
> This comes into play after we've switched a device into vdpa mode (using
> this new file).
> For a network device there is also "devlink" to consider. That could be used
> to switch a device into vdpa mode, but it cannot be used to switch it
> back (there is no netdev to operate on).
> My current understanding is that we won't have this issue for RDMA.
> For FPGA management there is no general configuration tool, just what
> fpga_mgr exposes (drivers/fpga). We intend to remove the special PF
> devices we have for this (PCI space is valuable), and use the normal
> network device in stead. I can give more details on this if you want.
> Worst case a special BAR config would be needed for this, but if needed I
> expect we can restrict this to the NIC provisioning stage.
> 
> So there is a general problem I think. The solution here is something at
> lower level, which is PCI in this case.
> Another solution would be a proprietary tool, something we are off course
> keen to avoid.

Okay. Indeed, we could easily bolt something onto devlink, I'd think
but I don't know the space enough to push for one solution over
another. 

Please try to document the problem and the solution... somewhere, tho.
Otherwise the chances that the next vendor with this problem follows
the same approach fall from low to none.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
  2022-07-13 18:48         ` Jakub Kicinski
@ 2022-07-14 11:32           ` Martin Habets
  2022-07-14 16:05             ` Jakub Kicinski
  0 siblings, 1 reply; 14+ messages in thread
From: Martin Habets @ 2022-07-14 11:32 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Bjorn Helgaas, davem, pabeni, edumazet, netdev, ecree.xilinx,
	linux-pci, virtualization

On Wed, Jul 13, 2022 at 11:48:04AM -0700, Jakub Kicinski wrote:
> On Wed, 13 Jul 2022 09:40:01 +0100 Martin Habets wrote:
> > > So it's switching between ethernet and vdpa? Isn't there a general
> > > problem for configuring vdpa capabilities (net vs storage etc) and
> > > shouldn't we seek to solve your BAR format switch in a similar fashion
> > > rather than adding PCI device attrs, which I believe is not done for
> > > anything vDPA-related?  
> > 
> > The initial support will be for vdpa net. vdpa block and RDMA will follow
> > later, and we also need to consider FPGA management.
> > 
> > When it comes to vDPA there is a "vdpa" tool that we intend to support.
> > This comes into play after we've switched a device into vdpa mode (using
> > this new file).
> > For a network device there is also "devlink" to consider. That could be used
> > to switch a device into vdpa mode, but it cannot be used to switch it
> > back (there is no netdev to operate on).
> > My current understanding is that we won't have this issue for RDMA.
> > For FPGA management there is no general configuration tool, just what
> > fpga_mgr exposes (drivers/fpga). We intend to remove the special PF
> > devices we have for this (PCI space is valuable), and use the normal
> > network device in stead. I can give more details on this if you want.
> > Worst case a special BAR config would be needed for this, but if needed I
> > expect we can restrict this to the NIC provisioning stage.
> > 
> > So there is a general problem I think. The solution here is something at
> > lower level, which is PCI in this case.
> > Another solution would be a proprietary tool, something we are off course
> > keen to avoid.
> 
> Okay. Indeed, we could easily bolt something onto devlink, I'd think
> but I don't know the space enough to push for one solution over
> another. 
> 
> Please try to document the problem and the solution... somewhere, tho.
> Otherwise the chances that the next vendor with this problem follows
> the same approach fall from low to none.

Yeah, good point. The obvious thing would be to create a
 Documentation/networking/device_drivers/ethernet/sfc/sfc/rst
Is that generic enough for other vendors to find out, or there a better place?
I can do a follow-up patch for this.

Martin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
  2022-07-14 11:32           ` Martin Habets
@ 2022-07-14 16:05             ` Jakub Kicinski
  2022-08-03  7:57               ` Jason Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2022-07-14 16:05 UTC (permalink / raw)
  To: Martin Habets
  Cc: Bjorn Helgaas, davem, pabeni, edumazet, netdev, ecree.xilinx,
	linux-pci, virtualization

On Thu, 14 Jul 2022 12:32:12 +0100 Martin Habets wrote:
> > Okay. Indeed, we could easily bolt something onto devlink, I'd think
> > but I don't know the space enough to push for one solution over
> > another. 
> > 
> > Please try to document the problem and the solution... somewhere, tho.
> > Otherwise the chances that the next vendor with this problem follows
> > the same approach fall from low to none.  
> 
> Yeah, good point. The obvious thing would be to create a
>  Documentation/networking/device_drivers/ethernet/sfc/sfc/rst
> Is that generic enough for other vendors to find out, or there a better place?

Documentation/vdpa.rst ? I don't see any kernel level notes on
implementing vDPA perhaps virt folks can suggest something.
I don't think people would be looking into driver-specific docs
when trying to implement an interface, so sfc is not a great option
IMHO.

> I can do a follow-up patch for this.

Let's make it part of the same series.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
  2022-07-14 16:05             ` Jakub Kicinski
@ 2022-08-03  7:57               ` Jason Wang
  2022-08-12  9:38                 ` Martin Habets
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Wang @ 2022-08-03  7:57 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Martin Habets, Bjorn Helgaas, davem, Paolo Abeni, Eric Dumazet,
	netdev, ecree.xilinx, linux-pci, virtualization, mst

On Fri, Jul 15, 2022 at 12:05 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 14 Jul 2022 12:32:12 +0100 Martin Habets wrote:
> > > Okay. Indeed, we could easily bolt something onto devlink, I'd think
> > > but I don't know the space enough to push for one solution over
> > > another.
> > >
> > > Please try to document the problem and the solution... somewhere, tho.
> > > Otherwise the chances that the next vendor with this problem follows
> > > the same approach fall from low to none.
> >
> > Yeah, good point. The obvious thing would be to create a
> >  Documentation/networking/device_drivers/ethernet/sfc/sfc/rst
> > Is that generic enough for other vendors to find out, or there a better place?
>
> Documentation/vdpa.rst ? I don't see any kernel level notes on
> implementing vDPA perhaps virt folks can suggest something.

Not sure, since it's not a vDPA general thing but a vendor/parent
specific thing.

Or maybe Documentation/vdpa/sfc ?

Thanks

> I don't think people would be looking into driver-specific docs
> when trying to implement an interface, so sfc is not a great option
> IMHO.
>
> > I can do a follow-up patch for this.
>
> Let's make it part of the same series.
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
  2022-08-03  7:57               ` Jason Wang
@ 2022-08-12  9:38                 ` Martin Habets
  2022-08-12 19:18                   ` Jakub Kicinski
  0 siblings, 1 reply; 14+ messages in thread
From: Martin Habets @ 2022-08-12  9:38 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Bjorn Helgaas, davem, Paolo Abeni, Eric Dumazet, netdev,
	ecree.xilinx, linux-pci, virtualization, mst

FYI, during my holiday my colleagues found a way to use the vdpa tool for this.
That means we should not need this series, at least for vDPA.
So we can drop this series.

Thanks,
Martin

On Wed, Aug 03, 2022 at 03:57:34PM +0800, Jason Wang wrote:
> On Fri, Jul 15, 2022 at 12:05 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Thu, 14 Jul 2022 12:32:12 +0100 Martin Habets wrote:
> > > > Okay. Indeed, we could easily bolt something onto devlink, I'd think
> > > > but I don't know the space enough to push for one solution over
> > > > another.
> > > >
> > > > Please try to document the problem and the solution... somewhere, tho.
> > > > Otherwise the chances that the next vendor with this problem follows
> > > > the same approach fall from low to none.
> > >
> > > Yeah, good point. The obvious thing would be to create a
> > >  Documentation/networking/device_drivers/ethernet/sfc/sfc/rst
> > > Is that generic enough for other vendors to find out, or there a better place?
> >
> > Documentation/vdpa.rst ? I don't see any kernel level notes on
> > implementing vDPA perhaps virt folks can suggest something.
> 
> Not sure, since it's not a vDPA general thing but a vendor/parent
> specific thing.
> 
> Or maybe Documentation/vdpa/sfc ?
> 
> Thanks
> 
> > I don't think people would be looking into driver-specific docs
> > when trying to implement an interface, so sfc is not a great option
> > IMHO.
> >
> > > I can do a follow-up patch for this.
> >
> > Let's make it part of the same series.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support
  2022-08-12  9:38                 ` Martin Habets
@ 2022-08-12 19:18                   ` Jakub Kicinski
  0 siblings, 0 replies; 14+ messages in thread
From: Jakub Kicinski @ 2022-08-12 19:18 UTC (permalink / raw)
  To: Martin Habets
  Cc: Bjorn Helgaas, davem, Paolo Abeni, Eric Dumazet, netdev,
	ecree.xilinx, linux-pci, virtualization, mst

On Fri, 12 Aug 2022 10:38:35 +0100 Martin Habets wrote:
> FYI, during my holiday my colleagues found a way to use the vdpa tool for this.
> That means we should not need this series, at least for vDPA.
> So we can drop this series.

🎉 small victories :)

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-08-12 19:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-07 13:07 [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support Martin Habets
2022-07-07 13:07 ` [PATCH net-next v2 1/2] " Martin Habets
2022-07-07 13:07 ` [PATCH net-next v2 2/2] sfc: Implement change of BAR configuration Martin Habets
2022-07-07 15:55 ` [PATCH net-next v2 0/2] sfc: Add EF100 BAR config support Bjorn Helgaas
2022-07-11 13:38   ` Martin Habets
2022-07-11 18:48     ` Jakub Kicinski
2022-07-13  8:40       ` Martin Habets
2022-07-13 18:48         ` Jakub Kicinski
2022-07-14 11:32           ` Martin Habets
2022-07-14 16:05             ` Jakub Kicinski
2022-08-03  7:57               ` Jason Wang
2022-08-12  9:38                 ` Martin Habets
2022-08-12 19:18                   ` Jakub Kicinski
2022-07-11 22:00     ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).