[PATCHv2 0/5] Extend Intel service layer, FPGA manager and region

[PATCHv2 0/5] Extend Intel service layer, FPGA manager and region

[richard.gong]

From: Richard Gong <richard.gong@intel.com>

This is 2nd submission of Intel service layer and FPGA patches.

The customer wants to verify that a FPGA bitstream can be started properly
before saving the bitstream to the QSPI flash memory.

Bitstream authentication makes sure a signed bitstream has valid signatures.

The customer sends the bitstream via FPGA framework and overlay, the
firmware will authenticate the bitstream but not program the bitstream to
device. If the authentication passes, the bitstream will be programmed into
QSPI flash and will be expected to boot without issues.

Extend Intel service layer, FPGA manager and region drivers to support the
bitstream authentication feature.

Richard Gong (5):
  firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag
  fpga: fpga-mgr: add FPGA_MGR_BITSTREM_AUTHENTICATION flag
  fpga: of-fpga-region: add authenticate-fpga-config property
  dt-bindings: fpga: add authenticate-fpga-config property
  fpga: stratix10-soc: entend driver for bitstream authentication

 Documentation/devicetree/bindings/fpga/fpga-region.txt |  1 +
 drivers/fpga/of-fpga-region.c                          |  3 +++
 drivers/fpga/stratix10-soc.c                           |  3 +++
 include/linux/firmware/intel/stratix10-svc-client.h    | 11 ++++++++---
 include/linux/fpga/fpga-mgr.h                          | 13 ++++++++-----
 5 files changed, 23 insertions(+), 8 deletions(-)

-- 
2.7.4

[PATCHv2 1/5] firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag

[richard.gong]

From: Richard Gong <richard.gong@intel.com>

Add COMMAND_AUTHENTICATE_BITSTREAM command flag for new added bitstream
authentication feature. Authenticating a bistream 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>
---
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 a93d859..85463c8 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 bit 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:
-- 
2.7.4

Re: [PATCHv2 1/5] firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag

[Moritz Fischer]

On Wed, Nov 18, 2020 at 08:29:09AM -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 bistream 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.

Should this be down with the v2: ?
> 
> Signed-off-by: Richard Gong <richard.gong@intel.com>
> ---
> 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 a93d859..85463c8 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 bit 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

Can you explain how this commit by itself doesn't break things?

Before this change firmware expected BIT(0) to be set for partial
reconfiguration, now BIT(0) suddenly means authentication? How doest his
work? :)

Was there a firmware version change? Did this never work before?

If this is version depenedent for firmware, then this might need a
different compatible string / id / some form of probing?

Entirely possible that I'm missing something, but it doesn't *seem*
right.
>  
>  /**
>   * Timeout settings for service clients:
> -- 
> 2.7.4
> 

Cheers,
Moritz

Re: [PATCHv2 1/5] firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag

[Richard Gong]

Hi Moritz,

On 11/18/20 9:30 AM, Moritz Fischer wrote:
> On Wed, Nov 18, 2020 at 08:29:09AM -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 bistream 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.
> 
> Should this be down with the v2: ?

I think the commit message should describe all the changes made in the 
patch, is it?

>>
>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>> ---
>> 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 a93d859..85463c8 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 bit 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
> 
> Can you explain how this commit by itself doesn't break things?
> 
> Before this change firmware expected BIT(0) to be set for partial
> reconfiguration, now BIT(0) suddenly means authentication? How doest his
> work? :)
>  > Was there a firmware version change? Did this never work before?
> 
> If this is version depenedent for firmware, then this might need a
> different compatible string / id / some form of probing?
> 
> Entirely possible that I'm missing something, but it doesn't *seem*
> right.
>>   

It did work before.

Before this change, firmware only checks if the received flag value is 
zero. If the value is zero, it preforms full reconfiguration. Otherwise 
it does partial reconfiguration.

To support bitstream authentication feature, firmware is updated to 
check the received flag value as below:
	0	--- full reconfiguration
	BIT(0) 	--- partial reconfiguration
	BIT(1) 	--- bitstream authentication

Therefore I have updated the command flag setting at Intel service layer 
driver to align with firmware.

Regards,
Richard

>>   /**
>>    * Timeout settings for service clients:
>> -- 
>> 2.7.4
>>
> 
> Cheers,
> Moritz
> 

Re: [PATCHv2 1/5] firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag

[Moritz Fischer]

Richard,

On Wed, Nov 18, 2020 at 12:16:09PM -0600, Richard Gong wrote:

> > > -#define COMMAND_RECONFIG_FLAG_PARTIAL	1
> > > +#define COMMAND_RECONFIG_FLAG_PARTIAL	0
> > > +#define COMMAND_AUTHENTICATE_BITSTREAM	1
> > 
> > Can you explain how this commit by itself doesn't break things?
> > 
> > Before this change firmware expected BIT(0) to be set for partial
> > reconfiguration, now BIT(0) suddenly means authentication? How doest his
> > work? :)
> >  > Was there a firmware version change? Did this never work before?
> > 
> > If this is version depenedent for firmware, then this might need a
> > different compatible string / id / some form of probing?
> > 
> > Entirely possible that I'm missing something, but it doesn't *seem*
> > right.
> 
> It did work before.
> 
> Before this change, firmware only checks if the received flag value is zero.
> If the value is zero, it preforms full reconfiguration. Otherwise it does
> partial reconfiguration.
> 
> To support bitstream authentication feature, firmware is updated to check
> the received flag value as below:
> 	0	--- full reconfiguration
> 	BIT(0) 	--- partial reconfiguration
> 	BIT(1) 	--- bitstream authentication

So there are two different versions of firmware involved that behave
differently? 

Old firmware:
- ctype.flags  = 0x0 -> Full reconfig
- ctype.flags != 0 -> Partial reconfig

New firmware:
- ctype.flags = 0x0 -> Full reconfig
- ctype.flags = 0x1 -> Partial reconfig
- ctype.flags = 0x2 -> Authenticate

Old software:
- Send 0x0 for Full
- Send 0x1 for Partial

New software:
- Send 0x0 for Full
- Send 0x1 for Partial
- Send 0x2 for Auth

If I send request for authentication BIT(1) (new software) to old
firmware it'd try and attempt a partial reconfiguration with the data I
send? Is that safe?

Is there a way for software to figure out the firmware version and do
the right thing?

> Therefore I have updated the command flag setting at Intel service layer
> driver to align with firmware.
> 
> Regards,
> Richard
> 
> > >   /**
> > >    * Timeout settings for service clients:
> > > -- 
> > > 2.7.4
> > > 
> > 
> > Cheers,
> > Moritz
> > 

Thanks,
Moritz

Re: [PATCHv2 1/5] firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag

[Richard Gong]

Hi Moritz,

Sorry for late reply, I was out last week.

On 11/21/20 7:10 PM, Moritz Fischer wrote:
> Richard,
> 
> On Wed, Nov 18, 2020 at 12:16:09PM -0600, Richard Gong wrote:
> 
>>>> -#define COMMAND_RECONFIG_FLAG_PARTIAL	1
>>>> +#define COMMAND_RECONFIG_FLAG_PARTIAL	0
>>>> +#define COMMAND_AUTHENTICATE_BITSTREAM	1
>>>
>>> Can you explain how this commit by itself doesn't break things?
>>>
>>> Before this change firmware expected BIT(0) to be set for partial
>>> reconfiguration, now BIT(0) suddenly means authentication? How doest his
>>> work? :)
>>>   > Was there a firmware version change? Did this never work before?
>>>
>>> If this is version depenedent for firmware, then this might need a
>>> different compatible string / id / some form of probing?
>>>
>>> Entirely possible that I'm missing something, but it doesn't *seem*
>>> right.
>>
>> It did work before.
>>
>> Before this change, firmware only checks if the received flag value is zero.
>> If the value is zero, it preforms full reconfiguration. Otherwise it does
>> partial reconfiguration.
>>
>> To support bitstream authentication feature, firmware is updated to check
>> the received flag value as below:
>> 	0	--- full reconfiguration
>> 	BIT(0) 	--- partial reconfiguration
>> 	BIT(1) 	--- bitstream authentication
> 
> So there are two different versions of firmware involved that behave
> differently?
> 
> Old firmware:
> - ctype.flags  = 0x0 -> Full reconfig
> - ctype.flags != 0 -> Partial reconfig
> 
> New firmware:
> - ctype.flags = 0x0 -> Full reconfig
> - ctype.flags = 0x1 -> Partial reconfig
> - ctype.flags = 0x2 -> Authenticate
> 
> Old software:
> - Send 0x0 for Full
> - Send 0x1 for Partial
> 
> New software:
> - Send 0x0 for Full
> - Send 0x1 for Partial
> - Send 0x2 for Auth
> 
> If I send request for authentication BIT(1) (new software) to old
> firmware it'd try and attempt a partial reconfiguration with the data I
> send? Is that safe?
> 

Yes, it is possible and it is not safe. But we will inform our customers 
they should update to the latest firmware (SDM firmware and ATF) if they 
want to have authentication feature.

We are migrating boot loader boot flow to the new ATF boot flow, which 
is SDM firmware -> SPL -> ATF -> U-boot proper -> Linux. The new 
authentication feature is supported only in the new ATF boot flow. ATF 
communicates with SDM firmware via mailbox, and SDM firmware performs 
the actual full/partial reconfiguration and bitstream authentication. 
ATF sets up EL3 environment and initializes PSCI services.

The old boot flow is SDM firmware -> SPL -> U-boot proper -> Linux, 
which SPL/U-boot handles PSCI services and communicates with SDM 
firmware via mailbox. SDM firmware performs the actual full/partial 
reconfiguration.

ATF = Arm Trust Firmware, SDM = Secure Device Manager

> Is there a way for software to figure out the firmware version and do
> the right thing?

It is not feasible for kernel driver to get the firmware version per 
current designs and implementations. I don't think there is other way 
around this.

> 
>> Therefore I have updated the command flag setting at Intel service layer
>> driver to align with firmware.
>>
>> Regards,
>> Richard
>>
>>>>    /**
>>>>     * Timeout settings for service clients:
>>>> -- 
>>>> 2.7.4
>>>>
>>>
>>> Cheers,
>>> Moritz
>>>
> 
> Thanks,
> Moritz
> 
Regards,
Richard

Re: [PATCHv2 1/5] firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag

[Moritz Fischer]

Hi Richard,

On Mon, Nov 30, 2020 at 12:55:44PM -0600, Richard Gong wrote:
> 
> Hi Moritz,
> 
> Sorry for late reply, I was out last week.

No worries, usually I'm late with replies ;-)
> 
> On 11/21/20 7:10 PM, Moritz Fischer wrote:
> > Richard,
> > 
> > On Wed, Nov 18, 2020 at 12:16:09PM -0600, Richard Gong wrote:
> > 
> > > > > -#define COMMAND_RECONFIG_FLAG_PARTIAL	1
> > > > > +#define COMMAND_RECONFIG_FLAG_PARTIAL	0
> > > > > +#define COMMAND_AUTHENTICATE_BITSTREAM	1
> > > > 
> > > > Can you explain how this commit by itself doesn't break things?
> > > > 
> > > > Before this change firmware expected BIT(0) to be set for partial
> > > > reconfiguration, now BIT(0) suddenly means authentication? How doest his
> > > > work? :)
> > > >   > Was there a firmware version change? Did this never work before?
> > > > 
> > > > If this is version depenedent for firmware, then this might need a
> > > > different compatible string / id / some form of probing?
> > > > 
> > > > Entirely possible that I'm missing something, but it doesn't *seem*
> > > > right.
> > > 
> > > It did work before.
> > > 
> > > Before this change, firmware only checks if the received flag value is zero.
> > > If the value is zero, it preforms full reconfiguration. Otherwise it does
> > > partial reconfiguration.
> > > 
> > > To support bitstream authentication feature, firmware is updated to check
> > > the received flag value as below:
> > > 	0	--- full reconfiguration
> > > 	BIT(0) 	--- partial reconfiguration
> > > 	BIT(1) 	--- bitstream authentication
> > 
> > So there are two different versions of firmware involved that behave
> > differently?
> > 
> > Old firmware:
> > - ctype.flags  = 0x0 -> Full reconfig
> > - ctype.flags != 0 -> Partial reconfig
> > 
> > New firmware:
> > - ctype.flags = 0x0 -> Full reconfig
> > - ctype.flags = 0x1 -> Partial reconfig
> > - ctype.flags = 0x2 -> Authenticate
> > 
> > Old software:
> > - Send 0x0 for Full
> > - Send 0x1 for Partial
> > 
> > New software:
> > - Send 0x0 for Full
> > - Send 0x1 for Partial
> > - Send 0x2 for Auth
> > 
> > If I send request for authentication BIT(1) (new software) to old
> > firmware it'd try and attempt a partial reconfiguration with the data I
> > send? Is that safe?
> > 
> 
> Yes, it is possible and it is not safe. But we will inform our customers
> they should update to the latest firmware (SDM firmware and ATF) if they
> want to have authentication feature.
> 
> We are migrating boot loader boot flow to the new ATF boot flow, which is
> SDM firmware -> SPL -> ATF -> U-boot proper -> Linux. The new authentication
> feature is supported only in the new ATF boot flow. ATF communicates with
> SDM firmware via mailbox, and SDM firmware performs the actual full/partial
> reconfiguration and bitstream authentication. ATF sets up EL3 environment
> and initializes PSCI services.

Can U-Boot determine whether it's the new or old flow? Can you set a
different compatible value in your device-tree, to disambiguate
behaviors?

> The old boot flow is SDM firmware -> SPL -> U-boot proper -> Linux, which
> SPL/U-boot handles PSCI services and communicates with SDM firmware via
> mailbox. SDM firmware performs the actual full/partial reconfiguration.
> 
> ATF = Arm Trust Firmware, SDM = Secure Device Manager
> 
> > Is there a way for software to figure out the firmware version and do
> > the right thing?
> 
> It is not feasible for kernel driver to get the firmware version per current
> designs and implementations. I don't think there is other way around this.
> 
> > 
> > > Therefore I have updated the command flag setting at Intel service layer
> > > driver to align with firmware.
> > > 
> > > Regards,
> > > Richard
> > > 
> > > > >    /**
> > > > >     * Timeout settings for service clients:
> > > > > -- 
> > > > > 2.7.4
> > > > > 
> > > > 
> > > > Cheers,
> > > > Moritz
> > > > 
> > 
> > Thanks,
> > Moritz
> > 
> Regards,
> Richard

Thanks,
Moritz

Re: [PATCHv2 1/5] firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag

[Richard Gong]

Hi Moritz,

On 11/30/20 10:31 PM, Moritz Fischer wrote:
> Hi Richard,
> 
> On Mon, Nov 30, 2020 at 12:55:44PM -0600, Richard Gong wrote:
>>
>> Hi Moritz,
>>
>> Sorry for late reply, I was out last week.
> 
> No worries, usually I'm late with replies ;-)
>>
>> On 11/21/20 7:10 PM, Moritz Fischer wrote:
>>> Richard,
>>>
>>> On Wed, Nov 18, 2020 at 12:16:09PM -0600, Richard Gong wrote:
>>>
>>>>>> -#define COMMAND_RECONFIG_FLAG_PARTIAL	1
>>>>>> +#define COMMAND_RECONFIG_FLAG_PARTIAL	0
>>>>>> +#define COMMAND_AUTHENTICATE_BITSTREAM	1
>>>>>
>>>>> Can you explain how this commit by itself doesn't break things?
>>>>>
>>>>> Before this change firmware expected BIT(0) to be set for partial
>>>>> reconfiguration, now BIT(0) suddenly means authentication? How doest his
>>>>> work? :)
>>>>>    > Was there a firmware version change? Did this never work before?
>>>>>
>>>>> If this is version depenedent for firmware, then this might need a
>>>>> different compatible string / id / some form of probing?
>>>>>
>>>>> Entirely possible that I'm missing something, but it doesn't *seem*
>>>>> right.
>>>>
>>>> It did work before.
>>>>
>>>> Before this change, firmware only checks if the received flag value is zero.
>>>> If the value is zero, it preforms full reconfiguration. Otherwise it does
>>>> partial reconfiguration.
>>>>
>>>> To support bitstream authentication feature, firmware is updated to check
>>>> the received flag value as below:
>>>> 	0	--- full reconfiguration
>>>> 	BIT(0) 	--- partial reconfiguration
>>>> 	BIT(1) 	--- bitstream authentication
>>>
>>> So there are two different versions of firmware involved that behave
>>> differently?
>>>
>>> Old firmware:
>>> - ctype.flags  = 0x0 -> Full reconfig
>>> - ctype.flags != 0 -> Partial reconfig
>>>
>>> New firmware:
>>> - ctype.flags = 0x0 -> Full reconfig
>>> - ctype.flags = 0x1 -> Partial reconfig
>>> - ctype.flags = 0x2 -> Authenticate
>>>
>>> Old software:
>>> - Send 0x0 for Full
>>> - Send 0x1 for Partial
>>>
>>> New software:
>>> - Send 0x0 for Full
>>> - Send 0x1 for Partial
>>> - Send 0x2 for Auth
>>>
>>> If I send request for authentication BIT(1) (new software) to old
>>> firmware it'd try and attempt a partial reconfiguration with the data I
>>> send? Is that safe?
>>>
>>
>> Yes, it is possible and it is not safe. But we will inform our customers
>> they should update to the latest firmware (SDM firmware and ATF) if they
>> want to have authentication feature.
>>
>> We are migrating boot loader boot flow to the new ATF boot flow, which is
>> SDM firmware -> SPL -> ATF -> U-boot proper -> Linux. The new authentication
>> feature is supported only in the new ATF boot flow. ATF communicates with
>> SDM firmware via mailbox, and SDM firmware performs the actual full/partial
>> reconfiguration and bitstream authentication. ATF sets up EL3 environment
>> and initializes PSCI services.
> 
> Can U-Boot determine whether it's the new or old flow? Can you set a
> different compatible value in your device-tree, to disambiguate
> behaviors?
> 

The boot flow is determined by defconfig during compilation, which means 
each boot flow will have its own defconfig.

SDM firmware loads SPL into OCRAM, then SPL will load the apporiate ATF 
or U-boot into the DRAM according to the setting of CONFIG_SPL_ATF. If 
CONFIG_SPL_ATF=y, SPL loads ATF and then jumps to ATF. ATF setups EL3 
environment and initialize the PSCI services.

CONFIG_SPL_ATF is not set for the old boot flow.

>> The old boot flow is SDM firmware -> SPL -> U-boot proper -> Linux, which
>> SPL/U-boot handles PSCI services and communicates with SDM firmware via
>> mailbox. SDM firmware performs the actual full/partial reconfiguration.
>>
>> ATF = Arm Trust Firmware, SDM = Secure Device Manager
>>
>>> Is there a way for software to figure out the firmware version and do
>>> the right thing?
>>
>> It is not feasible for kernel driver to get the firmware version per current
>> designs and implementations. I don't think there is other way around this.
>>
>>>
>>>> Therefore I have updated the command flag setting at Intel service layer
>>>> driver to align with firmware.
>>>>
>>>> Regards,
>>>> Richard
>>>>
>>>>>>     /**
>>>>>>      * Timeout settings for service clients:
>>>>>> -- 
>>>>>> 2.7.4
>>>>>>
>>>>>
>>>>> Cheers,
>>>>> Moritz
>>>>>
>>>
>>> Thanks,
>>> Moritz
>>>
>> Regards,
>> Richard
> 
> Thanks,
> Moritz
> 
Regards,
Richard

Re: [PATCHv2 1/5] firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag

[Moritz Fischer]

Hi Richard,

On Tue, Dec 01, 2020 at 01:30:16PM -0600, Richard Gong wrote:

> > Can U-Boot determine whether it's the new or old flow? Can you set a
> > different compatible value in your device-tree, to disambiguate
> > behaviors?
> > 
> 
> The boot flow is determined by defconfig during compilation, which means
> each boot flow will have its own defconfig.
> 
> SDM firmware loads SPL into OCRAM, then SPL will load the apporiate ATF or
> U-boot into the DRAM according to the setting of CONFIG_SPL_ATF. If
> CONFIG_SPL_ATF=y, SPL loads ATF and then jumps to ATF. ATF setups EL3
> environment and initialize the PSCI services.
> 
> CONFIG_SPL_ATF is not set for the old boot flow.

So you know at (U-Boot) build time? Can you just pass a different DT to
the kernel in that case?

- Moritz

Re: [PATCHv2 1/5] firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag

[Richard Gong]

Hi Moritz,

On 12/1/20 1:19 PM, Moritz Fischer wrote:
> Hi Richard,
> 
> On Tue, Dec 01, 2020 at 01:30:16PM -0600, Richard Gong wrote:
> 
>>> Can U-Boot determine whether it's the new or old flow? Can you set a
>>> different compatible value in your device-tree, to disambiguate
>>> behaviors?
>>>
>>
>> The boot flow is determined by defconfig during compilation, which means
>> each boot flow will have its own defconfig.
>>
>> SDM firmware loads SPL into OCRAM, then SPL will load the apporiate ATF or
>> U-boot into the DRAM according to the setting of CONFIG_SPL_ATF. If
>> CONFIG_SPL_ATF=y, SPL loads ATF and then jumps to ATF. ATF setups EL3
>> environment and initialize the PSCI services.
>>
>> CONFIG_SPL_ATF is not set for the old boot flow.
> 
> So you know at (U-Boot) build time? Can you just pass a different DT to
> the kernel in that case?
> 

Yes, we have decided the boot flow at build time. Starting from the next 
release, our U-boot will use the ATF boot flow.

Per my limited knowledge in U-boot, I don't think we can follow your 
suggestion. Or it will take a lot of efforts to achieve.

I think that back compatibility is your main concern, correct? the issue 
does exist with the old boot flow and old firmware, whenever the 
customers try to use authentication. Unfortunately we can't update 
U-boot or firmware that has been released.

The authentication feature is supported only at the ATF boot flow, 
updated kernel and firmware. We will have a well-documented document to 
inform our customers that if they want to have authentication feature, 
they need to upgrade the latest U-boot, kernel and firmware.

We always encourage our customers to take the latest U-boot, kernel and 
firmware releases in their developments.

> - Moritz
> 

Regards,
Richard

[PATCHv2 2/5] fpga: fpga-mgr: add FPGA_MGR_BITSTREM_AUTHENTICATION flag

[richard.gong]

From: Richard Gong <richard.gong@intel.com>

Add FPGA_MGR_BITSTREM_AUTHENTICATION flag for FPGA bitstream
authentication, which makes sure a signed bitstream has valid signatures.

Except for the actual configuration of the device, the authentication works
the same way as FPGA configuration does. If the authentication passes, the
bitstream will be programmed into QSPI flash and will be expected to boot
without issues.

Signed-off-by: Richard Gong <richard.gong@intel.com>
---
v2: align all FPGA_MGR_* flags
    update the commit messages
---
 include/linux/fpga/fpga-mgr.h | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 2bc3030..4fb3400 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -67,12 +67,15 @@ enum fpga_mgr_states {
  * %FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
  *
  * %FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
+ *
+ * %FPGA_MGR_BITSTREM_AUTHENTICATION: do FPGA bitstream authentication only
  */
-#define FPGA_MGR_PARTIAL_RECONFIG	BIT(0)
-#define FPGA_MGR_EXTERNAL_CONFIG	BIT(1)
-#define FPGA_MGR_ENCRYPTED_BITSTREAM	BIT(2)
-#define FPGA_MGR_BITSTREAM_LSB_FIRST	BIT(3)
-#define FPGA_MGR_COMPRESSED_BITSTREAM	BIT(4)
+#define FPGA_MGR_PARTIAL_RECONFIG		BIT(0)
+#define FPGA_MGR_EXTERNAL_CONFIG		BIT(1)
+#define FPGA_MGR_ENCRYPTED_BITSTREAM		BIT(2)
+#define FPGA_MGR_BITSTREAM_LSB_FIRST		BIT(3)
+#define FPGA_MGR_COMPRESSED_BITSTREAM		BIT(4)
+#define FPGA_MGR_BITSTREM_AUTHENTICATION	BIT(5)
 
 /**
  * struct fpga_image_info - information specific to a FPGA image
-- 
2.7.4

[PATCHv2 3/5] fpga: of-fpga-region: add authenticate-fpga-config property

[richard.gong]

From: Richard Gong <richard.gong@intel.com>

Add authenticate-fpga-config property to support FPGA bitstream
authentication, which makes sure a signed bitstream has valid signatures.

Signed-off-by: Richard Gong <richard.gong@intel.com>
---
v2: changed in alphabetical order
---
 drivers/fpga/of-fpga-region.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index e405309..3840883 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -219,6 +219,9 @@ static struct fpga_image_info *of_fpga_region_parse_ov(
 	info->overlay = overlay;
 
 	/* Read FPGA region properties from the overlay */
+	if (of_property_read_bool(overlay, "authenticate-fpga-config"))
+		info->flags |= FPGA_MGR_BITSTREM_AUTHENTICATION;
+
 	if (of_property_read_bool(overlay, "partial-fpga-config"))
 		info->flags |= FPGA_MGR_PARTIAL_RECONFIG;
 
-- 
2.7.4

[PATCHv2 4/5] dt-bindings: fpga: add authenticate-fpga-config property

[richard.gong]

From: Richard Gong <richard.gong@intel.com>

Add authenticate-fpga-config property for FPGA bitstream authentication,
which makes sure a signed bitstream has valid signatures.

Signed-off-by: Richard Gong <richard.gong@intel.com>
---
v2: put authenticate-fpga-config above partial-fpga-config
    update commit messages
---
 Documentation/devicetree/bindings/fpga/fpga-region.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt b/Documentation/devicetree/bindings/fpga/fpga-region.txt
index e811cf8..d0d3234 100644
--- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
+++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
@@ -182,6 +182,7 @@ Optional properties:
 	This property is optional if the FPGA Manager handles the bridges.
         If the fpga-region is  the child of a fpga-bridge, the list should not
         contain the parent bridge.
+- authenticate-fpga-config : boolean, set if do bitstream authentication only.
 - partial-fpga-config : boolean, set if partial reconfiguration is to be done,
 	otherwise full reconfiguration is done.
 - external-fpga-config : boolean, set if the FPGA has already been configured
-- 
2.7.4

[PATCHv2 5/5] fpga: stratix10-soc: extend driver for bitstream authentication

[richard.gong]

From: Richard Gong <richard.gong@intel.com>

Extend FPGA manager driver to support FPGA bitstream authentication on
Intel SocFPGA platforms.

Signed-off-by: Richard Gong <richard.gong@intel.com>
---
v2: use flag defined in stratix10-svc driver
---
 drivers/fpga/stratix10-soc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index 657a70c..b77067e 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -186,6 +186,9 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
 	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
 		dev_dbg(dev, "Requesting partial reconfiguration.\n");
 		ctype.flags |= BIT(COMMAND_RECONFIG_FLAG_PARTIAL);
+	} else if (info->flags & FPGA_MGR_BITSTREM_AUTHENTICATION) {
+		dev_dbg(dev, "Requesting bitstream authentication.\n");
+		ctype.flags |= BIT(COMMAND_AUTHENTICATE_BITSTREAM);
 	} else {
 		dev_dbg(dev, "Requesting full reconfiguration.\n");
 	}
-- 
2.7.4

Re: [PATCHv2 0/5] Extend Intel service layer, FPGA manager and region

[Richard Gong]

Hi Moritz, Greg,

Sorry for asking.

Any comment on Intel service layer and FPGA patches submitted on 11/18/20?

Regards,
Richard

On 11/18/20 8:29 AM, richard.gong@linux.intel.com wrote:
> From: Richard Gong <richard.gong@intel.com>
> 
> This is 2nd submission of Intel service layer and FPGA patches.
> 
> The customer wants to verify that a FPGA bitstream can be started properly
> before saving the bitstream to the QSPI flash memory.
> 
> Bitstream authentication makes sure a signed bitstream has valid signatures.
> 
> The customer sends the bitstream via FPGA framework and overlay, the
> firmware will authenticate the bitstream but not program the bitstream to
> device. If the authentication passes, the bitstream will be programmed into
> QSPI flash and will be expected to boot without issues.
> 
> Extend Intel service layer, FPGA manager and region drivers to support the
> bitstream authentication feature.
> 
> Richard Gong (5):
>    firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag
>    fpga: fpga-mgr: add FPGA_MGR_BITSTREM_AUTHENTICATION flag
>    fpga: of-fpga-region: add authenticate-fpga-config property
>    dt-bindings: fpga: add authenticate-fpga-config property
>    fpga: stratix10-soc: entend driver for bitstream authentication
> 
>   Documentation/devicetree/bindings/fpga/fpga-region.txt |  1 +
>   drivers/fpga/of-fpga-region.c                          |  3 +++
>   drivers/fpga/stratix10-soc.c                           |  3 +++
>   include/linux/firmware/intel/stratix10-svc-client.h    | 11 ++++++++---
>   include/linux/fpga/fpga-mgr.h                          | 13 ++++++++-----
>   5 files changed, 23 insertions(+), 8 deletions(-)
> 

Re: [PATCHv2 0/5] Extend Intel service layer, FPGA manager and region

[Greg KH]

On Mon, Dec 14, 2020 at 08:03:07AM -0600, Richard Gong wrote:
> 
> Hi Moritz, Greg,
> 
> Sorry for asking.
> 
> Any comment on Intel service layer and FPGA patches submitted on 11/18/20?

I don't see them in my review queue, sorry.

greg k-h

Re: [PATCHv2 0/5] Extend Intel service layer, FPGA manager and region

[Richard Gong]

Hi Greg,

On 12/14/20 8:05 AM, Greg KH wrote:
> On Mon, Dec 14, 2020 at 08:03:07AM -0600, Richard Gong wrote:
>>
>> Hi Moritz, Greg,
>>
>> Sorry for asking.
>>
>> Any comment on Intel service layer and FPGA patches submitted on 11/18/20?
> 
> I don't see them in my review queue, sorry.
> 

I just re-submitted the patches.

> greg k-h >
Regards,
Richard