Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: JcAYSx8dS08FD+BlSjmtlAgeGrVnvrEStZ59bxq/TKikXQG/deoC0XLcaZVY9qp8NlOpxKVQ/V
 DwDgWz7pk61A==
IronPort-SDR: asmWp1QY24TIDG7pef9yM7XBm68sR8mtJyGYqbxJfxtwO2nGiDRsrz4Il07yUK/wCjTvZ41CW2
 VM++/ccXZJ2Q==
Subject: Re: [PATCHv4 5/6] dt-bindings: fpga: add authenticate-fpga-config
 property
To:     Moritz Fischer <mdf@kernel.org>
Cc:     trix@redhat.com, gregkh@linuxfoundation.org,
        linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org,
        Richard Gong <richard.gong@intel.com>
References: <1612192919-4069-1-git-send-email-richard.gong@linux.intel.com>
 <1612192919-4069-6-git-send-email-richard.gong@linux.intel.com>
 <YBjUyc2ea51S4Wzp@epycbox.lan>
From:   Richard Gong <richard.gong@linux.intel.com>
Message-ID: <9752d0eb-962a-f308-24a3-aedd4fecf25d@linux.intel.com>
Date:   Tue, 2 Feb 2021 16:01:37 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <YBjUyc2ea51S4Wzp@epycbox.lan>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


Hi Moritz,

On 2/1/21 10:27 PM, Moritz Fischer wrote:
> On Mon, Feb 01, 2021 at 09:21:58AM -0600, richard.gong@linux.intel.com wrote:
>> 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>
>> ---
>> v4: explain authenticate-fpga-config flag further
>> v3: no change
>> v2: put authenticate-fpga-config above partial-fpga-config
>>      update commit messages
>> ---
>>   Documentation/devicetree/bindings/fpga/fpga-region.txt | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt b/Documentation/devicetree/bindings/fpga/fpga-region.txt
>> index e811cf8..e2740b6 100644
>> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
>> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
>> @@ -182,6 +182,10 @@ 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.
>> +	flag authenticate-fpga-config is used to first check the integrity of
>> +	the bitstream. If the authentication is passed, the user can perform
>> +	other operations.
>  From the other commits it looks like it *also* writes to QSPI? If so
> please document that.
> 
> If not, feel free to ignore :)
> 
> Maybe I would highlight two things:
> a) If you add 'authenticate-fpga-config' you are not allowed to add new
>     nodes
> b) If you add 'authenticate-fpga-config' you are not alllowed to add
>     other operations

How about the descriptions below?

- authenticate-fpga-config : boolean, set if do bitstream authentication 
only.
If 'authenticate-fpga-config' is added then adding a new node or another 
operation is not allowed.
Flag authenticate-fpga-config is used to check the integrity of the 
bitstream.
Except for the actual configuration of the device, the authentication 
works in the same way as FPGA configuration. If the authentication 
passes, other operations such as full or partial reconfiguration can be 
performed. When the bitstream into QSPI flash memory at device is 
programmed, it is expected that there will be no issue when starting the 
device.

>>   - 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
>>
> Thanks,
> Moritz
> 
Regards,
Richard