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:

quoted

On 1/19/23 05:33, Greg KH wrote:

quoted

On Wed, Jan 18, 2023 at 08:35:50PM -0500, Tianfei Zhang wrote:

quoted

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