Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sat, 9 Oct 2021 16:08:59 +0800
From:   Xu Yilun <yilun.xu@intel.com>
To:     Russ Weight <russell.h.weight@intel.com>
Cc:     mdf@kernel.org, linux-fpga@vger.kernel.org,
        linux-kernel@vger.kernel.org, trix@redhat.com, lgoncalv@redhat.com,
        hao.wu@intel.com, matthew.gerlach@intel.com
Subject: Re: [PATCH v17 0/5] FPGA Image Load (previously Security Manager)
Message-ID: <20211009080859.GA85181@yilunxu-OptiPlex-7050>
References: <20210929230025.68961-1-russell.h.weight@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210929230025.68961-1-russell.h.weight@intel.com>
Precedence: bulk

On Wed, Sep 29, 2021 at 04:00:20PM -0700, Russ Weight wrote:
> The FPGA Image Load framework provides an API to upload image
> files to an FPGA device. Image files are self-describing. They could
> contain FPGA images, BMC images, Root Entry Hashes, or other device
> specific files. It is up to the lower-level device driver and the
> target device to authenticate and disposition the file data.

I've reconsider the FPGA persistent image update again, and think we
may include it in FPGA manager framework.

Sorry I raised this topic again when it is already at patch v17, but now
I need to consider more seriously than before.

We have consensus the FPGA persistent image update is just like a normal
firmware update which finally writes the nvmem like flash or eeprom,
while the current FPGA manager deals with the active FPGA region update
and re-activation. Could we just expand the FPGA manager and let it handle
the nvmem update as well? Many FPGA cards have nvmem and downloaders
supports updating both FPGA region and nvmem.

According to the patchset, the basic workflow of the 2 update types are
quite similar, get the data, prepare for the HW, write and complete.
They are already implemented in FPGA manager. We've discussed some
differences like threading or canceling the update, which are
not provided by FPGA manager but they may also nice to have for FPGA
region update. An FPGA region update may also last for a long time??
So I think having 2 sets of similar frameworks in FPGA is unnecessary.

My quick mind is that we add some flags in struct fpga_mgr & struct
fpga_image_info to indicate the HW capability (support FPGA region
update or nvmem update or both) of the download engine and the provided
image type. Then the low-level driver knows how to download if it
supports both image types.

An char device could be added for each fpga manager dev, providing the
user APIs for nvmem update. We may not use the char dev for FPGA region
update cause it changes the system HW devices and needs device hotplug
in FPGA region. We'd better leave it to FPGA region class, this is
another topic.

More discussion is appreciated.

Thanks,
Yilun