From: Richard Gong <[email protected]>
This is 3rd submission of Intel service layer and FPGA patches.
This submission include 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.
The customer wants to verify that a FPGA bitstream can be started properly
before saving the bitstream to the QSPI flash memory.
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 (6):
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_BITSTREM_AUTHENTICATION 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 | 1 +
drivers/firmware/stratix10-svc.c | 12 ++++-
drivers/fpga/of-fpga-region.c | 3 ++
drivers/fpga/stratix10-soc.c | 62 +++++++++++++++++++---
include/linux/firmware/intel/stratix10-smc.h | 21 +++++++-
.../linux/firmware/intel/stratix10-svc-client.h | 15 ++++--
include/linux/fpga/fpga-mgr.h | 13 +++--
7 files changed, 109 insertions(+), 18 deletions(-)
--
2.7.4
From: Richard Gong <[email protected]>
Add authenticate-fpga-config property to support FPGA bitstream
authentication, which makes sure a signed bitstream has valid signatures.
Signed-off-by: Richard Gong <[email protected]>
---
v3: no change
v2: changed in alphabetical order
---
drivers/fpga/of-fpga-region.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index e405309..3840883 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -219,6 +219,9 @@ static struct fpga_image_info *of_fpga_region_parse_ov(
info->overlay = overlay;
/* Read FPGA region properties from the overlay */
+ if (of_property_read_bool(overlay, "authenticate-fpga-config"))
+ info->flags |= FPGA_MGR_BITSTREM_AUTHENTICATION;
+
if (of_property_read_bool(overlay, "partial-fpga-config"))
info->flags |= FPGA_MGR_PARTIAL_RECONFIG;
--
2.7.4
From: Richard Gong <[email protected]>
Add authenticate-fpga-config property for FPGA bitstream authentication,
which makes sure a signed bitstream has valid signatures.
Signed-off-by: Richard Gong <[email protected]>
---
v3: no change
v2: put authenticate-fpga-config above partial-fpga-config
update commit messages
---
Documentation/devicetree/bindings/fpga/fpga-region.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt b/Documentation/devicetree/bindings/fpga/fpga-region.txt
index e811cf8..d0d3234 100644
--- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
+++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
@@ -182,6 +182,7 @@ Optional properties:
This property is optional if the FPGA Manager handles the bridges.
If the fpga-region is the child of a fpga-bridge, the list should not
contain the parent bridge.
+- authenticate-fpga-config : boolean, set if do bitstream authentication only.
- partial-fpga-config : boolean, set if partial reconfiguration is to be done,
otherwise full reconfiguration is done.
- external-fpga-config : boolean, set if the FPGA has already been configured
--
2.7.4
From: Richard Gong <[email protected]>
Extend FPGA manager driver to support FPGA bitstream authentication on
Intel SocFPGA platforms.
Signed-off-by: Richard Gong <[email protected]>
---
v3: add handle to retriev the firmware version to keep driver
back compatible
v2: use flag defined in stratix10-svc driver
---
drivers/fpga/stratix10-soc.c | 62 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 56 insertions(+), 6 deletions(-)
diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index 657a70c..59d738c 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -24,6 +24,10 @@
#define S10_BUFFER_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_BUFFER_TIMEOUT_MS))
#define S10_RECONFIG_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_REQUEST_TIMEOUT_MS))
+#define INVALID_FIRMWARE_VERSION 0xFFFF
+typedef void (*s10_callback)(struct stratix10_svc_client *client,
+ struct stratix10_svc_cb_data *data);
+
/*
* struct s10_svc_buf
* buf: virtual address of buf provided by service layer
@@ -40,11 +44,13 @@ struct s10_priv {
struct completion status_return_completion;
struct s10_svc_buf svc_bufs[NUM_SVC_BUFS];
unsigned long status;
+ unsigned int fw_version;
};
static int s10_svc_send_msg(struct s10_priv *priv,
enum stratix10_svc_command_code command,
- void *payload, u32 payload_length)
+ void *payload, u32 payload_length,
+ s10_callback callback)
{
struct stratix10_svc_chan *chan = priv->chan;
struct device *dev = priv->client.dev;
@@ -57,6 +63,7 @@ static int s10_svc_send_msg(struct s10_priv *priv,
msg.command = command;
msg.payload = payload;
msg.payload_length = payload_length;
+ priv->client.receive_cb = callback;
ret = stratix10_svc_send(chan, &msg);
dev_dbg(dev, "stratix10_svc_send returned status %d\n", ret);
@@ -134,6 +141,29 @@ static void s10_unlock_bufs(struct s10_priv *priv, void *kaddr)
}
/*
+ * s10_fw_version_callback - callback for the version of running firmware
+ * @client: service layer client struct
+ * @data: message from service layer
+ */
+static void s10_fw_version_callback(struct stratix10_svc_client *client,
+ struct stratix10_svc_cb_data *data)
+{
+ struct s10_priv *priv = client->priv;
+ unsigned int *version = (unsigned int *)data->kaddr1;
+
+ if (data->status == BIT(SVC_STATUS_OK))
+ priv->fw_version = *version;
+ else if (data->status == BIT(SVC_STATUS_NO_SUPPORT))
+ dev_warn(client->dev,
+ "FW doesn't support bitstream authentication\n");
+ else
+ dev_err(client->dev, "Failed to get FW version %lu\n",
+ BIT(data->status));
+
+ complete(&priv->status_return_completion);
+}
+
+/*
* s10_receive_callback - callback for service layer to use to provide client
* (this driver) messages received through the mailbox.
* client: service layer client struct
@@ -186,13 +216,22 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
dev_dbg(dev, "Requesting partial reconfiguration.\n");
ctype.flags |= BIT(COMMAND_RECONFIG_FLAG_PARTIAL);
+ } else if (info->flags & FPGA_MGR_BITSTREM_AUTHENTICATION) {
+ if (priv->fw_version == INVALID_FIRMWARE_VERSION) {
+ dev_err(dev, "FW doesn't support\n");
+ return -EINVAL;
+ }
+
+ dev_dbg(dev, "Requesting bitstream authentication.\n");
+ ctype.flags |= BIT(COMMAND_AUTHENTICATE_BITSTREAM);
} else {
dev_dbg(dev, "Requesting full reconfiguration.\n");
}
reinit_completion(&priv->status_return_completion);
ret = s10_svc_send_msg(priv, COMMAND_RECONFIG,
- &ctype, sizeof(ctype));
+ &ctype, sizeof(ctype),
+ s10_receive_callback);
if (ret < 0)
goto init_done;
@@ -259,7 +298,7 @@ static int s10_send_buf(struct fpga_manager *mgr, const char *buf, size_t count)
svc_buf = priv->svc_bufs[i].buf;
memcpy(svc_buf, buf, xfer_sz);
ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_DATA_SUBMIT,
- svc_buf, xfer_sz);
+ svc_buf, xfer_sz, s10_receive_callback);
if (ret < 0) {
dev_err(dev,
"Error while sending data to service layer (%d)", ret);
@@ -303,7 +342,7 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
ret = s10_svc_send_msg(
priv, COMMAND_RECONFIG_DATA_CLAIM,
- NULL, 0);
+ NULL, 0, s10_receive_callback);
if (ret < 0)
break;
}
@@ -357,7 +396,8 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
do {
reinit_completion(&priv->status_return_completion);
- ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_STATUS, NULL, 0);
+ ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_STATUS,
+ NULL, 0, s10_receive_callback);
if (ret < 0)
break;
@@ -411,8 +451,9 @@ static int s10_probe(struct platform_device *pdev)
if (!priv)
return -ENOMEM;
+ priv->fw_version = INVALID_FIRMWARE_VERSION;
priv->client.dev = dev;
- priv->client.receive_cb = s10_receive_callback;
+ priv->client.receive_cb = NULL;
priv->client.priv = priv;
priv->chan = stratix10_svc_request_channel_byname(&priv->client,
@@ -440,6 +481,15 @@ static int s10_probe(struct platform_device *pdev)
goto probe_err;
}
+ /* get the running firmware version */
+ ret = s10_svc_send_msg(priv, COMMAND_FIRMWARE_VERSION,
+ NULL, 0, s10_fw_version_callback);
+ if (ret) {
+ dev_err(dev, "couldn't get firmware version\n");
+ fpga_mgr_free(mgr);
+ goto probe_err;
+ }
+
platform_set_drvdata(pdev, mgr);
return ret;
--
2.7.4
From: Richard Gong <[email protected]>
Add FPGA_MGR_BITSTREM_AUTHENTICATION flag for FPGA bitstream
authentication, which makes sure a signed bitstream has valid signatures.
Except for the actual configuration of the device, the authentication works
the same way as FPGA configuration does. If the authentication passes, the
bitstream will be programmed into QSPI flash and will be expected to boot
without issues.
Signed-off-by: Richard Gong <[email protected]>
---
v3: no change
v2: align all FPGA_MGR_* flags
update the commit messages
---
include/linux/fpga/fpga-mgr.h | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 2bc3030..4fb3400 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -67,12 +67,15 @@ enum fpga_mgr_states {
* %FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
*
* %FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
+ *
+ * %FPGA_MGR_BITSTREM_AUTHENTICATION: do FPGA bitstream authentication only
*/
-#define FPGA_MGR_PARTIAL_RECONFIG BIT(0)
-#define FPGA_MGR_EXTERNAL_CONFIG BIT(1)
-#define FPGA_MGR_ENCRYPTED_BITSTREAM BIT(2)
-#define FPGA_MGR_BITSTREAM_LSB_FIRST BIT(3)
-#define FPGA_MGR_COMPRESSED_BITSTREAM BIT(4)
+#define FPGA_MGR_PARTIAL_RECONFIG BIT(0)
+#define FPGA_MGR_EXTERNAL_CONFIG BIT(1)
+#define FPGA_MGR_ENCRYPTED_BITSTREAM BIT(2)
+#define FPGA_MGR_BITSTREAM_LSB_FIRST BIT(3)
+#define FPGA_MGR_COMPRESSED_BITSTREAM BIT(4)
+#define FPGA_MGR_BITSTREM_AUTHENTICATION BIT(5)
/**
* struct fpga_image_info - information specific to a FPGA image
--
2.7.4
Hi Moritz,
On 1/25/21 11:04 PM, Moritz Fischer wrote:
> On Mon, Jan 25, 2021 at 02:56:25PM -0600, [email protected] wrote:
>> From: Richard Gong <[email protected]>
>>
>> Add FPGA_MGR_BITSTREM_AUTHENTICATION flag for FPGA bitstream
>> authentication, which makes sure a signed bitstream has valid signatures.
>>
>> Except for the actual configuration of the device, the authentication works
>> the same way as FPGA configuration does. If the authentication passes, the
>> bitstream will be programmed into QSPI flash and will be expected to boot
>> without issues.
>>
>> Signed-off-by: Richard Gong <[email protected]>
>> ---
>> v3: no change
>> v2: align all FPGA_MGR_* flags
>> update the commit messages
>> ---
>> include/linux/fpga/fpga-mgr.h | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
>> index 2bc3030..4fb3400 100644
>> --- a/include/linux/fpga/fpga-mgr.h
>> +++ b/include/linux/fpga/fpga-mgr.h
>> @@ -67,12 +67,15 @@ enum fpga_mgr_states {
>> * %FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
>> *
>> * %FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
>> + *
>> + * %FPGA_MGR_BITSTREM_AUTHENTICATION: do FPGA bitstream authentication only
>> */
>> -#define FPGA_MGR_PARTIAL_RECONFIG BIT(0)
>> -#define FPGA_MGR_EXTERNAL_CONFIG BIT(1)
>> -#define FPGA_MGR_ENCRYPTED_BITSTREAM BIT(2)
>> -#define FPGA_MGR_BITSTREAM_LSB_FIRST BIT(3)
>> -#define FPGA_MGR_COMPRESSED_BITSTREAM BIT(4)
>> +#define FPGA_MGR_PARTIAL_RECONFIG BIT(0)
>> +#define FPGA_MGR_EXTERNAL_CONFIG BIT(1)
>> +#define FPGA_MGR_ENCRYPTED_BITSTREAM BIT(2)
>> +#define FPGA_MGR_BITSTREAM_LSB_FIRST BIT(3)
>> +#define FPGA_MGR_COMPRESSED_BITSTREAM BIT(4)
>> +#define FPGA_MGR_BITSTREM_AUTHENTICATION BIT(5)
> Consider FPGA_MGR_BITSTREAM_AUTHENTICATE (and fix typo)
Thanks, I will correct that in next submission.
>>
>> /**
>> * struct fpga_image_info - information specific to a FPGA image
>> --
>> 2.7.4
>>
>
> Thanks,
> Moritz
>
Regards,
Richard
On 1/25/21 11:05 PM, Moritz Fischer wrote:
> On Mon, Jan 25, 2021 at 02:56:27PM -0600, [email protected] wrote:
>> From: Richard Gong <[email protected]>
>>
>> Add authenticate-fpga-config property for FPGA bitstream authentication,
>> which makes sure a signed bitstream has valid signatures.
>>
>> Signed-off-by: Richard Gong <[email protected]>
>> ---
>> v3: no change
>> v2: put authenticate-fpga-config above partial-fpga-config
>> update commit messages
>> ---
>> Documentation/devicetree/bindings/fpga/fpga-region.txt | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt b/Documentation/devicetree/bindings/fpga/fpga-region.txt
>> index e811cf8..d0d3234 100644
>> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
>> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
>> @@ -182,6 +182,7 @@ Optional properties:
>> This property is optional if the FPGA Manager handles the bridges.
>> If the fpga-region is the child of a fpga-bridge, the list should not
>> contain the parent bridge.
>> +- authenticate-fpga-config : boolean, set if do bitstream authentication only.
> I don't understand. Can I do authenticate-fpga-config AND
> partial-fpga-config?
Yes, but not simultaneously.
Flag authenticate-fpga-config is used to first check the integrity of
the bitstream. If the authentication passes, the user can perform a full
or partial configuration to actually configure the bistream to device.
>> - partial-fpga-config : boolean, set if partial reconfiguration is to be done,
>> otherwise full reconfiguration is done.
>> - external-fpga-config : boolean, set if the FPGA has already been configured
>> --
>> 2.7.4
>>
> Please clarify,
>
> Moritz
>
Regards,
Richard
Hi Moritz,
On 1/25/21 11:10 PM, Moritz Fischer wrote:
> On Mon, Jan 25, 2021 at 02:56:26PM -0600, [email protected] wrote:
>> From: Richard Gong <[email protected]>
>>
>> Add authenticate-fpga-config property to support FPGA bitstream
>> authentication, which makes sure a signed bitstream has valid signatures.
>>
>> Signed-off-by: Richard Gong <[email protected]>
>> ---
>> v3: no change
>> v2: changed in alphabetical order
>> ---
>> drivers/fpga/of-fpga-region.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
>> index e405309..3840883 100644
>> --- a/drivers/fpga/of-fpga-region.c
>> +++ b/drivers/fpga/of-fpga-region.c
>> @@ -219,6 +219,9 @@ static struct fpga_image_info *of_fpga_region_parse_ov(
>> info->overlay = overlay;
>>
>> /* Read FPGA region properties from the overlay */
>> + if (of_property_read_bool(overlay, "authenticate-fpga-config"))
>> + info->flags |= FPGA_MGR_BITSTREM_AUTHENTICATION;
>> +
> Should you check here that no new nodes are being added as you *only*
> authenticate?
Sure, I will add additional checks in next submission.
>
>> if (of_property_read_bool(overlay, "partial-fpga-config"))
>> info->flags |= FPGA_MGR_PARTIAL_RECONFIG;
>>
>> --
>> 2.7.4
>>
>
> Thanks,
> Moritz
>
Regards,
Richard
Hi Moritz,
On 1/25/21 11:09 PM, Moritz Fischer wrote:
> On Mon, Jan 25, 2021 at 02:56:28PM -0600, [email protected] wrote:
>> From: Richard Gong <[email protected]>
>>
>> Extend FPGA manager driver to support FPGA bitstream authentication on
>> Intel SocFPGA platforms.
>>
>> Signed-off-by: Richard Gong <[email protected]>
>> ---
>> v3: add handle to retriev the firmware version to keep driver
>> back compatible
>> v2: use flag defined in stratix10-svc driver
>> ---
>> drivers/fpga/stratix10-soc.c | 62 +++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 56 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
>> index 657a70c..59d738c 100644
>> --- a/drivers/fpga/stratix10-soc.c
>> +++ b/drivers/fpga/stratix10-soc.c
>> @@ -24,6 +24,10 @@
>> #define S10_BUFFER_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_BUFFER_TIMEOUT_MS))
>> #define S10_RECONFIG_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_REQUEST_TIMEOUT_MS))
>>
>> +#define INVALID_FIRMWARE_VERSION 0xFFFF
>> +typedef void (*s10_callback)(struct stratix10_svc_client *client,
>> + struct stratix10_svc_cb_data *data);
>> +
>> /*
>> * struct s10_svc_buf
>> * buf: virtual address of buf provided by service layer
>> @@ -40,11 +44,13 @@ struct s10_priv {
>> struct completion status_return_completion;
>> struct s10_svc_buf svc_bufs[NUM_SVC_BUFS];
>> unsigned long status;
>> + unsigned int fw_version;
>> };
>>
>> static int s10_svc_send_msg(struct s10_priv *priv,
>> enum stratix10_svc_command_code command,
>> - void *payload, u32 payload_length)
>> + void *payload, u32 payload_length,
>> + s10_callback callback)
>> {
>> struct stratix10_svc_chan *chan = priv->chan;
>> struct device *dev = priv->client.dev;
>> @@ -57,6 +63,7 @@ static int s10_svc_send_msg(struct s10_priv *priv,
>> msg.command = command;
>> msg.payload = payload;
>> msg.payload_length = payload_length;
>> + priv->client.receive_cb = callback;
>>
>> ret = stratix10_svc_send(chan, &msg);
>> dev_dbg(dev, "stratix10_svc_send returned status %d\n", ret);
>> @@ -134,6 +141,29 @@ static void s10_unlock_bufs(struct s10_priv *priv, void *kaddr)
>> }
>>
>> /*
>> + * s10_fw_version_callback - callback for the version of running firmware
>> + * @client: service layer client struct
>> + * @data: message from service layer
>> + */
>> +static void s10_fw_version_callback(struct stratix10_svc_client *client,
>> + struct stratix10_svc_cb_data *data)
>> +{
>> + struct s10_priv *priv = client->priv;
>> + unsigned int *version = (unsigned int *)data->kaddr1;
>> +
>> + if (data->status == BIT(SVC_STATUS_OK))
>> + priv->fw_version = *version;
>> + else if (data->status == BIT(SVC_STATUS_NO_SUPPORT))
>> + dev_warn(client->dev,
>> + "FW doesn't support bitstream authentication\n");
>> + else
>> + dev_err(client->dev, "Failed to get FW version %lu\n",
>> + BIT(data->status));
>> +
>> + complete(&priv->status_return_completion);
>> +}
>> +
>> +/*
>> * s10_receive_callback - callback for service layer to use to provide client
>> * (this driver) messages received through the mailbox.
>> * client: service layer client struct
>> @@ -186,13 +216,22 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
>> if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
>> dev_dbg(dev, "Requesting partial reconfiguration.\n");
>> ctype.flags |= BIT(COMMAND_RECONFIG_FLAG_PARTIAL);
>> + } else if (info->flags & FPGA_MGR_BITSTREM_AUTHENTICATION) {
> BITSTREAM ;-)
>
> Nit: Maybe the flag could be FPGA_MGR_BITSTREAM_AUTHENTICATE and/or
> FPGA_MGR_BITSTREAM_AUTH_AND_PERSIST.
Thanks, I will correct both in next submission.
>> + if (priv->fw_version == INVALID_FIRMWARE_VERSION) {
>> + dev_err(dev, "FW doesn't support\n");
>> + return -EINVAL;
>> + }
>> +
>> + dev_dbg(dev, "Requesting bitstream authentication.\n");
>> + ctype.flags |= BIT(COMMAND_AUTHENTICATE_BITSTREAM);
>> } else {
>> dev_dbg(dev, "Requesting full reconfiguration.\n");
>> }
>>
>> reinit_completion(&priv->status_return_completion);
>> ret = s10_svc_send_msg(priv, COMMAND_RECONFIG,
>> - &ctype, sizeof(ctype));
>> + &ctype, sizeof(ctype),
>> + s10_receive_callback);
>> if (ret < 0)
>> goto init_done;
>>
>> @@ -259,7 +298,7 @@ static int s10_send_buf(struct fpga_manager *mgr, const char *buf, size_t count)
>> svc_buf = priv->svc_bufs[i].buf;
>> memcpy(svc_buf, buf, xfer_sz);
>> ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_DATA_SUBMIT,
>> - svc_buf, xfer_sz);
>> + svc_buf, xfer_sz, s10_receive_callback);
>> if (ret < 0) {
>> dev_err(dev,
>> "Error while sending data to service layer (%d)", ret);
>> @@ -303,7 +342,7 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>>
>> ret = s10_svc_send_msg(
>> priv, COMMAND_RECONFIG_DATA_CLAIM,
>> - NULL, 0);
>> + NULL, 0, s10_receive_callback);
>> if (ret < 0)
>> break;
>> }
>> @@ -357,7 +396,8 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>> do {
>> reinit_completion(&priv->status_return_completion);
>>
>> - ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_STATUS, NULL, 0);
>> + ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_STATUS,
>> + NULL, 0, s10_receive_callback);
>> if (ret < 0)
>> break;
>>
>> @@ -411,8 +451,9 @@ static int s10_probe(struct platform_device *pdev)
>> if (!priv)
>> return -ENOMEM;
>>
>> + priv->fw_version = INVALID_FIRMWARE_VERSION;
>> priv->client.dev = dev;
>> - priv->client.receive_cb = s10_receive_callback;
>> + priv->client.receive_cb = NULL;
>> priv->client.priv = priv;
>>
>> priv->chan = stratix10_svc_request_channel_byname(&priv->client,
>> @@ -440,6 +481,15 @@ static int s10_probe(struct platform_device *pdev)
>> goto probe_err;
>> }
>>
>> + /* get the running firmware version */
>> + ret = s10_svc_send_msg(priv, COMMAND_FIRMWARE_VERSION,
>> + NULL, 0, s10_fw_version_callback);
>> + if (ret) {
>> + dev_err(dev, "couldn't get firmware version\n");
>> + fpga_mgr_free(mgr);
>> + goto probe_err;
>> + }
>> +
>> platform_set_drvdata(pdev, mgr);
>> return ret;
>>
>> --
>> 2.7.4
>>
> Thanks,
> Moritz
>
Regards,
Richard
On Mon, Jan 25, 2021 at 02:56:27PM -0600, [email protected] wrote:
> From: Richard Gong <[email protected]>
>
> Add authenticate-fpga-config property for FPGA bitstream authentication,
> which makes sure a signed bitstream has valid signatures.
>
> Signed-off-by: Richard Gong <[email protected]>
> ---
> v3: no change
> v2: put authenticate-fpga-config above partial-fpga-config
> update commit messages
> ---
> Documentation/devicetree/bindings/fpga/fpga-region.txt | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> index e811cf8..d0d3234 100644
> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> @@ -182,6 +182,7 @@ Optional properties:
> This property is optional if the FPGA Manager handles the bridges.
> If the fpga-region is the child of a fpga-bridge, the list should not
> contain the parent bridge.
> +- authenticate-fpga-config : boolean, set if do bitstream authentication only.
I don't understand. Can I do authenticate-fpga-config AND
partial-fpga-config?
> - partial-fpga-config : boolean, set if partial reconfiguration is to be done,
> otherwise full reconfiguration is done.
> - external-fpga-config : boolean, set if the FPGA has already been configured
> --
> 2.7.4
>
Please clarify,
Moritz
On Mon, Jan 25, 2021 at 02:56:28PM -0600, [email protected] wrote:
> From: Richard Gong <[email protected]>
>
> Extend FPGA manager driver to support FPGA bitstream authentication on
> Intel SocFPGA platforms.
>
> Signed-off-by: Richard Gong <[email protected]>
> ---
> v3: add handle to retriev the firmware version to keep driver
> back compatible
> v2: use flag defined in stratix10-svc driver
> ---
> drivers/fpga/stratix10-soc.c | 62 +++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 56 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> index 657a70c..59d738c 100644
> --- a/drivers/fpga/stratix10-soc.c
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -24,6 +24,10 @@
> #define S10_BUFFER_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_BUFFER_TIMEOUT_MS))
> #define S10_RECONFIG_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_REQUEST_TIMEOUT_MS))
>
> +#define INVALID_FIRMWARE_VERSION 0xFFFF
> +typedef void (*s10_callback)(struct stratix10_svc_client *client,
> + struct stratix10_svc_cb_data *data);
> +
> /*
> * struct s10_svc_buf
> * buf: virtual address of buf provided by service layer
> @@ -40,11 +44,13 @@ struct s10_priv {
> struct completion status_return_completion;
> struct s10_svc_buf svc_bufs[NUM_SVC_BUFS];
> unsigned long status;
> + unsigned int fw_version;
> };
>
> static int s10_svc_send_msg(struct s10_priv *priv,
> enum stratix10_svc_command_code command,
> - void *payload, u32 payload_length)
> + void *payload, u32 payload_length,
> + s10_callback callback)
> {
> struct stratix10_svc_chan *chan = priv->chan;
> struct device *dev = priv->client.dev;
> @@ -57,6 +63,7 @@ static int s10_svc_send_msg(struct s10_priv *priv,
> msg.command = command;
> msg.payload = payload;
> msg.payload_length = payload_length;
> + priv->client.receive_cb = callback;
>
> ret = stratix10_svc_send(chan, &msg);
> dev_dbg(dev, "stratix10_svc_send returned status %d\n", ret);
> @@ -134,6 +141,29 @@ static void s10_unlock_bufs(struct s10_priv *priv, void *kaddr)
> }
>
> /*
> + * s10_fw_version_callback - callback for the version of running firmware
> + * @client: service layer client struct
> + * @data: message from service layer
> + */
> +static void s10_fw_version_callback(struct stratix10_svc_client *client,
> + struct stratix10_svc_cb_data *data)
> +{
> + struct s10_priv *priv = client->priv;
> + unsigned int *version = (unsigned int *)data->kaddr1;
> +
> + if (data->status == BIT(SVC_STATUS_OK))
> + priv->fw_version = *version;
> + else if (data->status == BIT(SVC_STATUS_NO_SUPPORT))
> + dev_warn(client->dev,
> + "FW doesn't support bitstream authentication\n");
> + else
> + dev_err(client->dev, "Failed to get FW version %lu\n",
> + BIT(data->status));
> +
> + complete(&priv->status_return_completion);
> +}
> +
> +/*
> * s10_receive_callback - callback for service layer to use to provide client
> * (this driver) messages received through the mailbox.
> * client: service layer client struct
> @@ -186,13 +216,22 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
> if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> dev_dbg(dev, "Requesting partial reconfiguration.\n");
> ctype.flags |= BIT(COMMAND_RECONFIG_FLAG_PARTIAL);
> + } else if (info->flags & FPGA_MGR_BITSTREM_AUTHENTICATION) {
BITSTREAM ;-)
Nit: Maybe the flag could be FPGA_MGR_BITSTREAM_AUTHENTICATE and/or
FPGA_MGR_BITSTREAM_AUTH_AND_PERSIST.
> + if (priv->fw_version == INVALID_FIRMWARE_VERSION) {
> + dev_err(dev, "FW doesn't support\n");
> + return -EINVAL;
> + }
> +
> + dev_dbg(dev, "Requesting bitstream authentication.\n");
> + ctype.flags |= BIT(COMMAND_AUTHENTICATE_BITSTREAM);
> } else {
> dev_dbg(dev, "Requesting full reconfiguration.\n");
> }
>
> reinit_completion(&priv->status_return_completion);
> ret = s10_svc_send_msg(priv, COMMAND_RECONFIG,
> - &ctype, sizeof(ctype));
> + &ctype, sizeof(ctype),
> + s10_receive_callback);
> if (ret < 0)
> goto init_done;
>
> @@ -259,7 +298,7 @@ static int s10_send_buf(struct fpga_manager *mgr, const char *buf, size_t count)
> svc_buf = priv->svc_bufs[i].buf;
> memcpy(svc_buf, buf, xfer_sz);
> ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_DATA_SUBMIT,
> - svc_buf, xfer_sz);
> + svc_buf, xfer_sz, s10_receive_callback);
> if (ret < 0) {
> dev_err(dev,
> "Error while sending data to service layer (%d)", ret);
> @@ -303,7 +342,7 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>
> ret = s10_svc_send_msg(
> priv, COMMAND_RECONFIG_DATA_CLAIM,
> - NULL, 0);
> + NULL, 0, s10_receive_callback);
> if (ret < 0)
> break;
> }
> @@ -357,7 +396,8 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
> do {
> reinit_completion(&priv->status_return_completion);
>
> - ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_STATUS, NULL, 0);
> + ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_STATUS,
> + NULL, 0, s10_receive_callback);
> if (ret < 0)
> break;
>
> @@ -411,8 +451,9 @@ static int s10_probe(struct platform_device *pdev)
> if (!priv)
> return -ENOMEM;
>
> + priv->fw_version = INVALID_FIRMWARE_VERSION;
> priv->client.dev = dev;
> - priv->client.receive_cb = s10_receive_callback;
> + priv->client.receive_cb = NULL;
> priv->client.priv = priv;
>
> priv->chan = stratix10_svc_request_channel_byname(&priv->client,
> @@ -440,6 +481,15 @@ static int s10_probe(struct platform_device *pdev)
> goto probe_err;
> }
>
> + /* get the running firmware version */
> + ret = s10_svc_send_msg(priv, COMMAND_FIRMWARE_VERSION,
> + NULL, 0, s10_fw_version_callback);
> + if (ret) {
> + dev_err(dev, "couldn't get firmware version\n");
> + fpga_mgr_free(mgr);
> + goto probe_err;
> + }
> +
> platform_set_drvdata(pdev, mgr);
> return ret;
>
> --
> 2.7.4
>
Thanks,
Moritz
On Mon, Jan 25, 2021 at 02:56:26PM -0600, [email protected] wrote:
> From: Richard Gong <[email protected]>
>
> Add authenticate-fpga-config property to support FPGA bitstream
> authentication, which makes sure a signed bitstream has valid signatures.
>
> Signed-off-by: Richard Gong <[email protected]>
> ---
> v3: no change
> v2: changed in alphabetical order
> ---
> drivers/fpga/of-fpga-region.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
> index e405309..3840883 100644
> --- a/drivers/fpga/of-fpga-region.c
> +++ b/drivers/fpga/of-fpga-region.c
> @@ -219,6 +219,9 @@ static struct fpga_image_info *of_fpga_region_parse_ov(
> info->overlay = overlay;
>
> /* Read FPGA region properties from the overlay */
> + if (of_property_read_bool(overlay, "authenticate-fpga-config"))
> + info->flags |= FPGA_MGR_BITSTREM_AUTHENTICATION;
> +
Should you check here that no new nodes are being added as you *only*
authenticate?
> if (of_property_read_bool(overlay, "partial-fpga-config"))
> info->flags |= FPGA_MGR_PARTIAL_RECONFIG;
>
> --
> 2.7.4
>
Thanks,
Moritz
On Mon, Jan 25, 2021 at 02:56:25PM -0600, [email protected] wrote:
> From: Richard Gong <[email protected]>
>
> Add FPGA_MGR_BITSTREM_AUTHENTICATION flag for FPGA bitstream
> authentication, which makes sure a signed bitstream has valid signatures.
>
> Except for the actual configuration of the device, the authentication works
> the same way as FPGA configuration does. If the authentication passes, the
> bitstream will be programmed into QSPI flash and will be expected to boot
> without issues.
>
> Signed-off-by: Richard Gong <[email protected]>
> ---
> v3: no change
> v2: align all FPGA_MGR_* flags
> update the commit messages
> ---
> include/linux/fpga/fpga-mgr.h | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> index 2bc3030..4fb3400 100644
> --- a/include/linux/fpga/fpga-mgr.h
> +++ b/include/linux/fpga/fpga-mgr.h
> @@ -67,12 +67,15 @@ enum fpga_mgr_states {
> * %FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
> *
> * %FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
> + *
> + * %FPGA_MGR_BITSTREM_AUTHENTICATION: do FPGA bitstream authentication only
> */
> -#define FPGA_MGR_PARTIAL_RECONFIG BIT(0)
> -#define FPGA_MGR_EXTERNAL_CONFIG BIT(1)
> -#define FPGA_MGR_ENCRYPTED_BITSTREAM BIT(2)
> -#define FPGA_MGR_BITSTREAM_LSB_FIRST BIT(3)
> -#define FPGA_MGR_COMPRESSED_BITSTREAM BIT(4)
> +#define FPGA_MGR_PARTIAL_RECONFIG BIT(0)
> +#define FPGA_MGR_EXTERNAL_CONFIG BIT(1)
> +#define FPGA_MGR_ENCRYPTED_BITSTREAM BIT(2)
> +#define FPGA_MGR_BITSTREAM_LSB_FIRST BIT(3)
> +#define FPGA_MGR_COMPRESSED_BITSTREAM BIT(4)
> +#define FPGA_MGR_BITSTREM_AUTHENTICATION BIT(5)
Consider FPGA_MGR_BITSTREAM_AUTHENTICATE (and fix typo)
>
> /**
> * struct fpga_image_info - information specific to a FPGA image
> --
> 2.7.4
>
Thanks,
Moritz