diff mbox series

[PATCHv1,3/4] dt-bindings: fpga: add authenticate-fpga-config property

Message ID 1605204403-6663-4-git-send-email-richard.gong@linux.intel.com (mailing list archive)
State New
Headers show
Series Extend FPGA manager and region drivers for | expand

Commit Message

Richard Gong Nov. 12, 2020, 6:06 p.m. UTC
From: Richard Gong <richard.gong@intel.com>

Add authenticate-fpga-config property for FPGA bitstream authentication.

Signed-off-by: Richard Gong <richard.gong@intel.com>
---
 Documentation/devicetree/bindings/fpga/fpga-region.txt | 1 +
 1 file changed, 1 insertion(+)

Comments

Tom Rix Nov. 13, 2020, 8:28 p.m. UTC | #1
On 11/12/20 10:06 AM, richard.gong@linux.intel.com wrote:
> From: Richard Gong <richard.gong@intel.com>
>
> Add authenticate-fpga-config property for FPGA bitstream authentication.
>
> Signed-off-by: Richard Gong <richard.gong@intel.com>
> ---
>  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..7a512bc 100644
> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> @@ -187,6 +187,7 @@ Optional properties:
>  - external-fpga-config : boolean, set if the FPGA has already been configured
>  	prior to OS boot up.
>  - encrypted-fpga-config : boolean, set if the bitstream is encrypted
> +- authenticate-fpga-config : boolean, set if do bitstream authentication

The list is mostly in alphabetical order so the new 'authenticate-... ' should go at the top.

Improve what you mean by 'authentication' similar to my comment in the first patch.

Tom

>  - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
>  	bridges to successfully become enabled after the region has been
>  	programmed.
Richard Gong Nov. 14, 2020, 2:52 p.m. UTC | #2
Hi Tom,

On 11/13/20 2:28 PM, Tom Rix wrote:
> 
> On 11/12/20 10:06 AM, richard.gong@linux.intel.com wrote:
>> From: Richard Gong <richard.gong@intel.com>
>>
>> Add authenticate-fpga-config property for FPGA bitstream authentication.
>>
>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>> ---
>>   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..7a512bc 100644
>> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
>> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
>> @@ -187,6 +187,7 @@ Optional properties:
>>   - external-fpga-config : boolean, set if the FPGA has already been configured
>>   	prior to OS boot up.
>>   - encrypted-fpga-config : boolean, set if the bitstream is encrypted
>> +- authenticate-fpga-config : boolean, set if do bitstream authentication
> 
> The list is mostly in alphabetical order so the new 'authenticate-... ' should go at the top.
> 

The original list is not in alphabetical order. The order of 
partial-fpga-config, external-fpga-config and encrypted-fpga-config here 
follows the implementation in the of-fpga-region.c file.

So authenticate-fpga-config should follow the way, correct?

> Improve what you mean by 'authentication' similar to my comment in the first patch.
> 

Will do in the version 2 submission.

Regards,
Richard

> Tom
> 
>>   - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
>>   	bridges to successfully become enabled after the region has been
>>   	programmed.
>
Tom Rix Nov. 14, 2020, 3:59 p.m. UTC | #3
On 11/14/20 6:52 AM, Richard Gong wrote:
> Hi Tom,
>
>>>       prior to OS boot up.
>>>   - encrypted-fpga-config : boolean, set if the bitstream is encrypted
>>> +- authenticate-fpga-config : boolean, set if do bitstream authentication
>>
>> The list is mostly in alphabetical order so the new 'authenticate-... ' should go at the top.
>>
>
> The original list is not in alphabetical order. The order of partial-fpga-config, external-fpga-config and encrypted-fpga-config here follows the implementation in the of-fpga-region.c file.
>
> So authenticate-fpga-config should follow the way, correct?
>
This is why i say 'mostly' ..

In general when listing options for a user to read, you should make it easy for them to find

the option they are looking for.  Ordering them alphabetically is an obvious but not the only

way.  I am not asking for you to fix the whole table, just what you are adding. If there is

a better way to organize them please propose the method.

Tom

>> Improve what you mean by 'authentication' similar to my comment in the first patch.
>>
>
> Will do in the version 2 submission.
>
> Regards,
> Richard
>
>> Tom
>>
>>>   - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
>>>       bridges to successfully become enabled after the region has been
>>>       programmed.
>>
>
Moritz Fischer Nov. 15, 2020, 7:21 p.m. UTC | #4
Hi Richard,

On Thu, Nov 12, 2020 at 12:06:42PM -0600, richard.gong@linux.intel.com wrote:
> From: Richard Gong <richard.gong@intel.com>
> 
> Add authenticate-fpga-config property for FPGA bitstream authentication.
> 
> Signed-off-by: Richard Gong <richard.gong@intel.com>
> ---
>  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..7a512bc 100644
> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> @@ -187,6 +187,7 @@ Optional properties:
>  - external-fpga-config : boolean, set if the FPGA has already been configured
>  	prior to OS boot up.
>  - encrypted-fpga-config : boolean, set if the bitstream is encrypted
> +- authenticate-fpga-config : boolean, set if do bitstream authentication
It is unclear to me from the description whether this entails
authentication + reconfiguration or just authentication.

If the latter is the case this should probably be described as such.

>  - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
>  	bridges to successfully become enabled after the region has been
>  	programmed.
> -- 
> 2.7.4
> 

Thanks
Xu Yilun Nov. 16, 2020, 2:47 a.m. UTC | #5
On Sun, Nov 15, 2020 at 11:21:06AM -0800, Moritz Fischer wrote:
> Hi Richard,
> 
> On Thu, Nov 12, 2020 at 12:06:42PM -0600, richard.gong@linux.intel.com wrote:
> > From: Richard Gong <richard.gong@intel.com>
> > 
> > Add authenticate-fpga-config property for FPGA bitstream authentication.
> > 
> > Signed-off-by: Richard Gong <richard.gong@intel.com>
> > ---
> >  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..7a512bc 100644
> > --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > @@ -187,6 +187,7 @@ Optional properties:
> >  - external-fpga-config : boolean, set if the FPGA has already been configured
> >  	prior to OS boot up.
> >  - encrypted-fpga-config : boolean, set if the bitstream is encrypted
> > +- authenticate-fpga-config : boolean, set if do bitstream authentication
> It is unclear to me from the description whether this entails
> authentication + reconfiguration or just authentication.
> 
> If the latter is the case this should probably be described as such.

If it is just authentication, do we still need to disable bridges in
fpga_region_program_fpga?

I'm wondering if the FPGA functionalities could still be working when
the authenticating is ongoing, or when the authenticating is failed.

Thanks,
Yilun

> 
> >  - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
> >  	bridges to successfully become enabled after the region has been
> >  	programmed.
> > -- 
> > 2.7.4
> > 
> 
> Thanks
Richard Gong Nov. 16, 2020, 1:50 p.m. UTC | #6
Hi Tom,


On 11/14/20 9:59 AM, Tom Rix wrote:
> 
> On 11/14/20 6:52 AM, Richard Gong wrote:
>> Hi Tom,
>>
>>>>        prior to OS boot up.
>>>>    - encrypted-fpga-config : boolean, set if the bitstream is encrypted
>>>> +- authenticate-fpga-config : boolean, set if do bitstream authentication
>>>
>>> The list is mostly in alphabetical order so the new 'authenticate-... ' should go at the top.
>>>
>>
>> The original list is not in alphabetical order. The order of partial-fpga-config, external-fpga-config and encrypted-fpga-config here follows the implementation in the of-fpga-region.c file.
>>
>> So authenticate-fpga-config should follow the way, correct?
>>
> This is why i say 'mostly' ..
> 
> In general when listing options for a user to read, you should make it easy for them to find
> 
> the option they are looking for.  Ordering them alphabetically is an obvious but not the only
> 
> way.  I am not asking for you to fix the whole table, just what you are adding. If there is
> 
> a better way to organize them please propose the method.
> 

How about put authenticate-fpga-config above partial-fpga-config? I 
would like to group all xxx-fpga-config flags together.


Regards,
Richard

> Tom
> 
>>> Improve what you mean by 'authentication' similar to my comment in the first patch.
>>>
>>
>> Will do in the version 2 submission.
>>
>> Regards,
>> Richard
>>
>>> Tom
>>>
>>>>    - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
>>>>        bridges to successfully become enabled after the region has been
>>>>        programmed.
>>>
>>
>
Richard Gong Nov. 16, 2020, 1:59 p.m. UTC | #7
Hi Moritz,


On 11/15/20 1:21 PM, Moritz Fischer wrote:
> Hi Richard,
> 
> On Thu, Nov 12, 2020 at 12:06:42PM -0600, richard.gong@linux.intel.com wrote:
>> From: Richard Gong <richard.gong@intel.com>
>>
>> Add authenticate-fpga-config property for FPGA bitstream authentication.
>>
>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>> ---
>>   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..7a512bc 100644
>> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
>> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
>> @@ -187,6 +187,7 @@ Optional properties:
>>   - external-fpga-config : boolean, set if the FPGA has already been configured
>>   	prior to OS boot up.
>>   - encrypted-fpga-config : boolean, set if the bitstream is encrypted
>> +- authenticate-fpga-config : boolean, set if do bitstream authentication
> It is unclear to me from the description whether this entails
> authentication + reconfiguration or just authentication.
> 
> If the latter is the case this should probably be described as such.

It is for authentication only, just make the signed bitstream has the 
valid signatures.

Regards,
Richard

> 
>>   - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
>>   	bridges to successfully become enabled after the region has been
>>   	programmed.
>> -- 
>> 2.7.4
>>
> 
> Thanks
>
Richard Gong Nov. 16, 2020, 2:14 p.m. UTC | #8
Hi Yilun,

On 11/15/20 8:47 PM, Xu Yilun wrote:
> On Sun, Nov 15, 2020 at 11:21:06AM -0800, Moritz Fischer wrote:
>> Hi Richard,
>>
>> On Thu, Nov 12, 2020 at 12:06:42PM -0600, richard.gong@linux.intel.com wrote:
>>> From: Richard Gong <richard.gong@intel.com>
>>>
>>> Add authenticate-fpga-config property for FPGA bitstream authentication.
>>>
>>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>>> ---
>>>   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..7a512bc 100644
>>> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
>>> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
>>> @@ -187,6 +187,7 @@ Optional properties:
>>>   - external-fpga-config : boolean, set if the FPGA has already been configured
>>>   	prior to OS boot up.
>>>   - encrypted-fpga-config : boolean, set if the bitstream is encrypted
>>> +- authenticate-fpga-config : boolean, set if do bitstream authentication
>> It is unclear to me from the description whether this entails
>> authentication + reconfiguration or just authentication.
>>
>> If the latter is the case this should probably be described as such.
> 
> If it is just authentication, do we still need to disable bridges in
> fpga_region_program_fpga?
> 

Yes.

Except for the actual configuration of the device, the authentication 
feature is the same as FPGA configuration.

Regards,
Richard

> I'm wondering if the FPGA functionalities could still be working when
> the authenticating is ongoing, or when the authenticating is failed.
> 



> Thanks,
> Yilun
> 
>>
>>>   - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
>>>   	bridges to successfully become enabled after the region has been
>>>   	programmed.
>>> -- 
>>> 2.7.4
>>>
>>
>> Thanks
Tom Rix Nov. 16, 2020, 3:11 p.m. UTC | #9
On 11/16/20 5:50 AM, Richard Gong wrote:
> Hi Tom,
>
>
> On 11/14/20 9:59 AM, Tom Rix wrote:
>>
>> On 11/14/20 6:52 AM, Richard Gong wrote:
>>> Hi Tom,
>>>
>>>>>        prior to OS boot up.
>>>>>    - encrypted-fpga-config : boolean, set if the bitstream is encrypted
>>>>> +- authenticate-fpga-config : boolean, set if do bitstream authentication
>>>>
>>>> The list is mostly in alphabetical order so the new 'authenticate-... ' should go at the top.
>>>>
>>>
>>> The original list is not in alphabetical order. The order of partial-fpga-config, external-fpga-config and encrypted-fpga-config here follows the implementation in the of-fpga-region.c file.
>>>
>>> So authenticate-fpga-config should follow the way, correct?
>>>
>> This is why i say 'mostly' ..
>>
>> In general when listing options for a user to read, you should make it easy for them to find
>>
>> the option they are looking for.  Ordering them alphabetically is an obvious but not the only
>>
>> way.  I am not asking for you to fix the whole table, just what you are adding. If there is
>>
>> a better way to organize them please propose the method.
>>
>
> How about put authenticate-fpga-config above partial-fpga-config? I would like to group all xxx-fpga-config flags together.

Ok that sounds fine.

Tom

>
>
> Regards,
> Richard
>
>> Tom
>>
>>>> Improve what you mean by 'authentication' similar to my comment in the first patch.
>>>>
>>>
>>> Will do in the version 2 submission.
>>>
>>> Regards,
>>> Richard
>>>
>>>> Tom
>>>>
>>>>>    - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
>>>>>        bridges to successfully become enabled after the region has been
>>>>>        programmed.
>>>>
>>>
>>
>
Xu Yilun Nov. 17, 2020, 2:24 a.m. UTC | #10
On Mon, Nov 16, 2020 at 08:14:52AM -0600, Richard Gong wrote:
> 
> Hi Yilun,
> 
> On 11/15/20 8:47 PM, Xu Yilun wrote:
> >On Sun, Nov 15, 2020 at 11:21:06AM -0800, Moritz Fischer wrote:
> >>Hi Richard,
> >>
> >>On Thu, Nov 12, 2020 at 12:06:42PM -0600, richard.gong@linux.intel.com wrote:
> >>>From: Richard Gong <richard.gong@intel.com>
> >>>
> >>>Add authenticate-fpga-config property for FPGA bitstream authentication.
> >>>
> >>>Signed-off-by: Richard Gong <richard.gong@intel.com>
> >>>---
> >>>  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..7a512bc 100644
> >>>--- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> >>>+++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> >>>@@ -187,6 +187,7 @@ Optional properties:
> >>>  - external-fpga-config : boolean, set if the FPGA has already been configured
> >>>  	prior to OS boot up.
> >>>  - encrypted-fpga-config : boolean, set if the bitstream is encrypted
> >>>+- authenticate-fpga-config : boolean, set if do bitstream authentication
> >>It is unclear to me from the description whether this entails
> >>authentication + reconfiguration or just authentication.
> >>
> >>If the latter is the case this should probably be described as such.
> >
> >If it is just authentication, do we still need to disable bridges in
> >fpga_region_program_fpga?
> >
> 
> Yes.
> 
> Except for the actual configuration of the device, the authentication
> feature is the same as FPGA configuration.

FPGA Bridges gate bus signals between a host and FPGA. So the FPGA
region could not be accessed by host when doing configuration. But for
this authentication, we are just writing the flash, we don't actually
touch the FPGA soft logic. The host should still be able to operate on
the old logic before reboot, is it?

Thanks,
Yilun

> 
> Regards,
> Richard
> 
> >I'm wondering if the FPGA functionalities could still be working when
> >the authenticating is ongoing, or when the authenticating is failed.
> >
> 
> 
> 
> >Thanks,
> >Yilun
> >
> >>
> >>>  - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
> >>>  	bridges to successfully become enabled after the region has been
> >>>  	programmed.
> >>>-- 
> >>>2.7.4
> >>>
> >>
> >>Thanks
Richard Gong Nov. 17, 2020, 3:39 p.m. UTC | #11
On 11/16/20 8:24 PM, Xu Yilun wrote:
> On Mon, Nov 16, 2020 at 08:14:52AM -0600, Richard Gong wrote:
>>
>> Hi Yilun,
>>
>> On 11/15/20 8:47 PM, Xu Yilun wrote:
>>> On Sun, Nov 15, 2020 at 11:21:06AM -0800, Moritz Fischer wrote:
>>>> Hi Richard,
>>>>
>>>> On Thu, Nov 12, 2020 at 12:06:42PM -0600, richard.gong@linux.intel.com wrote:
>>>>> From: Richard Gong <richard.gong@intel.com>
>>>>>
>>>>> Add authenticate-fpga-config property for FPGA bitstream authentication.
>>>>>
>>>>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>>>>> ---
>>>>>   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..7a512bc 100644
>>>>> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
>>>>> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
>>>>> @@ -187,6 +187,7 @@ Optional properties:
>>>>>   - external-fpga-config : boolean, set if the FPGA has already been configured
>>>>>   	prior to OS boot up.
>>>>>   - encrypted-fpga-config : boolean, set if the bitstream is encrypted
>>>>> +- authenticate-fpga-config : boolean, set if do bitstream authentication
>>>> It is unclear to me from the description whether this entails
>>>> authentication + reconfiguration or just authentication.
>>>>
>>>> If the latter is the case this should probably be described as such.
>>>
>>> If it is just authentication, do we still need to disable bridges in
>>> fpga_region_program_fpga?
>>>
>>
>> Yes.
>>
>> Except for the actual configuration of the device, the authentication
>> feature is the same as FPGA configuration.
> 
> FPGA Bridges gate bus signals between a host and FPGA. So the FPGA
> region could not be accessed by host when doing configuration. But for
> this authentication, we are just writing the flash, we don't actually
> touch the FPGA soft logic. The host should still be able to operate on
> the old logic before reboot, is it?
>
Yes, it's feasible in theory but doesn't make much sense in practice. I 
prefer to keep fpga_region_program_fpga() unchanged.

Regards,
Richard

> Thanks,
> Yilun
> 
>>
>> Regards,
>> Richard
>>
>>> I'm wondering if the FPGA functionalities could still be working when
>>> the authenticating is ongoing, or when the authenticating is failed.
>>>
>>
>>
>>
>>> Thanks,
>>> Yilun
>>>
>>>>
>>>>>   - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
>>>>>   	bridges to successfully become enabled after the region has been
>>>>>   	programmed.
>>>>> -- 
>>>>> 2.7.4
>>>>>
>>>>
>>>> Thanks
Xu Yilun Nov. 18, 2020, 5:47 a.m. UTC | #12
On Tue, Nov 17, 2020 at 09:39:55AM -0600, Richard Gong wrote:
> 
> 
> On 11/16/20 8:24 PM, Xu Yilun wrote:
> >On Mon, Nov 16, 2020 at 08:14:52AM -0600, Richard Gong wrote:
> >>
> >>Hi Yilun,
> >>
> >>On 11/15/20 8:47 PM, Xu Yilun wrote:
> >>>On Sun, Nov 15, 2020 at 11:21:06AM -0800, Moritz Fischer wrote:
> >>>>Hi Richard,
> >>>>
> >>>>On Thu, Nov 12, 2020 at 12:06:42PM -0600, richard.gong@linux.intel.com wrote:
> >>>>>From: Richard Gong <richard.gong@intel.com>
> >>>>>
> >>>>>Add authenticate-fpga-config property for FPGA bitstream authentication.
> >>>>>
> >>>>>Signed-off-by: Richard Gong <richard.gong@intel.com>
> >>>>>---
> >>>>>  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..7a512bc 100644
> >>>>>--- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> >>>>>+++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> >>>>>@@ -187,6 +187,7 @@ Optional properties:
> >>>>>  - external-fpga-config : boolean, set if the FPGA has already been configured
> >>>>>  	prior to OS boot up.
> >>>>>  - encrypted-fpga-config : boolean, set if the bitstream is encrypted
> >>>>>+- authenticate-fpga-config : boolean, set if do bitstream authentication
> >>>>It is unclear to me from the description whether this entails
> >>>>authentication + reconfiguration or just authentication.
> >>>>
> >>>>If the latter is the case this should probably be described as such.
> >>>
> >>>If it is just authentication, do we still need to disable bridges in
> >>>fpga_region_program_fpga?
> >>>
> >>
> >>Yes.
> >>
> >>Except for the actual configuration of the device, the authentication
> >>feature is the same as FPGA configuration.
> >
> >FPGA Bridges gate bus signals between a host and FPGA. So the FPGA
> >region could not be accessed by host when doing configuration. But for
> >this authentication, we are just writing the flash, we don't actually
> >touch the FPGA soft logic. The host should still be able to operate on
> >the old logic before reboot, is it?
> >
> Yes, it's feasible in theory but doesn't make much sense in practice. I
> prefer to keep fpga_region_program_fpga() unchanged.

I'm thinking of the case of inband reprograming, that the QSPI flash
controller itself is embedded in FPGA soft logic, then maybe host still
need to access FPGA on authentication.

Thanks,
Yilun

> >>
> >>>I'm wondering if the FPGA functionalities could still be working when
> >>>the authenticating is ongoing, or when the authenticating is failed.
> >>>
> >>
> >>
> >>
> >>>Thanks,
> >>>Yilun
> >>>
> >>>>
> >>>>>  - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
> >>>>>  	bridges to successfully become enabled after the region has been
> >>>>>  	programmed.
> >>>>>-- 
> >>>>>2.7.4
> >>>>>
> >>>>
> >>>>Thanks
Richard Gong Nov. 18, 2020, 1:38 p.m. UTC | #13
On 11/17/20 11:47 PM, Xu Yilun wrote:
> On Tue, Nov 17, 2020 at 09:39:55AM -0600, Richard Gong wrote:
>>
>>
>> On 11/16/20 8:24 PM, Xu Yilun wrote:
>>> On Mon, Nov 16, 2020 at 08:14:52AM -0600, Richard Gong wrote:
>>>>
>>>> Hi Yilun,
>>>>
>>>> On 11/15/20 8:47 PM, Xu Yilun wrote:
>>>>> On Sun, Nov 15, 2020 at 11:21:06AM -0800, Moritz Fischer wrote:
>>>>>> Hi Richard,
>>>>>>
>>>>>> On Thu, Nov 12, 2020 at 12:06:42PM -0600, richard.gong@linux.intel.com wrote:
>>>>>>> From: Richard Gong <richard.gong@intel.com>
>>>>>>>
>>>>>>> Add authenticate-fpga-config property for FPGA bitstream authentication.
>>>>>>>
>>>>>>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>>>>>>> ---
>>>>>>>   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..7a512bc 100644
>>>>>>> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
>>>>>>> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
>>>>>>> @@ -187,6 +187,7 @@ Optional properties:
>>>>>>>   - external-fpga-config : boolean, set if the FPGA has already been configured
>>>>>>>   	prior to OS boot up.
>>>>>>>   - encrypted-fpga-config : boolean, set if the bitstream is encrypted
>>>>>>> +- authenticate-fpga-config : boolean, set if do bitstream authentication
>>>>>> It is unclear to me from the description whether this entails
>>>>>> authentication + reconfiguration or just authentication.
>>>>>>
>>>>>> If the latter is the case this should probably be described as such.
>>>>>
>>>>> If it is just authentication, do we still need to disable bridges in
>>>>> fpga_region_program_fpga?
>>>>>
>>>>
>>>> Yes.
>>>>
>>>> Except for the actual configuration of the device, the authentication
>>>> feature is the same as FPGA configuration.
>>>
>>> FPGA Bridges gate bus signals between a host and FPGA. So the FPGA
>>> region could not be accessed by host when doing configuration. But for
>>> this authentication, we are just writing the flash, we don't actually
>>> touch the FPGA soft logic. The host should still be able to operate on
>>> the old logic before reboot, is it?
>>>
>> Yes, it's feasible in theory but doesn't make much sense in practice. I
>> prefer to keep fpga_region_program_fpga() unchanged.
> 
> I'm thinking of the case of inband reprograming, that the QSPI flash
> controller itself is embedded in FPGA soft logic, then maybe host still
> need to access FPGA on authentication.

We can decide whether we should update fpga_region_program_fpga() 
function when you update for inband reprogramming case.

Regards,
Richard
> 
> Thanks,
> Yilun
> 
>>>>
>>>>> I'm wondering if the FPGA functionalities could still be working when
>>>>> the authenticating is ongoing, or when the authenticating is failed.
>>>>>
>>>>
>>>>
>>>>
>>>>> Thanks,
>>>>> Yilun
>>>>>
>>>>>>
>>>>>>>   - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
>>>>>>>   	bridges to successfully become enabled after the region has been
>>>>>>>   	programmed.
>>>>>>> -- 
>>>>>>> 2.7.4
>>>>>>>
>>>>>>
>>>>>> Thanks
Xu Yilun Nov. 19, 2020, 11:14 a.m. UTC | #14
On Wed, Nov 18, 2020 at 07:38:31AM -0600, Richard Gong wrote:
> 
> 
> On 11/17/20 11:47 PM, Xu Yilun wrote:
> >On Tue, Nov 17, 2020 at 09:39:55AM -0600, Richard Gong wrote:
> >>
> >>
> >>On 11/16/20 8:24 PM, Xu Yilun wrote:
> >>>On Mon, Nov 16, 2020 at 08:14:52AM -0600, Richard Gong wrote:
> >>>>
> >>>>Hi Yilun,
> >>>>
> >>>>On 11/15/20 8:47 PM, Xu Yilun wrote:
> >>>>>On Sun, Nov 15, 2020 at 11:21:06AM -0800, Moritz Fischer wrote:
> >>>>>>Hi Richard,
> >>>>>>
> >>>>>>On Thu, Nov 12, 2020 at 12:06:42PM -0600, richard.gong@linux.intel.com wrote:
> >>>>>>>From: Richard Gong <richard.gong@intel.com>
> >>>>>>>
> >>>>>>>Add authenticate-fpga-config property for FPGA bitstream authentication.
> >>>>>>>
> >>>>>>>Signed-off-by: Richard Gong <richard.gong@intel.com>
> >>>>>>>---
> >>>>>>>  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..7a512bc 100644
> >>>>>>>--- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> >>>>>>>+++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> >>>>>>>@@ -187,6 +187,7 @@ Optional properties:
> >>>>>>>  - external-fpga-config : boolean, set if the FPGA has already been configured
> >>>>>>>  	prior to OS boot up.
> >>>>>>>  - encrypted-fpga-config : boolean, set if the bitstream is encrypted
> >>>>>>>+- authenticate-fpga-config : boolean, set if do bitstream authentication
> >>>>>>It is unclear to me from the description whether this entails
> >>>>>>authentication + reconfiguration or just authentication.
> >>>>>>
> >>>>>>If the latter is the case this should probably be described as such.
> >>>>>
> >>>>>If it is just authentication, do we still need to disable bridges in
> >>>>>fpga_region_program_fpga?
> >>>>>
> >>>>
> >>>>Yes.
> >>>>
> >>>>Except for the actual configuration of the device, the authentication
> >>>>feature is the same as FPGA configuration.
> >>>
> >>>FPGA Bridges gate bus signals between a host and FPGA. So the FPGA
> >>>region could not be accessed by host when doing configuration. But for
> >>>this authentication, we are just writing the flash, we don't actually
> >>>touch the FPGA soft logic. The host should still be able to operate on
> >>>the old logic before reboot, is it?
> >>>
> >>Yes, it's feasible in theory but doesn't make much sense in practice. I
> >>prefer to keep fpga_region_program_fpga() unchanged.
> >
> >I'm thinking of the case of inband reprograming, that the QSPI flash
> >controller itself is embedded in FPGA soft logic, then maybe host still
> >need to access FPGA on authentication.
> 
> We can decide whether we should update fpga_region_program_fpga() function
> when you update for inband reprogramming case.

Sure, we could think about it later.

Thanks,
Yilun

> 
> Regards,
> Richard
> >
> >Thanks,
> >Yilun
> >
> >>>>
> >>>>>I'm wondering if the FPGA functionalities could still be working when
> >>>>>the authenticating is ongoing, or when the authenticating is failed.
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>>>Thanks,
> >>>>>Yilun
> >>>>>
> >>>>>>
> >>>>>>>  - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
> >>>>>>>  	bridges to successfully become enabled after the region has been
> >>>>>>>  	programmed.
> >>>>>>>-- 
> >>>>>>>2.7.4
> >>>>>>>
> >>>>>>
> >>>>>>Thanks
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt b/Documentation/devicetree/bindings/fpga/fpga-region.txt
index e811cf8..7a512bc 100644
--- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
+++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
@@ -187,6 +187,7 @@  Optional properties:
 - external-fpga-config : boolean, set if the FPGA has already been configured
 	prior to OS boot up.
 - encrypted-fpga-config : boolean, set if the bitstream is encrypted
+- authenticate-fpga-config : boolean, set if do bitstream authentication
 - region-unfreeze-timeout-us : The maximum time in microseconds to wait for
 	bridges to successfully become enabled after the region has been
 	programmed.