linux-fpga.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Wu, Hao" <hao.wu@intel.com>
To: "Weight, Russell H" <russell.h.weight@intel.com>,
	"Xu, Yilun" <yilun.xu@intel.com>
Cc: Tom Rix <trix@redhat.com>, "mdf@kernel.org" <mdf@kernel.org>,
	"linux-fpga@vger.kernel.org" <linux-fpga@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"lgoncalv@redhat.com" <lgoncalv@redhat.com>,
	"Gerlach, Matthew" <matthew.gerlach@intel.com>
Subject: RE: [PATCH v17 0/5] FPGA Image Load (previously Security Manager)
Date: Wed, 27 Oct 2021 03:29:46 +0000	[thread overview]
Message-ID: <DM6PR11MB38192DC4A39D654F88322D0C85859@DM6PR11MB3819.namprd11.prod.outlook.com> (raw)
In-Reply-To: <03ff4983-d8a9-6ad7-a655-a8dcde3da360@intel.com>

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

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

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? 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


  reply	other threads:[~2021-10-27  3:29 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-29 23:00 [PATCH v17 0/5] FPGA Image Load (previously Security Manager) Russ Weight
2021-09-29 23:00 ` [PATCH v17 1/5] fpga: image-load: fpga image load framework Russ Weight
2021-09-29 23:00 ` [PATCH v17 2/5] fpga: image-load: enable image uploads Russ Weight
2021-10-06 18:13   ` Russ Weight
2021-09-29 23:00 ` [PATCH v17 3/5] fpga: image-load: signal eventfd when complete Russ Weight
2021-09-29 23:00 ` [PATCH v17 4/5] fpga: image-load: add status ioctl Russ Weight
2021-10-15 20:22   ` Lizhi Hou
2021-10-20 21:42     ` Russ Weight
2021-10-26  0:07       ` Lizhi Hou
2021-09-29 23:00 ` [PATCH v17 5/5] fpga: image-load: enable cancel of image upload Russ Weight
2021-10-09  8:08 ` [PATCH v17 0/5] FPGA Image Load (previously Security Manager) Xu Yilun
2021-10-09 12:11   ` Tom Rix
2021-10-11  1:41     ` Xu Yilun
2021-10-11 12:35       ` Tom Rix
2021-10-12  1:00         ` Russ Weight
2021-10-12  7:47           ` Xu Yilun
2021-10-12  7:56             ` Xu Yilun
2021-10-12 17:20             ` Russ Weight
2021-10-13  1:06               ` Xu Yilun
2021-10-13 18:09                 ` Russ Weight
2021-10-14  1:49                   ` Xu Yilun
     [not found]                     ` <7d1971d0-b50b-077f-2a82-83d822cd2ad7@intel.com>
2021-10-15  2:51                       ` Xu Yilun
2021-10-15 17:34                         ` Russ Weight
2021-10-18  8:13                           ` Xu Yilun
2021-10-18 16:24                             ` Russ Weight
2021-10-19  2:53                               ` Xu Yilun
2021-10-19 15:09                                 ` Russ Weight
2021-10-20  1:16                                   ` Xu Yilun
2021-10-20 16:27                                     ` Russ Weight
2021-10-26  6:45                                       ` Wu, Hao
2021-10-26 17:41                                         ` Russ Weight
2021-10-27  3:29                                           ` Wu, Hao [this message]
2021-10-27 15:11                                             ` Russ Weight
2021-10-27 15:34                                               ` Tom Rix
2021-10-28 15:09                                                 ` Xu Yilun
2021-10-28 16:08                                                   ` Tom Rix
2021-10-29  2:05                                                     ` Xu Yilun
2021-10-12  7:49         ` Xu Yilun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR11MB38192DC4A39D654F88322D0C85859@DM6PR11MB3819.namprd11.prod.outlook.com \
    --to=hao.wu@intel.com \
    --cc=lgoncalv@redhat.com \
    --cc=linux-fpga@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew.gerlach@intel.com \
    --cc=mdf@kernel.org \
    --cc=russell.h.weight@intel.com \
    --cc=trix@redhat.com \
    --cc=yilun.xu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).