Moritz, On 3/28/21 10:20 AM, Moritz Fischer wrote: > Tom, > > On Sun, Mar 28, 2021 at 08:40:24AM -0700, Tom Rix wrote: >> On 3/27/21 11:09 AM, Moritz Fischer wrote: >>> Hi Richard, Russ, >>> >>> On Thu, Feb 25, 2021 at 01:07:14PM +0000, Gong, Richard wrote: >>>> Hi Moritz, >>>> >>>> Sorry for asking. >>>> >>>> When you have chance, can you help review the version 5 patchset submitted >>>> on 02/09/21? >>>> >>>> Regards, >>>> Richard >>>> >>>> -----Original Message----- >>>> From: richard.g...@linux.intel.com <richard.g...@linux.intel.com> >>>> Sent: Tuesday, February 9, 2021 4:20 PM >>>> To: m...@kernel.org; t...@redhat.com; gre...@linuxfoundation.org; >>>> linux-f...@vger.kernel.org; linux-kernel@vger.kernel.org >>>> Cc: Gong, Richard <richard.g...@intel.com> >>>> Subject: [PATCHv5 0/7] Extend Intel service layer, FPGA manager and region >>>> >>>> From: Richard Gong <richard.g...@intel.com> >>>> >>>> This is 5th submission of Intel service layer and FPGA patches, which >>>> includes the missing standalone patch in the 4th submission. >>>> >>>> This submission includes additional changes for Intel service layer driver >>>> to get the firmware version running at FPGA SoC device. Then FPGA manager >>>> driver, one of Intel service layer driver's client, can decide whether to >>>> handle the newly added bitstream authentication function based on the >>>> retrieved firmware version. So that we can maintain FPGA manager driver >>>> the back compatible. >>>> >>>> Bitstream authentication makes sure a signed bitstream has valid >>>> signatures. >>>> >>>> The customer sends the bitstream via FPGA framework and overlay, the >>>> firmware will authenticate the bitstream but not program the bitstream to >>>> device. If the authentication passes, the bitstream will be programmed >>>> into QSPI flash and will be expected to boot without issues. >>>> >>>> Extend Intel service layer, FPGA manager and region drivers to support the >>>> bitstream authentication feature. >>>> >>>> Richard Gong (7): >>>> firmware: stratix10-svc: reset COMMAND_RECONFIG_FLAG_PARTIAL to 0 >>>> firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag >>>> firmware: stratix10-svc: extend SVC driver to get the firmware version >>>> fpga: fpga-mgr: add FPGA_MGR_BITSTREAM_AUTHENTICATE flag >>>> fpga: of-fpga-region: add authenticate-fpga-config property >>>> dt-bindings: fpga: add authenticate-fpga-config property >>>> fpga: stratix10-soc: extend driver for bitstream authentication >>>> >>>> .../devicetree/bindings/fpga/fpga-region.txt | 10 ++++ >>>> drivers/firmware/stratix10-svc.c | 12 ++++- >>>> drivers/fpga/of-fpga-region.c | 24 ++++++--- >>>> drivers/fpga/stratix10-soc.c | 62 >>>> +++++++++++++++++++--- >>>> include/linux/firmware/intel/stratix10-smc.h | 21 +++++++- >>>> .../linux/firmware/intel/stratix10-svc-client.h | 11 +++- >>>> include/linux/fpga/fpga-mgr.h | 3 ++ >>>> 7 files changed, 125 insertions(+), 18 deletions(-) >>>> >>>> -- >>>> 2.7.4 >>>> >>> Apologies for the epic delay in getting back to this, I took another >>> look at this patchset and Russ' patchset. >>> >>> TL;DR I'm not really a fan of using device-tree overlays for this (and >>> again, apologies, I should've voiced this earlier ...). >>> >>> Anyways, let's find a common API for this and Russ' work, they're trying >>> to achieve the same / similar thing, they should use the same API. >>> >>> I'd like to re-invetigate the possiblity to extend FPGA Manager with >>> 'secure update' ops that work for both these use-cases (and I susspect >>> hte XRT patchset will follow with a similar requirement, right after).
Richard and I had an initial conversation today. I'll start looking at how secure operations can be integrated into the fpga manager. More to come... Thanks, - Russ >> The xrt patchset makes heavy use of device trees. >> >> What is the general guidance for device tree usage ? > I'm not generally against using device tree, it has its place. To > describe hardware (and hardware *changes* with overlays) :) > > What I don't like about this particular implementation w.r.t device-tree > usage is that it uses DT overlays as a mechanism to program the flash -- > in place of having an API to do so. > > One could add device-nodes during the DT overlay application, while the > FPGA doesn't actually get programmed with a new runtime image -- meaning > live DT and actual hardware state diverged -- worst case it'd crash. > > So when roughly at the same time (from the same company even) we have two > patchsets that do similar things with radically different APIs I think > we should pause, and reflect on whether we can come up with something > that works for both :) > > TL;DR the firmware parts to authenticate the bitstream look fine to me, the > way we tie it into the FPGA region I'm not a fan of. > > - Moritz
