| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <41b42ae2-6cbc-fec2-44b3-6353507e1b02@redhat.com>
Date: Mon, 13 Sep 2021 05:12:32 -0700
From: Tom Rix <trix@...hat.com>
To: Xu Yilun <yilun.xu@...el.com>
Cc: Russ Weight <russell.h.weight@...el.com>, mdf@...nel.org,
linux-fpga@...r.kernel.org, linux-kernel@...r.kernel.org,
lgoncalv@...hat.com, hao.wu@...el.com, matthew.gerlach@...el.com
Subject: Re: [PATCH v14 1/4] fpga: m10bmc-sec: create max10 bmc secure update
driver
On 9/12/21 10:37 PM, Xu Yilun wrote:
> On Sat, Sep 11, 2021 at 12:04:07PM -0700, Tom Rix wrote:
>> On 9/10/21 1:27 PM, Russ Weight wrote:
>>> On 9/10/21 8:13 AM, Xu Yilun wrote:
>>>> On Thu, Sep 09, 2021 at 04:33:01PM -0700, Russ Weight wrote:
>>>>> Create a sub driver for the FPGA Card BMC in order to support secure
>>>>> updates. This sub-driver will invoke an instance of the FPGA Image Load
>>>>> class driver for the image load portion of the update.
>>>>>
>>>>> This patch creates the MAX10 BMC Secure Update driver and provides sysfs
>>>>> files for displaying the current root entry hashes for the FPGA static
>>>>> region, the FPGA PR region, and the MAX10 BMC.
>>>>>
>>>>> Signed-off-by: Russ Weight <russell.h.weight@...el.com>
>>>>> Reviewed-by: Tom Rix <trix@...hat.com>
>>>>> ---
>>>>> v14:
>>>>> - Changed symbol and text references to reflect the renaming of the
>>>>> Security Manager Class driver to FPGA Image Load.
>>>>> v13:
>>>>> - Updated copyright to 2021
>>>>> - Updated ABI documentation date and kernel version
>>>>> - Call updated fpga_sec_mgr_register() and fpga_sec_mgr_unregister()
>>>>> functions instead of devm_fpga_sec_mgr_create() and
>>>>> devm_fpga_sec_mgr_register().
>>>>> v12:
>>>>> - Updated Date and KernelVersion fields in ABI documentation
>>>>> v11:
>>>>> - Added Reviewed-by tag
>>>>> v10:
>>>>> - Changed the path expressions in the sysfs documentation to
>>>>> replace the n3000 reference with something more generic to
>>>>> accomodate other devices that use the same driver.
>>>>> v9:
>>>>> - Rebased to 5.12-rc2 next
>>>>> - Updated Date and KernelVersion in ABI documentation
>>>>> v8:
>>>>> - Previously patch 2/6, otherwise no change
>>>>> v7:
>>>>> - Updated Date and KernelVersion in ABI documentation
>>>>> v6:
>>>>> - Added WARN_ON() call for (sha_num_bytes / stride) to assert
>>>>> that the proper count is passed to regmap_bulk_read().
>>>>> v5:
>>>>> - No change
>>>>> v4:
>>>>> - Moved sysfs files for displaying the root entry hashes (REH)
>>>>> from the FPGA Security Manager class driver to here. The
>>>>> m10bmc_reh() and m10bmc_reh_size() functions are removed and
>>>>> the functionality from these functions is moved into a
>>>>> show_root_entry_hash() function for displaying the REHs.
>>>>> - Added ABI documentation for the new sysfs entries:
>>>>> sysfs-driver-intel-m10-bmc-secure
>>>>> - Updated the MAINTAINERS file to add the new ABI documentation
>>>>> file: sysfs-driver-intel-m10-bmc-secure
>>>>> - Removed unnecessary ret variable from m10bmc_secure_probe()
>>>>> - Incorporated new devm_fpga_sec_mgr_register() function into
>>>>> m10bmc_secure_probe() and removed the m10bmc_secure_remove()
>>>>> function.
>>>>> v3:
>>>>> - Changed from "Intel FPGA Security Manager" to FPGA Security Manager"
>>>>> - Changed: iops -> sops, imgr -> smgr, IFPGA_ -> FPGA_, ifpga_ to fpga_
>>>>> - Changed "MAX10 BMC Secure Engine driver" to "MAX10 BMC Secure
>>>>> Update driver"
>>>>> - Removed wrapper functions (m10bmc_raw_*, m10bmc_sys_*). The
>>>>> underlying functions are now called directly.
>>>>> - Changed "_root_entry_hash" to "_reh", with a comment explaining
>>>>> what reh is.
>>>>> v2:
>>>>> - Added drivers/fpga/intel-m10-bmc-secure.c file to MAINTAINERS.
>>>>> - Switched to GENMASK(31, 16) for a couple of mask definitions.
>>>>> - Moved MAX10 BMC address and function definitions to a separate
>>>>> patch.
>>>>> - Replaced small function-creation macros with explicit function
>>>>> declarations.
>>>>> - Removed ifpga_sec_mgr_init() and ifpga_sec_mgr_uinit() functions.
>>>>> - Adapted to changes in the Intel FPGA Security Manager by splitting
>>>>> the single call to ifpga_sec_mgr_register() into two function
>>>>> calls: devm_ifpga_sec_mgr_create() and ifpga_sec_mgr_register().
>>>>> ---
>>>>> .../testing/sysfs-driver-intel-m10-bmc-secure | 29 ++++
>>>>> MAINTAINERS | 2 +
>>>>> drivers/fpga/Kconfig | 11 ++
>>>>> drivers/fpga/Makefile | 3 +
>>>>> drivers/fpga/intel-m10-bmc-secure.c | 145 ++++++++++++++++++
>>>>> 5 files changed, 190 insertions(+)
>>>>> create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-secure
>>>>> create mode 100644 drivers/fpga/intel-m10-bmc-secure.c
>>>>>
>>>>> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-secure b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-secure
>>>>> new file mode 100644
>>>>> index 000000000000..363403ce992d
>>>>> --- /dev/null
>>>>> +++ b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-secure
>>>>> @@ -0,0 +1,29 @@
>>>>> +What: /sys/bus/platform/drivers/intel-m10bmc-secure/.../security/sr_root_entry_hash
>>>>> +Date: Aug 2021
>>>>> +KernelVersion: 5.15
>>>>> +Contact: Russ Weight <russell.h.weight@...el.com>
>>>>> +Description: Read only. Returns the root entry hash for the static
>>>>> + region if one is programmed, else it returns the
>>>>> + string: "hash not programmed". This file is only
>>>>> + visible if the underlying device supports it.
>>>>> + Format: "0x%x".
>>>>> +
>>>>> +What: /sys/bus/platform/drivers/intel-m10bmc-secure/.../security/pr_root_entry_hash
>>>>> +Date: Aug 2021
>>>>> +KernelVersion: 5.15
>>>>> +Contact: Russ Weight <russell.h.weight@...el.com>
>>>>> +Description: Read only. Returns the root entry hash for the partial
>>>>> + reconfiguration region if one is programmed, else it
>>>>> + returns the string: "hash not programmed". This file
>>>>> + is only visible if the underlying device supports it.
>>>>> + Format: "0x%x".
>>>>> +
>>>>> +What: /sys/bus/platform/drivers/intel-m10bmc-secure/.../security/bmc_root_entry_hash
>>>>> +Date: Aug 2021
>>>>> +KernelVersion: 5.15
>>>>> +Contact: Russ Weight <russell.h.weight@...el.com>
>>>>> +Description: Read only. Returns the root entry hash for the BMC image
>>>>> + if one is programmed, else it returns the string:
>>>>> + "hash not programmed". This file is only visible if the
>>>>> + underlying device supports it.
>>>>> + Format: "0x%x".
>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>> index e3fbc1bde9bc..cf93835b4775 100644
>>>>> --- a/MAINTAINERS
>>>>> +++ b/MAINTAINERS
>>>>> @@ -7363,8 +7363,10 @@ M: Russ Weight <russell.h.weight@...el.com>
>>>>> L: linux-fpga@...r.kernel.org
>>>>> S: Maintained
>>>>> F: Documentation/ABI/testing/sysfs-class-fpga-image-load
>>>>> +F: Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-secure
>>>> Should we change the name of the driver? Some keywords like "image load"
>>>> or "firmware update" should be in the name.
>>> I considered that. The image-upload functionality is a subset of this
>>> driver. It also exposes security collateral via sysfs, and the image-load
>>> triggers and power-on-image sysfs files will probably end up in this
>>> driver too.
>>>
>>> The current driver name is intel-m10-bmc-secure. Do we need to keep
>>> "intel-m10-bmc" in the name?
>>>
>>> intel-m10-bmc-sec-fw-update?
>>> intel-m10-bmc-sec-update?
>>>
>>> What do you think? Any other suggestions?
> The single word "secure" is quite indistinct to me. I think
> intel-m10-bmc-sec-update is much better.
This fine.
Should it move to mfd/ ?
Tom
>
>> The prefix intel-m10-bmc-sec is clunky and confuses me because I think of
>> mfd/intel-m10-bmc.c
> The secure update engine is now implemented in MAX10 bmc. The driver
> code also assumes it is always a sub device of MAX10 bmc. So my
> preference is we keep the prefix.
>
>> How about
>>
>> dfl-image-load ?
> There may be several secure update engines for DFL based FPGAs. So we
> may be more specific.
>
> Thanks,
> Yilun
>
Powered by blists - more mailing lists