linux-fpga.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 00/12] add FPGA hotplug manager driver
@ 2023-01-19  1:35 Tianfei Zhang
  2023-01-19  1:35 ` [PATCH v1 01/12] PCI: hotplug: add new callbacks on hotplug_slot_ops Tianfei Zhang
                   ` (13 more replies)
  0 siblings, 14 replies; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

This patchset introduces the FPGA hotplug manager (fpgahp) driver which 
has been verified on the Intel N3000 card.

When a PCIe-based FPGA card is reprogrammed, it temporarily disappears
from the PCIe bus. This needs to be managed to avoid PCIe errors and to
reprobe the device after reprogramming.

To change the FPGA image, the kernel burns a new image into the flash on
the card, and then triggers the card BMC to load the new image into FPGA.
A new FPGA hotplug manager driver is introduced that leverages the PCIe
hotplug framework to trigger and manage the update of the FPGA image,
including the disappearance and reappearance of the card on the PCIe bus.
The fpgahp driver uses APIs from the pciehp driver. Two new operation
callbacks are defined in hotplug_slot_ops:

  - available_images: Optional: available FPGA images
  - image_load: Optional: trigger the FPGA to load a new image


The process of reprogramming an FPGA card begins by removing all devices
associated with the card that are not required for the reprogramming of
the card. This includes PCIe devices (PFs and VFs) associated with the
card as well as any other types of devices (platform, etc.) defined within
the FPGA. The remaining devices are referred to here as "reserved" devices.
After triggering the update of the FPGA card, the reserved devices are also
removed.

The complete process for reprogramming the FPGA are:
    1. remove all PFs and VFs except for PF0 (reserved).
    2. remove all non-reserved devices of PF0.
    3. trigger FPGA card to do the image update.
    4. disable the link of the hotplug bridge.
    5. remove all reserved devices under hotplug bridge.
    6. wait for image reload done via BMC, e.g. 10s.
    7. re-enable the link of hotplug bridge
    8. enumerate PCI devices below the hotplug bridge

usage example:
[root@localhost]# cd /sys/bus/pci/slot/X-X/

Get the available images.
[root@localhost 2-1]# cat available_images
bmc_factory bmc_user retimer_fw

Load the request images for FPGA Card, for example load the BMC user image:
[root@localhost 2-1]# echo bmc_user > image_load

Tianfei Zhang (12):
  PCI: hotplug: add new callbacks on hotplug_slot_ops
  PCI: hotplug: expose APIs from pciehp driver
  PCI: hotplug: add and expose link disable API
  PCI: hotplug: add FPGA PCI hotplug manager driver
  fpga: dfl: register dfl-pci device into fpgahph driver
  driver core: expose device_is_ancestor() API
  PCI: hotplug: add register/unregister function for BMC device
  fpga: m10bmc-sec: register BMC device into fpgahp driver
  fpga: dfl: remove non-reserved devices
  PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp
  fpga: m10bmc-sec: add m10bmc_sec_retimer_load callback
  Documentation: fpga: add description of fpgahp driver

 Documentation/ABI/testing/sysfs-driver-fpgahp |  21 +
 Documentation/fpga/fpgahp.rst                 |  29 +
 Documentation/fpga/index.rst                  |   1 +
 MAINTAINERS                                   |  10 +
 drivers/base/core.c                           |   3 +-
 drivers/fpga/Kconfig                          |   2 +
 drivers/fpga/dfl-pci.c                        |  95 +++-
 drivers/fpga/dfl.c                            |  58 ++
 drivers/fpga/dfl.h                            |   4 +
 drivers/fpga/intel-m10-bmc-sec-update.c       | 246 ++++++++
 drivers/pci/hotplug/Kconfig                   |  14 +
 drivers/pci/hotplug/Makefile                  |   1 +
 drivers/pci/hotplug/fpgahp.c                  | 526 ++++++++++++++++++
 drivers/pci/hotplug/pci_hotplug_core.c        |  88 +++
 drivers/pci/hotplug/pciehp.h                  |   3 +
 drivers/pci/hotplug/pciehp_hpc.c              |  11 +-
 drivers/pci/hotplug/pciehp_pci.c              |   2 +
 include/linux/device.h                        |   1 +
 include/linux/fpga/fpgahp_manager.h           | 100 ++++
 include/linux/mfd/intel-m10-bmc.h             |  31 ++
 include/linux/pci_hotplug.h                   |   5 +
 21 files changed, 1243 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-fpgahp
 create mode 100644 Documentation/fpga/fpgahp.rst
 create mode 100644 drivers/pci/hotplug/fpgahp.c
 create mode 100644 include/linux/fpga/fpgahp_manager.h


base-commit: 5dc4c995db9eb45f6373a956eb1f69460e69e6d4
-- 
2.38.1


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

* [PATCH v1 01/12] PCI: hotplug: add new callbacks on hotplug_slot_ops
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
@ 2023-01-19  1:35 ` Tianfei Zhang
  2023-01-19 13:31   ` Greg KH
  2023-01-19  1:35 ` [PATCH v1 02/12] PCI: hotplug: expose APIs from pciehp driver Tianfei Zhang
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

To reprogram an PCIe-based FPGA card, a new image is
burned into FLASH on the card and then the card BMC is
triggered to reboot the card and load the new image.

Two new operation callbacks are defined in hotplug_slot_ops
to trigger the reprogramming of an FPGA-based PCIe card:

  - available_images: Optional: available FPGA images
  - image_load: Optional: trigger the FPGA to load a new image

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/hotplug/pci_hotplug_core.c | 88 ++++++++++++++++++++++++++
 include/linux/pci_hotplug.h            |  5 ++
 2 files changed, 93 insertions(+)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 058d5937d8a9..2b14b6513a03 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -231,6 +231,52 @@ static struct pci_slot_attribute hotplug_slot_attr_test = {
 	.store = test_write_file
 };
 
+static ssize_t available_images_read_file(struct pci_slot *pci_slot, char *buf)
+{
+	struct hotplug_slot *slot = pci_slot->hotplug;
+	ssize_t count = 0;
+
+	if (!try_module_get(slot->owner))
+		return -ENODEV;
+
+	if (slot->ops->available_images(slot, buf))
+		count = slot->ops->available_images(slot, buf);
+
+	module_put(slot->owner);
+
+	return count;
+}
+
+static struct pci_slot_attribute hotplug_slot_attr_available_images = {
+	.attr = { .name = "available_images", .mode = 0444 },
+	.show = available_images_read_file,
+};
+
+static ssize_t image_load_write_file(struct pci_slot *pci_slot,
+				     const char *buf, size_t count)
+{
+	struct hotplug_slot *slot = pci_slot->hotplug;
+	int retval = 0;
+
+	if (!try_module_get(slot->owner))
+		return -ENODEV;
+
+	if (slot->ops->image_load)
+		retval = slot->ops->image_load(slot, buf);
+
+	module_put(slot->owner);
+
+	if (retval)
+		return retval;
+
+	return count;
+}
+
+static struct pci_slot_attribute hotplug_slot_attr_image_load = {
+	.attr = { .name = "image_load", .mode = 0644 },
+	.store = image_load_write_file,
+};
+
 static bool has_power_file(struct pci_slot *pci_slot)
 {
 	struct hotplug_slot *slot = pci_slot->hotplug;
@@ -289,6 +335,20 @@ static bool has_test_file(struct pci_slot *pci_slot)
 	return false;
 }
 
+static bool has_available_images_file(struct pci_slot *pci_slot)
+{
+	struct hotplug_slot *slot = pci_slot->hotplug;
+
+	return slot && slot->ops && slot->ops->available_images;
+}
+
+static bool has_image_load_file(struct pci_slot *pci_slot)
+{
+	struct hotplug_slot *slot = pci_slot->hotplug;
+
+	return slot && slot->ops && slot->ops->image_load;
+}
+
 static int fs_add_slot(struct pci_slot *pci_slot)
 {
 	int retval = 0;
@@ -331,8 +391,30 @@ static int fs_add_slot(struct pci_slot *pci_slot)
 			goto exit_test;
 	}
 
+	if (has_available_images_file(pci_slot)) {
+		retval = sysfs_create_file(&pci_slot->kobj,
+					   &hotplug_slot_attr_available_images.attr);
+		if (retval)
+			goto exit_available_images;
+	}
+
+	if (has_image_load_file(pci_slot)) {
+		retval = sysfs_create_file(&pci_slot->kobj,
+					   &hotplug_slot_attr_image_load.attr);
+		if (retval)
+			goto exit_image_load;
+	}
+
 	goto exit;
 
+exit_image_load:
+	if (has_adapter_file(pci_slot))
+		sysfs_remove_file(&pci_slot->kobj,
+				  &hotplug_slot_attr_available_images.attr);
+exit_available_images:
+	if (has_adapter_file(pci_slot))
+		sysfs_remove_file(&pci_slot->kobj,
+				  &hotplug_slot_attr_test.attr);
 exit_test:
 	if (has_adapter_file(pci_slot))
 		sysfs_remove_file(&pci_slot->kobj,
@@ -372,6 +454,12 @@ static void fs_remove_slot(struct pci_slot *pci_slot)
 	if (has_test_file(pci_slot))
 		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_test.attr);
 
+	if (has_available_images_file(pci_slot))
+		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_available_images.attr);
+
+	if (has_image_load_file(pci_slot))
+		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_image_load.attr);
+
 	pci_hp_remove_module_link(pci_slot);
 }
 
diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h
index 3a10d6ec3ee7..b7f39c20ad8b 100644
--- a/include/linux/pci_hotplug.h
+++ b/include/linux/pci_hotplug.h
@@ -29,6 +29,9 @@
  * @reset_slot: Optional interface to allow override of a bus reset for the
  *	slot for cases where a secondary bus reset can result in spurious
  *	hotplug events or where a slot can be reset independent of the bus.
+ * @available_images: Optional: called to get the available images for accelerator,
+ *	like FPGA.
+ * @image_load: Optional: called to load a new image for accelerator like FPGA.
  *
  * The table of function pointers that is passed to the hotplug pci core by a
  * hotplug pci driver.  These functions are called by the hotplug pci core when
@@ -45,6 +48,8 @@ struct hotplug_slot_ops {
 	int (*get_latch_status)		(struct hotplug_slot *slot, u8 *value);
 	int (*get_adapter_status)	(struct hotplug_slot *slot, u8 *value);
 	int (*reset_slot)		(struct hotplug_slot *slot, bool probe);
+	int (*available_images)		(struct hotplug_slot *slot, char *buf);
+	int (*image_load)		(struct hotplug_slot *slot, const char *buf);
 };
 
 /**
-- 
2.38.1


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

* [PATCH v1 02/12] PCI: hotplug: expose APIs from pciehp driver
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
  2023-01-19  1:35 ` [PATCH v1 01/12] PCI: hotplug: add new callbacks on hotplug_slot_ops Tianfei Zhang
@ 2023-01-19  1:35 ` Tianfei Zhang
  2023-01-19  1:35 ` [PATCH v1 03/12] PCI: hotplug: add and expose link disable API Tianfei Zhang
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

When a PCIe-based FPGA card is reprogrammed, it temporarily
disappears from the PCIe bus. This needs to be managed by
disabling the link to avoid PCIe errors while the device is
not present. Also, re-enabling the link and rescan the PCI
devices must be performed after loading the new images. Export
functions from pciehp driver necessary for removing and
reconfiguring the PCI devices below a PCI hotplug bridge.

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/hotplug/pciehp.h     | 2 ++
 drivers/pci/hotplug/pciehp_hpc.c | 5 ++++-
 drivers/pci/hotplug/pciehp_pci.c | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index e0a614acee05..c7f455a3b08f 100644
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -194,6 +194,8 @@ int pciehp_get_raw_indicator_status(struct hotplug_slot *h_slot, u8 *status);
 
 int pciehp_slot_reset(struct pcie_device *dev);
 
+int pciehp_link_enable(struct controller *ctrl);
+
 static inline const char *slot_name(struct controller *ctrl)
 {
 	return hotplug_slot_name(&ctrl->hotplug_slot);
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 10e9670eea0b..11e4bc58aec0 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -328,6 +328,7 @@ int pciehp_check_link_status(struct controller *ctrl)
 
 	return 0;
 }
+EXPORT_SYMBOL_NS_GPL(pciehp_check_link_status, PCIEHP);
 
 static int __pciehp_link_set(struct controller *ctrl, bool enable)
 {
@@ -346,10 +347,11 @@ static int __pciehp_link_set(struct controller *ctrl, bool enable)
 	return 0;
 }
 
-static int pciehp_link_enable(struct controller *ctrl)
+int pciehp_link_enable(struct controller *ctrl)
 {
 	return __pciehp_link_set(ctrl, true);
 }
+EXPORT_SYMBOL_NS_GPL(pciehp_link_enable, PCIEHP);
 
 int pciehp_get_raw_indicator_status(struct hotplug_slot *hotplug_slot,
 				    u8 *status)
@@ -482,6 +484,7 @@ int pciehp_query_power_fault(struct controller *ctrl)
 	pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
 	return !!(slot_status & PCI_EXP_SLTSTA_PFD);
 }
+EXPORT_SYMBOL_NS_GPL(pciehp_query_power_fault, PCIEHP);
 
 int pciehp_set_raw_indicator_status(struct hotplug_slot *hotplug_slot,
 				    u8 status)
diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index d17f3bf36f70..dd44e999b777 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -69,6 +69,7 @@ int pciehp_configure_device(struct controller *ctrl)
 	pci_unlock_rescan_remove();
 	return ret;
 }
+EXPORT_SYMBOL_NS_GPL(pciehp_configure_device, PCIEHP);
 
 /**
  * pciehp_unconfigure_device() - remove PCI devices below a hotplug bridge
@@ -120,3 +121,4 @@ void pciehp_unconfigure_device(struct controller *ctrl, bool presence)
 
 	pci_unlock_rescan_remove();
 }
+EXPORT_SYMBOL_NS_GPL(pciehp_unconfigure_device, PCIEHP);
-- 
2.38.1


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

* [PATCH v1 03/12] PCI: hotplug: add and expose link disable API
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
  2023-01-19  1:35 ` [PATCH v1 01/12] PCI: hotplug: add new callbacks on hotplug_slot_ops Tianfei Zhang
  2023-01-19  1:35 ` [PATCH v1 02/12] PCI: hotplug: expose APIs from pciehp driver Tianfei Zhang
@ 2023-01-19  1:35 ` Tianfei Zhang
  2023-01-19  1:35 ` [PATCH v1 04/12] PCI: hotplug: add FPGA PCI hotplug manager driver Tianfei Zhang
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

When a PCIe-based FPGA card is reprogrammed, it temporarily
disappears from the PCIe bus. This needs to be managed to
avoid PCIe errors while the device is not present. Also,
re-probing and rescan the PCI devices must be performed after
loading the new images. Export functions from pciehp driver
necessary for disable and enable the PCI link of a PCI hotplug
bridge.

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/pci/hotplug/pciehp.h     | 1 +
 drivers/pci/hotplug/pciehp_hpc.c | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index c7f455a3b08f..8b58d0e84644 100644
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -195,6 +195,7 @@ int pciehp_get_raw_indicator_status(struct hotplug_slot *h_slot, u8 *status);
 int pciehp_slot_reset(struct pcie_device *dev);
 
 int pciehp_link_enable(struct controller *ctrl);
+int pciehp_link_disable(struct controller *ctrl);
 
 static inline const char *slot_name(struct controller *ctrl)
 {
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 11e4bc58aec0..729546c0c1f9 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -353,6 +353,12 @@ int pciehp_link_enable(struct controller *ctrl)
 }
 EXPORT_SYMBOL_NS_GPL(pciehp_link_enable, PCIEHP);
 
+int pciehp_link_disable(struct controller *ctrl)
+{
+	return __pciehp_link_set(ctrl, false);
+}
+EXPORT_SYMBOL_NS_GPL(pciehp_link_disable, PCIEHP);
+
 int pciehp_get_raw_indicator_status(struct hotplug_slot *hotplug_slot,
 				    u8 *status)
 {
-- 
2.38.1


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

* [PATCH v1 04/12] PCI: hotplug: add FPGA PCI hotplug manager driver
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
                   ` (2 preceding siblings ...)
  2023-01-19  1:35 ` [PATCH v1 03/12] PCI: hotplug: add and expose link disable API Tianfei Zhang
@ 2023-01-19  1:35 ` Tianfei Zhang
  2023-01-19  1:35 ` [PATCH v1 05/12] fpga: dfl: register dfl-pci device into fpgahph driver Tianfei Zhang
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

After burning a new FPGA/BMC image, the driver should stop the
current running FPGA image and load a new image from the flash
to FPGA. Before reloading a new image, the driver needs to remove
all of PCI devices like PFs/VFs and as well as any other types of
devices (platform, etc.) defined within the FPGA which are running
in the old image. To help manage the PCIe-based FPGA card, leverage
the PCIe hotplug framework to implement the card management during
loading the new FPGA image.

Introduce new APIs to register/unregister a PCI device into PCI
hotplug core. The fpgahp driver instances a hotplug controller and
then registers into pci hotplug core which leverages the hotplug_slot_ops
callbacks to manage the FPGA card.

The new data structure fpgahp_manager is used for a fpga hotplug manager
instance. The fpgahp_manager has some callbacks in fpgahp_manager_ops.
The hotplug_prepare callback does some preparations, like removing
sub-devices below the PCI device to avoid data corruption during the
hotplug.

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 MAINTAINERS                         |   8 +
 drivers/pci/hotplug/Kconfig         |  14 ++
 drivers/pci/hotplug/Makefile        |   1 +
 drivers/pci/hotplug/fpgahp.c        | 269 ++++++++++++++++++++++++++++
 include/linux/fpga/fpgahp_manager.h |  62 +++++++
 5 files changed, 354 insertions(+)
 create mode 100644 drivers/pci/hotplug/fpgahp.c
 create mode 100644 include/linux/fpga/fpgahp_manager.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 42fc47c6edfd..7ac38b7cc44c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8163,6 +8163,14 @@ F:	drivers/uio/uio_dfl.c
 F:	include/linux/dfl.h
 F:	include/uapi/linux/fpga-dfl.h
 
+FPGA PCI Hotplug Driver
+M:	Tianfei Zhang <tianfei.zhang@intel.com>
+L:	linux-fpga@vger.kernel.org
+L:	linux-pci@vger.kernel.org
+S:	Maintained
+F:	drivers/pci/hotplug/fpgahp.c
+F:	include/linux/fpga/fpgahp_manager.h
+
 FPGA MANAGER FRAMEWORK
 M:	Moritz Fischer <mdf@kernel.org>
 M:	Wu Hao <hao.wu@intel.com>
diff --git a/drivers/pci/hotplug/Kconfig b/drivers/pci/hotplug/Kconfig
index 48113b210cf9..57a20e60afd4 100644
--- a/drivers/pci/hotplug/Kconfig
+++ b/drivers/pci/hotplug/Kconfig
@@ -61,6 +61,20 @@ config HOTPLUG_PCI_ACPI
 
 	  When in doubt, say N.
 
+config HOTPLUG_PCI_FPGA
+	tristate "FPGA PCI Hotplug Manager Driver"
+	depends on HOTPLUG_PCI_PCIE
+	help
+          Select this option to enable FPGA hotplug driver for PCIe-based
+          Field-Programmable Gate Array (FPGA) solutions. This driver provides
+          sysfs files for userspace applications to manager the FPGA card like
+          load a new FPGA image, reset the FPGA card.
+
+          To compile this driver as a module, choose M here: the
+          module will be called fpgahp.
+
+          When in doubt, say N.
+
 config HOTPLUG_PCI_ACPI_IBM
 	tristate "ACPI PCI Hotplug driver IBM extensions"
 	depends on HOTPLUG_PCI_ACPI
diff --git a/drivers/pci/hotplug/Makefile b/drivers/pci/hotplug/Makefile
index 5196983220df..b055592924ea 100644
--- a/drivers/pci/hotplug/Makefile
+++ b/drivers/pci/hotplug/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_HOTPLUG_PCI_POWERNV)	+= pnv-php.o
 obj-$(CONFIG_HOTPLUG_PCI_RPA)		+= rpaphp.o
 obj-$(CONFIG_HOTPLUG_PCI_RPA_DLPAR)	+= rpadlpar_io.o
 obj-$(CONFIG_HOTPLUG_PCI_ACPI)		+= acpiphp.o
+obj-$(CONFIG_HOTPLUG_PCI_FPGA)	        += fpgahp.o
 obj-$(CONFIG_HOTPLUG_PCI_S390)		+= s390_pci_hpc.o
 
 # acpiphp_ibm extends acpiphp, so should be linked afterwards.
diff --git a/drivers/pci/hotplug/fpgahp.c b/drivers/pci/hotplug/fpgahp.c
new file mode 100644
index 000000000000..71cee65383e2
--- /dev/null
+++ b/drivers/pci/hotplug/fpgahp.c
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * FPGA PCI Hotplug Manager Driver
+ *
+ * Copyright (C) 2023 Intel Corporation
+ */
+
+#include <linux/fpga/fpgahp_manager.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/pci_hotplug.h>
+
+#include "pciehp.h"
+
+/*
+ * a global fhpc_list is used to manage all
+ * registered FPGA hotplug controllers.
+ */
+static LIST_HEAD(fhpc_list);
+static DEFINE_MUTEX(fhpc_lock);
+
+struct fpgahp_controller {
+	struct list_head node;
+	struct fpgahp_manager mgr;
+	struct pcie_device *pcie;
+	struct controller ctrl;
+	struct pci_dev *hotplug_bridge;
+};
+
+static void fpgahp_add_fhpc(struct fpgahp_controller *fhpc)
+{
+	mutex_lock(&fhpc_lock);
+	list_add_tail(&fhpc->node, &fhpc_list);
+	mutex_unlock(&fhpc_lock);
+}
+
+static int fpgahp_init_controller(struct controller *ctrl, struct pcie_device *dev)
+{
+	struct pci_dev *hotplug_bridge = dev->port;
+	u32 slot_cap;
+
+	ctrl->pcie = dev;
+
+	if (pcie_capability_read_dword(hotplug_bridge, PCI_EXP_SLTCAP, &slot_cap))
+		return -EINVAL;
+
+	ctrl->slot_cap = slot_cap;
+
+	return 0;
+}
+
+static const struct hotplug_slot_ops fpgahp_slot_ops = {
+};
+
+static int fpgahp_init_slot(struct controller *ctrl)
+{
+	char name[SLOT_NAME_SIZE];
+	struct pci_dev *hotplug_bridge = ctrl->pcie->port;
+	int ret;
+
+	snprintf(name, sizeof(name), "%u", PSN(ctrl));
+
+	ctrl->hotplug_slot.ops = &fpgahp_slot_ops;
+
+	ret = pci_hp_register(&ctrl->hotplug_slot, hotplug_bridge->subordinate,
+			      PCI_SLOT(hotplug_bridge->devfn), name);
+	if (ret) {
+		ctrl_err(ctrl, "Register PCI hotplug core failed with error %d\n", ret);
+		return ret;
+	}
+
+	ctrl_info(ctrl, "Slot [%s] registered\n", hotplug_slot_name(&ctrl->hotplug_slot));
+
+	return 0;
+}
+
+static int
+fpgahp_create_new_fhpc(struct fpgahp_controller *fhpc, struct pci_dev *hotplug_bridge,
+		       const char *name, const struct fpgahp_manager_ops *ops)
+{
+	struct fpgahp_manager *mgr = &fhpc->mgr;
+	struct controller *ctrl = &fhpc->ctrl;
+	struct pcie_device *pcie;
+	int ret;
+
+	pcie = kzalloc(sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->port = hotplug_bridge;
+	fhpc->hotplug_bridge = hotplug_bridge;
+	fhpc->pcie = pcie;
+
+	ret = fpgahp_init_controller(ctrl, pcie);
+	if (ret)
+		goto free_pcie;
+
+	ret = fpgahp_init_slot(ctrl);
+	if (ret) {
+		if (ret == -EBUSY)
+			ctrl_warn(ctrl, "Slot already registered by another hotplug driver\n");
+		else
+			ctrl_err(ctrl, "Slot initialization failed (%d)\n", ret);
+		goto free_pcie;
+	}
+
+	mutex_init(&mgr->lock);
+
+	fpgahp_add_fhpc(fhpc);
+
+	return 0;
+
+free_pcie:
+	kfree(pcie);
+	return ret;
+}
+
+static struct fpgahp_controller *
+fpgahp_find_exist_fhpc(struct pci_dev *hotplug_bridge,
+		       struct pci_dev *pcidev, const struct fpgahp_manager_ops *ops)
+{
+	struct fpgahp_controller *iter, *fhpc = NULL;
+
+	mutex_lock(&fhpc_lock);
+
+	list_for_each_entry(iter, &fhpc_list, node) {
+		struct controller *ctrl = &iter->ctrl;
+
+		if (!iter->mgr.registered)
+			continue;
+
+		if (iter->hotplug_bridge == hotplug_bridge &&
+		    iter->mgr.priv == pcidev && iter->mgr.ops == ops) {
+			fhpc = iter;
+			ctrl_dbg(ctrl, "Found existing fhpc slot(%s)\n", slot_name(ctrl));
+			break;
+		}
+	}
+
+	mutex_unlock(&fhpc_lock);
+
+	return fhpc;
+}
+
+static struct fpgahp_controller *fpgahp_reclaim_fhpc(struct pci_dev *hotplug_bridge)
+{
+	struct fpgahp_controller *iter, *fhpc = NULL;
+
+	mutex_lock(&fhpc_lock);
+
+	list_for_each_entry(iter, &fhpc_list, node) {
+		struct controller *ctrl = &iter->ctrl;
+
+		if (iter->mgr.registered)
+			continue;
+
+		/* reclaim unused fhpc, will reuse it later */
+		if (iter->hotplug_bridge == hotplug_bridge) {
+			fhpc = iter;
+			ctrl_dbg(ctrl, "Found unused fhpc, reuse slot(%s)\n", slot_name(ctrl));
+			break;
+		}
+	}
+
+	mutex_unlock(&fhpc_lock);
+
+	return fhpc;
+}
+
+static void fpgahp_remove_fhpc(void)
+{
+	struct fpgahp_controller *fhpc, *tmp;
+
+	mutex_lock(&fhpc_lock);
+
+	list_for_each_entry_safe(fhpc, tmp, &fhpc_list, node) {
+		struct controller *ctrl = &fhpc->ctrl;
+
+		list_del(&fhpc->node);
+		pci_hp_deregister(&ctrl->hotplug_slot);
+		kfree(fhpc);
+	}
+
+	mutex_unlock(&fhpc_lock);
+}
+
+/**
+ * fpgahp_register - register FPGA device into fpgahp driver
+ * @hotplug_bridge: the hotplug bridge of the FPGA device
+ * @name: the name of the FPGA device
+ * @ops: pointer to structure of fpgahp manager ops
+ * @priv: private data for FPGA device
+ *
+ * Return: pointer to struct fpgahp_manager pointer or ERR_PTR()
+ */
+struct fpgahp_manager *fpgahp_register(struct pci_dev *hotplug_bridge, const char *name,
+				       const struct fpgahp_manager_ops *ops, void *priv)
+{
+	struct fpgahp_controller *fhpc;
+	struct pci_dev *pcidev = priv;
+	int ret;
+
+	if (!hotplug_bridge || !ops || !pcidev)
+		return ERR_PTR(-EINVAL);
+
+	dev_dbg(&pcidev->dev, "Register hotplug bridge: %04x:%02x:%02x\n",
+		pci_domain_nr(hotplug_bridge->bus), hotplug_bridge->bus->number,
+		PCI_SLOT(hotplug_bridge->devfn));
+
+	/* find existing matching fpgahp_controller */
+	fhpc = fpgahp_find_exist_fhpc(hotplug_bridge, pcidev, ops);
+	if (fhpc)
+		return &fhpc->mgr;
+
+	/* can it reuse the free fpgahp_controller? */
+	fhpc = fpgahp_reclaim_fhpc(hotplug_bridge);
+	if (fhpc)
+		goto reuse;
+
+	fhpc = kzalloc(sizeof(*fhpc), GFP_KERNEL);
+	if (!fhpc)
+		return ERR_PTR(-ENOMEM);
+
+	ret = fpgahp_create_new_fhpc(fhpc, hotplug_bridge, name, ops);
+	if (ret) {
+		kfree(fhpc);
+		return ERR_PTR(ret);
+	}
+
+reuse:
+	mutex_lock(&fhpc->mgr.lock);
+	fhpc->mgr.ops = ops;
+	fhpc->mgr.name = name;
+	fhpc->mgr.priv = pcidev;
+	fhpc->mgr.registered = true;
+	fhpc->mgr.state = FPGAHP_MGR_UNKNOWN;
+	mutex_unlock(&fhpc->mgr.lock);
+
+	return &fhpc->mgr;
+}
+EXPORT_SYMBOL_NS_GPL(fpgahp_register, FPGAHP);
+
+/**
+ * fpgahp_unregister - unregister FPGA device from fpgahp driver
+ * @mgr: point to the fpgahp_manager
+ */
+void fpgahp_unregister(struct fpgahp_manager *mgr)
+{
+	mutex_lock(&mgr->lock);
+	mgr->registered = false;
+	mutex_unlock(&mgr->lock);
+}
+EXPORT_SYMBOL_NS_GPL(fpgahp_unregister, FPGAHP);
+
+static int __init fpgahp_init(void)
+{
+	return 0;
+}
+module_init(fpgahp_init);
+
+static void __exit fpgahp_exit(void)
+{
+	fpgahp_remove_fhpc();
+}
+module_exit(fpgahp_exit);
+
+MODULE_DESCRIPTION("FPGA PCI Hotplug Manager Driver");
+MODULE_AUTHOR("Tianfei Zhang <tianfei.zhang@intel.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/fpga/fpgahp_manager.h b/include/linux/fpga/fpgahp_manager.h
new file mode 100644
index 000000000000..5e31877f03de
--- /dev/null
+++ b/include/linux/fpga/fpgahp_manager.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Driver Header File for FPGA PCI Hotplug Driver
+ *
+ * Copyright (C) 2023 Intel Corporation
+ */
+#ifndef _LINUX_FPGAHP_MANAGER_H
+#define _LINUX_FPGAHP_MANAGER_H
+
+#include <linux/mutex.h>
+
+struct pci_dev;
+struct fpgahp_manager;
+
+/**
+ * struct fpgahp_manager_ops - fpgahp manager specific operations
+ * @hotplug_prepare: Required: hotplug prepare like removing subdevices
+ *                   below the PCI device.
+ */
+struct fpgahp_manager_ops {
+	int (*hotplug_prepare)(struct fpgahp_manager *mgr);
+};
+
+/**
+ * enum fpgahp_manager_states - FPGA hotplug states
+ * @FPGAHP_MGR_UNKNOWN: can't determine state
+ * @FPGAHP_MGR_LOADING: image loading
+ * @FPGAHP_MGR_LOAD_DONE: image load done
+ * @FPGAHP_MGR_HP_FAIL: hotplug failed
+ */
+enum fpgahp_manager_states {
+	FPGAHP_MGR_UNKNOWN,
+	FPGAHP_MGR_LOADING,
+	FPGAHP_MGR_LOAD_DONE,
+	FPGAHP_MGR_HP_FAIL,
+};
+
+/**
+ * struct fpgahp_manager - represent a FPGA hotplug manager instance
+ *
+ * @lock: mutex to protect fpgahp manager data
+ * @priv: private data for fpgahp manager
+ * @ops: ops of this fpgahp_manager
+ * @state: the status of fpgahp_manager
+ * @name: name of the fpgahp_manager
+ * @registered: register status
+ */
+struct fpgahp_manager {
+	struct mutex lock; /* protect registered state of fpgahp_manager */
+	void *priv;
+	const struct fpgahp_manager_ops *ops;
+	enum fpgahp_manager_states state;
+	const char *name;
+	bool registered;
+};
+
+struct fpgahp_manager *fpgahp_register(struct pci_dev *hotplug_bridge,
+				       const char *name, const struct fpgahp_manager_ops *ops,
+				       void *priv);
+void fpgahp_unregister(struct fpgahp_manager *mgr);
+
+#endif
-- 
2.38.1


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

* [PATCH v1 05/12] fpga: dfl: register dfl-pci device into fpgahph driver
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
                   ` (3 preceding siblings ...)
  2023-01-19  1:35 ` [PATCH v1 04/12] PCI: hotplug: add FPGA PCI hotplug manager driver Tianfei Zhang
@ 2023-01-19  1:35 ` Tianfei Zhang
  2023-01-19  1:35 ` [PATCH v1 06/12] driver core: expose device_is_ancestor() API Tianfei Zhang
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

Add a registration of dfl-pci device into fpgahp driver by
fpgahp_register() API.

For Intel N3000 Card, FPGA devices like PFs/VFs and some ethernet
controllers are connected with a PCI switch, so the hotplug bridge
is the root hub of PCI device. This patch instances this hotplug
bridge as a hotplug controller.

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/fpga/Kconfig   |  1 +
 drivers/fpga/dfl-pci.c | 77 ++++++++++++++++++++++++++++++++++++++----
 drivers/fpga/dfl.h     |  2 ++
 3 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 6ce143dafd04..2188c5658e06 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -213,6 +213,7 @@ config FPGA_DFL_NIOS_INTEL_PAC_N3000
 config FPGA_DFL_PCI
 	tristate "FPGA DFL PCIe Device Driver"
 	depends on PCI && FPGA_DFL
+	depends on HOTPLUG_PCI_FPGA
 	help
 	  Select this option to enable PCIe driver for PCIe-based
 	  Field-Programmable Gate Array (FPGA) solutions which implement
diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
index 0914e7328b1a..0409cb30e563 100644
--- a/drivers/fpga/dfl-pci.c
+++ b/drivers/fpga/dfl-pci.c
@@ -23,6 +23,8 @@
 #include <linux/errno.h>
 #include <linux/aer.h>
 
+#include <linux/fpga/fpgahp_manager.h>
+
 #include "dfl.h"
 
 #define DRV_VERSION	"0.8"
@@ -40,6 +42,9 @@ struct cci_drvdata {
 	struct dfl_fpga_cdev *cdev;	/* container device */
 };
 
+static const struct fpgahp_manager_ops fpgahp_ops = {
+};
+
 static void __iomem *cci_pci_ioremap_bar0(struct pci_dev *pcidev)
 {
 	if (pcim_iomap_regions(pcidev, BIT(0), DRV_NAME))
@@ -118,6 +123,15 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
 };
 MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
 
+/*
+ * List the FPGA cards which have some ethernet controllers connected to a PCI
+ * switch like PAC N3000, used to find hotplug bridge for fpgahp driver.
+ */
+static const struct pci_device_id has_pci_switch_devids[] = {
+	{ PCI_VDEVICE(INTEL, PCIE_DEVICE_ID_INTEL_PAC_N3000) },
+	{}
+};
+
 static int cci_init_drvdata(struct pci_dev *pcidev)
 {
 	struct cci_drvdata *drvdata;
@@ -306,12 +320,33 @@ static int find_dfls_by_default(struct pci_dev *pcidev,
 	return ret;
 }
 
+/*
+ * On N3000 Card, FPGA devices like PFs/VFs and some ethernet controllers
+ * are connected to a PCI switch, so the hotplug bridge on the root port of
+ * FPGA PF0 PCI device.
+ */
+static struct pci_dev *cci_find_hotplug_bridge(struct pci_dev *pcidev)
+{
+	struct pci_dev *hotplug_bridge;
+
+	if (!pci_match_id(has_pci_switch_devids, pcidev))
+		return pcidev;
+
+	hotplug_bridge = pcie_find_root_port(pcidev);
+	if (!hotplug_bridge)
+		return NULL;
+
+	return hotplug_bridge;
+}
+
 /* enumerate feature devices under pci device */
 static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
 {
 	struct cci_drvdata *drvdata = pci_get_drvdata(pcidev);
 	struct dfl_fpga_enum_info *info;
+	struct pci_dev *hotplug_bridge;
 	struct dfl_fpga_cdev *cdev;
+	struct fpgahp_manager *mgr;
 	int nvec, ret = 0;
 	int *irq_table;
 
@@ -346,23 +381,47 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
 	if (ret)
 		goto irq_free_exit;
 
+	/* register hotplug bridge of PF0 device into fpgahp driver */
+	if (dev_is_pf(&pcidev->dev)) {
+		hotplug_bridge = cci_find_hotplug_bridge(pcidev);
+		if (!hotplug_bridge) {
+			dev_err(&pcidev->dev, "Hotplug bridge not found\n");
+			ret = -ENODEV;
+			goto irq_free_exit;
+		}
+
+		mgr = fpgahp_register(hotplug_bridge, dev_name(&pcidev->dev),
+				      &fpgahp_ops, pcidev);
+		if (IS_ERR(mgr)) {
+			dev_err(&pcidev->dev, "Registering fpga hotplug failed\n");
+			ret = PTR_ERR(mgr);
+			goto irq_free_exit;
+		}
+	}
+
 	/* start enumeration with prepared enumeration information */
 	cdev = dfl_fpga_feature_devs_enumerate(info);
 	if (IS_ERR(cdev)) {
 		dev_err(&pcidev->dev, "Enumeration failure\n");
 		ret = PTR_ERR(cdev);
-		goto irq_free_exit;
+		goto free_register;
 	}
 
+	if (dev_is_pf(&pcidev->dev))
+		cdev->fpgahp_mgr = mgr;
+
 	drvdata->cdev = cdev;
 
-irq_free_exit:
-	if (ret)
-		cci_pci_free_irq(pcidev);
 enum_info_free_exit:
 	dfl_fpga_enum_info_free(info);
-
 	return ret;
+
+free_register:
+	if (dev_is_pf(&pcidev->dev))
+		fpgahp_unregister(mgr);
+irq_free_exit:
+	cci_pci_free_irq(pcidev);
+	goto enum_info_free_exit;
 }
 
 static
@@ -444,8 +503,13 @@ static int cci_pci_sriov_configure(struct pci_dev *pcidev, int num_vfs)
 
 static void cci_pci_remove(struct pci_dev *pcidev)
 {
-	if (dev_is_pf(&pcidev->dev))
+	struct cci_drvdata *drvdata = pci_get_drvdata(pcidev);
+	struct dfl_fpga_cdev *cdev = drvdata->cdev;
+
+	if (dev_is_pf(&pcidev->dev)) {
 		cci_pci_sriov_configure(pcidev, 0);
+		fpgahp_unregister(cdev->fpgahp_mgr);
+	}
 
 	cci_remove_feature_devs(pcidev);
 	pci_disable_pcie_error_reporting(pcidev);
@@ -464,3 +528,4 @@ module_pci_driver(cci_pci_driver);
 MODULE_DESCRIPTION("FPGA DFL PCIe Device Driver");
 MODULE_AUTHOR("Intel Corporation");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(FPGAHP);
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 06cfcd5e84bb..898c05c269fb 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -469,6 +469,7 @@ void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info);
  * @fme_dev: FME feature device under this container device.
  * @lock: mutex lock to protect the port device list.
  * @port_dev_list: list of all port feature devices under this container device.
+ * @fpgahp_mgr: fpga hotplug manager.
  * @released_port_num: released port number under this container device.
  */
 struct dfl_fpga_cdev {
@@ -477,6 +478,7 @@ struct dfl_fpga_cdev {
 	struct device *fme_dev;
 	struct mutex lock;
 	struct list_head port_dev_list;
+	struct fpgahp_manager *fpgahp_mgr;
 	int released_port_num;
 };
 
-- 
2.38.1


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

* [PATCH v1 06/12] driver core: expose device_is_ancestor() API
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
                   ` (4 preceding siblings ...)
  2023-01-19  1:35 ` [PATCH v1 05/12] fpga: dfl: register dfl-pci device into fpgahph driver Tianfei Zhang
@ 2023-01-19  1:35 ` Tianfei Zhang
  2023-01-19  1:35 ` [PATCH v1 07/12] PCI: hotplug: add register/unregister function for BMC device Tianfei Zhang
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

Exposes device_is_ancestor() API which can be used
for check the device is the target device's ancestor or not.

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/core.c    | 3 ++-
 include/linux/device.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index a3e14143ec0c..597192b38198 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -262,7 +262,7 @@ static void device_link_remove_from_lists(struct device_link *link)
 }
 #endif /* !CONFIG_SRCU */
 
-static bool device_is_ancestor(struct device *dev, struct device *target)
+bool device_is_ancestor(struct device *dev, struct device *target)
 {
 	while (target->parent) {
 		target = target->parent;
@@ -271,6 +271,7 @@ static bool device_is_ancestor(struct device *dev, struct device *target)
 	}
 	return false;
 }
+EXPORT_SYMBOL_GPL(device_is_ancestor);
 
 /**
  * device_is_dependent - Check if one device depends on another one
diff --git a/include/linux/device.h b/include/linux/device.h
index 44e3acae7b36..35e35982d9a5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -909,6 +909,7 @@ int device_move(struct device *dev, struct device *new_parent,
 int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid);
 const char *device_get_devnode(struct device *dev, umode_t *mode, kuid_t *uid,
 			       kgid_t *gid, const char **tmp);
+bool device_is_ancestor(struct device *dev, struct device *target);
 int device_is_dependent(struct device *dev, void *target);
 
 static inline bool device_supports_offline(struct device *dev)
-- 
2.38.1


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

* [PATCH v1 07/12] PCI: hotplug: add register/unregister function for BMC device
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
                   ` (5 preceding siblings ...)
  2023-01-19  1:35 ` [PATCH v1 06/12] driver core: expose device_is_ancestor() API Tianfei Zhang
@ 2023-01-19  1:35 ` Tianfei Zhang
  2023-01-19  1:35 ` [PATCH v1 08/12] fpga: m10bmc-sec: register BMC device into fpgahp driver Tianfei Zhang
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

Introduce new APIs to register/unregister a BMC device instance
for fpgahp driver.

The fpgahp_bmc_device data structure represents a FPGA BMC device
which has some specific callbacks for FPGA hotplug operations.
The first one is available_images, which will return a list of
available images for FPGA. The second one is the image_load,
which will provide a reload trigger operation to BMC via secure
update driver for Intel PAC N3000 Card.

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/pci/hotplug/fpgahp.c        | 78 +++++++++++++++++++++++++++++
 include/linux/fpga/fpgahp_manager.h | 38 ++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/drivers/pci/hotplug/fpgahp.c b/drivers/pci/hotplug/fpgahp.c
index 71cee65383e2..79bae97a1d39 100644
--- a/drivers/pci/hotplug/fpgahp.c
+++ b/drivers/pci/hotplug/fpgahp.c
@@ -34,6 +34,84 @@ static void fpgahp_add_fhpc(struct fpgahp_controller *fhpc)
 	mutex_unlock(&fhpc_lock);
 }
 
+static struct fpgahp_bmc_device *fpgahp_find_bmc(struct device *bmc_device)
+{
+	struct fpgahp_bmc_device *bmc = NULL;
+	struct fpgahp_controller *fhpc;
+
+	mutex_lock(&fhpc_lock);
+
+	list_for_each_entry(fhpc, &fhpc_list, node) {
+		struct fpgahp_manager *mgr = &fhpc->mgr;
+		struct pci_dev *pcidev = mgr->priv;
+
+		if (!mgr->registered)
+			continue;
+
+		/*
+		 * BMC device (like security dev) is a subordinate device under
+		 * PCI device, so check if the parent device of BMC device recursively
+		 */
+		if (device_is_ancestor(&pcidev->dev, bmc_device)) {
+			bmc = &mgr->bmc;
+			break;
+		}
+	}
+
+	mutex_unlock(&fhpc_lock);
+
+	return bmc;
+}
+
+/**
+ * fpgahp_bmc_device_register - register FPGA BMC device into fpgahp driver
+ * @ops: pointer to structure of fpgahp manager ops
+ * @dev: device struct of BMC device
+ * @priv: private data for FPGA device
+ *
+ * Return: pointer to struct fpgahp_manager pointer or ERR_PTR()
+ */
+struct fpgahp_bmc_device *
+fpgahp_bmc_device_register(const struct fpgahp_bmc_ops *ops,
+			   struct device *dev, void *priv)
+{
+	struct fpgahp_manager *mgr;
+	struct fpgahp_bmc_device *bmc;
+
+	if (!ops)
+		return ERR_PTR(-EINVAL);
+
+	bmc = fpgahp_find_bmc(dev);
+	if (!bmc)
+		return ERR_PTR(-EINVAL);
+
+	mgr = to_fpgahp_mgr(bmc);
+
+	mutex_lock(&mgr->lock);
+	bmc->priv = priv;
+	bmc->device = dev;
+	bmc->ops = ops;
+	bmc->registered = true;
+	mutex_unlock(&mgr->lock);
+
+	return bmc;
+}
+EXPORT_SYMBOL_NS_GPL(fpgahp_bmc_device_register, FPGAHP);
+
+/**
+ * fpgahp_bmc_device_unregister - unregister FPGA BMC device from fpgahp driver
+ * @bmc: point to the fpgahp_bmc_device
+ */
+void fpgahp_bmc_device_unregister(struct fpgahp_bmc_device *bmc)
+{
+	struct fpgahp_manager *mgr = to_fpgahp_mgr(bmc);
+
+	mutex_lock(&mgr->lock);
+	bmc->registered = false;
+	mutex_unlock(&mgr->lock);
+}
+EXPORT_SYMBOL_NS_GPL(fpgahp_bmc_device_unregister, FPGAHP);
+
 static int fpgahp_init_controller(struct controller *ctrl, struct pcie_device *dev)
 {
 	struct pci_dev *hotplug_bridge = dev->port;
diff --git a/include/linux/fpga/fpgahp_manager.h b/include/linux/fpga/fpgahp_manager.h
index 5e31877f03de..982fbc661f55 100644
--- a/include/linux/fpga/fpgahp_manager.h
+++ b/include/linux/fpga/fpgahp_manager.h
@@ -11,6 +11,33 @@
 
 struct pci_dev;
 struct fpgahp_manager;
+struct fpgahp_bmc_device;
+
+/**
+ * struct fpgahp_bmc_ops - fpga hotplug BMC specific operations
+ * @available_images: Required: available images for fpgahp trigger
+ * @image_trigger: Required: trigger the image reload on BMC
+ */
+struct fpgahp_bmc_ops {
+	ssize_t (*available_images)(struct fpgahp_bmc_device *bmc, char *buf);
+	int (*image_trigger)(struct fpgahp_bmc_device *bmc, const char *buf,
+			     u32 *wait_time_msec);
+};
+
+/**
+ * struct fpgahp_bmc_device - represent a fpga hotplug BMC device
+ *
+ * @ops: ops of this fpgahp_bmc_device
+ * @priv: private data for fpgahp_bmc_device
+ * @device: device of BMC device
+ * @registered: register status
+ */
+struct fpgahp_bmc_device {
+	const struct fpgahp_bmc_ops *ops;
+	void *priv;
+	struct device *device;
+	bool registered;
+};
 
 /**
  * struct fpgahp_manager_ops - fpgahp manager specific operations
@@ -43,6 +70,7 @@ enum fpgahp_manager_states {
  * @ops: ops of this fpgahp_manager
  * @state: the status of fpgahp_manager
  * @name: name of the fpgahp_manager
+ * @bmc: fpgahp BMC device
  * @registered: register status
  */
 struct fpgahp_manager {
@@ -51,12 +79,22 @@ struct fpgahp_manager {
 	const struct fpgahp_manager_ops *ops;
 	enum fpgahp_manager_states state;
 	const char *name;
+	struct fpgahp_bmc_device bmc;
 	bool registered;
 };
 
+static inline struct fpgahp_manager *to_fpgahp_mgr(struct fpgahp_bmc_device *bmc)
+{
+	return container_of(bmc, struct fpgahp_manager, bmc);
+}
+
 struct fpgahp_manager *fpgahp_register(struct pci_dev *hotplug_bridge,
 				       const char *name, const struct fpgahp_manager_ops *ops,
 				       void *priv);
 void fpgahp_unregister(struct fpgahp_manager *mgr);
 
+struct fpgahp_bmc_device *fpgahp_bmc_device_register(const struct fpgahp_bmc_ops *ops,
+						     struct device *dev, void *priv);
+void fpgahp_bmc_device_unregister(struct fpgahp_bmc_device *bmc);
+
 #endif
-- 
2.38.1


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

* [PATCH v1 08/12] fpga: m10bmc-sec: register BMC device into fpgahp driver
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
                   ` (6 preceding siblings ...)
  2023-01-19  1:35 ` [PATCH v1 07/12] PCI: hotplug: add register/unregister function for BMC device Tianfei Zhang
@ 2023-01-19  1:35 ` Tianfei Zhang
  2023-01-19  1:35 ` [PATCH v1 09/12] fpga: dfl: remove non-reserved devices Tianfei Zhang
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

Add a registration of Intel M10 BMC security update device
into fpgahp driver by fpgahp_bmc_device_register() API.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/fpga/Kconfig                    |   1 +
 drivers/fpga/intel-m10-bmc-sec-update.c | 110 ++++++++++++++++++++++++
 2 files changed, 111 insertions(+)

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 2188c5658e06..f83682d00a1b 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -248,6 +248,7 @@ config FPGA_MGR_VERSAL_FPGA
 config FPGA_M10_BMC_SEC_UPDATE
 	tristate "Intel MAX10 BMC Secure Update driver"
 	depends on MFD_INTEL_M10_BMC
+	depends on HOTPLUG_PCI_FPGA
 	select FW_LOADER
 	select FW_UPLOAD
 	help
diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c
index 79d48852825e..647531094b3b 100644
--- a/drivers/fpga/intel-m10-bmc-sec-update.c
+++ b/drivers/fpga/intel-m10-bmc-sec-update.c
@@ -8,6 +8,7 @@
 #include <linux/bitfield.h>
 #include <linux/device.h>
 #include <linux/firmware.h>
+#include <linux/fpga/fpgahp_manager.h>
 #include <linux/mfd/intel-m10-bmc.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -19,10 +20,21 @@ struct m10bmc_sec {
 	struct intel_m10bmc *m10bmc;
 	struct fw_upload *fwl;
 	char *fw_name;
+	struct image_load *image_load;
+	struct fpgahp_bmc_device *fpgahp_bmc;
 	u32 fw_name_id;
 	bool cancel_request;
 };
 
+struct image_load {
+	const char *name;
+	int (*load_image)(struct m10bmc_sec *sec);
+	u32 wait_time_msec;
+};
+
+/* default wait 10 seconds for FPGA/BMC image reload */
+#define RELOAD_DEFAULT_WAIT_MSECS  (10 * MSEC_PER_SEC)
+
 static DEFINE_XARRAY_ALLOC(fw_upload_xa);
 
 /* Root Entry Hash (REH) support */
@@ -137,6 +149,43 @@ DEVICE_ATTR_SEC_CSK_RO(pr, PR_PROG_ADDR + CSK_VEC_OFFSET);
 
 #define FLASH_COUNT_SIZE 4096	/* count stored as inverted bit vector */
 
+static ssize_t m10bmc_available_images(struct fpgahp_bmc_device *bmc, char *buf)
+{
+	struct m10bmc_sec *sec = bmc->priv;
+	const struct image_load *hndlr;
+	ssize_t count = 0;
+
+	for (hndlr = sec->image_load; hndlr->name; hndlr++)
+		count += scnprintf(buf + count, PAGE_SIZE - count, "%s ", hndlr->name);
+
+	buf[count - 1] = '\n';
+
+	return count;
+}
+
+static int m10bmc_image_trigger(struct fpgahp_bmc_device *bmc, const char *buf,
+				u32 *wait_time_msec)
+{
+	struct m10bmc_sec *sec = bmc->priv;
+	const struct image_load *hndlr;
+	int ret = -EINVAL;
+
+	for (hndlr = sec->image_load; hndlr->name; hndlr++) {
+		if (sysfs_streq(buf, hndlr->name)) {
+			ret = hndlr->load_image(sec);
+			*wait_time_msec = hndlr->wait_time_msec;
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static const struct fpgahp_bmc_ops fpgahp_bmc_ops = {
+	.image_trigger = m10bmc_image_trigger,
+	.available_images = m10bmc_available_images,
+};
+
 static ssize_t flash_count_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
@@ -208,6 +257,54 @@ static void log_error_regs(struct m10bmc_sec *sec, u32 doorbell)
 		dev_err(sec->dev, "RSU auth result: 0x%08x\n", auth_result);
 }
 
+static int m10bmc_sec_bmc_image_load(struct m10bmc_sec *sec,
+				     unsigned int val)
+{
+	u32 doorbell;
+	int ret;
+
+	if (val > 1) {
+		dev_err(sec->dev, "invalid reload val = %u\n", val);
+		return -EINVAL;
+	}
+
+	ret = m10bmc_sys_read(sec->m10bmc, M10BMC_DOORBELL, &doorbell);
+	if (ret)
+		return ret;
+
+	if (doorbell & DRBL_REBOOT_DISABLED)
+		return -EBUSY;
+
+	return regmap_update_bits(sec->m10bmc->regmap,
+				  M10BMC_SYS_BASE + M10BMC_DOORBELL,
+				  DRBL_CONFIG_SEL | DRBL_REBOOT_REQ,
+				  FIELD_PREP(DRBL_CONFIG_SEL, val) | DRBL_REBOOT_REQ);
+}
+
+static int m10bmc_sec_bmc_image_load_0(struct m10bmc_sec *sec)
+{
+	return m10bmc_sec_bmc_image_load(sec, 0);
+}
+
+static int m10bmc_sec_bmc_image_load_1(struct m10bmc_sec *sec)
+{
+	return m10bmc_sec_bmc_image_load(sec, 1);
+}
+
+static struct image_load m10bmc_image_load_hndlrs[] = {
+	{
+		.name = "bmc_factory",
+		.load_image = m10bmc_sec_bmc_image_load_1,
+		.wait_time_msec = RELOAD_DEFAULT_WAIT_MSECS,
+	},
+	{
+		.name = "bmc_user",
+		.load_image = m10bmc_sec_bmc_image_load_0,
+		.wait_time_msec = RELOAD_DEFAULT_WAIT_MSECS,
+	},
+	{}
+};
+
 static enum fw_upload_err rsu_check_idle(struct m10bmc_sec *sec)
 {
 	u32 doorbell;
@@ -565,6 +662,7 @@ static int m10bmc_sec_probe(struct platform_device *pdev)
 	sec->dev = &pdev->dev;
 	sec->m10bmc = dev_get_drvdata(pdev->dev.parent);
 	dev_set_drvdata(&pdev->dev, sec);
+	sec->image_load = m10bmc_image_load_hndlrs;
 
 	ret = xa_alloc(&fw_upload_xa, &sec->fw_name_id, sec,
 		       xa_limit_32b, GFP_KERNEL);
@@ -587,6 +685,16 @@ static int m10bmc_sec_probe(struct platform_device *pdev)
 	}
 
 	sec->fwl = fwl;
+
+	sec->fpgahp_bmc = fpgahp_bmc_device_register(&fpgahp_bmc_ops, sec->dev, sec);
+	if (IS_ERR(sec->fpgahp_bmc)) {
+		dev_err(sec->dev, "register hotplug bmc failed\n");
+		kfree(sec->fw_name);
+		xa_erase(&fw_upload_xa, sec->fw_name_id);
+		firmware_upload_unregister(sec->fwl);
+		return PTR_ERR(sec->fpgahp_bmc);
+	}
+
 	return 0;
 }
 
@@ -594,6 +702,7 @@ static int m10bmc_sec_remove(struct platform_device *pdev)
 {
 	struct m10bmc_sec *sec = dev_get_drvdata(&pdev->dev);
 
+	fpgahp_bmc_device_unregister(sec->fpgahp_bmc);
 	firmware_upload_unregister(sec->fwl);
 	kfree(sec->fw_name);
 	xa_erase(&fw_upload_xa, sec->fw_name_id);
@@ -626,3 +735,4 @@ module_platform_driver(intel_m10bmc_sec_driver);
 MODULE_AUTHOR("Intel Corporation");
 MODULE_DESCRIPTION("Intel MAX10 BMC Secure Update");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(FPGAHP);
-- 
2.38.1


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

* [PATCH v1 09/12] fpga: dfl: remove non-reserved devices
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
                   ` (7 preceding siblings ...)
  2023-01-19  1:35 ` [PATCH v1 08/12] fpga: m10bmc-sec: register BMC device into fpgahp driver Tianfei Zhang
@ 2023-01-19  1:35 ` Tianfei Zhang
  2023-01-19  1:36 ` [PATCH v1 10/12] PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp Tianfei Zhang
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

Introduce a new concept of non-reserved devices and reserved devices
on DFL bus. Reserved devices mean that devices under DFL bus will be
reserved before triggering the image load, because those devices are
provided a communication link to BMC during the trigger, for example
SPI/NIOS private feature device on Intel PAC N3000 card, security
update device. On the other hand, the reset of devices are
non-reserved devices, which will be removed before triggering the
image load.

After loading a new image, all of reserved and non-reserved devices
will be removed.

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/fpga/dfl-pci.c | 18 +++++++++++++
 drivers/fpga/dfl.c     | 58 ++++++++++++++++++++++++++++++++++++++++++
 drivers/fpga/dfl.h     |  2 ++
 3 files changed, 78 insertions(+)

diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
index 0409cb30e563..64cf6c623a5a 100644
--- a/drivers/fpga/dfl-pci.c
+++ b/drivers/fpga/dfl-pci.c
@@ -42,7 +42,24 @@ struct cci_drvdata {
 	struct dfl_fpga_cdev *cdev;	/* container device */
 };
 
+static int dfl_hp_prepare(struct fpgahp_manager *mgr)
+{
+	struct pci_dev *pcidev = mgr->priv;
+	struct cci_drvdata *drvdata = pci_get_drvdata(pcidev);
+	struct dfl_fpga_cdev *cdev = drvdata->cdev;
+	struct platform_device *fme = to_platform_device(cdev->fme_dev);
+
+	/* remove all of non-reserved fme devices of PF0 */
+	dfl_reload_remove_non_reserved_devs(fme, mgr->bmc.device);
+
+	/* remove all AFU devices of PF0 */
+	dfl_reload_remove_afus(cdev);
+
+	return 0;
+}
+
 static const struct fpgahp_manager_ops fpgahp_ops = {
+	.hotplug_prepare = dfl_hp_prepare,
 };
 
 static void __iomem *cci_pci_ioremap_bar0(struct pci_dev *pcidev)
@@ -529,3 +546,4 @@ MODULE_DESCRIPTION("FPGA DFL PCIe Device Driver");
 MODULE_AUTHOR("Intel Corporation");
 MODULE_LICENSE("GPL v2");
 MODULE_IMPORT_NS(FPGAHP);
+MODULE_IMPORT_NS(DFL_CORE);
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index b9aae85ba930..613a8fef47d8 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -486,6 +486,60 @@ EXPORT_SYMBOL(dfl_driver_unregister);
 
 #define is_header_feature(feature) ((feature)->id == FEATURE_ID_FIU_HEADER)
 
+static void dfl_devs_remove_non_reserved(struct dfl_feature_platform_data *pdata,
+					 struct device *trigger_dev)
+{
+	struct dfl_feature *feature;
+
+	dfl_fpga_dev_for_each_feature(pdata, feature) {
+		if (!feature->ddev)
+			continue;
+
+		/* find and skip reserved dfl device */
+		if (device_is_ancestor(&feature->ddev->dev, trigger_dev))
+			continue;
+
+		device_unregister(&feature->ddev->dev);
+		feature->ddev = NULL;
+	}
+}
+
+void dfl_reload_remove_non_reserved_devs(struct platform_device *pdev, struct device *trigger_dev)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	struct dfl_feature *feature;
+
+	dfl_devs_remove_non_reserved(pdata, trigger_dev);
+
+	dfl_fpga_dev_for_each_feature(pdata, feature) {
+		if (feature->ops) {
+			if (feature->ops->uinit)
+				feature->ops->uinit(pdev, feature);
+			feature->ops = NULL;
+		}
+	}
+}
+EXPORT_SYMBOL_NS_GPL(dfl_reload_remove_non_reserved_devs, DFL_CORE);
+
+void dfl_reload_remove_afus(struct dfl_fpga_cdev *cdev)
+{
+	struct dfl_feature_platform_data *pdata, *ptmp;
+
+	mutex_lock(&cdev->lock);
+
+	list_for_each_entry_safe(pdata, ptmp, &cdev->port_dev_list, node) {
+		struct platform_device *port_dev = pdata->dev;
+		enum dfl_id_type type = feature_dev_id_type(port_dev);
+		int id = port_dev->id;
+
+		list_del(&pdata->node);
+		platform_device_unregister(port_dev);
+		dfl_id_free(type, id);
+	}
+	mutex_unlock(&cdev->lock);
+}
+EXPORT_SYMBOL_NS_GPL(dfl_reload_remove_afus, DFL_CORE);
+
 /**
  * dfl_fpga_dev_feature_uinit - uinit for sub features of dfl feature device
  * @pdev: feature device.
@@ -1376,6 +1430,10 @@ static int remove_feature_dev(struct device *dev, void *data)
 	enum dfl_id_type type = feature_dev_id_type(pdev);
 	int id = pdev->id;
 
+	/* pdev has been released */
+	if (!device_is_registered(&pdev->dev))
+		return 0;
+
 	platform_device_unregister(pdev);
 
 	dfl_id_free(type, id);
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 898c05c269fb..3cbe1b21f001 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -485,6 +485,8 @@ struct dfl_fpga_cdev {
 struct dfl_fpga_cdev *
 dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info);
 void dfl_fpga_feature_devs_remove(struct dfl_fpga_cdev *cdev);
+void dfl_reload_remove_afus(struct dfl_fpga_cdev *cdev);
+void dfl_reload_remove_non_reserved_devs(struct platform_device *pdev, struct device *trigger_dev);
 
 /*
  * need to drop the device reference with put_device() after use port platform
-- 
2.38.1


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

* [PATCH v1 10/12] PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
                   ` (8 preceding siblings ...)
  2023-01-19  1:35 ` [PATCH v1 09/12] fpga: dfl: remove non-reserved devices Tianfei Zhang
@ 2023-01-19  1:36 ` Tianfei Zhang
  2023-01-19 13:28   ` Greg KH
  2023-01-19  1:36 ` [PATCH v1 11/12] fpga: m10bmc-sec: add m10bmc_sec_retimer_load callback Tianfei Zhang
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:36 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

Implement the image_load and available_images callback functions
for fpgahp driver. This patch leverages some APIs from pciehp
driver to implement the device reconfiguration below the PCI hotplug
bridge.

Here are the steps for a process of image load.
1. remove all PFs and VFs except the PF0.
2. remove all non-reserved devices of PF0.
3. trigger a image load via BMC.
4. disable the link of the hotplug bridge.
5. remove all reserved devices under PF0 and PCI devices
   below the hotplug bridge.
6. wait for image load done via BMC, e.g. 10s.
7. re-enable the link of the hotplug bridge.
8. re-enumerate PCI devices below the hotplug bridge.

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 Documentation/ABI/testing/sysfs-driver-fpgahp |  21 ++
 MAINTAINERS                                   |   1 +
 drivers/pci/hotplug/fpgahp.c                  | 179 ++++++++++++++++++
 3 files changed, 201 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-fpgahp

diff --git a/Documentation/ABI/testing/sysfs-driver-fpgahp b/Documentation/ABI/testing/sysfs-driver-fpgahp
new file mode 100644
index 000000000000..8d4b1bfc4012
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-fpgahp
@@ -0,0 +1,21 @@
+What:		/sys/bus/pci/slots/X-X/available_images
+Date:		May 2023
+KernelVersion:	6.3
+Contact:	Tianfei Zhang <tianfei.zhang@intel.com>
+Description:	Read-only. This file returns a space separated list of
+		key words that may be written into the image_load file
+		described below. These keywords decribe an FPGA, BMC,
+		or firmware image in FLASH or EEPROM storage that may
+		be loaded.
+
+What:		/sys/bus/pci/slots/X-X/image_load
+Date:		May 2023
+KernelVersion:	6.3
+Contact:	Tianfei Zhang <tianfei.zhang@intel.com>
+Description:	Write-only. A key word may be written to this file to
+		trigger a new image loading of an FPGA, BMC, or firmware
+		image from FLASH or EEPROM. Refer to the available_images
+		file for a list of supported key words for the underlying
+		device.
+		Writing an unsupported string to this file will result in
+		EINVAL being returned.
diff --git a/MAINTAINERS b/MAINTAINERS
index 7ac38b7cc44c..85d4e3a0e986 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8168,6 +8168,7 @@ M:	Tianfei Zhang <tianfei.zhang@intel.com>
 L:	linux-fpga@vger.kernel.org
 L:	linux-pci@vger.kernel.org
 S:	Maintained
+F:	Documentation/ABI/testing/sysfs-driver-fpgahp
 F:	drivers/pci/hotplug/fpgahp.c
 F:	include/linux/fpga/fpgahp_manager.h
 
diff --git a/drivers/pci/hotplug/fpgahp.c b/drivers/pci/hotplug/fpgahp.c
index 79bae97a1d39..3fdf37b238c6 100644
--- a/drivers/pci/hotplug/fpgahp.c
+++ b/drivers/pci/hotplug/fpgahp.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/pci_hotplug.h>
+#include <linux/pm_runtime.h>
 
 #include "pciehp.h"
 
@@ -25,8 +26,182 @@ struct fpgahp_controller {
 	struct pcie_device *pcie;
 	struct controller ctrl;
 	struct pci_dev *hotplug_bridge;
+	struct mutex lock;  /* parallel access into image_load callback */
 };
 
+static inline struct fpgahp_controller *to_fhpc(struct controller *ctrl)
+{
+	return container_of(ctrl, struct fpgahp_controller, ctrl);
+}
+
+static int fpgahp_available_images(struct hotplug_slot *slot, char *buf)
+{
+	struct controller *ctrl = to_ctrl(slot);
+	struct fpgahp_controller *fhpc = to_fhpc(ctrl);
+	struct fpgahp_manager *mgr = &fhpc->mgr;
+	struct fpgahp_bmc_device *bmc = &mgr->bmc;
+	ssize_t count;
+
+	mutex_lock(&mgr->lock);
+
+	if (!mgr->registered || !bmc->registered)
+		goto err_unlock;
+
+	if (!bmc->ops->available_images)
+		goto err_unlock;
+
+	count = bmc->ops->available_images(bmc, buf);
+
+	mutex_unlock(&mgr->lock);
+
+	return count;
+
+err_unlock:
+	mutex_unlock(&mgr->lock);
+	return -EINVAL;
+}
+
+static void fpgahp_remove_sibling_pci_dev(struct pci_dev *pcidev)
+{
+	struct pci_bus *bus = pcidev->bus;
+	struct pci_dev *sibling, *tmp;
+
+	if (!bus)
+		return;
+
+	list_for_each_entry_safe_reverse(sibling, tmp, &bus->devices, bus_list)
+		if (sibling != pcidev)
+			pci_stop_and_remove_bus_device_locked(sibling);
+}
+
+static int fpgahp_link_enable(struct controller *ctrl)
+{
+	int retval;
+
+	retval = pciehp_link_enable(ctrl);
+	if (retval) {
+		ctrl_err(ctrl, "Can not enable the link!\n");
+		return retval;
+	}
+
+	retval = pciehp_check_link_status(ctrl);
+	if (retval) {
+		ctrl_err(ctrl, "Check link status fail!\n");
+		return retval;
+	}
+
+	retval = pciehp_query_power_fault(ctrl);
+	if (retval)
+		ctrl_err(ctrl, "Slot(%s): Power fault\n", slot_name(ctrl));
+
+	return retval;
+}
+
+static int fpgahp_rescan_slot(struct controller *ctrl)
+{
+	int retval;
+	struct pci_bus *parent = ctrl->pcie->port->subordinate;
+
+	retval = pciehp_configure_device(ctrl);
+	if (retval && retval != -EEXIST)
+		ctrl_err(ctrl, "Cannot add device at %04x:%02x:00\n",
+			 pci_domain_nr(parent), parent->number);
+
+	return retval;
+}
+
+static int __fpgahp_image_load(struct fpgahp_controller *fhpc, const char *buf)
+{
+	struct pci_dev *hotplug_bridge = fhpc->hotplug_bridge;
+	struct fpgahp_manager *mgr = &fhpc->mgr;
+	struct fpgahp_bmc_device *bmc = &mgr->bmc;
+	struct controller *ctrl = &fhpc->ctrl;
+	struct pci_dev *pcidev = mgr->priv;
+	u32 wait_time_msec;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(&hotplug_bridge->dev);
+	if (ret)
+		return ret;
+
+	mutex_lock(&mgr->lock);
+
+	if (!pcidev || !mgr->registered || !bmc->registered || !bmc->ops->image_trigger) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	mgr->state = FPGAHP_MGR_LOADING;
+
+	/* 1. remove all PFs and VFs except the PF0 */
+	fpgahp_remove_sibling_pci_dev(pcidev);
+
+	/* 2. remove all non-reserved devices */
+	if (mgr->ops->hotplug_prepare) {
+		ret = mgr->ops->hotplug_prepare(mgr);
+		if (ret) {
+			ctrl_err(ctrl, "Prepare hotplug failed\n");
+			goto out_unlock;
+		}
+	}
+
+	/* 3. trigger loading a new image of BMC */
+	ret = bmc->ops->image_trigger(bmc, buf, &wait_time_msec);
+	if (ret) {
+		ctrl_err(ctrl, "Image trigger failed\n");
+		goto out_unlock;
+	}
+
+	/* 4. disable link of hotplug bridge */
+	pciehp_link_disable(ctrl);
+
+	/*
+	 * unlock the mrg->lock temporarily to avoid the dead lock while re-gain
+	 * the same lock on fpgahp_unregister() during remove PCI devices below the
+	 * hotplug bridge
+	 */
+	mutex_unlock(&mgr->lock);
+
+	/* 5. remove PCI devices below hotplug bridge */
+	pciehp_unconfigure_device(ctrl, true);
+
+	/* 6. wait for FPGA/BMC load done */
+	msleep(wait_time_msec);
+
+	mutex_lock(&mgr->lock);
+
+	/* 7. re-enable link */
+	ret = fpgahp_link_enable(ctrl);
+
+out_unlock:
+	if (ret)
+		mgr->state = FPGAHP_MGR_HP_FAIL;
+	else
+		mgr->state = FPGAHP_MGR_LOAD_DONE;
+
+	mutex_unlock(&mgr->lock);
+
+	/* re-enumerate PCI devices below hotplug bridge */
+	if (!ret)
+		ret = fpgahp_rescan_slot(ctrl);
+
+	pm_runtime_put(&hotplug_bridge->dev);
+	return ret;
+}
+
+static int fpgahp_image_load(struct hotplug_slot *slot, const char *buf)
+{
+	struct controller *ctrl = to_ctrl(slot);
+	struct fpgahp_controller *fhpc = to_fhpc(ctrl);
+	int ret;
+
+	mutex_lock(&fhpc->lock);
+	ret = __fpgahp_image_load(fhpc, buf);
+	mutex_unlock(&fhpc->lock);
+
+	return ret;
+}
+
 static void fpgahp_add_fhpc(struct fpgahp_controller *fhpc)
 {
 	mutex_lock(&fhpc_lock);
@@ -128,6 +303,8 @@ static int fpgahp_init_controller(struct controller *ctrl, struct pcie_device *d
 }
 
 static const struct hotplug_slot_ops fpgahp_slot_ops = {
+	.available_images	= fpgahp_available_images,
+	.image_load		= fpgahp_image_load,
 };
 
 static int fpgahp_init_slot(struct controller *ctrl)
@@ -183,6 +360,7 @@ fpgahp_create_new_fhpc(struct fpgahp_controller *fhpc, struct pci_dev *hotplug_b
 	}
 
 	mutex_init(&mgr->lock);
+	mutex_init(&fhpc->lock);
 
 	fpgahp_add_fhpc(fhpc);
 
@@ -345,3 +523,4 @@ module_exit(fpgahp_exit);
 MODULE_DESCRIPTION("FPGA PCI Hotplug Manager Driver");
 MODULE_AUTHOR("Tianfei Zhang <tianfei.zhang@intel.com>");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(PCIEHP);
-- 
2.38.1


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

* [PATCH v1 11/12] fpga: m10bmc-sec: add m10bmc_sec_retimer_load callback
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
                   ` (9 preceding siblings ...)
  2023-01-19  1:36 ` [PATCH v1 10/12] PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp Tianfei Zhang
@ 2023-01-19  1:36 ` Tianfei Zhang
  2023-01-19 14:22   ` Lee Jones
  2023-01-19  1:36 ` [PATCH v1 12/12] Documentation: fpga: add description of fpgahp driver Tianfei Zhang
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:36 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

Create m10bmc_sec_retimer_load() callback function to provide
a trigger to update a new retimer (Intel C827 Ethernet
transceiver) firmware on Intel PAC N3000 Card.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 136 ++++++++++++++++++++++++
 include/linux/mfd/intel-m10-bmc.h       |  31 ++++++
 2 files changed, 167 insertions(+)

diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c
index 647531094b3b..053be33713c5 100644
--- a/drivers/fpga/intel-m10-bmc-sec-update.c
+++ b/drivers/fpga/intel-m10-bmc-sec-update.c
@@ -291,6 +291,137 @@ static int m10bmc_sec_bmc_image_load_1(struct m10bmc_sec *sec)
 	return m10bmc_sec_bmc_image_load(sec, 1);
 }
 
+static int trigger_retimer_eeprom_load(struct m10bmc_sec *sec)
+{
+	struct intel_m10bmc *m10bmc = sec->m10bmc;
+	unsigned int val;
+	int ret;
+
+	ret = regmap_update_bits(m10bmc->regmap, M10BMC_SYS_BASE + M10BMC_DOORBELL,
+				 DRBL_PKVL_EEPROM_LOAD_SEC, DRBL_PKVL_EEPROM_LOAD_SEC);
+	if (ret)
+		return ret;
+
+	/*
+	 * If the current NIOS FW supports this retimer update feature, then
+	 * it will clear the same PKVL_EEPROM_LOAD bit in 2 seconds. Otherwise
+	 * the driver needs to clear the PKVL_EEPROM_LOAD bit manually and
+	 * return an error code.
+	 */
+	ret = regmap_read_poll_timeout(m10bmc->regmap, M10BMC_SYS_BASE + M10BMC_DOORBELL,
+				       val, !(val & DRBL_PKVL_EEPROM_LOAD_SEC),
+				       M10BMC_PKVL_LOAD_INTERVAL_US, M10BMC_PKVL_LOAD_TIMEOUT_US);
+	if (ret == -ETIMEDOUT) {
+		dev_err(sec->dev, "PKVL_EEPROM_LOAD clear timeout\n");
+		regmap_update_bits(m10bmc->regmap, M10BMC_SYS_BASE + M10BMC_DOORBELL,
+				   DRBL_PKVL_EEPROM_LOAD_SEC, 0);
+	} else if (ret) {
+		dev_err(sec->dev, "EEPROM_LOAD poll error %d\n", ret);
+	}
+
+	return ret;
+}
+
+static int poll_retimer_eeprom_load_done(struct m10bmc_sec *sec)
+{
+	struct intel_m10bmc *m10bmc = sec->m10bmc;
+	unsigned int doorbell;
+	int ret;
+
+	/*
+	 * RSU_STAT_PKVL_REJECT indicates that the current image is
+	 * already programmed. RSU_PROG_PKVL_PROM_DONE that the firmware
+	 * update process has finished, but does not necessarily indicate
+	 * a successful update.
+	 */
+	ret = regmap_read_poll_timeout(m10bmc->regmap, M10BMC_SYS_BASE + M10BMC_DOORBELL,
+				       doorbell, (rsu_prog(doorbell) == RSU_PROG_PKVL_PROM_DONE) ||
+				       (rsu_stat(doorbell) == RSU_STAT_PKVL_REJECT),
+				       M10BMC_PKVL_PRELOAD_INTERVAL_US,
+				       M10BMC_PKVL_PRELOAD_TIMEOUT_US);
+	if (ret) {
+		if (ret == -ETIMEDOUT)
+			dev_err(sec->dev,
+				"Doorbell check timeout, last value 0x%08x\n", doorbell);
+		else
+			dev_err(sec->dev, "Doorbell poll error\n");
+		return ret;
+	}
+
+	if (rsu_stat(doorbell) == RSU_STAT_PKVL_REJECT) {
+		dev_err(sec->dev, "Duplicate image rejected\n");
+		return -EEXIST;
+	}
+
+	return 0;
+}
+
+static int poll_retimer_preload_done(struct m10bmc_sec *sec)
+{
+	struct intel_m10bmc *m10bmc = sec->m10bmc;
+	unsigned int val;
+	int ret;
+
+	/*
+	 * Wait for the updated firmware to be loaded by the PKVL device
+	 * and confirm that the updated firmware is operational
+	 */
+	ret = regmap_read_poll_timeout(m10bmc->regmap,
+				       M10BMC_SYS_BASE + M10BMC_PKVL_POLL_CTRL, val,
+				       (val & M10BMC_PKVL_PRELOAD) == M10BMC_PKVL_PRELOAD,
+				       M10BMC_PKVL_PRELOAD_INTERVAL_US,
+				       M10BMC_PKVL_PRELOAD_TIMEOUT_US);
+	if (ret) {
+		dev_err(sec->dev, "M10BMC_PKVL_PRELOAD poll error %d\n", ret);
+		return ret;
+	}
+
+	if ((val & M10BMC_PKVL_UPG_STATUS_MASK) != M10BMC_PKVL_UPG_STATUS_GOOD) {
+		dev_err(sec->dev, "Error detected during upgrade\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int retimer_check_idle(struct m10bmc_sec *sec)
+{
+	u32 doorbell;
+	int ret;
+
+	ret = m10bmc_sys_read(sec->m10bmc, M10BMC_DOORBELL, &doorbell);
+	if (ret)
+		return -EIO;
+
+	if (rsu_prog(doorbell) != RSU_PROG_IDLE &&
+	    rsu_prog(doorbell) != RSU_PROG_RSU_DONE &&
+	    rsu_prog(doorbell) != RSU_PROG_PKVL_PROM_DONE) {
+		log_error_regs(sec, doorbell);
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
+static int m10bmc_sec_retimer_eeprom_load(struct m10bmc_sec *sec)
+{
+	int ret;
+
+	ret = retimer_check_idle(sec);
+	if (ret)
+		return ret;
+
+	ret = trigger_retimer_eeprom_load(sec);
+	if (ret)
+		return ret;
+
+	ret = poll_retimer_eeprom_load_done(sec);
+	if (ret)
+		return ret;
+
+	return poll_retimer_preload_done(sec);
+}
+
 static struct image_load m10bmc_image_load_hndlrs[] = {
 	{
 		.name = "bmc_factory",
@@ -302,6 +433,11 @@ static struct image_load m10bmc_image_load_hndlrs[] = {
 		.load_image = m10bmc_sec_bmc_image_load_0,
 		.wait_time_msec = RELOAD_DEFAULT_WAIT_MSECS,
 	},
+	{
+		.name = "retimer_fw",
+		.load_image = m10bmc_sec_retimer_eeprom_load,
+		.wait_time_msec = 2 * RELOAD_DEFAULT_WAIT_MSECS,
+	},
 	{}
 };
 
diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
index f0044b14136e..35ce0c35138b 100644
--- a/include/linux/mfd/intel-m10-bmc.h
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -36,6 +36,37 @@
 #define M10BMC_VER_PCB_INFO_MSK		GENMASK(31, 24)
 #define M10BMC_VER_LEGACY_INVALID	0xffffffff
 
+/* Retimer related registers, in system register region */
+#define M10BMC_PKVL_POLL_CTRL		0x80
+#define M10BMC_PKVL_A_PRELOAD		BIT(16)
+#define M10BMC_PKVL_A_PRELOAD_TO	BIT(17)
+#define M10BMC_PKVL_A_DATA_TOO_BIG	BIT(18)
+#define M10BMC_PKVL_A_HDR_CKSUM	BIT(20)
+#define M10BMC_PKVL_B_PRELOAD		BIT(24)
+#define M10BMC_PKVL_B_PRELOAD_TO	BIT(25)
+#define M10BMC_PKVL_B_DATA_TOO_BIG	BIT(26)
+#define M10BMC_PKVL_B_HDR_CKSUM	BIT(28)
+
+#define M10BMC_PKVL_PRELOAD		(M10BMC_PKVL_A_PRELOAD | M10BMC_PKVL_B_PRELOAD)
+#define M10BMC_PKVL_PRELOAD_TIMEOUT	(M10BMC_PKVL_A_PRELOAD_TO | M10BMC_PKVL_B_PRELOAD_TO)
+#define M10BMC_PKVL_DATA_TOO_BIG	(M10BMC_PKVL_A_DATA_TOO_BIG | M10BMC_PKVL_B_DATA_TOO_BIG)
+#define M10BMC_PKVL_HDR_CHECKSUM	(M10BMC_PKVL_A_HDR_CKSUM | M10BMC_PKVL_B_HDR_CKSUM)
+
+#define M10BMC_PKVL_UPG_STATUS_MASK \
+	(M10BMC_PKVL_PRELOAD | \
+	M10BMC_PKVL_PRELOAD_TIMEOUT | \
+	M10BMC_PKVL_DATA_TOO_BIG | \
+	M10BMC_PKVL_HDR_CHECKSUM)
+#define M10BMC_PKVL_UPG_STATUS_GOOD	(M10BMC_PKVL_PRELOAD | M10BMC_PKVL_HDR_CHECKSUM)
+
+/* interval 100ms and timeout 2s */
+#define M10BMC_PKVL_LOAD_INTERVAL_US	(100 * USEC_PER_MSEC)
+#define M10BMC_PKVL_LOAD_TIMEOUT_US	(2 * USEC_PER_SEC)
+
+/* interval 100ms and timeout 30s */
+#define M10BMC_PKVL_PRELOAD_INTERVAL_US (100 * USEC_PER_MSEC)
+#define M10BMC_PKVL_PRELOAD_TIMEOUT_US  (30 * USEC_PER_SEC)
+
 /* Secure update doorbell register, in system register region */
 #define M10BMC_DOORBELL			0x400
 
-- 
2.38.1


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

* [PATCH v1 12/12] Documentation: fpga: add description of fpgahp driver
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
                   ` (10 preceding siblings ...)
  2023-01-19  1:36 ` [PATCH v1 11/12] fpga: m10bmc-sec: add m10bmc_sec_retimer_load callback Tianfei Zhang
@ 2023-01-19  1:36 ` Tianfei Zhang
  2023-01-19  9:38   ` Bagas Sanjaya
  2023-01-19  8:06 ` [PATCH v1 00/12] add FPGA hotplug manager driver Pali Rohár
  2023-01-19 13:33 ` Greg KH
  13 siblings, 1 reply; 29+ messages in thread
From: Tianfei Zhang @ 2023-01-19  1:36 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach
  Cc: Tianfei Zhang

Add the description of fpgahp driver.

Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 Documentation/fpga/fpgahp.rst | 29 +++++++++++++++++++++++++++++
 Documentation/fpga/index.rst  |  1 +
 MAINTAINERS                   |  1 +
 3 files changed, 31 insertions(+)
 create mode 100644 Documentation/fpga/fpgahp.rst

diff --git a/Documentation/fpga/fpgahp.rst b/Documentation/fpga/fpgahp.rst
new file mode 100644
index 000000000000..3ec34bbffde1
--- /dev/null
+++ b/Documentation/fpga/fpgahp.rst
@@ -0,0 +1,29 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===========================
+FPGA Hotplug Manager Driver
+===========================
+
+Authors:
+
+- Tianfei Zhang <tianfei.zhang@intel.com>
+
+There are some board managements for PCIe-based FPGA card like burning the entire
+image, loading a new FPGA image or BMC firmware in FPGA deployment of data center
+or cloud. For example, loading a new FPGA image, the driver needs to remove all of
+PCI devices like PFs/VFs and as well as any other types of devices (platform, etc.)
+defined within the FPGA. After triggering the image load of the FPGA card via BMC,
+the driver reconfigures the PCI bus. The FPGA Hotplug Manager (fpgahp) driver manages
+those devices and functions leveraging the PCI hotplug framework to deal with the
+reconfiguration of the PCI bus and removal/probe of PCI devices below the FPGA card.
+
+This fpgahp driver adds 2 new callbacks to extend the hotplug mechanism to
+allow selecting and loading a new FPGA image.
+
+ - available_images: Optional: called to return the available images of a FPGA card.
+ - image_load: Optional: called to load a new image for a FPGA card.
+
+In general, the fpgahp driver provides some sysfs files::
+
+        /sys/bus/pci/slots/<X-X>/available_images
+        /sys/bus/pci/slots/<X-X>/image_load
diff --git a/Documentation/fpga/index.rst b/Documentation/fpga/index.rst
index f80f95667ca2..8973a8a3f066 100644
--- a/Documentation/fpga/index.rst
+++ b/Documentation/fpga/index.rst
@@ -8,6 +8,7 @@ fpga
     :maxdepth: 1
 
     dfl
+    fpgahp
 
 .. only::  subproject and html
 
diff --git a/MAINTAINERS b/MAINTAINERS
index 85d4e3a0e986..569c7f680229 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8169,6 +8169,7 @@ L:	linux-fpga@vger.kernel.org
 L:	linux-pci@vger.kernel.org
 S:	Maintained
 F:	Documentation/ABI/testing/sysfs-driver-fpgahp
+F:	Documentation/fpga/fpgahp.rst
 F:	drivers/pci/hotplug/fpgahp.c
 F:	include/linux/fpga/fpgahp_manager.h
 
-- 
2.38.1


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

* Re: [PATCH v1 00/12] add FPGA hotplug manager driver
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
                   ` (11 preceding siblings ...)
  2023-01-19  1:36 ` [PATCH v1 12/12] Documentation: fpga: add description of fpgahp driver Tianfei Zhang
@ 2023-01-19  8:06 ` Pali Rohár
  2023-01-19  8:17   ` Zhang, Tianfei
  2023-01-19 13:33 ` Greg KH
  13 siblings, 1 reply; 29+ messages in thread
From: Pali Rohár @ 2023-01-19  8:06 UTC (permalink / raw)
  To: Tianfei Zhang
  Cc: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, mdf, hao.wu,
	yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach

Hello!

On Wednesday 18 January 2023 20:35:50 Tianfei Zhang wrote:
> This patchset introduces the FPGA hotplug manager (fpgahp) driver which 
> has been verified on the Intel N3000 card.
> 
> When a PCIe-based FPGA card is reprogrammed, it temporarily disappears
> from the PCIe bus. This needs to be managed to avoid PCIe errors and to
> reprobe the device after reprogramming.
> 
> To change the FPGA image, the kernel burns a new image into the flash on
> the card, and then triggers the card BMC to load the new image into FPGA.
> A new FPGA hotplug manager driver is introduced that leverages the PCIe
> hotplug framework to trigger and manage the update of the FPGA image,
> including the disappearance and reappearance of the card on the PCIe bus.
> The fpgahp driver uses APIs from the pciehp driver.

Just I'm thinking about one thing. PCIe cards can support PCIe hotplug
mechanism (via standard PCIe capabilities). So what would happen when
FPGA based PCIe card is also hotplug-able? Will be there two PCI hotplug
drivers/devices (one fpgahp and one pciehp)? Or just one and which?

> Two new operation
> callbacks are defined in hotplug_slot_ops:
> 
>   - available_images: Optional: available FPGA images
>   - image_load: Optional: trigger the FPGA to load a new image
> 
> 
> The process of reprogramming an FPGA card begins by removing all devices
> associated with the card that are not required for the reprogramming of
> the card. This includes PCIe devices (PFs and VFs) associated with the
> card as well as any other types of devices (platform, etc.) defined within
> the FPGA. The remaining devices are referred to here as "reserved" devices.
> After triggering the update of the FPGA card, the reserved devices are also
> removed.
> 
> The complete process for reprogramming the FPGA are:
>     1. remove all PFs and VFs except for PF0 (reserved).
>     2. remove all non-reserved devices of PF0.
>     3. trigger FPGA card to do the image update.
>     4. disable the link of the hotplug bridge.
>     5. remove all reserved devices under hotplug bridge.
>     6. wait for image reload done via BMC, e.g. 10s.
>     7. re-enable the link of hotplug bridge
>     8. enumerate PCI devices below the hotplug bridge
> 
> usage example:
> [root@localhost]# cd /sys/bus/pci/slot/X-X/
> 
> Get the available images.
> [root@localhost 2-1]# cat available_images
> bmc_factory bmc_user retimer_fw
> 
> Load the request images for FPGA Card, for example load the BMC user image:
> [root@localhost 2-1]# echo bmc_user > image_load
> 
> Tianfei Zhang (12):
>   PCI: hotplug: add new callbacks on hotplug_slot_ops
>   PCI: hotplug: expose APIs from pciehp driver
>   PCI: hotplug: add and expose link disable API
>   PCI: hotplug: add FPGA PCI hotplug manager driver
>   fpga: dfl: register dfl-pci device into fpgahph driver
>   driver core: expose device_is_ancestor() API
>   PCI: hotplug: add register/unregister function for BMC device
>   fpga: m10bmc-sec: register BMC device into fpgahp driver
>   fpga: dfl: remove non-reserved devices
>   PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp
>   fpga: m10bmc-sec: add m10bmc_sec_retimer_load callback
>   Documentation: fpga: add description of fpgahp driver
> 
>  Documentation/ABI/testing/sysfs-driver-fpgahp |  21 +
>  Documentation/fpga/fpgahp.rst                 |  29 +
>  Documentation/fpga/index.rst                  |   1 +
>  MAINTAINERS                                   |  10 +
>  drivers/base/core.c                           |   3 +-
>  drivers/fpga/Kconfig                          |   2 +
>  drivers/fpga/dfl-pci.c                        |  95 +++-
>  drivers/fpga/dfl.c                            |  58 ++
>  drivers/fpga/dfl.h                            |   4 +
>  drivers/fpga/intel-m10-bmc-sec-update.c       | 246 ++++++++
>  drivers/pci/hotplug/Kconfig                   |  14 +
>  drivers/pci/hotplug/Makefile                  |   1 +
>  drivers/pci/hotplug/fpgahp.c                  | 526 ++++++++++++++++++
>  drivers/pci/hotplug/pci_hotplug_core.c        |  88 +++
>  drivers/pci/hotplug/pciehp.h                  |   3 +
>  drivers/pci/hotplug/pciehp_hpc.c              |  11 +-
>  drivers/pci/hotplug/pciehp_pci.c              |   2 +
>  include/linux/device.h                        |   1 +
>  include/linux/fpga/fpgahp_manager.h           | 100 ++++
>  include/linux/mfd/intel-m10-bmc.h             |  31 ++
>  include/linux/pci_hotplug.h                   |   5 +
>  21 files changed, 1243 insertions(+), 8 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-fpgahp
>  create mode 100644 Documentation/fpga/fpgahp.rst
>  create mode 100644 drivers/pci/hotplug/fpgahp.c
>  create mode 100644 include/linux/fpga/fpgahp_manager.h
> 
> 
> base-commit: 5dc4c995db9eb45f6373a956eb1f69460e69e6d4
> -- 
> 2.38.1
> 

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

* RE: [PATCH v1 00/12] add FPGA hotplug manager driver
  2023-01-19  8:06 ` [PATCH v1 00/12] add FPGA hotplug manager driver Pali Rohár
@ 2023-01-19  8:17   ` Zhang, Tianfei
  2023-01-19 11:27     ` andriy.shevchenko
  0 siblings, 1 reply; 29+ messages in thread
From: Zhang, Tianfei @ 2023-01-19  8:17 UTC (permalink / raw)
  To: Pali Rohár
  Cc: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, mdf, Wu,
	Hao, Xu, Yilun, Rix, Tom, jgg, Weiny, Ira, andriy.shevchenko,
	Williams, Dan J, keescook, rafael, Weight, Russell H, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach



> -----Original Message-----
> From: Pali Rohár <pali@kernel.org>
> Sent: Thursday, January 19, 2023 4:06 PM
> To: Zhang, Tianfei <tianfei.zhang@intel.com>
> Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linux-fpga@vger.kernel.org;
> lukas@wunner.de; kabel@kernel.org; mani@kernel.org; mdf@kernel.org; Wu, Hao
> <hao.wu@intel.com>; Xu, Yilun <yilun.xu@intel.com>; Rix, Tom <trix@redhat.com>;
> jgg@ziepe.ca; Weiny, Ira <ira.weiny@intel.com>;
> andriy.shevchenko@linux.intel.com; Williams, Dan J <dan.j.williams@intel.com>;
> keescook@chromium.org; rafael@kernel.org; Weight, Russell H
> <russell.h.weight@intel.com>; corbet@lwn.net; linux-doc@vger.kernel.org;
> ilpo.jarvinen@linux.intel.com; lee@kernel.org; gregkh@linuxfoundation.org;
> matthew.gerlach@linux.intel.com
> Subject: Re: [PATCH v1 00/12] add FPGA hotplug manager driver
> 
> Hello!
> 
> On Wednesday 18 January 2023 20:35:50 Tianfei Zhang wrote:
> > This patchset introduces the FPGA hotplug manager (fpgahp) driver
> > which has been verified on the Intel N3000 card.
> >
> > When a PCIe-based FPGA card is reprogrammed, it temporarily disappears
> > from the PCIe bus. This needs to be managed to avoid PCIe errors and
> > to reprobe the device after reprogramming.
> >
> > To change the FPGA image, the kernel burns a new image into the flash
> > on the card, and then triggers the card BMC to load the new image into FPGA.
> > A new FPGA hotplug manager driver is introduced that leverages the
> > PCIe hotplug framework to trigger and manage the update of the FPGA
> > image, including the disappearance and reappearance of the card on the PCIe bus.
> > The fpgahp driver uses APIs from the pciehp driver.
> 
> Just I'm thinking about one thing. PCIe cards can support PCIe hotplug mechanism
> (via standard PCIe capabilities). So what would happen when FPGA based PCIe card is
> also hotplug-able? Will be there two PCI hotplug drivers/devices (one fpgahp and
> one pciehp)? Or just one and which?

For our Intel PAC N3000 and N6000 FPGA card, there are not support PCIe hotplug capability from hardware side now,
but from software perspective, the process of FPGA image load is very similar with PCIe hotplug, like removing all of devices under 
PCIe bridge, re-scan the PCIe device under the bridge, so we are looking for the PCIe hotplug framework and APIs from pciehp 
driver to manager this process, and reduce some duplicate code.

> 
> > Two new operation
> > callbacks are defined in hotplug_slot_ops:
> >
> >   - available_images: Optional: available FPGA images
> >   - image_load: Optional: trigger the FPGA to load a new image
> >
> >
> > The process of reprogramming an FPGA card begins by removing all
> > devices associated with the card that are not required for the
> > reprogramming of the card. This includes PCIe devices (PFs and VFs)
> > associated with the card as well as any other types of devices
> > (platform, etc.) defined within the FPGA. The remaining devices are referred to
> here as "reserved" devices.
> > After triggering the update of the FPGA card, the reserved devices are
> > also removed.
> >
> > The complete process for reprogramming the FPGA are:
> >     1. remove all PFs and VFs except for PF0 (reserved).
> >     2. remove all non-reserved devices of PF0.
> >     3. trigger FPGA card to do the image update.
> >     4. disable the link of the hotplug bridge.
> >     5. remove all reserved devices under hotplug bridge.
> >     6. wait for image reload done via BMC, e.g. 10s.
> >     7. re-enable the link of hotplug bridge
> >     8. enumerate PCI devices below the hotplug bridge
> >
> > usage example:
> > [root@localhost]# cd /sys/bus/pci/slot/X-X/
> >
> > Get the available images.
> > [root@localhost 2-1]# cat available_images bmc_factory bmc_user
> > retimer_fw
> >
> > Load the request images for FPGA Card, for example load the BMC user image:
> > [root@localhost 2-1]# echo bmc_user > image_load
> >
> > Tianfei Zhang (12):
> >   PCI: hotplug: add new callbacks on hotplug_slot_ops
> >   PCI: hotplug: expose APIs from pciehp driver
> >   PCI: hotplug: add and expose link disable API
> >   PCI: hotplug: add FPGA PCI hotplug manager driver
> >   fpga: dfl: register dfl-pci device into fpgahph driver
> >   driver core: expose device_is_ancestor() API
> >   PCI: hotplug: add register/unregister function for BMC device
> >   fpga: m10bmc-sec: register BMC device into fpgahp driver
> >   fpga: dfl: remove non-reserved devices
> >   PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp
> >   fpga: m10bmc-sec: add m10bmc_sec_retimer_load callback
> >   Documentation: fpga: add description of fpgahp driver
> >
> >  Documentation/ABI/testing/sysfs-driver-fpgahp |  21 +
> >  Documentation/fpga/fpgahp.rst                 |  29 +
> >  Documentation/fpga/index.rst                  |   1 +
> >  MAINTAINERS                                   |  10 +
> >  drivers/base/core.c                           |   3 +-
> >  drivers/fpga/Kconfig                          |   2 +
> >  drivers/fpga/dfl-pci.c                        |  95 +++-
> >  drivers/fpga/dfl.c                            |  58 ++
> >  drivers/fpga/dfl.h                            |   4 +
> >  drivers/fpga/intel-m10-bmc-sec-update.c       | 246 ++++++++
> >  drivers/pci/hotplug/Kconfig                   |  14 +
> >  drivers/pci/hotplug/Makefile                  |   1 +
> >  drivers/pci/hotplug/fpgahp.c                  | 526 ++++++++++++++++++
> >  drivers/pci/hotplug/pci_hotplug_core.c        |  88 +++
> >  drivers/pci/hotplug/pciehp.h                  |   3 +
> >  drivers/pci/hotplug/pciehp_hpc.c              |  11 +-
> >  drivers/pci/hotplug/pciehp_pci.c              |   2 +
> >  include/linux/device.h                        |   1 +
> >  include/linux/fpga/fpgahp_manager.h           | 100 ++++
> >  include/linux/mfd/intel-m10-bmc.h             |  31 ++
> >  include/linux/pci_hotplug.h                   |   5 +
> >  21 files changed, 1243 insertions(+), 8 deletions(-)  create mode
> > 100644 Documentation/ABI/testing/sysfs-driver-fpgahp
> >  create mode 100644 Documentation/fpga/fpgahp.rst  create mode 100644
> > drivers/pci/hotplug/fpgahp.c  create mode 100644
> > include/linux/fpga/fpgahp_manager.h
> >
> >
> > base-commit: 5dc4c995db9eb45f6373a956eb1f69460e69e6d4
> > --
> > 2.38.1
> >

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

* Re: [PATCH v1 12/12] Documentation: fpga: add description of fpgahp driver
  2023-01-19  1:36 ` [PATCH v1 12/12] Documentation: fpga: add description of fpgahp driver Tianfei Zhang
@ 2023-01-19  9:38   ` Bagas Sanjaya
  0 siblings, 0 replies; 29+ messages in thread
From: Bagas Sanjaya @ 2023-01-19  9:38 UTC (permalink / raw)
  To: Tianfei Zhang, bhelgaas, linux-pci, linux-fpga, lukas, kabel,
	mani, pali, mdf, hao.wu, yilun.xu, trix, jgg, ira.weiny,
	andriy.shevchenko, dan.j.williams, keescook, rafael,
	russell.h.weight, corbet, linux-doc, ilpo.jarvinen, lee, gregkh,
	matthew.gerlach

[-- Attachment #1: Type: text/plain, Size: 3942 bytes --]

On Wed, Jan 18, 2023 at 08:36:02PM -0500, Tianfei Zhang wrote:
> +===========================
> +FPGA Hotplug Manager Driver
> +===========================
> +
> +Authors:
> +
> +- Tianfei Zhang <tianfei.zhang@intel.com>
> +
> +There are some board managements for PCIe-based FPGA card like burning the entire
> +image, loading a new FPGA image or BMC firmware in FPGA deployment of data center
> +or cloud. For example, loading a new FPGA image, the driver needs to remove all of
> +PCI devices like PFs/VFs and as well as any other types of devices (platform, etc.)
> +defined within the FPGA. After triggering the image load of the FPGA card via BMC,
> +the driver reconfigures the PCI bus. The FPGA Hotplug Manager (fpgahp) driver manages
> +those devices and functions leveraging the PCI hotplug framework to deal with the
> +reconfiguration of the PCI bus and removal/probe of PCI devices below the FPGA card.
> +
> +This fpgahp driver adds 2 new callbacks to extend the hotplug mechanism to
> +allow selecting and loading a new FPGA image.
> +
> + - available_images: Optional: called to return the available images of a FPGA card.
> + - image_load: Optional: called to load a new image for a FPGA card.
> +
> +In general, the fpgahp driver provides some sysfs files::
> +
> +        /sys/bus/pci/slots/<X-X>/available_images
> +        /sys/bus/pci/slots/<X-X>/image_load

The doc reads a rather confused to me, so I have to make wording improv:

---- >8 ----
diff --git a/Documentation/fpga/fpgahp.rst b/Documentation/fpga/fpgahp.rst
index 3ec34bbffde10c..73f1b53de1cf85 100644
--- a/Documentation/fpga/fpgahp.rst
+++ b/Documentation/fpga/fpgahp.rst
@@ -8,22 +8,22 @@ Authors:
 
 - Tianfei Zhang <tianfei.zhang@intel.com>
 
-There are some board managements for PCIe-based FPGA card like burning the entire
-image, loading a new FPGA image or BMC firmware in FPGA deployment of data center
-or cloud. For example, loading a new FPGA image, the driver needs to remove all of
-PCI devices like PFs/VFs and as well as any other types of devices (platform, etc.)
-defined within the FPGA. After triggering the image load of the FPGA card via BMC,
-the driver reconfigures the PCI bus. The FPGA Hotplug Manager (fpgahp) driver manages
-those devices and functions leveraging the PCI hotplug framework to deal with the
-reconfiguration of the PCI bus and removal/probe of PCI devices below the FPGA card.
 
-This fpgahp driver adds 2 new callbacks to extend the hotplug mechanism to
-allow selecting and loading a new FPGA image.
+The FPGA Hotplug Manager (fpgahp) manages PCIe-based FPGA card devices.
+The PCI bus reconfiguration and device probe for devices below the FPGA
+card are done by leveraging the PCI hotplug framework.
 
- - available_images: Optional: called to return the available images of a FPGA card.
- - image_load: Optional: called to load a new image for a FPGA card.
+The driver can be helpful in device management tasks like burning the entire
+image and loading a new FPGA image or BMC firmware in FPGA deployment of data
+center or cloud. For example, when loading the image, the driver needs to
+remove all of PCI devices like PFs/VFs and as well as any other types of
+devices (platform, etc.) defined within the FPGA. After triggering the image
+load of the FPGA card via BMC, the driver reconfigures the appropriate PCI bus.
 
-In general, the fpgahp driver provides some sysfs files::
+The driver adds 2 new sysfs callbacks to extend the hotplug mechanism to
+allow selecting and loading a new FPGA image:
+
+ - ``/sys/bus/pci/slots/<X-X>/available_images``: list available images for
+   a FPGA card.
+ - ``/sys/bus/pci/slots/<X-X>/image_load``: load the image.
 
-        /sys/bus/pci/slots/<X-X>/available_images
-        /sys/bus/pci/slots/<X-X>/image_load

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v1 00/12] add FPGA hotplug manager driver
  2023-01-19  8:17   ` Zhang, Tianfei
@ 2023-01-19 11:27     ` andriy.shevchenko
  2023-01-19 12:09       ` Zhang, Tianfei
  0 siblings, 1 reply; 29+ messages in thread
From: andriy.shevchenko @ 2023-01-19 11:27 UTC (permalink / raw)
  To: Zhang, Tianfei
  Cc: Pali Rohár, bhelgaas, linux-pci, linux-fpga, lukas, kabel,
	mani, mdf, Wu, Hao, Xu, Yilun, Rix, Tom, jgg, Weiny, Ira,
	Williams, Dan J, keescook, rafael, Weight, Russell H, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach

On Thu, Jan 19, 2023 at 08:17:05AM +0000, Zhang, Tianfei wrote:
> > From: Pali Rohár <pali@kernel.org>
> > Sent: Thursday, January 19, 2023 4:06 PM
> > On Wednesday 18 January 2023 20:35:50 Tianfei Zhang wrote:

...

> > > To change the FPGA image, the kernel burns a new image into the flash
> > > on the card, and then triggers the card BMC to load the new image into FPGA.
> > > A new FPGA hotplug manager driver is introduced that leverages the
> > > PCIe hotplug framework to trigger and manage the update of the FPGA
> > > image, including the disappearance and reappearance of the card on the PCIe bus.
> > > The fpgahp driver uses APIs from the pciehp driver.
> > 
> > Just I'm thinking about one thing. PCIe cards can support PCIe hotplug mechanism
> > (via standard PCIe capabilities). So what would happen when FPGA based PCIe card is
> > also hotplug-able? Will be there two PCI hotplug drivers/devices (one fpgahp and
> > one pciehp)? Or just one and which?
> 
> For our Intel PAC N3000 and N6000 FPGA card, there are not support PCIe
> hotplug capability from hardware side now, but from software perspective, the
> process of FPGA image load is very similar with PCIe hotplug, like removing
> all of devices under PCIe bridge, re-scan the PCIe device under the bridge,
> so we are looking for the PCIe hotplug framework and APIs from pciehp driver
> to manager this process, and reduce some duplicate code.

Exactly, from the OS perspective they both should be equivalent.

-- 
With Best Regards,
Andy Shevchenko



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

* RE: [PATCH v1 00/12] add FPGA hotplug manager driver
  2023-01-19 11:27     ` andriy.shevchenko
@ 2023-01-19 12:09       ` Zhang, Tianfei
  0 siblings, 0 replies; 29+ messages in thread
From: Zhang, Tianfei @ 2023-01-19 12:09 UTC (permalink / raw)
  To: andriy.shevchenko
  Cc: Pali Rohár, bhelgaas, linux-pci, linux-fpga, lukas, kabel,
	mani, mdf, Wu, Hao, Xu, Yilun, Rix, Tom, jgg, Weiny, Ira,
	Williams, Dan J, keescook, rafael, Weight, Russell H, corbet,
	linux-doc, ilpo.jarvinen, lee, gregkh, matthew.gerlach



> -----Original Message-----
> From: andriy.shevchenko@linux.intel.com <andriy.shevchenko@linux.intel.com>
> Sent: Thursday, January 19, 2023 7:28 PM
> To: Zhang, Tianfei <tianfei.zhang@intel.com>
> Cc: Pali Rohár <pali@kernel.org>; bhelgaas@google.com; linux-
> pci@vger.kernel.org; linux-fpga@vger.kernel.org; lukas@wunner.de;
> kabel@kernel.org; mani@kernel.org; mdf@kernel.org; Wu, Hao
> <hao.wu@intel.com>; Xu, Yilun <yilun.xu@intel.com>; Rix, Tom <trix@redhat.com>;
> jgg@ziepe.ca; Weiny, Ira <ira.weiny@intel.com>; Williams, Dan J
> <dan.j.williams@intel.com>; keescook@chromium.org; rafael@kernel.org; Weight,
> Russell H <russell.h.weight@intel.com>; corbet@lwn.net; linux-
> doc@vger.kernel.org; ilpo.jarvinen@linux.intel.com; lee@kernel.org;
> gregkh@linuxfoundation.org; matthew.gerlach@linux.intel.com
> Subject: Re: [PATCH v1 00/12] add FPGA hotplug manager driver
> 
> On Thu, Jan 19, 2023 at 08:17:05AM +0000, Zhang, Tianfei wrote:
> > > From: Pali Rohár <pali@kernel.org>
> > > Sent: Thursday, January 19, 2023 4:06 PM On Wednesday 18 January
> > > 2023 20:35:50 Tianfei Zhang wrote:
> 
> ...
> 
> > > > To change the FPGA image, the kernel burns a new image into the
> > > > flash on the card, and then triggers the card BMC to load the new image into
> FPGA.
> > > > A new FPGA hotplug manager driver is introduced that leverages the
> > > > PCIe hotplug framework to trigger and manage the update of the
> > > > FPGA image, including the disappearance and reappearance of the card on the
> PCIe bus.
> > > > The fpgahp driver uses APIs from the pciehp driver.
> > >
> > > Just I'm thinking about one thing. PCIe cards can support PCIe
> > > hotplug mechanism (via standard PCIe capabilities). So what would
> > > happen when FPGA based PCIe card is also hotplug-able? Will be there
> > > two PCI hotplug drivers/devices (one fpgahp and one pciehp)? Or just one and
> which?
> >
> > For our Intel PAC N3000 and N6000 FPGA card, there are not support
> > PCIe hotplug capability from hardware side now, but from software
> > perspective, the process of FPGA image load is very similar with PCIe
> > hotplug, like removing all of devices under PCIe bridge, re-scan the
> > PCIe device under the bridge, so we are looking for the PCIe hotplug
> > framework and APIs from pciehp driver to manager this process, and reduce some
> duplicate code.
> 
> Exactly, from the OS perspective they both should be equivalent.

Additionally, for FPGA usage, it doesn't need physical hot-add and hot-removal of the card for FPGA card managements 
like load a new FPGA image in FPGA deployment of data center or cloud.  So whatever the card support
PCIe hotplug capability or not, it can be done by  leveraging the PCI hotplug framework and APIs from pciehp driver.
I think it also has benefits for other vendor's FPGA Card to do some card managements by using  fpgahp driver.


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

* Re: [PATCH v1 10/12] PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp
  2023-01-19  1:36 ` [PATCH v1 10/12] PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp Tianfei Zhang
@ 2023-01-19 13:28   ` Greg KH
  2023-01-20 22:38     ` Russ Weight
  0 siblings, 1 reply; 29+ messages in thread
From: Greg KH @ 2023-01-19 13:28 UTC (permalink / raw)
  To: Tianfei Zhang
  Cc: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, matthew.gerlach

On Wed, Jan 18, 2023 at 08:36:00PM -0500, Tianfei Zhang wrote:
> Implement the image_load and available_images callback functions
> for fpgahp driver. This patch leverages some APIs from pciehp
> driver to implement the device reconfiguration below the PCI hotplug
> bridge.
> 
> Here are the steps for a process of image load.
> 1. remove all PFs and VFs except the PF0.
> 2. remove all non-reserved devices of PF0.
> 3. trigger a image load via BMC.
> 4. disable the link of the hotplug bridge.
> 5. remove all reserved devices under PF0 and PCI devices
>    below the hotplug bridge.
> 6. wait for image load done via BMC, e.g. 10s.
> 7. re-enable the link of the hotplug bridge.
> 8. re-enumerate PCI devices below the hotplug bridge.
> 
> Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
> ---
>  Documentation/ABI/testing/sysfs-driver-fpgahp |  21 ++
>  MAINTAINERS                                   |   1 +
>  drivers/pci/hotplug/fpgahp.c                  | 179 ++++++++++++++++++
>  3 files changed, 201 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-fpgahp
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-fpgahp b/Documentation/ABI/testing/sysfs-driver-fpgahp
> new file mode 100644
> index 000000000000..8d4b1bfc4012
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-fpgahp
> @@ -0,0 +1,21 @@
> +What:		/sys/bus/pci/slots/X-X/available_images
> +Date:		May 2023
> +KernelVersion:	6.3
> +Contact:	Tianfei Zhang <tianfei.zhang@intel.com>
> +Description:	Read-only. This file returns a space separated list of
> +		key words that may be written into the image_load file
> +		described below. These keywords decribe an FPGA, BMC,
> +		or firmware image in FLASH or EEPROM storage that may
> +		be loaded.

No, sysfs is "one value per file", why is this a list?

And what exactly defines the values in this list?

> +
> +What:		/sys/bus/pci/slots/X-X/image_load
> +Date:		May 2023
> +KernelVersion:	6.3
> +Contact:	Tianfei Zhang <tianfei.zhang@intel.com>
> +Description:	Write-only. A key word may be written to this file to
> +		trigger a new image loading of an FPGA, BMC, or firmware
> +		image from FLASH or EEPROM. Refer to the available_images
> +		file for a list of supported key words for the underlying
> +		device.
> +		Writing an unsupported string to this file will result in
> +		EINVAL being returned.

Why is this a separate file from the "read the list" file?

That feels wrong.

thanks,

greg k-h

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

* Re: [PATCH v1 01/12] PCI: hotplug: add new callbacks on hotplug_slot_ops
  2023-01-19  1:35 ` [PATCH v1 01/12] PCI: hotplug: add new callbacks on hotplug_slot_ops Tianfei Zhang
@ 2023-01-19 13:31   ` Greg KH
  0 siblings, 0 replies; 29+ messages in thread
From: Greg KH @ 2023-01-19 13:31 UTC (permalink / raw)
  To: Tianfei Zhang
  Cc: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, matthew.gerlach

On Wed, Jan 18, 2023 at 08:35:51PM -0500, Tianfei Zhang wrote:
> To reprogram an PCIe-based FPGA card, a new image is
> burned into FLASH on the card and then the card BMC is
> triggered to reboot the card and load the new image.
> 
> Two new operation callbacks are defined in hotplug_slot_ops
> to trigger the reprogramming of an FPGA-based PCIe card:
> 
>   - available_images: Optional: available FPGA images
>   - image_load: Optional: trigger the FPGA to load a new image
> 
> Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/pci/hotplug/pci_hotplug_core.c | 88 ++++++++++++++++++++++++++
>  include/linux/pci_hotplug.h            |  5 ++
>  2 files changed, 93 insertions(+)
> 
> diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
> index 058d5937d8a9..2b14b6513a03 100644
> --- a/drivers/pci/hotplug/pci_hotplug_core.c
> +++ b/drivers/pci/hotplug/pci_hotplug_core.c
> @@ -231,6 +231,52 @@ static struct pci_slot_attribute hotplug_slot_attr_test = {
>  	.store = test_write_file
>  };
>  
> +static ssize_t available_images_read_file(struct pci_slot *pci_slot, char *buf)
> +{
> +	struct hotplug_slot *slot = pci_slot->hotplug;
> +	ssize_t count = 0;
> +
> +	if (!try_module_get(slot->owner))
> +		return -ENODEV;
> +
> +	if (slot->ops->available_images(slot, buf))
> +		count = slot->ops->available_images(slot, buf);
> +
> +	module_put(slot->owner);
> +
> +	return count;
> +}
> +
> +static struct pci_slot_attribute hotplug_slot_attr_available_images = {
> +	.attr = { .name = "available_images", .mode = 0444 },
> +	.show = available_images_read_file,

If you name things properly, you can use the correct macros and not have
to open-code any of this :(

> +static ssize_t image_load_write_file(struct pci_slot *pci_slot,
> +				     const char *buf, size_t count)
> +{
> +	struct hotplug_slot *slot = pci_slot->hotplug;
> +	int retval = 0;
> +
> +	if (!try_module_get(slot->owner))
> +		return -ENODEV;
> +
> +	if (slot->ops->image_load)
> +		retval = slot->ops->image_load(slot, buf);
> +
> +	module_put(slot->owner);
> +
> +	if (retval)
> +		return retval;
> +
> +	return count;
> +}
> +
> +static struct pci_slot_attribute hotplug_slot_attr_image_load = {
> +	.attr = { .name = "image_load", .mode = 0644 },
> +	.store = image_load_write_file,

Same here, don't open-code this.

> +};
> +
>  static bool has_power_file(struct pci_slot *pci_slot)
>  {
>  	struct hotplug_slot *slot = pci_slot->hotplug;
> @@ -289,6 +335,20 @@ static bool has_test_file(struct pci_slot *pci_slot)
>  	return false;
>  }
>  
> +static bool has_available_images_file(struct pci_slot *pci_slot)
> +{
> +	struct hotplug_slot *slot = pci_slot->hotplug;
> +
> +	return slot && slot->ops && slot->ops->available_images;
> +}
> +
> +static bool has_image_load_file(struct pci_slot *pci_slot)
> +{
> +	struct hotplug_slot *slot = pci_slot->hotplug;
> +
> +	return slot && slot->ops && slot->ops->image_load;
> +}
> +
>  static int fs_add_slot(struct pci_slot *pci_slot)
>  {
>  	int retval = 0;
> @@ -331,8 +391,30 @@ static int fs_add_slot(struct pci_slot *pci_slot)
>  			goto exit_test;
>  	}
>  
> +	if (has_available_images_file(pci_slot)) {
> +		retval = sysfs_create_file(&pci_slot->kobj,
> +					   &hotplug_slot_attr_available_images.attr);
> +		if (retval)
> +			goto exit_available_images;
> +	}
> +
> +	if (has_image_load_file(pci_slot)) {
> +		retval = sysfs_create_file(&pci_slot->kobj,
> +					   &hotplug_slot_attr_image_load.attr);
> +		if (retval)
> +			goto exit_image_load;
> +	}
> +
>  	goto exit;
>  
> +exit_image_load:
> +	if (has_adapter_file(pci_slot))
> +		sysfs_remove_file(&pci_slot->kobj,
> +				  &hotplug_slot_attr_available_images.attr);
> +exit_available_images:
> +	if (has_adapter_file(pci_slot))
> +		sysfs_remove_file(&pci_slot->kobj,
> +				  &hotplug_slot_attr_test.attr);
>  exit_test:
>  	if (has_adapter_file(pci_slot))
>  		sysfs_remove_file(&pci_slot->kobj,
> @@ -372,6 +454,12 @@ static void fs_remove_slot(struct pci_slot *pci_slot)
>  	if (has_test_file(pci_slot))
>  		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_test.attr);
>  
> +	if (has_available_images_file(pci_slot))
> +		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_available_images.attr);
> +
> +	if (has_image_load_file(pci_slot))
> +		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_image_load.attr);
> +

Ick no, please just make this an attribute group that properly shows or
does not show, the attribute when created.  Do not manually add
individual sysfs files.

Yes, I know the existing code does this, so it's not really your fault,
but let's not persist in making this code even messier.  Convert to a
group first and then your new files will be added automagically without
having to care about anything here at all.

thanks,

greg k-h

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

* Re: [PATCH v1 00/12] add FPGA hotplug manager driver
  2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
                   ` (12 preceding siblings ...)
  2023-01-19  8:06 ` [PATCH v1 00/12] add FPGA hotplug manager driver Pali Rohár
@ 2023-01-19 13:33 ` Greg KH
  2023-01-19 13:43   ` Rafael J. Wysocki
  2023-01-20 16:28   ` Russ Weight
  13 siblings, 2 replies; 29+ messages in thread
From: Greg KH @ 2023-01-19 13:33 UTC (permalink / raw)
  To: Tianfei Zhang
  Cc: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, lee, matthew.gerlach

On Wed, Jan 18, 2023 at 08:35:50PM -0500, Tianfei Zhang wrote:
> This patchset introduces the FPGA hotplug manager (fpgahp) driver which 
> has been verified on the Intel N3000 card.
> 
> When a PCIe-based FPGA card is reprogrammed, it temporarily disappears
> from the PCIe bus. This needs to be managed to avoid PCIe errors and to
> reprobe the device after reprogramming.
> 
> To change the FPGA image, the kernel burns a new image into the flash on
> the card, and then triggers the card BMC to load the new image into FPGA.
> A new FPGA hotplug manager driver is introduced that leverages the PCIe
> hotplug framework to trigger and manage the update of the FPGA image,
> including the disappearance and reappearance of the card on the PCIe bus.
> The fpgahp driver uses APIs from the pciehp driver. Two new operation
> callbacks are defined in hotplug_slot_ops:
> 
>   - available_images: Optional: available FPGA images
>   - image_load: Optional: trigger the FPGA to load a new image
> 
> 
> The process of reprogramming an FPGA card begins by removing all devices
> associated with the card that are not required for the reprogramming of
> the card. This includes PCIe devices (PFs and VFs) associated with the
> card as well as any other types of devices (platform, etc.) defined within
> the FPGA. The remaining devices are referred to here as "reserved" devices.
> After triggering the update of the FPGA card, the reserved devices are also
> removed.
> 
> The complete process for reprogramming the FPGA are:
>     1. remove all PFs and VFs except for PF0 (reserved).
>     2. remove all non-reserved devices of PF0.
>     3. trigger FPGA card to do the image update.
>     4. disable the link of the hotplug bridge.
>     5. remove all reserved devices under hotplug bridge.
>     6. wait for image reload done via BMC, e.g. 10s.
>     7. re-enable the link of hotplug bridge
>     8. enumerate PCI devices below the hotplug bridge
> 
> usage example:
> [root@localhost]# cd /sys/bus/pci/slot/X-X/
> 
> Get the available images.
> [root@localhost 2-1]# cat available_images
> bmc_factory bmc_user retimer_fw
> 
> Load the request images for FPGA Card, for example load the BMC user image:
> [root@localhost 2-1]# echo bmc_user > image_load

Why is all of this tied into the pci hotplug code? Shouldn't it be
specific to this one driver instead?  pci hotplug is for removing/adding
PCI devices to the system, not messing with FPGA images.

This feels like an abuse of the pci hotplug bus to me as this is NOT
really a PCI hotplug bus at all, right?

Or is it?  If so, then the slots should show up under the PCI device
itself, not in /sys/bus/pci/slot/.  That location is there for old old
stuff, we probably should move it one of these days as there's lots of
special-cases in the driver core just because of that :(

thanks,

greg k-h

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

* Re: [PATCH v1 00/12] add FPGA hotplug manager driver
  2023-01-19 13:33 ` Greg KH
@ 2023-01-19 13:43   ` Rafael J. Wysocki
  2023-01-19 15:33     ` Greg KH
  2023-01-20 16:28   ` Russ Weight
  1 sibling, 1 reply; 29+ messages in thread
From: Rafael J. Wysocki @ 2023-01-19 13:43 UTC (permalink / raw)
  To: Greg KH
  Cc: Tianfei Zhang, bhelgaas, linux-pci, linux-fpga, lukas, kabel,
	mani, pali, mdf, hao.wu, yilun.xu, trix, jgg, ira.weiny,
	andriy.shevchenko, dan.j.williams, keescook, rafael,
	russell.h.weight, corbet, linux-doc, ilpo.jarvinen, lee,
	matthew.gerlach

On Thu, Jan 19, 2023 at 2:34 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jan 18, 2023 at 08:35:50PM -0500, Tianfei Zhang wrote:
> > This patchset introduces the FPGA hotplug manager (fpgahp) driver which
> > has been verified on the Intel N3000 card.
> >
> > When a PCIe-based FPGA card is reprogrammed, it temporarily disappears
> > from the PCIe bus. This needs to be managed to avoid PCIe errors and to
> > reprobe the device after reprogramming.
> >
> > To change the FPGA image, the kernel burns a new image into the flash on
> > the card, and then triggers the card BMC to load the new image into FPGA.
> > A new FPGA hotplug manager driver is introduced that leverages the PCIe
> > hotplug framework to trigger and manage the update of the FPGA image,
> > including the disappearance and reappearance of the card on the PCIe bus.
> > The fpgahp driver uses APIs from the pciehp driver. Two new operation
> > callbacks are defined in hotplug_slot_ops:
> >
> >   - available_images: Optional: available FPGA images
> >   - image_load: Optional: trigger the FPGA to load a new image
> >
> >
> > The process of reprogramming an FPGA card begins by removing all devices
> > associated with the card that are not required for the reprogramming of
> > the card. This includes PCIe devices (PFs and VFs) associated with the
> > card as well as any other types of devices (platform, etc.) defined within
> > the FPGA. The remaining devices are referred to here as "reserved" devices.
> > After triggering the update of the FPGA card, the reserved devices are also
> > removed.
> >
> > The complete process for reprogramming the FPGA are:
> >     1. remove all PFs and VFs except for PF0 (reserved).
> >     2. remove all non-reserved devices of PF0.
> >     3. trigger FPGA card to do the image update.
> >     4. disable the link of the hotplug bridge.
> >     5. remove all reserved devices under hotplug bridge.
> >     6. wait for image reload done via BMC, e.g. 10s.
> >     7. re-enable the link of hotplug bridge
> >     8. enumerate PCI devices below the hotplug bridge
> >
> > usage example:
> > [root@localhost]# cd /sys/bus/pci/slot/X-X/
> >
> > Get the available images.
> > [root@localhost 2-1]# cat available_images
> > bmc_factory bmc_user retimer_fw
> >
> > Load the request images for FPGA Card, for example load the BMC user image:
> > [root@localhost 2-1]# echo bmc_user > image_load
>
> Why is all of this tied into the pci hotplug code? Shouldn't it be
> specific to this one driver instead?  pci hotplug is for removing/adding
> PCI devices to the system, not messing with FPGA images.
>
> This feels like an abuse of the pci hotplug bus to me as this is NOT
> really a PCI hotplug bus at all, right?
>
> Or is it?  If so, then the slots should show up under the PCI device
> itself, not in /sys/bus/pci/slot/.  That location is there for old old
> stuff, we probably should move it one of these days as there's lots of
> special-cases in the driver core just because of that :(

I'm not sure if I can agree with this statement.

The slot here is what is registered via pci_hp_register(), isn't it?

There are multiple users of this in the tree, including ACPI-based PCI
hotplug, which is not really that old.

Are you saying that this should not be used?

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

* Re: [PATCH v1 11/12] fpga: m10bmc-sec: add m10bmc_sec_retimer_load callback
  2023-01-19  1:36 ` [PATCH v1 11/12] fpga: m10bmc-sec: add m10bmc_sec_retimer_load callback Tianfei Zhang
@ 2023-01-19 14:22   ` Lee Jones
  0 siblings, 0 replies; 29+ messages in thread
From: Lee Jones @ 2023-01-19 14:22 UTC (permalink / raw)
  To: Tianfei Zhang
  Cc: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, russell.h.weight, corbet,
	linux-doc, ilpo.jarvinen, gregkh, matthew.gerlach

On Wed, 18 Jan 2023, Tianfei Zhang wrote:

> Create m10bmc_sec_retimer_load() callback function to provide
> a trigger to update a new retimer (Intel C827 Ethernet
> transceiver) firmware on Intel PAC N3000 Card.
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
> ---
>  drivers/fpga/intel-m10-bmc-sec-update.c | 136 ++++++++++++++++++++++++
>  include/linux/mfd/intel-m10-bmc.h       |  31 ++++++

Looks mostly fine - no need to send this to me again, thanks:

Acked-by: Lee Jones <lee@kernel.org>

-- 
Lee Jones [李琼斯]

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

* Re: [PATCH v1 00/12] add FPGA hotplug manager driver
  2023-01-19 13:43   ` Rafael J. Wysocki
@ 2023-01-19 15:33     ` Greg KH
  0 siblings, 0 replies; 29+ messages in thread
From: Greg KH @ 2023-01-19 15:33 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Tianfei Zhang, bhelgaas, linux-pci, linux-fpga, lukas, kabel,
	mani, pali, mdf, hao.wu, yilun.xu, trix, jgg, ira.weiny,
	andriy.shevchenko, dan.j.williams, keescook, russell.h.weight,
	corbet, linux-doc, ilpo.jarvinen, lee, matthew.gerlach

On Thu, Jan 19, 2023 at 02:43:21PM +0100, Rafael J. Wysocki wrote:
> On Thu, Jan 19, 2023 at 2:34 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Jan 18, 2023 at 08:35:50PM -0500, Tianfei Zhang wrote:
> > > This patchset introduces the FPGA hotplug manager (fpgahp) driver which
> > > has been verified on the Intel N3000 card.
> > >
> > > When a PCIe-based FPGA card is reprogrammed, it temporarily disappears
> > > from the PCIe bus. This needs to be managed to avoid PCIe errors and to
> > > reprobe the device after reprogramming.
> > >
> > > To change the FPGA image, the kernel burns a new image into the flash on
> > > the card, and then triggers the card BMC to load the new image into FPGA.
> > > A new FPGA hotplug manager driver is introduced that leverages the PCIe
> > > hotplug framework to trigger and manage the update of the FPGA image,
> > > including the disappearance and reappearance of the card on the PCIe bus.
> > > The fpgahp driver uses APIs from the pciehp driver. Two new operation
> > > callbacks are defined in hotplug_slot_ops:
> > >
> > >   - available_images: Optional: available FPGA images
> > >   - image_load: Optional: trigger the FPGA to load a new image
> > >
> > >
> > > The process of reprogramming an FPGA card begins by removing all devices
> > > associated with the card that are not required for the reprogramming of
> > > the card. This includes PCIe devices (PFs and VFs) associated with the
> > > card as well as any other types of devices (platform, etc.) defined within
> > > the FPGA. The remaining devices are referred to here as "reserved" devices.
> > > After triggering the update of the FPGA card, the reserved devices are also
> > > removed.
> > >
> > > The complete process for reprogramming the FPGA are:
> > >     1. remove all PFs and VFs except for PF0 (reserved).
> > >     2. remove all non-reserved devices of PF0.
> > >     3. trigger FPGA card to do the image update.
> > >     4. disable the link of the hotplug bridge.
> > >     5. remove all reserved devices under hotplug bridge.
> > >     6. wait for image reload done via BMC, e.g. 10s.
> > >     7. re-enable the link of hotplug bridge
> > >     8. enumerate PCI devices below the hotplug bridge
> > >
> > > usage example:
> > > [root@localhost]# cd /sys/bus/pci/slot/X-X/
> > >
> > > Get the available images.
> > > [root@localhost 2-1]# cat available_images
> > > bmc_factory bmc_user retimer_fw
> > >
> > > Load the request images for FPGA Card, for example load the BMC user image:
> > > [root@localhost 2-1]# echo bmc_user > image_load
> >
> > Why is all of this tied into the pci hotplug code? Shouldn't it be
> > specific to this one driver instead?  pci hotplug is for removing/adding
> > PCI devices to the system, not messing with FPGA images.
> >
> > This feels like an abuse of the pci hotplug bus to me as this is NOT
> > really a PCI hotplug bus at all, right?
> >
> > Or is it?  If so, then the slots should show up under the PCI device
> > itself, not in /sys/bus/pci/slot/.  That location is there for old old
> > stuff, we probably should move it one of these days as there's lots of
> > special-cases in the driver core just because of that :(
> 
> I'm not sure if I can agree with this statement.
> 
> The slot here is what is registered via pci_hp_register(), isn't it?

Yes, but is it really a "slot" like a normal PCI slot?

> There are multiple users of this in the tree, including ACPI-based PCI
> hotplug, which is not really that old.

It's really old, I think I worked on that in the 2.4/2.5 days?  Anyway,
it's been around a long time.

> Are you saying that this should not be used?

I'm saying that PCI is the only subsystem/bus that has something like
this and we have a number of functions exported in the driver core only
for the pci hotplug slot list.  Which kind of implies that maybe it
should be moved to something else?  I don't have any specific ideas what
it should be, just that it feels really odd as-is still.

thanks,

greg k-h

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

* Re: [PATCH v1 00/12] add FPGA hotplug manager driver
  2023-01-19 13:33 ` Greg KH
  2023-01-19 13:43   ` Rafael J. Wysocki
@ 2023-01-20 16:28   ` Russ Weight
  2023-01-20 18:42     ` Lukas Wunner
  1 sibling, 1 reply; 29+ messages in thread
From: Russ Weight @ 2023-01-20 16:28 UTC (permalink / raw)
  To: Greg KH, Tianfei Zhang
  Cc: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, corbet, linux-doc,
	ilpo.jarvinen, lee, matthew.gerlach



On 1/19/23 05:33, Greg KH wrote:
> On Wed, Jan 18, 2023 at 08:35:50PM -0500, Tianfei Zhang wrote:
>> This patchset introduces the FPGA hotplug manager (fpgahp) driver which 
>> has been verified on the Intel N3000 card.
>>
>> When a PCIe-based FPGA card is reprogrammed, it temporarily disappears
>> from the PCIe bus. This needs to be managed to avoid PCIe errors and to
>> reprobe the device after reprogramming.
>>
>> To change the FPGA image, the kernel burns a new image into the flash on
>> the card, and then triggers the card BMC to load the new image into FPGA.
>> A new FPGA hotplug manager driver is introduced that leverages the PCIe
>> hotplug framework to trigger and manage the update of the FPGA image,
>> including the disappearance and reappearance of the card on the PCIe bus.
>> The fpgahp driver uses APIs from the pciehp driver. Two new operation
>> callbacks are defined in hotplug_slot_ops:
>>
>>   - available_images: Optional: available FPGA images
>>   - image_load: Optional: trigger the FPGA to load a new image
>>
>>
>> The process of reprogramming an FPGA card begins by removing all devices
>> associated with the card that are not required for the reprogramming of
>> the card. This includes PCIe devices (PFs and VFs) associated with the
>> card as well as any other types of devices (platform, etc.) defined within
>> the FPGA. The remaining devices are referred to here as "reserved" devices.
>> After triggering the update of the FPGA card, the reserved devices are also
>> removed.
>>
>> The complete process for reprogramming the FPGA are:
>>     1. remove all PFs and VFs except for PF0 (reserved).
>>     2. remove all non-reserved devices of PF0.
>>     3. trigger FPGA card to do the image update.
>>     4. disable the link of the hotplug bridge.
>>     5. remove all reserved devices under hotplug bridge.
>>     6. wait for image reload done via BMC, e.g. 10s.
>>     7. re-enable the link of hotplug bridge
>>     8. enumerate PCI devices below the hotplug bridge
>>
>> usage example:
>> [root@localhost]# cd /sys/bus/pci/slot/X-X/
>>
>> Get the available images.
>> [root@localhost 2-1]# cat available_images
>> bmc_factory bmc_user retimer_fw
>>
>> Load the request images for FPGA Card, for example load the BMC user image:
>> [root@localhost 2-1]# echo bmc_user > image_load
> Why is all of this tied into the pci hotplug code? Shouldn't it be
> specific to this one driver instead?  pci hotplug is for removing/adding
> PCI devices to the system, not messing with FPGA images.
>
> This feels like an abuse of the pci hotplug bus to me as this is NOT
> really a PCI hotplug bus at all, right?
While it is true that triggering an FPGA image-load does not involve
hotplug specific registers to be managed, the RTL that comprises
the PCIe interface will disappear and then reappear after the FPGA
is reprogrammed. When it reappears, it_could/_/have a different PCI
ID. The process of managing this event has a lot of similarity to a
PCIe hotplug event; there is a lot of existing PCIe hotplug related
code that could be leveraged.

As alternatives to the idea of creating a hotplug driver, we have
considered creating a new PCIe service driver specifically to
handle FPGA reprogramming, or modifying the existing hotplug
driver(s) to add FPGA support. We have also considered a separate
fpga-reload driver that would not be bound to a PCIe interface,
but would still leverage the PCIe code to manage the event. Do
any of these options sound preferable to creating an FPGA hotplug
driver?
>
> Or is it?  If so, then the slots should show up under the PCI device
> itself, not in /sys/bus/pci/slot/.  That location is there for old old
> stuff, we probably should move it one of these days as there's lots of
> special-cases in the driver core just because of that :(
>
> thanks,
>
> greg k-h


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

* Re: [PATCH v1 00/12] add FPGA hotplug manager driver
  2023-01-20 16:28   ` Russ Weight
@ 2023-01-20 18:42     ` Lukas Wunner
  2023-01-21  7:34       ` Greg KH
  0 siblings, 1 reply; 29+ messages in thread
From: Lukas Wunner @ 2023-01-20 18:42 UTC (permalink / raw)
  To: Russ Weight
  Cc: Greg KH, Tianfei Zhang, bhelgaas, linux-pci, linux-fpga, kabel,
	mani, pali, mdf, hao.wu, yilun.xu, trix, jgg, ira.weiny,
	andriy.shevchenko, dan.j.williams, keescook, rafael, corbet,
	linux-doc, ilpo.jarvinen, lee, matthew.gerlach

On Fri, Jan 20, 2023 at 08:28:51AM -0800, Russ Weight wrote:
> On 1/19/23 05:33, Greg KH wrote:
> > On Wed, Jan 18, 2023 at 08:35:50PM -0500, Tianfei Zhang wrote:
> > > This patchset introduces the FPGA hotplug manager (fpgahp) driver which
> > > has been verified on the Intel N3000 card.
> > >
> > > When a PCIe-based FPGA card is reprogrammed, it temporarily disappears
> > > from the PCIe bus. This needs to be managed to avoid PCIe errors and to
> > > reprobe the device after reprogramming.
> > >
> > > To change the FPGA image, the kernel burns a new image into the flash on
> > > the card, and then triggers the card BMC to load the new image into FPGA.
> > > A new FPGA hotplug manager driver is introduced that leverages the PCIe
> > > hotplug framework to trigger and manage the update of the FPGA image,
> > > including the disappearance and reappearance of the card on the PCIe bus.
> > > The fpgahp driver uses APIs from the pciehp driver. Two new operation
> > > callbacks are defined in hotplug_slot_ops:
> > >
> > >   - available_images: Optional: available FPGA images
> > >   - image_load: Optional: trigger the FPGA to load a new image
> > 
> > Why is all of this tied into the pci hotplug code? Shouldn't it be
> > specific to this one driver instead?  pci hotplug is for removing/adding
> > PCI devices to the system, not messing with FPGA images.
> >
> > This feels like an abuse of the pci hotplug bus to me as this is NOT
> > really a PCI hotplug bus at all, right?
> 
> While it is true that triggering an FPGA image-load does not involve
> hotplug specific registers to be managed, the RTL that comprises
> the PCIe interface will disappear and then reappear after the FPGA
> is reprogrammed. When it reappears, it_could/_/have a different PCI
> ID. The process of managing this event has a lot of similarity to a
> PCIe hotplug event; there is a lot of existing PCIe hotplug related
> code that could be leveraged.

It sounds like the N3000 is a PCI endpoint device which, when reprogrammed,
briefly disappears from the bus and then may reappear under a different
device ID.

What you want to do then is make sure that the slot into which the N3000
is plugged is hotplug-capable.  In that case, pciehp will handle
disappearance and reappearance of the card just fine.  Once the N3000
disables the link, pciehp will bring down the slot.  Once it re-enables
the link, it will bring the slot up again.  It's as if the card was
removed and replaced with a different one.  pciehp will bind to the
Root Port or Downstream Port associated with the hotplug slot.

The pci_hotplug_port infrastructure is for hotplug controllers which
handle devices disappearing and reappearing *below* them.  It is not
for endpoint devices.

Thanks,

Lukas

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

* Re: [PATCH v1 10/12] PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp
  2023-01-19 13:28   ` Greg KH
@ 2023-01-20 22:38     ` Russ Weight
  2023-01-21  7:35       ` Greg KH
  0 siblings, 1 reply; 29+ messages in thread
From: Russ Weight @ 2023-01-20 22:38 UTC (permalink / raw)
  To: Greg KH, Tianfei Zhang
  Cc: bhelgaas, linux-pci, linux-fpga, lukas, kabel, mani, pali, mdf,
	hao.wu, yilun.xu, trix, jgg, ira.weiny, andriy.shevchenko,
	dan.j.williams, keescook, rafael, corbet, linux-doc,
	ilpo.jarvinen, lee, matthew.gerlach



On 1/19/23 05:28, Greg KH wrote:
> On Wed, Jan 18, 2023 at 08:36:00PM -0500, Tianfei Zhang wrote:
>> Implement the image_load and available_images callback functions
>> for fpgahp driver. This patch leverages some APIs from pciehp
>> driver to implement the device reconfiguration below the PCI hotplug
>> bridge.
>>
>> Here are the steps for a process of image load.
>> 1. remove all PFs and VFs except the PF0.
>> 2. remove all non-reserved devices of PF0.
>> 3. trigger a image load via BMC.
>> 4. disable the link of the hotplug bridge.
>> 5. remove all reserved devices under PF0 and PCI devices
>>    below the hotplug bridge.
>> 6. wait for image load done via BMC, e.g. 10s.
>> 7. re-enable the link of the hotplug bridge.
>> 8. re-enumerate PCI devices below the hotplug bridge.
>>
>> Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
>> ---
>>  Documentation/ABI/testing/sysfs-driver-fpgahp |  21 ++
>>  MAINTAINERS                                   |   1 +
>>  drivers/pci/hotplug/fpgahp.c                  | 179 ++++++++++++++++++
>>  3 files changed, 201 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/sysfs-driver-fpgahp
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-fpgahp b/Documentation/ABI/testing/sysfs-driver-fpgahp
>> new file mode 100644
>> index 000000000000..8d4b1bfc4012
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-fpgahp
>> @@ -0,0 +1,21 @@
>> +What:		/sys/bus/pci/slots/X-X/available_images
>> +Date:		May 2023
>> +KernelVersion:	6.3
>> +Contact:	Tianfei Zhang <tianfei.zhang@intel.com>
>> +Description:	Read-only. This file returns a space separated list of
>> +		key words that may be written into the image_load file
>> +		described below. These keywords decribe an FPGA, BMC,
>> +		or firmware image in FLASH or EEPROM storage that may
>> +		be loaded.
> No, sysfs is "one value per file", why is this a list?
>
> And what exactly defines the values in this list?
>
>> +
>> +What:		/sys/bus/pci/slots/X-X/image_load
>> +Date:		May 2023
>> +KernelVersion:	6.3
>> +Contact:	Tianfei Zhang <tianfei.zhang@intel.com>
>> +Description:	Write-only. A key word may be written to this file to
>> +		trigger a new image loading of an FPGA, BMC, or firmware
>> +		image from FLASH or EEPROM. Refer to the available_images
>> +		file for a list of supported key words for the underlying
>> +		device.
>> +		Writing an unsupported string to this file will result in
>> +		EINVAL being returned.
> Why is this a separate file from the "read the list" file?

The intended usage is like this:

$ cat available_images
bmc_factory bmc_user fpga_factory fpga_user1 fpga_user2
$ echo bmc_user > image_load

This specifies which image stored in flash that you want to have activated
on the device.

An existing example of something like this is in the tracing code:
available_tracers and current_tracer

Would it be preferable to just create a file for each possible image,
and echo 1 to trigger the event? (echo 1 > bmc_user)

Thanks,
- Russ

> That feels wrong.
>
> thanks,
>
> greg k-h


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

* Re: [PATCH v1 00/12] add FPGA hotplug manager driver
  2023-01-20 18:42     ` Lukas Wunner
@ 2023-01-21  7:34       ` Greg KH
  0 siblings, 0 replies; 29+ messages in thread
From: Greg KH @ 2023-01-21  7:34 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Russ Weight, Tianfei Zhang, bhelgaas, linux-pci, linux-fpga,
	kabel, mani, pali, mdf, hao.wu, yilun.xu, trix, jgg, ira.weiny,
	andriy.shevchenko, dan.j.williams, keescook, rafael, corbet,
	linux-doc, ilpo.jarvinen, lee, matthew.gerlach

On Fri, Jan 20, 2023 at 07:42:53PM +0100, Lukas Wunner wrote:
> On Fri, Jan 20, 2023 at 08:28:51AM -0800, Russ Weight wrote:
> > On 1/19/23 05:33, Greg KH wrote:
> > > On Wed, Jan 18, 2023 at 08:35:50PM -0500, Tianfei Zhang wrote:
> > > > This patchset introduces the FPGA hotplug manager (fpgahp) driver which
> > > > has been verified on the Intel N3000 card.
> > > >
> > > > When a PCIe-based FPGA card is reprogrammed, it temporarily disappears
> > > > from the PCIe bus. This needs to be managed to avoid PCIe errors and to
> > > > reprobe the device after reprogramming.
> > > >
> > > > To change the FPGA image, the kernel burns a new image into the flash on
> > > > the card, and then triggers the card BMC to load the new image into FPGA.
> > > > A new FPGA hotplug manager driver is introduced that leverages the PCIe
> > > > hotplug framework to trigger and manage the update of the FPGA image,
> > > > including the disappearance and reappearance of the card on the PCIe bus.
> > > > The fpgahp driver uses APIs from the pciehp driver. Two new operation
> > > > callbacks are defined in hotplug_slot_ops:
> > > >
> > > >   - available_images: Optional: available FPGA images
> > > >   - image_load: Optional: trigger the FPGA to load a new image
> > > 
> > > Why is all of this tied into the pci hotplug code? Shouldn't it be
> > > specific to this one driver instead?  pci hotplug is for removing/adding
> > > PCI devices to the system, not messing with FPGA images.
> > >
> > > This feels like an abuse of the pci hotplug bus to me as this is NOT
> > > really a PCI hotplug bus at all, right?
> > 
> > While it is true that triggering an FPGA image-load does not involve
> > hotplug specific registers to be managed, the RTL that comprises
> > the PCIe interface will disappear and then reappear after the FPGA
> > is reprogrammed. When it reappears, it_could/_/have a different PCI
> > ID. The process of managing this event has a lot of similarity to a
> > PCIe hotplug event; there is a lot of existing PCIe hotplug related
> > code that could be leveraged.
> 
> It sounds like the N3000 is a PCI endpoint device which, when reprogrammed,
> briefly disappears from the bus and then may reappear under a different
> device ID.
> 
> What you want to do then is make sure that the slot into which the N3000
> is plugged is hotplug-capable.  In that case, pciehp will handle
> disappearance and reappearance of the card just fine.  Once the N3000
> disables the link, pciehp will bring down the slot.  Once it re-enables
> the link, it will bring the slot up again.  It's as if the card was
> removed and replaced with a different one.  pciehp will bind to the
> Root Port or Downstream Port associated with the hotplug slot.
> 
> The pci_hotplug_port infrastructure is for hotplug controllers which
> handle devices disappearing and reappearing *below* them.  It is not
> for endpoint devices.

Yes, thank you for expressing my concerns about this design much better
than I originally did.  I totally agree.

thanks,

greg k-h

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

* Re: [PATCH v1 10/12] PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp
  2023-01-20 22:38     ` Russ Weight
@ 2023-01-21  7:35       ` Greg KH
  0 siblings, 0 replies; 29+ messages in thread
From: Greg KH @ 2023-01-21  7:35 UTC (permalink / raw)
  To: Russ Weight
  Cc: Tianfei Zhang, bhelgaas, linux-pci, linux-fpga, lukas, kabel,
	mani, pali, mdf, hao.wu, yilun.xu, trix, jgg, ira.weiny,
	andriy.shevchenko, dan.j.williams, keescook, rafael, corbet,
	linux-doc, ilpo.jarvinen, lee, matthew.gerlach

On Fri, Jan 20, 2023 at 02:38:43PM -0800, Russ Weight wrote:
> 
> 
> On 1/19/23 05:28, Greg KH wrote:
> > On Wed, Jan 18, 2023 at 08:36:00PM -0500, Tianfei Zhang wrote:
> >> Implement the image_load and available_images callback functions
> >> for fpgahp driver. This patch leverages some APIs from pciehp
> >> driver to implement the device reconfiguration below the PCI hotplug
> >> bridge.
> >>
> >> Here are the steps for a process of image load.
> >> 1. remove all PFs and VFs except the PF0.
> >> 2. remove all non-reserved devices of PF0.
> >> 3. trigger a image load via BMC.
> >> 4. disable the link of the hotplug bridge.
> >> 5. remove all reserved devices under PF0 and PCI devices
> >>    below the hotplug bridge.
> >> 6. wait for image load done via BMC, e.g. 10s.
> >> 7. re-enable the link of the hotplug bridge.
> >> 8. re-enumerate PCI devices below the hotplug bridge.
> >>
> >> Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
> >> ---
> >>  Documentation/ABI/testing/sysfs-driver-fpgahp |  21 ++
> >>  MAINTAINERS                                   |   1 +
> >>  drivers/pci/hotplug/fpgahp.c                  | 179 ++++++++++++++++++
> >>  3 files changed, 201 insertions(+)
> >>  create mode 100644 Documentation/ABI/testing/sysfs-driver-fpgahp
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-driver-fpgahp b/Documentation/ABI/testing/sysfs-driver-fpgahp
> >> new file mode 100644
> >> index 000000000000..8d4b1bfc4012
> >> --- /dev/null
> >> +++ b/Documentation/ABI/testing/sysfs-driver-fpgahp
> >> @@ -0,0 +1,21 @@
> >> +What:		/sys/bus/pci/slots/X-X/available_images
> >> +Date:		May 2023
> >> +KernelVersion:	6.3
> >> +Contact:	Tianfei Zhang <tianfei.zhang@intel.com>
> >> +Description:	Read-only. This file returns a space separated list of
> >> +		key words that may be written into the image_load file
> >> +		described below. These keywords decribe an FPGA, BMC,
> >> +		or firmware image in FLASH or EEPROM storage that may
> >> +		be loaded.
> > No, sysfs is "one value per file", why is this a list?
> >
> > And what exactly defines the values in this list?
> >
> >> +
> >> +What:		/sys/bus/pci/slots/X-X/image_load
> >> +Date:		May 2023
> >> +KernelVersion:	6.3
> >> +Contact:	Tianfei Zhang <tianfei.zhang@intel.com>
> >> +Description:	Write-only. A key word may be written to this file to
> >> +		trigger a new image loading of an FPGA, BMC, or firmware
> >> +		image from FLASH or EEPROM. Refer to the available_images
> >> +		file for a list of supported key words for the underlying
> >> +		device.
> >> +		Writing an unsupported string to this file will result in
> >> +		EINVAL being returned.
> > Why is this a separate file from the "read the list" file?
> 
> The intended usage is like this:
> 
> $ cat available_images
> bmc_factory bmc_user fpga_factory fpga_user1 fpga_user2
> $ echo bmc_user > image_load
> 
> This specifies which image stored in flash that you want to have activated
> on the device.
> 
> An existing example of something like this is in the tracing code:
> available_tracers and current_tracer
> 
> Would it be preferable to just create a file for each possible image,
> and echo 1 to trigger the event? (echo 1 > bmc_user)

That would make things much more simpler overall and not force people to
have to parse a sysfs file, which is the main reason we created sysfs in
the first place.

thanks,

greg k-h

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

end of thread, other threads:[~2023-01-21  7:37 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-19  1:35 [PATCH v1 00/12] add FPGA hotplug manager driver Tianfei Zhang
2023-01-19  1:35 ` [PATCH v1 01/12] PCI: hotplug: add new callbacks on hotplug_slot_ops Tianfei Zhang
2023-01-19 13:31   ` Greg KH
2023-01-19  1:35 ` [PATCH v1 02/12] PCI: hotplug: expose APIs from pciehp driver Tianfei Zhang
2023-01-19  1:35 ` [PATCH v1 03/12] PCI: hotplug: add and expose link disable API Tianfei Zhang
2023-01-19  1:35 ` [PATCH v1 04/12] PCI: hotplug: add FPGA PCI hotplug manager driver Tianfei Zhang
2023-01-19  1:35 ` [PATCH v1 05/12] fpga: dfl: register dfl-pci device into fpgahph driver Tianfei Zhang
2023-01-19  1:35 ` [PATCH v1 06/12] driver core: expose device_is_ancestor() API Tianfei Zhang
2023-01-19  1:35 ` [PATCH v1 07/12] PCI: hotplug: add register/unregister function for BMC device Tianfei Zhang
2023-01-19  1:35 ` [PATCH v1 08/12] fpga: m10bmc-sec: register BMC device into fpgahp driver Tianfei Zhang
2023-01-19  1:35 ` [PATCH v1 09/12] fpga: dfl: remove non-reserved devices Tianfei Zhang
2023-01-19  1:36 ` [PATCH v1 10/12] PCI: hotplug: implement the hotplug_slot_ops callback for fpgahp Tianfei Zhang
2023-01-19 13:28   ` Greg KH
2023-01-20 22:38     ` Russ Weight
2023-01-21  7:35       ` Greg KH
2023-01-19  1:36 ` [PATCH v1 11/12] fpga: m10bmc-sec: add m10bmc_sec_retimer_load callback Tianfei Zhang
2023-01-19 14:22   ` Lee Jones
2023-01-19  1:36 ` [PATCH v1 12/12] Documentation: fpga: add description of fpgahp driver Tianfei Zhang
2023-01-19  9:38   ` Bagas Sanjaya
2023-01-19  8:06 ` [PATCH v1 00/12] add FPGA hotplug manager driver Pali Rohár
2023-01-19  8:17   ` Zhang, Tianfei
2023-01-19 11:27     ` andriy.shevchenko
2023-01-19 12:09       ` Zhang, Tianfei
2023-01-19 13:33 ` Greg KH
2023-01-19 13:43   ` Rafael J. Wysocki
2023-01-19 15:33     ` Greg KH
2023-01-20 16:28   ` Russ Weight
2023-01-20 18:42     ` Lukas Wunner
2023-01-21  7:34       ` Greg KH

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).