Linux-FPGA Archive mirror
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: "Manne, Nava kishore" <nava.kishore.manne@amd.com>,
	"mdf@kernel.org" <mdf@kernel.org>,
	"hao.wu@intel.com" <hao.wu@intel.com>,
	"yilun.xu@intel.com" <yilun.xu@intel.com>,
	"trix@redhat.com" <trix@redhat.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"krzysztof.kozlowski+dt@linaro.org"
	<krzysztof.kozlowski+dt@linaro.org>,
	"conor+dt@kernel.org" <conor+dt@kernel.org>,
	"Simek, Michal" <michal.simek@amd.com>,
	"mathieu.poirier@linaro.org" <mathieu.poirier@linaro.org>,
	"Levinsky, Ben" <ben.levinsky@amd.com>,
	"Potthuri, Sai Krishna" <sai.krishna.potthuri@amd.com>,
	"Shah, Tanmay" <tanmay.shah@amd.com>,
	"dhaval.r.shah@amd.com" <dhaval.r.shah@amd.com>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"Datta, Shubhrajyoti" <shubhrajyoti.datta@amd.com>,
	"linux-fpga@vger.kernel.org" <linux-fpga@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCH 1/3] dt-bindings: fpga: Add support for user-key encrypted bitstream loading
Date: Fri, 22 Dec 2023 15:30:55 +0000	[thread overview]
Message-ID: <20231222-unisexual-construct-573e79a488c9@spud> (raw)
In-Reply-To: <a90d980e-71a1-4b90-b1cb-66ac45d79031@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 4258 bytes --]

On Fri, Nov 24, 2023 at 04:46:06PM +0100, Krzysztof Kozlowski wrote:
> On 24/11/2023 13:48, Conor Dooley wrote:
> > On Fri, Nov 24, 2023 at 06:35:19AM +0000, Manne, Nava kishore wrote:
> >> Hi Conor,
> >>
> >> 	Thanks for providing the review comments.
> >> Please find my response inline.
> >>
> >>> -----Original Message-----
> >>> From: Conor Dooley <conor@kernel.org>
> >>> Sent: Wednesday, November 22, 2023 10:21 PM
> >>> To: Manne, Nava kishore <nava.kishore.manne@amd.com>
> >>> Cc: mdf@kernel.org; hao.wu@intel.com; yilun.xu@intel.com;
> >>> trix@redhat.com; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> >>> conor+dt@kernel.org; Simek, Michal <michal.simek@amd.com>;
> >>> mathieu.poirier@linaro.org; Levinsky, Ben <ben.levinsky@amd.com>;
> >>> Potthuri, Sai Krishna <sai.krishna.potthuri@amd.com>; Shah, Tanmay
> >>> <tanmay.shah@amd.com>; dhaval.r.shah@amd.com; arnd@arndb.de;
> >>> Datta, Shubhrajyoti <shubhrajyoti.datta@amd.com>; linux-
> >>> fpga@vger.kernel.org; devicetree@vger.kernel.org; linux-
> >>> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> >>> Subject: Re: [RFC PATCH 1/3] dt-bindings: fpga: Add support for user-key
> >>> encrypted bitstream loading
> >>>
> >>> On Wed, Nov 22, 2023 at 11:14:02AM +0530, Nava kishore Manne wrote:
> >>>> Adds ‘encrypted-key-name’ property to support user-key encrypted
> >>>> bitstream loading use case.
> >>>>
> >>>> Signed-off-by: Nava kishore Manne <nava.kishore.manne@amd.com>
> >>>> ---
> >>>>  .../devicetree/bindings/fpga/fpga-region.txt  | 32
> >>>> +++++++++++++++++++
> >>>
> >>> Is there a reason that this has not yet been converted to yaml?
> >>>
> >> I am not sure about the complication involved here why it's not converted to yaml format.
> >> Due to time constraints, I couldn’t spend much time so I have used this existing legacy format
> >> to add my changes.
> >>
> >>>>  1 file changed, 32 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> >>>> b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> >>>> index 528df8a0e6d8..309334558b3f 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 an 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 with user-key.
> >>>
> >>> I might be misreading things, but your driver code seems to assume that this
> >>> is an aes key. Nothing here seems to document that this is supposed to be a
> >>> key of a particular type.
> >>>
> >>
> >> Yes, these changes are intended to add the support for Aes user-key encrypted bitstream loading use case.
> >> Will fix it in v2, something like below.
> >> aes-key-file-name : Should contain the AES key file name on the firmware search path.
> >> 		      The key file contains the AES key and it will be used to decrypt the FPGA image.
> > 
> > Then when someone comes along looking for a different type of encryption
> > we will end up with national-pride-foo-file-name etc. I think I'd rather
> > have a second property that notes what type of cipher is being used and
> > if that property is not present default to AES.
> 
> I wonder why does it need to be in DT in the first place? Why it cannot
> be appended to the FPGA binary image itself? Which also points to
> dubious security aspect of this approach... Shipping FPGA encrypted
> image with its decryption key sounds like marvelous idea.
> 
> Even if this is suitable, why not using more arguments of firmware-name?
> This would scale even for multiple FPGA firmwares with different keys
> (although such need seems unlikely).

In case it is not clear (given the month's delay here), this question is
for the submitter of the series to answer, not me.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2023-12-22 15:31 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-22  5:44 [RFC PATCH 0/3]fpga: Add encrypted Bitstream loading support Nava kishore Manne
2023-11-22  5:44 ` [RFC PATCH 1/3] dt-bindings: fpga: Add support for user-key encrypted bitstream loading Nava kishore Manne
2023-11-22 16:50   ` Conor Dooley
2023-11-24  6:35     ` Manne, Nava kishore
2023-11-24 12:48       ` Conor Dooley
2023-11-24 15:46         ` Krzysztof Kozlowski
2023-12-22 15:30           ` Conor Dooley [this message]
2023-11-22  5:44 ` [RFC PATCH 2/3] drivers: fpga: Add user-key encrypted FPGA Image loading support Nava kishore Manne
2023-11-22  5:44 ` [RFC PATCH 3/3] fpga: zynqmp: Add encrypted Bitstream " Nava kishore Manne
2023-11-24 15:49 ` [RFC PATCH 0/3]fpga: " Krzysztof Kozlowski

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=20231222-unisexual-construct-573e79a488c9@spud \
    --to=conor@kernel.org \
    --cc=arnd@arndb.de \
    --cc=ben.levinsky@amd.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dhaval.r.shah@amd.com \
    --cc=hao.wu@intel.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-fpga@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=mdf@kernel.org \
    --cc=michal.simek@amd.com \
    --cc=nava.kishore.manne@amd.com \
    --cc=robh+dt@kernel.org \
    --cc=sai.krishna.potthuri@amd.com \
    --cc=shubhrajyoti.datta@amd.com \
    --cc=tanmay.shah@amd.com \
    --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 read-only IMAP folder(s) and NNTP newsgroup(s).