[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 at kernel.org>
> > Sent: Wednesday, November 22, 2023 10:21 PM
> > To: Manne, Nava kishore <nava.kishore.manne at amd.com>
> > Cc: mdf at kernel.org; hao.wu at intel.com; yilun.xu at intel.com;
> > trix at redhat.com; robh+dt at kernel.org; krzysztof.kozlowski+dt at linaro.org;
> > conor+dt at kernel.org; Simek, Michal <michal.simek at amd.com>;
> > mathieu.poirier at linaro.org; Levinsky, Ben <ben.levinsky at amd.com>;
> > Potthuri, Sai Krishna <sai.krishna.potthuri at amd.com>; Shah, Tanmay
> > <tanmay.shah at amd.com>; dhaval.r.shah at amd.com; arnd at arndb.de;
> > Datta, Shubhrajyoti <shubhrajyoti.datta at amd.com>; linux-
> > fpga at vger.kernel.org; devicetree at vger.kernel.org; linux-
> > kernel at vger.kernel.org; linux-arm-kernel at 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 at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20231124/2733a634/attachment.sig>