Thanks for the comments! > -----Original Message----- > From: Antoine Tenart <antoine.tenart@xxxxxxxxxxx> > Sent: Wednesday, June 19, 2019 2:28 PM > To: Pascal van Leeuwen <pascalvanl@xxxxxxxxx> > Cc: linux-crypto@xxxxxxxxxxxxxxx; antoine.tenart@xxxxxxxxxxx; herbert@xxxxxxxxxxxxxxxxxxx; > davem@xxxxxxxxxxxxx; Pascal Van Leeuwen <pvanleeuwen@xxxxxxxxxxxxxxxx> > Subject: Re: [PATCH 3/3] crypto: inside-secure - add support for using the EIP197 without > firmware images > > Hi Pascal, > > On Tue, Jun 18, 2019 at 07:56:24AM +0200, Pascal van Leeuwen wrote: > > > > static int eip197_load_firmwares(struct safexcel_crypto_priv *priv) > > { > > + /* > > + * The embedded one-size-fits-all MiniFW is just for handling TR > > + * prefetch & invalidate. It does not support any FW flows, effectively > > + * turning the EIP197 into a glorified EIP97 > > + */ > > + const u32 ipue_minifw[] = { > > + 0x24808200, 0x2D008204, 0x2680E208, 0x2780E20C, > > + 0x2200F7FF, 0x38347000, 0x2300F000, 0x15200A80, > > + 0x01699003, 0x60038011, 0x38B57000, 0x0119F04C, > > + 0x01198548, 0x20E64000, 0x20E75000, 0x1E200000, > > + 0x30E11000, 0x103A93FF, 0x60830014, 0x5B8B0000, > > + 0xC0389000, 0x600B0018, 0x2300F000, 0x60800011, > > + 0x90800000, 0x10000000, 0x10000000}; > > + const u32 ifpp_minifw[] = { > > + 0x21008000, 0x260087FC, 0xF01CE4C0, 0x60830006, > > + 0x530E0000, 0x90800000, 0x23008004, 0x24808008, > > + 0x2580800C, 0x0D300000, 0x205577FC, 0x30D42000, > > + 0x20DAA7FC, 0x43107000, 0x42220004, 0x00000000, > > + 0x00000000, 0x00000000, 0x00000000, 0x00000000, > > + 0x00060004, 0x20337004, 0x90800000, 0x10000000, > > + 0x10000000}; > > What is the license of this firmware? With this patch, it would be > shipped with Linux kernel images and this question is then very > important. > I am very well aware of that. The driver under GPL 2.0 so that simply would also apply to this. This was written by me specifically for this particular driver anyway and my company allows me to GPL this stuff. > In addition to this, the direction the kernel has taken was to *remove* > binary firmwares from its source code. I'm afraid adding this is a > no-go. > Actually, I would like to argue against that here. For a HW engineer, there really is no fundamental difference between control register contents or an instruction word. They can both have the exact same effects internal to the HW. If I had disguised this as a handful of config reg writes writing some #define'd magic values, probably no one would have even noticed. This is not firmware for some general purpose CPU, it is firmware for some very dedicated microengine thingy controlling some local datapath. By that same definition, the tokens the driver generates for processing could be considered "firmware" as well (as they are used by the hardware in a very similar way) ... > The proper solution I believe would be to support loading this "MiniFW", > which (depending on the license) could be either distributed in the > rootfs and loaded (like what's done currently), or through > CONFIG_EXTRA_FIRMWARE. > That seems total overkill for just a handful of words though. But I can do that if the community insists ... > This should be discussed first before discussing the implementation of > this particular patch. > > Thanks! > Antoine > > -- > Antoine Ténart, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Inside Secure www.insidesecure.com