Re: [PATCH v17 0/5] FPGA Image Load (previously Security Manager)

[Tom Rix]

On 10/27/21 8:11 AM, Russ Weight wrote:
> On 10/26/21 8:29 PM, Wu, Hao wrote:
> > > > > > The API should not only define what it won't do, but also define what
> > > > > > it will do. But the "image load" just specifies the top half of the
> > > > > > process. So I don't think this API would be accepted.
> > > > > So what is the path forward. It seems like you are saying
> > > > > that the self-describing files do not fit in the fpga-mgr.
> > > > > Can we reconsider the FPGA Image Load Framework, which does
> > > > > not make any assumptions about the contents of the image
> > > > > files?
> > > > Why we need such "generic data transfer" interface in FPGA
> > > > framework?
> > > Are you referring to the use of self-describing files?
> > > or the generic nature of this class driver?
> > Yes, why this is under FPGA framework? Per your description that
> > it can be used to transfer any data, e.g. BMC images, some device
> > specific data (self-describing image?). Let's take this as example,
> > if FPGA device is replaced with ASIC on N3000, do you still want
> > to use FPGA image load framework to transfer your device specific
> > data, e.g. BMC images? I really hope that FPGA framework code only
> > focus on common usage of FPGA.
> >
> > > > we need to handle the common need for FPGA
> > > > devices only, not all devices, like programming FPGA images.
> > > > So far we even don't know, what's the hardware response on
> > > > these self-describing files, how we define it as a common need
> > > > interface in the framework?
> > > The class driver does not _need_ to reside in the FPGA
> > > framework. I sent an inquiry to the maintainer of the
> > > Firmware update subsystem (and cc'd the kernel mailing list)
> > > and received no responses. I placed it under the FPGA
> > > framework only because the first user of the class driver
> > > is an FPGA driver.
> > You must have enough justifications why this needs to be included
> > for everybody not for our own case.
> How do we justify it when there are currently no other known
> users? I can go ahead and work up some patches for the firmware
> subsystem, if we can resolve the other concerns below.
>
> > > > If you just want to reuse the
> > > > fpga-mgr/framework code for your own purpose, Yes, it seems
> > > > saving some code for you, but finally it loses flexibility, as it's
> > > > not possible to extend common framework for your own
> > > > purpose in the future.
> > > If I understand correctly, you are saying that it doesn't
> > > fit well in the FPGA manager, because not all file types
> > > fit the definition of a firmware update? And future file
> > > types may not fit in fpga-mgr context?
> > Let's split the use cases, I think the use case that update a persistent
> > storage for FPGA image, and later use hardware logic (FPGA loader)
> > to load it into FPGA. This sounds like a common usage for FPGA
> > devices, so I think this is why Yilun propose to have this part to be
> > covered by fpga-mgr. But for other cases in your description, e.g.
> > BMC images, device specific data, self-describing image and etc,
> > they are out of scope of FPGA.
> Self-describing files are not something new to us; _ALL_ of the image
> files that we send to our FPGA cards, including the N3000 FPGA and BMC
> images, root-entry hashes, key cancellations, etc. are self-describing
> files. They always have been.
>
>    
> > Actually I don't fully understand why we need to introduce the
> > "self-describing image" as a common data transfer interface, if
> > I remember correctly, for N3000, different sub drivers will own
> > different hardware sub function blocks, why expose such a new
> > shared communication channel?
> There is no change here. The N3000 files are self describing. The
> secure-update sub-driver of the MAX10 BMC invokes the class driver,
> funnels image data to the BMC, performs handshakes with the BMC,
> and ultimately returns status through the class driver. All images
> that are sent to the FPGA card follow this same path - and it works
> fine.
>
> To try to split out the purposes of each self-describing file to
> use different kernel APIs means interfacing multiple class drivers
> to the same MAX10 sub-driver. I think it also means replicating
> code.

Could the split be ?

add max10 bits mfd/

move image updating out of the kernel and into an uio driver

Tom

> - Russ
> > If "self-describing image" is a
> > request to one of the sub function block, why not just expose
> > new interface in such hardware block per modularization? I
> > have some concern that this new requirement may break
> > current driver architecture for N3000.
> >
> > Hao
> >
> > > - Russ
> > > > Thanks
> > > > Hao