On Mon, Feb 05, 2018 at 10:25:54PM -0600, Alan Tull wrote:

On Mon, Feb 5, 2018 at 7:47 PM, Wu Hao [off-list ref] wrote:

quoted

On Mon, Feb 05, 2018 at 10:36:45AM -0800, Luebbers, Enno wrote:

quoted

Hi Hao,

On Sun, Feb 04, 2018 at 05:37:06PM +0800, Wu Hao wrote:

quoted

On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:

quoted

Hi Hao, Alan,

On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:

quoted

On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:

quoted

On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao [off-list ref] wrote:

Hi Hao,

A few comments below.   Besides that, looks good.

quoted

This patch adds fpga manager driver for FPGA Management Engine (FME). It
implements fpga_manager_ops for FPGA Partial Reconfiguration function.

Signed-off-by: Tim Whisonant <redacted>
Signed-off-by: Enno Luebbers <redacted>
Signed-off-by: Shiva Rao <redacted>
Signed-off-by: Christopher Rauer <redacted>
Signed-off-by: Kang Luwei <redacted>
Signed-off-by: Xiao Guangrong <guangrong.xiao-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Wu Hao <redacted>
----
v3: rename driver to dfl-fpga-fme-mgr
    implemented status callback for fpga manager
    rebased due to fpga api changes
---
 .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr    |   8 +
 drivers/fpga/Kconfig                               |   6 +
 drivers/fpga/Makefile                              |   1 +
 drivers/fpga/fpga-dfl-fme-mgr.c                    | 318 +++++++++++++++++++++
 drivers/fpga/fpga-dfl.h                            |  39 ++-
 5 files changed, 371 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
 create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c

diff --git a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
new file mode 100644
index 0000000..2d4f917
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr

@@ -0,0 +1,8 @@
+What:          /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
+Date:          November 2017
+KernelVersion:  4.15
+Contact:       Wu Hao <hao.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
+Description:   Read-only. It returns interface id of partial reconfiguration
+               hardware. Userspace could use this information to check if
+               current hardware is compatible with given image before FPGA
+               programming.

I'm a little confused by this.  I can understand that the PR bitstream
has a dependency on the FPGA's static image, but I don't understand
the dependency of the bistream on the hardware that is used to program
the bitstream to the FPGA.

Sorry for the confusion, the interface_id is used to indicate the version of
the hardware for partial reconfiguration (it's part of the static image of
the FPGA device). Will improve the description on this.

The interface_id expresses the compatibility of the static region with PR
bitstreams generated for it. It changes every time a new static region is
generated.

Would it make more sense to have the interface_id exposed as part of the FME
device (which represents the static region)? I'm not sure - it kind of also
makes sense here, where you would have all the information in one place (if the
interface_id matches, I can use this component to program a bitstream).

Hi Enno

Yes, this interface is under fpga-dfl-fme-mgr.0, and fpga-dfl-fme-mgr.0 is
under fpga-dfl-fme.0. It's part of the FME device for sure. From another
point of view, it means if anyone wants to do PR on this Intel FPGA device,
he needs to find the FME device firstly, and then check if any fpga manager
created under this FME device, if yes, check the interface_id before PR via
the FME device node ioctl.

That sounds good, thank you!

quoted

quoted

Sorry for my limited understanding of the infrastructure - would this same
"fpga-dfl-fme-mgr.0" be used for PR if we had multiple PR regions? In that case
it would need to expose multiple interface_ids (or we'd have to track both
interface IDs and an identifier for the target PR region).

Yes, the fpga manager could be shared with different PR regions.

Sorry, I'm not sure where we need to expose multiple interface_ids and why.

It's basically a question of how to determine bitstream compatibility - either,
there's a separate interface_id per reconfigurable region, or there is a single
interface_id for the entire device. Both make sense from a certain perspective.

If there are multiple interface_ids per device (one per region), the driver
would need to expose all of them. If there's only a single one, the driver only
exposes that one ID - compatibility would be determined by looking at both that
single interface_id _and_ the identifier/number of the targeted region.

I would prefer a separate interface_id per region - it seems more generic and
flexible.

Hi Enno,

I agree with this.

quoted

It's possible to have per region interface_id (or even both per dev interface_id
and per region interface_id at the same time), but per FME PR sub feature
implementation, it supports multiple PR regions, but only provide one interface
id, so at least in this case, it's not per-region information per my
understanding. We can consider it later when hardware really supports it. : )

Hi Hao,

I understand that in the case of this PR hardware, the region to
program is selected when the region_id to program is written to a PR
hardware control register.  For another example, Arria10 has a hard PR
hardware and the PR bitstream lands in the area of the FPGA for which
it was compiled.  In that case, for the PR bitstream to be compatible
with a PR region, the layout of the edge connections also needs to be
compatible, so compatibility is per-region in that case instead of
per-PR hardware.

Hi Alan,

Thanks a lot for the explanation. :)

I fully understand the consideration of adding per-region interface_id.

And besides, as I said yesterday, the hard PR
hardware would not know what the static region ID is when this
framework is used with such a device.

Yes, is it possible that hard PR hardware with different versions, requires
different images or different methods for compatibility checking?

That's why I think making the id per-region may be more future proof,
even if it may see unnecessary in the case of the original blue bits
this was written for.

I feel that per-PR hardware interface id is useful in some cases, and maybe
in some cases, both per-PR hardware and per-region interface ids are needed
for its compatibility checking, so shall we leave developers to decide to
implement per-PR hardware or per-region or both interface ids based on their
own hardware implementations? How do you think? :)

Thanks
Hao

Alan