2021-05-04 10:24:30

by Nava kishore Manne

[permalink] [raw]
Subject: [RFC PATCH 0/4]Fpga: adds support to load the user-key encrypted FPGA Image loading

This patch series adds supports user-key encrypted FPGA Image loading using
FPGA Manager framework.

Nava kishore Manne (4):
drivers: firmware: Add user encrypted key load API support
fpga: Add new properties to support user-key encrypted bitstream
loading
drivers: fpga: Add user-key encrypted FPGA Image loading support
fpga: zynqmp: Add user-key encrypted FPGA Image loading support

.../devicetree/bindings/fpga/fpga-region.txt | 5 ++++
drivers/firmware/xilinx/zynqmp.c | 17 +++++++++++++
drivers/fpga/fpga-mgr.c | 15 ++++++++++++
drivers/fpga/of-fpga-region.c | 13 ++++++++++
drivers/fpga/zynqmp-fpga.c | 24 +++++++++++++++++--
include/linux/firmware/xlnx-zynqmp.h | 9 +++++++
include/linux/fpga/fpga-mgr.h | 7 ++++++
7 files changed, 88 insertions(+), 2 deletions(-)

--
2.17.1


2021-05-04 10:24:37

by Nava kishore Manne

[permalink] [raw]
Subject: [RFC PATCH 3/4] drivers: fpga: Add user-key encrypted FPGA Image loading support

This patch adds user-key encrypted FPGA Image loading support
to the framework.

Signed-off-by: Nava kishore Manne <[email protected]>
---
drivers/fpga/fpga-mgr.c | 15 +++++++++++++++
drivers/fpga/of-fpga-region.c | 13 +++++++++++++
include/linux/fpga/fpga-mgr.h | 7 +++++++
3 files changed, 35 insertions(+)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index b85bc47c91a9..3e79ab8cc86f 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -325,6 +325,7 @@ static int fpga_mgr_firmware_load(struct fpga_manager *mgr,
const char *image_name)
{
struct device *dev = &mgr->dev;
+ const struct firmware *enc_fw;
const struct firmware *fw;
int ret;

@@ -339,8 +340,22 @@ static int fpga_mgr_firmware_load(struct fpga_manager *mgr,
return ret;
}

+ if (info->encrypted_key_name) {
+ ret = request_firmware(&enc_fw, info->encrypted_key_name, dev);
+ if (ret) {
+ mgr->state = FPGA_MGR_STATE_FIRMWARE_REQ_ERR;
+ dev_err(dev, "Error requesting firmware %s\n",
+ info->encrypted_key_name);
+ return ret;
+ }
+ info->enc_key_buf = enc_fw->data;
+ info->enc_key_buf_size = enc_fw->size;
+ }
+
ret = fpga_mgr_buf_load(mgr, info, fw->data, fw->size);

+ if (info->encrypted_key_name)
+ release_firmware(enc_fw);
release_firmware(fw);

return ret;
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index e405309baadc..19faa463d96e 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -195,6 +195,7 @@ static struct fpga_image_info *of_fpga_region_parse_ov(
{
struct device *dev = &region->dev;
struct fpga_image_info *info;
+ const char *encrypted_key_name;
const char *firmware_name;
int ret;

@@ -228,6 +229,18 @@ static struct fpga_image_info *of_fpga_region_parse_ov(
if (of_property_read_bool(overlay, "encrypted-fpga-config"))
info->flags |= FPGA_MGR_ENCRYPTED_BITSTREAM;

+ if (of_property_read_bool(overlay, "encrypted-user-key-fpga-config")) {
+ if (!of_property_read_string(overlay, "encrypted-key-name",
+ &encrypted_key_name)) {
+ info->encrypted_key_name =
+ devm_kstrdup(dev, encrypted_key_name, GFP_KERNEL);
+ if (!info->encrypted_key_name)
+ return ERR_PTR(-ENOMEM);
+ }
+
+ info->flags |= FPGA_MGR_ENCRYPTED_USER_KEY_BITSTREAM;
+ }
+
if (!of_property_read_string(overlay, "firmware-name",
&firmware_name)) {
info->firmware_name = devm_kstrdup(dev, firmware_name,
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 2bc3030a69e5..ac86f4398c3c 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_ENCRYPTED_USER_KEY_BITSTREAM: indicates bitstream is encrypted
+ * with user-key
*/
#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_ENCRYPTED_USER_KEY_BITSTREAM BIT(5)

/**
* struct fpga_image_info - information specific to a FPGA image
@@ -82,6 +85,7 @@ enum fpga_mgr_states {
* @config_complete_timeout_us: maximum time for FPGA to switch to operating
* status in the write_complete op.
* @firmware_name: name of FPGA image firmware file
+ * @encrypted_key_name: name of the FPGA image encrypted user-key file
* @sgt: scatter/gather table containing FPGA image
* @buf: contiguous buffer containing FPGA image
* @count: size of buf
@@ -95,8 +99,11 @@ struct fpga_image_info {
u32 disable_timeout_us;
u32 config_complete_timeout_us;
char *firmware_name;
+ char *encrypted_key_name;
struct sg_table *sgt;
+ const char *enc_key_buf;
const char *buf;
+ size_t enc_key_buf_size;
size_t count;
int region_id;
struct device *dev;
--
2.17.1

2021-05-04 10:24:49

by Nava kishore Manne

[permalink] [raw]
Subject: [RFC PATCH 1/4] drivers: firmware: Add user encrypted key load API support

This patch adds user encrypted key load API to support
User key encrypted images loading use cases from Linux.

Signed-off-by: Nava kishore Manne <[email protected]>
---
drivers/firmware/xilinx/zynqmp.c | 17 +++++++++++++++++
include/linux/firmware/xlnx-zynqmp.h | 7 +++++++
2 files changed, 24 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 15b138326ecc..2fa5687a75f8 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -787,6 +787,23 @@ int zynqmp_pm_fpga_load(const u64 address, const u32 size, const u32 flags)
}
EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_load);

+/**
+ * zynqmp_pm_fpga_key_load - Perform to load the bitstream encrypted key
+ * @address: Address to write
+ * @size: encrypted key size
+ *
+ * This function provides access to pmufw. To transfer
+ * the required encrypted key.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+int zynqmp_pm_fpga_enc_key_load(const u64 address, const u32 size)
+{
+ return zynqmp_pm_invoke_fn(PM_ENC_KEY_LOAD, lower_32_bits(address),
+ upper_32_bits(address), size, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_enc_key_load);
+
/**
* zynqmp_pm_fpga_get_status - Read value from PCAP status register
* @value: Value to read
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 9d1a5c175065..7aa9ad40ff53 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -91,6 +91,7 @@ enum pm_api_id {
PM_CLOCK_GETPARENT = 44,
PM_SECURE_AES = 47,
PM_FEATURE_CHECK = 63,
+ PM_ENC_KEY_LOAD = 64,
};

/* PMU-FW return status codes */
@@ -411,6 +412,7 @@ int zynqmp_pm_pinctrl_get_config(const u32 pin, const u32 param,
u32 *value);
int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
u32 value);
+int zynqmp_pm_fpga_enc_key_load(const u64 address, const u32 size);
#else
static inline int zynqmp_pm_get_api_version(u32 *version)
{
@@ -622,6 +624,11 @@ static inline int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
{
return -ENODEV;
}
+
+static inline int zynqmp_pm_fpga_enc_key_load(const u64 address, const u32 size)
+{
+ return -ENODEV;
+}
#endif

#endif /* __FIRMWARE_ZYNQMP_H__ */
--
2.17.1

2021-05-04 10:25:20

by Nava kishore Manne

[permalink] [raw]
Subject: [RFC PATCH 2/4] fpga: Add new properties to support user-key encrypted bitstream loading

This patch Adds ‘encrypted-key-name’ and
‘encrypted-user-key-fpga-config’ properties
to support user-key encrypted bitstream loading
use case.

Signed-off-by: Nava kishore Manne <[email protected]>
---
Documentation/devicetree/bindings/fpga/fpga-region.txt | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt b/Documentation/devicetree/bindings/fpga/fpga-region.txt
index d787d57491a1..957dc6cbcd9e 100644
--- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
+++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
@@ -177,6 +177,9 @@ Optional properties:
it indicates that the FPGA has already been programmed with this image.
If this property is in an overlay targeting a FPGA region, it is a
request to program the FPGA with that image.
+- encrypted-key-name : should contain the name of an encrypted key file located
+ on the firmware search path. It will be used to decrypt the FPGA image
+ file.
- fpga-bridges : should contain a list of phandles to FPGA Bridges that must be
controlled during FPGA programming along with the parent FPGA bridge.
This property is optional if the FPGA Manager handles the bridges.
@@ -187,6 +190,8 @@ Optional properties:
- external-fpga-config : boolean, set if the FPGA has already been configured
prior to OS boot up.
- encrypted-fpga-config : boolean, set if the bitstream is encrypted
+- encrypted-user-key-fpga-config : boolean, set if the bitstream is encrypted
+ with user key.
- region-unfreeze-timeout-us : The maximum time in microseconds to wait for
bridges to successfully become enabled after the region has been
programmed.
--
2.17.1

2021-05-04 11:01:16

by Nava kishore Manne

[permalink] [raw]
Subject: [RFC PATCH 4/4] fpga: zynqmp: Add user-key encrypted FPGA Image loading support

This patch adds support to load the user-key encrypted FPGA
Image loading to the Xilinx ZynqMP Soc.

Signed-off-by: Nava kishore Manne <[email protected]>
---
drivers/fpga/zynqmp-fpga.c | 24 ++++++++++++++++++++++--
include/linux/firmware/xlnx-zynqmp.h | 2 ++
2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
index 125743c9797f..565ebe9e1610 100644
--- a/drivers/fpga/zynqmp-fpga.c
+++ b/drivers/fpga/zynqmp-fpga.c
@@ -22,6 +22,8 @@
*/
struct zynqmp_fpga_priv {
struct device *dev;
+ const char *key_buf;
+ size_t key_size;
u32 flags;
};

@@ -33,6 +35,8 @@ static int zynqmp_fpga_ops_write_init(struct fpga_manager *mgr,

priv = mgr->priv;
priv->flags = info->flags;
+ priv->key_buf = info->enc_key_buf;
+ priv->key_size = info->enc_key_buf_size;

return 0;
}
@@ -41,9 +45,9 @@ static int zynqmp_fpga_ops_write(struct fpga_manager *mgr,
const char *buf, size_t size)
{
struct zynqmp_fpga_priv *priv;
- dma_addr_t dma_addr;
+ dma_addr_t dma_addr, key_addr;
u32 eemi_flags = 0;
- char *kbuf;
+ char *kbuf, *key_kbuf;
int ret;

priv = mgr->priv;
@@ -54,13 +58,29 @@ static int zynqmp_fpga_ops_write(struct fpga_manager *mgr,

memcpy(kbuf, buf, size);

+ if (priv->flags & FPGA_MGR_ENCRYPTED_USER_KEY_BITSTREAM) {
+ eemi_flags |= XILINX_ZYNQMP_PM_FPGA_ENC_USER_KEY;
+ key_kbuf = dma_alloc_coherent(priv->dev, size, &key_addr,
+ GFP_KERNEL);
+ if (!key_kbuf)
+ return -ENOMEM;
+ memcpy(key_kbuf, priv->key_buf, priv->key_size);
+ }
+
wmb(); /* ensure all writes are done before initiate FW call */

if (priv->flags & FPGA_MGR_PARTIAL_RECONFIG)
eemi_flags |= XILINX_ZYNQMP_PM_FPGA_PARTIAL;

+ if (priv->flags & FPGA_MGR_ENCRYPTED_USER_KEY_BITSTREAM)
+ ret = zynqmp_pm_fpga_enc_key_load(key_addr, priv->key_size);
+
ret = zynqmp_pm_fpga_load(dma_addr, size, eemi_flags);

+ if (priv->flags & FPGA_MGR_ENCRYPTED_USER_KEY_BITSTREAM)
+ dma_free_coherent(priv->dev, priv->key_size,
+ key_kbuf, key_addr);
+
dma_free_coherent(priv->dev, size, kbuf, dma_addr);

return ret;
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 7aa9ad40ff53..a767386d930a 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -56,9 +56,11 @@
* Firmware FPGA Manager flags
* XILINX_ZYNQMP_PM_FPGA_FULL: FPGA full reconfiguration
* XILINX_ZYNQMP_PM_FPGA_PARTIAL: FPGA partial reconfiguration
+ * XILINX_ZYNQMP_PM_FPGA_ENC_USER_KEY: User-key Encrypted FPGA reconfiguration
*/
#define XILINX_ZYNQMP_PM_FPGA_FULL 0x0U
#define XILINX_ZYNQMP_PM_FPGA_PARTIAL BIT(0)
+#define XILINX_ZYNQMP_PM_FPGA_ENC_USER_KEY BIT(3)

enum pm_api_id {
PM_GET_API_VERSION = 1,
--
2.17.1

2021-05-13 02:32:34

by Rob Herring

[permalink] [raw]
Subject: Re: [RFC PATCH 2/4] fpga: Add new properties to support user-key encrypted bitstream loading

On Tue, May 04, 2021 at 03:52:25PM +0530, Nava kishore Manne wrote:
> This patch Adds ‘encrypted-key-name’ and
> ‘encrypted-user-key-fpga-config’ properties
> to support user-key encrypted bitstream loading
> use case.
>
> Signed-off-by: Nava kishore Manne <[email protected]>
> ---
> Documentation/devicetree/bindings/fpga/fpga-region.txt | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> index d787d57491a1..957dc6cbcd9e 100644
> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> @@ -177,6 +177,9 @@ Optional properties:
> it indicates that the FPGA has already been programmed with this image.
> If this property is in an overlay targeting a FPGA region, it is a
> request to program the FPGA with that image.
> +- encrypted-key-name : should contain the name of an encrypted key file located
> + on the firmware search path. It will be used to decrypt the FPGA image
> + file.
> - fpga-bridges : should contain a list of phandles to FPGA Bridges that must be
> controlled during FPGA programming along with the parent FPGA bridge.
> This property is optional if the FPGA Manager handles the bridges.
> @@ -187,6 +190,8 @@ Optional properties:
> - external-fpga-config : boolean, set if the FPGA has already been configured
> prior to OS boot up.
> - encrypted-fpga-config : boolean, set if the bitstream is encrypted
> +- encrypted-user-key-fpga-config : boolean, set if the bitstream is encrypted
> + with user key.

What's the relationship with encrypted-fpga-config? Both present or
mutually exclusive? Couldn't this be implied by encrypted-key-name being
present?

> - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
> bridges to successfully become enabled after the region has been
> programmed.
> --
> 2.17.1
>

2021-05-13 17:46:32

by Nava kishore Manne

[permalink] [raw]
Subject: RE: [RFC PATCH 2/4] fpga: Add new properties to support user-key encrypted bitstream loading

Hi Rob,

Please find my response inline.

> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: Thursday, May 13, 2021 8:01 AM
> To: Nava kishore Manne <[email protected]>
> Cc: [email protected]; [email protected]; Michal Simek <[email protected]>;
> [email protected]; Rajan Vaja <[email protected]>;
> [email protected]; [email protected]; Amit Sunil Dhamne
> <[email protected]>; Tejas Patel <[email protected]>;
> [email protected]; Manish Narani <[email protected]>; Sai Krishna
> Potthuri <[email protected]>; Jiaying Liang <[email protected]>; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; git
> <[email protected]>; [email protected]
> Subject: Re: [RFC PATCH 2/4] fpga: Add new properties to support user-key
> encrypted bitstream loading
>
> On Tue, May 04, 2021 at 03:52:25PM +0530, Nava kishore Manne wrote:
> > This patch Adds ‘encrypted-key-name’ and
> > ‘encrypted-user-key-fpga-config’ properties to support user-key
> > encrypted bitstream loading use case.
> >
> > Signed-off-by: Nava kishore Manne <[email protected]>
> > ---
> > Documentation/devicetree/bindings/fpga/fpga-region.txt | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > index d787d57491a1..957dc6cbcd9e 100644
> > --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > @@ -177,6 +177,9 @@ Optional properties:
> > it indicates that the FPGA has already been programmed with this
> image.
> > If this property is in an overlay targeting a FPGA region, it is a
> > request to program the FPGA with that image.
> > +- encrypted-key-name : should contain the name of an encrypted key file
> located
> > + on the firmware search path. It will be used to decrypt the FPGA
> image
> > + file.
> > - fpga-bridges : should contain a list of phandles to FPGA Bridges that must
> be
> > controlled during FPGA programming along with the parent FPGA
> bridge.
> > This property is optional if the FPGA Manager handles the bridges.
> > @@ -187,6 +190,8 @@ Optional properties:
> > - external-fpga-config : boolean, set if the FPGA has already been
> configured
> > prior to OS boot up.
> > - encrypted-fpga-config : boolean, set if the bitstream is encrypted
> > +- encrypted-user-key-fpga-config : boolean, set if the bitstream is
> encrypted
> > + with user key.
>
> What's the relationship with encrypted-fpga-config? Both present or
> mutually exclusive? Couldn't this be implied by encrypted-key-name being
> present?
>

In Encryption we have two kinds of use case one is Encrypted Bitstream loading with Device-key and
Other one is Encrypted Bitstream loading with User-key. encrypted-fpga-config and encrypted-user-key-fpga-config
are mutually exclusive. To differentiate both the use cases I have added this new flag and Aes Key file(encrypted-key-name)
is needed only for encrypted-user-key-fpga-config use cases.

Regards,
Navakishore.

2021-05-13 21:51:59

by Rob Herring

[permalink] [raw]
Subject: Re: [RFC PATCH 2/4] fpga: Add new properties to support user-key encrypted bitstream loading

On Thu, May 13, 2021 at 5:55 AM Nava kishore Manne <[email protected]> wrote:
>
> Hi Rob,
>
> Please find my response inline.
>
> > -----Original Message-----
> > From: Rob Herring <[email protected]>
> > Sent: Thursday, May 13, 2021 8:01 AM
> > To: Nava kishore Manne <[email protected]>
> > Cc: [email protected]; [email protected]; Michal Simek <[email protected]>;
> > [email protected]; Rajan Vaja <[email protected]>;
> > [email protected]; [email protected]; Amit Sunil Dhamne
> > <[email protected]>; Tejas Patel <[email protected]>;
> > [email protected]; Manish Narani <[email protected]>; Sai Krishna
> > Potthuri <[email protected]>; Jiaying Liang <[email protected]>; linux-
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; git
> > <[email protected]>; [email protected]
> > Subject: Re: [RFC PATCH 2/4] fpga: Add new properties to support user-key
> > encrypted bitstream loading
> >
> > On Tue, May 04, 2021 at 03:52:25PM +0530, Nava kishore Manne wrote:
> > > This patch Adds ‘encrypted-key-name’ and
> > > ‘encrypted-user-key-fpga-config’ properties to support user-key
> > > encrypted bitstream loading use case.
> > >
> > > Signed-off-by: Nava kishore Manne <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/fpga/fpga-region.txt | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > index d787d57491a1..957dc6cbcd9e 100644
> > > --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > @@ -177,6 +177,9 @@ Optional properties:
> > > it indicates that the FPGA has already been programmed with this
> > image.
> > > If this property is in an overlay targeting a FPGA region, it is a
> > > request to program the FPGA with that image.
> > > +- encrypted-key-name : should contain the name of an encrypted key file
> > located
> > > + on the firmware search path. It will be used to decrypt the FPGA
> > image
> > > + file.
> > > - fpga-bridges : should contain a list of phandles to FPGA Bridges that must
> > be
> > > controlled during FPGA programming along with the parent FPGA
> > bridge.
> > > This property is optional if the FPGA Manager handles the bridges.
> > > @@ -187,6 +190,8 @@ Optional properties:
> > > - external-fpga-config : boolean, set if the FPGA has already been
> > configured
> > > prior to OS boot up.
> > > - encrypted-fpga-config : boolean, set if the bitstream is encrypted
> > > +- encrypted-user-key-fpga-config : boolean, set if the bitstream is
> > encrypted
> > > + with user key.
> >
> > What's the relationship with encrypted-fpga-config? Both present or
> > mutually exclusive? Couldn't this be implied by encrypted-key-name being
> > present?
> >
>
> In Encryption we have two kinds of use case one is Encrypted Bitstream loading with Device-key and
> Other one is Encrypted Bitstream loading with User-key. encrypted-fpga-config and encrypted-user-key-fpga-config
> are mutually exclusive. To differentiate both the use cases I have added this new flag and Aes Key file(encrypted-key-name)
> is needed only for encrypted-user-key-fpga-config use cases.

If encrypted-key-name is required for a user key, then why do you need
encrypted-user-key-fpga-config also?

IOW, why have 3 properties (that's 9 possible combinations) for 2 modes?

Rob

2021-05-27 10:53:21

by Nava kishore Manne

[permalink] [raw]
Subject: RE: [RFC PATCH 2/4] fpga: Add new properties to support user-key encrypted bitstream loading

Hi Rob,

Please find my response inline.

> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: Thursday, May 13, 2021 8:05 PM
> To: Nava kishore Manne <[email protected]>
> Cc: [email protected]; [email protected]; Michal Simek <[email protected]>;
> [email protected]; Rajan Vaja <[email protected]>;
> [email protected]; [email protected]; Amit Sunil Dhamne
> <[email protected]>; Tejas Patel <[email protected]>;
> [email protected]; Manish Narani <[email protected]>; Sai Krishna
> Potthuri <[email protected]>; Jiaying Liang <[email protected]>; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; git
> <[email protected]>; [email protected]
> Subject: Re: [RFC PATCH 2/4] fpga: Add new properties to support user-key
> encrypted bitstream loading
>
> On Thu, May 13, 2021 at 5:55 AM Nava kishore Manne <[email protected]>
> wrote:
> >
> > Hi Rob,
> >
> > Please find my response inline.
> >
> > > -----Original Message-----
> > > From: Rob Herring <[email protected]>
> > > Sent: Thursday, May 13, 2021 8:01 AM
> > > To: Nava kishore Manne <[email protected]>
> > > Cc: [email protected]; [email protected]; Michal Simek
> > > <[email protected]>; [email protected]; Rajan Vaja
> <[email protected]>;
> > > [email protected]; [email protected]; Amit Sunil
> > > Dhamne <[email protected]>; Tejas Patel
> > > <[email protected]>; [email protected]; Manish Narani
> > > <[email protected]>; Sai Krishna Potthuri <[email protected]>;
> > > Jiaying Liang <[email protected]>; linux- [email protected];
> > > [email protected]; linux- [email protected];
> > > [email protected]; git <[email protected]>;
> > > [email protected]
> > > Subject: Re: [RFC PATCH 2/4] fpga: Add new properties to support
> > > user-key encrypted bitstream loading
> > >
> > > On Tue, May 04, 2021 at 03:52:25PM +0530, Nava kishore Manne wrote:
> > > > This patch Adds ‘encrypted-key-name’ and
> > > > ‘encrypted-user-key-fpga-config’ properties to support user-key
> > > > encrypted bitstream loading use case.
> > > >
> > > > Signed-off-by: Nava kishore Manne <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/fpga/fpga-region.txt | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > index d787d57491a1..957dc6cbcd9e 100644
> > > > --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > @@ -177,6 +177,9 @@ Optional properties:
> > > > it indicates that the FPGA has already been programmed with
> > > > this
> > > image.
> > > > If this property is in an overlay targeting a FPGA region, it is a
> > > > request to program the FPGA with that image.
> > > > +- encrypted-key-name : should contain the name of an encrypted
> > > > +key file
> > > located
> > > > + on the firmware search path. It will be used to decrypt the
> > > > + FPGA
> > > image
> > > > + file.
> > > > - fpga-bridges : should contain a list of phandles to FPGA
> > > > Bridges that must
> > > be
> > > > controlled during FPGA programming along with the parent FPGA
> > > bridge.
> > > > This property is optional if the FPGA Manager handles the bridges.
> > > > @@ -187,6 +190,8 @@ Optional properties:
> > > > - external-fpga-config : boolean, set if the FPGA has already
> > > > been
> > > configured
> > > > prior to OS boot up.
> > > > - encrypted-fpga-config : boolean, set if the bitstream is
> > > > encrypted
> > > > +- encrypted-user-key-fpga-config : boolean, set if the bitstream
> > > > +is
> > > encrypted
> > > > + with user key.
> > >
> > > What's the relationship with encrypted-fpga-config? Both present or
> > > mutually exclusive? Couldn't this be implied by encrypted-key-name
> > > being present?
> > >
> >
> > In Encryption we have two kinds of use case one is Encrypted Bitstream
> > loading with Device-key and Other one is Encrypted Bitstream loading
> > with User-key. encrypted-fpga-config and
> > encrypted-user-key-fpga-config are mutually exclusive. To differentiate
> both the use cases I have added this new flag and Aes Key file(encrypted-key-
> name) is needed only for encrypted-user-key-fpga-config use cases.
>
> If encrypted-key-name is required for a user key, then why do you need
> encrypted-user-key-fpga-config also?
>
> IOW, why have 3 properties (that's 9 possible combinations) for 2 modes?
>

Agree, we can use encrypted-key-name for user-key use cases instead of having both encrypted-key-name and encrypted-user-key-fpga-config flags.
Will fix this issue in v2.

Regards,
Navakishore.