All of
 help / color / mirror / Atom feed
From: Russ Weight <>
To: Greg KH <>, Tom Rix <>
Cc: Moritz Fischer <>, <>,
Subject: Re: [PATCH 02/12] fpga: sec-mgr: enable secure updates
Date: Thu, 29 Jul 2021 18:23:12 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 5/17/21 12:37 PM, Russ Weight wrote:
> On 5/16/21 10:32 PM, Greg KH wrote:
>> On Sun, May 16, 2021 at 07:31:50PM -0700, Moritz Fischer wrote:
>>> From: Russ Weight <>
>>> Extend the FPGA Security Manager class driver to
>>> include an update/filename sysfs node that can be used
>>> to initiate a secure update.  The filename of a secure
>>> update file (BMC image, FPGA image, Root Entry Hash image,
>>> or Code Signing Key cancellation image) can be written to
>>> this sysfs entry to cause a secure update to occur.
>> Why is userspace responsible for triggering this?  Passing a "filename"
>> into the kernel and having it do something with it is ripe for major
>> problems, please do not.
> I am using the "request_firmware" framework, which accepts a filename
> and finds the firmware file under /lib/firmware.
> Is this not an acceptable use for request_firmware?
> - Russ

Hi Greg,

The dev_release fixes that you asked for in the FPGA Manager, Bridge, and
Region code are almost complete. I'm trying to get back to the FPGA
security manager patch set. Your previous comments challenged some basic
assumptions. If it is OK, I would like to get some clarity before I rework
the patches.

Overview: The goal is to pass signed data to the PCIe FPGA Card's BMC. BMC
firmware will authenticate the data and disposition it. In our case, FPGA
image data, root entry hashes, and cancellation keys are authenticated
and then stored in FLASH memory. The patchset contains both a class
driver and a platform driver.

Example Output of Current Driver:

        [root@psera2-dell24 update]# echo -n intel/dcp_2_0_page0_0x2020002000000237_signed.bin > filename
        [root@psera2-dell24 update]# while :; do cat status remaining_size ; sleep 3; done
        <- snip ->
        <- snip ->
        <- snip ->
        <- snip ->
        [root@psera2-dell24 update]# cat error
        [root@psera2-dell24 update]#


(1) request_firmware(). We had assumed that making use of the existing
request_firmware() would be preferred. This requires providing a filename
under /lib/firmware to the framework. You commented (above): "Passing a
'filename' into the kernel and having it do something with it is ripe for
problems, please do not." Unless you have additional comments on this, I
will plan to NOT use the request_firmware framework.

(2) sysfs interface. We had assumed that a sysfs interface would
be preferred. If I am not passing a filename, then I think my only option
with sysfs is to use a binary attribute and cat the data in. Is that
acceptable, or would it be better to use IOCTLs to pass the data?

(3) Platform Driver. This driver is for the BMC on the FPGA Card.
I think that is similar to the SOC model. This is actually a sub-driver
for the MAX10 BMC driver. Other platform drivers (e.g. hwmon) have already
been accepted as subdrivers for the BMC. Is the platform driver the
right approach? If not, can you please point me in the right direction?

(4) Kernel worker thread: The worst case update is currently about 45
minutes (newer implementations are shorter). We chose to do the data
transfer in a kernel worker thread and then make it possible for
userspace to monitor the progress (currently via sysfs). Any concerns
about doing the transfer in a background thread?

(5) New vs modified driver: Perhaps "FPGA Security Manager" is not
a good name. Simply put, the driver passes signed data from the host
to the Card BMC for authentication and disposition. I looked at
merging the class driver with the FPGA Manager, but:

a) Secure updates make no use of the existing FPGA Manager code (FPGA
state management, etc). The only similarity is in the ops data structure.

b) Because of the kernel worker thread the driver remove functionality
adds complexity that is not helpful to the FPGA Manager.

I can, of course, combine the drivers anyway if you think that is better.

I appreciate any feedback/direction you can give.

- Russ

  reply	other threads:[~2021-07-30  1:24 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-17  2:31 [PATCH 00/12] FPGA Security Manager for 5.14 Moritz Fischer
2021-05-17  2:31 ` [PATCH 01/12] fpga: sec-mgr: fpga security manager class driver Moritz Fischer
2021-05-17  5:18   ` Greg KH
2021-05-17 17:45     ` Russ Weight
2021-05-17 17:55       ` Greg KH
2021-05-17 18:25         ` Russ Weight
2021-05-19 20:42           ` Tom Rix
2021-05-21  1:10             ` Russ Weight
2021-05-21  4:58               ` Greg KH
2021-05-21 15:15                 ` Russ Weight
2021-05-17  2:31 ` [PATCH 02/12] fpga: sec-mgr: enable secure updates Moritz Fischer
2021-05-17  5:32   ` Greg KH
2021-05-17 19:37     ` Russ Weight
2021-07-30  1:23       ` Russ Weight [this message]
2021-07-30 11:18         ` Greg KH
2021-08-02 18:31           ` Russ Weight
2021-08-03  5:49             ` Greg KH
2021-08-03 19:02               ` Russ Weight
2021-08-04  7:37                 ` Greg KH
2021-08-04 14:58                   ` Moritz Fischer
2021-08-04 15:12                     ` Greg KH
2021-08-04 19:47                       ` Moritz Fischer
2021-11-02 16:25                       ` Russ Weight
2021-11-02 17:06                         ` Greg KH
2021-05-17  2:31 ` [PATCH 03/12] fpga: sec-mgr: expose sec-mgr update status Moritz Fischer
2021-05-17  2:31 ` [PATCH 04/12] fpga: sec-mgr: expose sec-mgr update errors Moritz Fischer
2021-05-17  2:31 ` [PATCH 05/12] fpga: sec-mgr: expose sec-mgr update size Moritz Fischer
2021-05-17  2:31 ` [PATCH 06/12] fpga: sec-mgr: enable cancel of secure update Moritz Fischer
2021-05-17  2:31 ` [PATCH 07/12] fpga: sec-mgr: expose hardware error info Moritz Fischer
2021-05-17  7:10   ` Greg KH
2021-05-17 19:49     ` Russ Weight
2021-05-17  2:31 ` [PATCH 08/12] fpga: m10bmc-sec: create max10 bmc secure update driver Moritz Fischer
2021-05-17  5:30   ` Greg KH
2021-05-17 20:09     ` Russ Weight
2021-05-17  2:31 ` [PATCH 09/12] fpga: m10bmc-sec: expose max10 flash update count Moritz Fischer
2021-05-17  2:31 ` [PATCH 10/12] fpga: m10bmc-sec: expose max10 canceled keys in sysfs Moritz Fischer
2021-05-17  2:31 ` [PATCH 11/12] fpga: m10bmc-sec: add max10 secure update functions Moritz Fischer
2021-05-17  2:32 ` [PATCH 12/12] fpga: m10bmc-sec: add max10 get_hw_errinfo callback func Moritz Fischer

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:

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

  git send-email \ \ \ \ \ \ \ \

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.