From: Russ Weight <russell.h.weight@intel.com>
To: Tom Rix <trix@redhat.com>,
mdf@kernel.org, lee.jones@linaro.org, linux-fpga@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: lgoncalv@redhat.com, yilun.xu@intel.com, hao.wu@intel.com,
matthew.gerlach@intel.com
Subject: Re: [PATCH v1 03/12] fpga: expose max10 flash update counts in sysfs
Date: Wed, 16 Sep 2020 11:37:20 -0700 [thread overview]
Message-ID: <9cc5197e-72f9-e0de-c1c6-a0dc331fb48f@intel.com> (raw)
In-Reply-To: <a4ee319d-9f6f-5a27-415e-b1fabd1d5465@redhat.com>
On 9/5/20 1:39 PM, Tom Rix wrote:
> On 9/4/20 4:52 PM, Russ Weight wrote:
>> Extend the MAX10 BMC Security Engine driver to provide a
>> handler to expose the flash update count for the FPGA user
>> image.
>>
>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>> Reviewed-by: Wu Hao <hao.wu@intel.com>
>> ---
>> drivers/fpga/intel-m10-bmc-secure.c | 32 +++++++++++++++++++++++++++++
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/fpga/intel-m10-bmc-secure.c b/drivers/fpga/intel-m10-bmc-secure.c
>> index 1f86bfb694b4..b824790e43aa 100644
>> --- a/drivers/fpga/intel-m10-bmc-secure.c
>> +++ b/drivers/fpga/intel-m10-bmc-secure.c
>> @@ -10,6 +10,7 @@
>> #include <linux/mfd/intel-m10-bmc.h>
>> #include <linux/module.h>
>> #include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> #include <linux/vmalloc.h>
>>
>> struct m10bmc_sec {
>> @@ -99,7 +100,38 @@ SYSFS_GET_REH(bmc, BMC_REH_ADDR)
>> SYSFS_GET_REH(sr, SR_REH_ADDR)
>> SYSFS_GET_REH(pr, PR_REH_ADDR)
>>
>> +#define FLASH_COUNT_SIZE 4096
> This seems too high at most it should be 64.
The flash count size represents the size of the flash memory that stores the
count. The count is represented in flash as an inverted bit vector.
I suppose a comment would be helpful here...
>> +#define USER_FLASH_COUNT 0x17ffb000
> Why shouldn't this be in intel-m10-bmc.h ?
Sure - I'll move it there with the other addresses.
>> +
>> +static int get_qspi_flash_count(struct ifpga_sec_mgr *imgr)
> what does 'qspi' mean ?
It means Quad-SPI, the controller type that connects to the FLASH. This term
does seem out of place here. There is also a BMC flash count. I'll change this to
"user".
>
> unless there are going to be several *flash_count's consider
>
> removing this substring.
>
>> +{
>> + struct m10bmc_sec *sec = imgr->priv;
>> + unsigned int stride = regmap_get_reg_stride(sec->m10bmc->regmap);
>> + unsigned int cnt, num_bits = FLASH_COUNT_SIZE * 8;
>> + u8 *flash_buf;
>> + int ret;
>> +
>> + flash_buf = kmalloc(FLASH_COUNT_SIZE, GFP_KERNEL);
>> + if (!flash_buf)
>> + return -ENOMEM;
>> +
>> + ret = m10bmc_raw_bulk_read(sec->m10bmc, USER_FLASH_COUNT, flash_buf,
>> + FLASH_COUNT_SIZE / stride);
>> + if (ret) {
>> + dev_err(sec->dev, "%s failed to read %d\n", __func__, ret);
>> + goto exit_free;
>> + }
>> +
>> + cnt = num_bits - bitmap_weight((unsigned long *)flash_buf, num_bits);
> Simplify ret = num_bits...
yes - will do.
Thanks!
- Russ
>> +
>> +exit_free:
>> + kfree(flash_buf);
>> +
>> + return ret ? : cnt;
> Then simplify
>
> return ret;
>
> Tom
>
>> +}
>> +
>> static const struct ifpga_sec_mgr_ops m10bmc_iops = {
>> + .user_flash_count = get_qspi_flash_count,
>> .bmc_root_entry_hash = get_bmc_root_entry_hash,
>> .sr_root_entry_hash = get_sr_root_entry_hash,
>> .pr_root_entry_hash = get_pr_root_entry_hash,
next prev parent reply other threads:[~2020-09-16 18:37 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-04 23:52 [PATCH v1 00/12] Intel FPGA Security Manager Class Driver Russ Weight
2020-09-04 23:52 ` [PATCH v1 01/12] fpga: fpga security manager class driver Russ Weight
2020-09-04 23:57 ` Randy Dunlap
2020-09-05 0:23 ` Moritz Fischer
2020-09-05 0:44 ` Russ Weight
2020-09-05 13:39 ` Wu, Hao
2020-09-05 19:09 ` Tom Rix
[not found] ` <ebf251a0-5f13-d1a1-6915-e3c940bb19fe@intel.com>
2020-09-10 21:51 ` Tom Rix
2020-09-10 23:05 ` Russ Weight
2020-09-16 20:16 ` Moritz Fischer
2020-09-30 20:54 ` Russ Weight
2020-10-01 0:31 ` Moritz Fischer
2020-10-01 1:07 ` Russ Weight
2020-10-01 19:07 ` Moritz Fischer
2020-09-04 23:52 ` [PATCH v1 02/12] fpga: create intel max10 bmc security engine Russ Weight
2020-09-05 0:01 ` Randy Dunlap
2020-09-05 0:05 ` Russ Weight
2020-09-05 20:22 ` Tom Rix
2020-09-14 19:07 ` Russ Weight
2020-09-14 20:48 ` Tom Rix
2020-09-14 21:40 ` Russ Weight
2020-09-16 20:33 ` Moritz Fischer
2020-09-30 23:14 ` Russ Weight
2020-09-04 23:52 ` [PATCH v1 03/12] fpga: expose max10 flash update counts in sysfs Russ Weight
2020-09-05 20:39 ` Tom Rix
2020-09-16 18:37 ` Russ Weight [this message]
2020-09-04 23:52 ` [PATCH v1 04/12] fpga: expose max10 canceled keys " Russ Weight
2020-09-05 20:52 ` Tom Rix
2020-09-04 23:52 ` [PATCH v1 05/12] fpga: enable secure updates Russ Weight
2020-09-05 22:04 ` Tom Rix
[not found] ` <1d90bfb6-417c-55df-9290-991c391158a9@intel.com>
2020-09-20 15:24 ` Tom Rix
2020-09-04 23:52 ` [PATCH v1 06/12] fpga: add max10 secure update functions Russ Weight
2020-09-06 16:10 ` Tom Rix
2020-09-22 1:15 ` Russ Weight
2020-09-08 8:05 ` Lee Jones
2020-09-04 23:53 ` [PATCH v1 07/12] fpga: expose sec-mgr update status Russ Weight
2020-09-06 16:16 ` Tom Rix
2020-09-22 22:31 ` Russ Weight
2020-09-04 23:53 ` [PATCH v1 08/12] fpga: expose sec-mgr update errors Russ Weight
2020-09-06 16:27 ` Tom Rix
2020-09-22 23:42 ` Russ Weight
2020-09-23 12:52 ` Tom Rix
2020-09-04 23:53 ` [PATCH v1 09/12] fpga: expose sec-mgr update size Russ Weight
2020-09-06 16:39 ` Tom Rix
2020-09-04 23:53 ` [PATCH v1 10/12] fpga: enable sec-mgr update cancel Russ Weight
2020-09-06 17:00 ` Tom Rix
[not found] ` <678f8d39-a244-42d0-4c56-91eb859b43f0@intel.com>
2020-09-23 13:02 ` Tom Rix
2020-09-04 23:53 ` [PATCH v1 11/12] fpga: expose hardware error info in sysfs Russ Weight
2020-09-06 17:06 ` Tom Rix
2020-09-04 23:53 ` [PATCH v1 12/12] fpga: add max10 get_hw_errinfo callback func Russ Weight
2020-09-06 17:14 ` Tom Rix
2020-09-24 21:48 ` Russ Weight
2020-09-05 14:13 ` [PATCH v1 00/12] Intel FPGA Security Manager Class Driver Wu, Hao
2020-10-01 20:42 ` Russ Weight
2020-09-05 16:10 ` Tom Rix
2020-09-05 17:16 ` Tom Rix
2020-10-01 0:19 ` Russ Weight
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=9cc5197e-72f9-e0de-c1c6-a0dc331fb48f@intel.com \
--to=russell.h.weight@intel.com \
--cc=hao.wu@intel.com \
--cc=lee.jones@linaro.org \
--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=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).