| Message ID | 1611608188-25621-2-git-send-email-richard.gong@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Extend Intel service layer, FPGA manager and region | expand |
On 1/25/21 12:56 PM, richard.gong@linux.intel.com wrote: > From: Richard Gong <richard.gong@intel.com> > > Add COMMAND_AUTHENTICATE_BITSTREAM command flag for new added bitstream > authentication feature. Authenticating a bitstream is to make sure a signed > bitstream has the valid signatures. > > Except for the actual configuration of the device, the bitstream > authentication works the same way as FPGA configuration does. If the > authentication passes, the signed bitstream will be programmed into QSPI > flash memory and will be expected to boot without issues. > > Clean up COMMAND_RECONFIG_FLAG_PARTIAL flag by resetting it to 0, which > aligns with the firmware settings. > > Signed-off-by: Richard Gong <richard.gong@intel.com> > --- > v3: no change > v2: new added > --- > include/linux/firmware/intel/stratix10-svc-client.h | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h > index ebc2956..7ada1f2 100644 > --- a/include/linux/firmware/intel/stratix10-svc-client.h > +++ b/include/linux/firmware/intel/stratix10-svc-client.h > @@ -51,12 +51,17 @@ > #define SVC_STATUS_NO_SUPPORT 6 > > /* This patch fails to apply, i believe the conflict is because in mainline this is '/**' not '/*' Please check or point me at the branch/tag you are using. I am using char-misc-next. Tom Tom
Hi Tom, On 1/25/21 4:56 PM, Tom Rix wrote: > > On 1/25/21 12:56 PM, richard.gong@linux.intel.com wrote: >> From: Richard Gong <richard.gong@intel.com> >> >> Add COMMAND_AUTHENTICATE_BITSTREAM command flag for new added bitstream >> authentication feature. Authenticating a bitstream is to make sure a signed >> bitstream has the valid signatures. >> >> Except for the actual configuration of the device, the bitstream >> authentication works the same way as FPGA configuration does. If the >> authentication passes, the signed bitstream will be programmed into QSPI >> flash memory and will be expected to boot without issues. >> >> Clean up COMMAND_RECONFIG_FLAG_PARTIAL flag by resetting it to 0, which >> aligns with the firmware settings. >> >> Signed-off-by: Richard Gong <richard.gong@intel.com> >> --- >> v3: no change >> v2: new added >> --- >> include/linux/firmware/intel/stratix10-svc-client.h | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h >> index ebc2956..7ada1f2 100644 >> --- a/include/linux/firmware/intel/stratix10-svc-client.h >> +++ b/include/linux/firmware/intel/stratix10-svc-client.h >> @@ -51,12 +51,17 @@ >> #define SVC_STATUS_NO_SUPPORT 6 >> >> /* > > This patch fails to apply, i believe the conflict is because in mainline this is '/**' not '/*' > > Please check or point me at the branch/tag you are using. > I am using next-20210125 tag. > I am using char-misc-next. > > Tom > > Tom > Regards, Richard
On Mon, Jan 25, 2021 at 02:56:23PM -0600, richard.gong@linux.intel.com wrote: > From: Richard Gong <richard.gong@intel.com> > > Add COMMAND_AUTHENTICATE_BITSTREAM command flag for new added bitstream > authentication feature. Authenticating a bitstream is to make sure a signed > bitstream has the valid signatures. > > Except for the actual configuration of the device, the bitstream > authentication works the same way as FPGA configuration does. If the > authentication passes, the signed bitstream will be programmed into QSPI > flash memory and will be expected to boot without issues. > > Clean up COMMAND_RECONFIG_FLAG_PARTIAL flag by resetting it to 0, which > aligns with the firmware settings. > > Signed-off-by: Richard Gong <richard.gong@intel.com> > --- > v3: no change > v2: new added > --- > include/linux/firmware/intel/stratix10-svc-client.h | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h > index ebc2956..7ada1f2 100644 > --- a/include/linux/firmware/intel/stratix10-svc-client.h > +++ b/include/linux/firmware/intel/stratix10-svc-client.h > @@ -51,12 +51,17 @@ > #define SVC_STATUS_NO_SUPPORT 6 > > /* > - * Flag bit for COMMAND_RECONFIG > + * Flag for COMMAND_RECONFIG, in bit number > * > * COMMAND_RECONFIG_FLAG_PARTIAL: > - * Set to FPGA configuration type (full or partial). > + * Set for partial FPGA configuration. > + * > + * COMMAND_AUTHENTICATE_BITSTREAM: > + * Set for bitstream authentication, which makes sure a signed bitstream > + * has valid signatures before committing it to QSPI flash memory. > */ > -#define COMMAND_RECONFIG_FLAG_PARTIAL 1 > +#define COMMAND_RECONFIG_FLAG_PARTIAL 0 So is this a bugfix, changing this value to the correct one? If so, shouldn't this be a stand-alone patch and get backported to stable kernel releases? If not, then no one uses this flag today? thanks, greg k-h
Hi Greg, Thanks for review! On 1/27/21 6:04 AM, Greg KH wrote: > On Mon, Jan 25, 2021 at 02:56:23PM -0600, richard.gong@linux.intel.com wrote: >> From: Richard Gong <richard.gong@intel.com> >> >> Add COMMAND_AUTHENTICATE_BITSTREAM command flag for new added bitstream >> authentication feature. Authenticating a bitstream is to make sure a signed >> bitstream has the valid signatures. >> >> Except for the actual configuration of the device, the bitstream >> authentication works the same way as FPGA configuration does. If the >> authentication passes, the signed bitstream will be programmed into QSPI >> flash memory and will be expected to boot without issues. >> >> Clean up COMMAND_RECONFIG_FLAG_PARTIAL flag by resetting it to 0, which >> aligns with the firmware settings. >> >> Signed-off-by: Richard Gong <richard.gong@intel.com> >> --- >> v3: no change >> v2: new added >> --- >> include/linux/firmware/intel/stratix10-svc-client.h | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h >> index ebc2956..7ada1f2 100644 >> --- a/include/linux/firmware/intel/stratix10-svc-client.h >> +++ b/include/linux/firmware/intel/stratix10-svc-client.h >> @@ -51,12 +51,17 @@ >> #define SVC_STATUS_NO_SUPPORT 6 >> >> /* >> - * Flag bit for COMMAND_RECONFIG >> + * Flag for COMMAND_RECONFIG, in bit number >> * >> * COMMAND_RECONFIG_FLAG_PARTIAL: >> - * Set to FPGA configuration type (full or partial). >> + * Set for partial FPGA configuration. >> + * >> + * COMMAND_AUTHENTICATE_BITSTREAM: >> + * Set for bitstream authentication, which makes sure a signed bitstream >> + * has valid signatures before committing it to QSPI flash memory. >> */ >> -#define COMMAND_RECONFIG_FLAG_PARTIAL 1 >> +#define COMMAND_RECONFIG_FLAG_PARTIAL 0 > > So is this a bugfix, changing this value to the correct one? Yes, it is a bug fix. > > If so, shouldn't this be a stand-alone patch and get backported to > stable kernel releases? Sure, I will make change and submit again as a standalone patch. > > If not, then no one uses this flag today? > > thanks, > > greg k-h > Regards, Richard
On Wed, Jan 27, 2021 at 07:05:41AM -0600, Richard Gong wrote: > > Hi Greg, > > Thanks for review! > > On 1/27/21 6:04 AM, Greg KH wrote: > > On Mon, Jan 25, 2021 at 02:56:23PM -0600, richard.gong@linux.intel.com wrote: > > > From: Richard Gong <richard.gong@intel.com> > > > > > > Add COMMAND_AUTHENTICATE_BITSTREAM command flag for new added bitstream > > > authentication feature. Authenticating a bitstream is to make sure a signed > > > bitstream has the valid signatures. > > > > > > Except for the actual configuration of the device, the bitstream > > > authentication works the same way as FPGA configuration does. If the > > > authentication passes, the signed bitstream will be programmed into QSPI > > > flash memory and will be expected to boot without issues. > > > > > > Clean up COMMAND_RECONFIG_FLAG_PARTIAL flag by resetting it to 0, which > > > aligns with the firmware settings. > > > > > > Signed-off-by: Richard Gong <richard.gong@intel.com> > > > --- > > > v3: no change > > > v2: new added > > > --- > > > include/linux/firmware/intel/stratix10-svc-client.h | 11 ++++++++--- > > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > > > diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h > > > index ebc2956..7ada1f2 100644 > > > --- a/include/linux/firmware/intel/stratix10-svc-client.h > > > +++ b/include/linux/firmware/intel/stratix10-svc-client.h > > > @@ -51,12 +51,17 @@ > > > #define SVC_STATUS_NO_SUPPORT 6 > > > /* > > > - * Flag bit for COMMAND_RECONFIG > > > + * Flag for COMMAND_RECONFIG, in bit number > > > * > > > * COMMAND_RECONFIG_FLAG_PARTIAL: > > > - * Set to FPGA configuration type (full or partial). > > > + * Set for partial FPGA configuration. > > > + * > > > + * COMMAND_AUTHENTICATE_BITSTREAM: > > > + * Set for bitstream authentication, which makes sure a signed bitstream > > > + * has valid signatures before committing it to QSPI flash memory. > > > */ > > > -#define COMMAND_RECONFIG_FLAG_PARTIAL 1 > > > +#define COMMAND_RECONFIG_FLAG_PARTIAL 0 > > > > So is this a bugfix, changing this value to the correct one? > > Yes, it is a bug fix. Wat? This is a change in interface spec with the firmware. I thought the whole point of the firmware version SVC call was to prevent breaking old firmware? Didn't we discuss this earlier? > > > > If so, shouldn't this be a stand-alone patch and get backported to > > stable kernel releases? > > Sure, I will make change and submit again as a standalone patch. > > > > > If not, then no one uses this flag today? > > > > thanks, > > > > greg k-h > > > Regards, > Richard - Moritz
Hi Moritz, Sorry for the confusion. On 1/27/21 3:41 PM, Moritz Fischer wrote: > On Wed, Jan 27, 2021 at 07:05:41AM -0600, Richard Gong wrote: >> >> Hi Greg, >> >> Thanks for review! >> >> On 1/27/21 6:04 AM, Greg KH wrote: >>> On Mon, Jan 25, 2021 at 02:56:23PM -0600, richard.gong@linux.intel.com wrote: >>>> From: Richard Gong <richard.gong@intel.com> >>>> >>>> Add COMMAND_AUTHENTICATE_BITSTREAM command flag for new added bitstream >>>> authentication feature. Authenticating a bitstream is to make sure a signed >>>> bitstream has the valid signatures. >>>> >>>> Except for the actual configuration of the device, the bitstream >>>> authentication works the same way as FPGA configuration does. If the >>>> authentication passes, the signed bitstream will be programmed into QSPI >>>> flash memory and will be expected to boot without issues. >>>> >>>> Clean up COMMAND_RECONFIG_FLAG_PARTIAL flag by resetting it to 0, which >>>> aligns with the firmware settings. >>>> >>>> Signed-off-by: Richard Gong <richard.gong@intel.com> >>>> --- >>>> v3: no change >>>> v2: new added >>>> --- >>>> include/linux/firmware/intel/stratix10-svc-client.h | 11 ++++++++--- >>>> 1 file changed, 8 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h >>>> index ebc2956..7ada1f2 100644 >>>> --- a/include/linux/firmware/intel/stratix10-svc-client.h >>>> +++ b/include/linux/firmware/intel/stratix10-svc-client.h >>>> @@ -51,12 +51,17 @@ >>>> #define SVC_STATUS_NO_SUPPORT 6 >>>> /* >>>> - * Flag bit for COMMAND_RECONFIG >>>> + * Flag for COMMAND_RECONFIG, in bit number >>>> * >>>> * COMMAND_RECONFIG_FLAG_PARTIAL: >>>> - * Set to FPGA configuration type (full or partial). >>>> + * Set for partial FPGA configuration. >>>> + * >>>> + * COMMAND_AUTHENTICATE_BITSTREAM: >>>> + * Set for bitstream authentication, which makes sure a signed bitstream >>>> + * has valid signatures before committing it to QSPI flash memory. >>>> */ >>>> -#define COMMAND_RECONFIG_FLAG_PARTIAL 1 >>>> +#define COMMAND_RECONFIG_FLAG_PARTIAL 0 >>> >>> So is this a bugfix, changing this value to the correct one? >> >> Yes, it is a bug fix. > Wat? This is a change in interface spec with the firmware. I thought the > whole point of the firmware version SVC call was to prevent breaking old > firmware? > > Didn't we discuss this earlier? > We discussed before and I thought we were all aligned. There are 2 aspects: 1. The purpose I changed COMMAND_RECONFIG_FLAG_PARTIAL to 0 from 1 is to align with the current firmware setting. This change will NOT break old firmware since always treats request with non-zero value as partial reconfiguration. 2. When we add new bitstream authentication function, the old firmware couldn't distinguish partial reconfiguration or bitstream authentication since the value of both requests were not zero. To address this back compatible issue, I extend Intel service layer driver for FPGA manager driver to get the running firmware version via SMC call. Then FPGA manager driver can decide whether to handle the newly added bitstream authentication based on the retrieved firmware version. >>> >>> If so, shouldn't this be a stand-alone patch and get backported to >>> stable kernel releases? >> >> Sure, I will make change and submit again as a standalone patch. >> >>> >>> If not, then no one uses this flag today? >>> >>> thanks, >>> >>> greg k-h >>> >> Regards, >> Richard > > - Moritz > Regards, Richard
diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h index ebc2956..7ada1f2 100644 --- a/include/linux/firmware/intel/stratix10-svc-client.h +++ b/include/linux/firmware/intel/stratix10-svc-client.h @@ -51,12 +51,17 @@ #define SVC_STATUS_NO_SUPPORT 6 /* - * Flag bit for COMMAND_RECONFIG + * Flag for COMMAND_RECONFIG, in bit number * * COMMAND_RECONFIG_FLAG_PARTIAL: - * Set to FPGA configuration type (full or partial). + * Set for partial FPGA configuration. + * + * COMMAND_AUTHENTICATE_BITSTREAM: + * Set for bitstream authentication, which makes sure a signed bitstream + * has valid signatures before committing it to QSPI flash memory. */ -#define COMMAND_RECONFIG_FLAG_PARTIAL 1 +#define COMMAND_RECONFIG_FLAG_PARTIAL 0 +#define COMMAND_AUTHENTICATE_BITSTREAM 1 /* * Timeout settings for service clients: