Re: [RFC PATCH 1/3] dt-bindings: fpga: Add support for user-key encrypted bitstream loading

[Conor Dooley]

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@xxxxxxxxxx>
> > Sent: Wednesday, November 22, 2023 10:21 PM
> > To: Manne, Nava kishore <nava.kishore.manne@xxxxxxx>
> > Cc: mdf@xxxxxxxxxx; hao.wu@xxxxxxxxx; yilun.xu@xxxxxxxxx;
> > trix@xxxxxxxxxx; robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx;
> > conor+dt@xxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>;
> > mathieu.poirier@xxxxxxxxxx; Levinsky, Ben <ben.levinsky@xxxxxxx>;
> > Potthuri, Sai Krishna <sai.krishna.potthuri@xxxxxxx>; Shah, Tanmay
> > <tanmay.shah@xxxxxxx>; dhaval.r.shah@xxxxxxx; arnd@xxxxxxxx;
> > Datta, Shubhrajyoti <shubhrajyoti.datta@xxxxxxx>; linux-
> > fpga@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> > kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > 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@xxxxxxx>
> > > ---
> > >  .../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.

Attachment: signature.asc
Description: PGP signature