| Message ID | 1605204403-6663-4-git-send-email-richard.gong@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Extend FPGA manager and region drivers for | expand |
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.
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. >
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. >> >
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
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
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. >>> >> >
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 >
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
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. >>>> >>> >> >
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
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
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
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
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 --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.