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

From: Richard Gong <richard.gong@linux.intel.com>
To: Moritz Fischer <mdf@kernel.org>
Cc: gregkh@linuxfoundation.org, trix@redhat.com,
	linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org,
	dinguyen@kernel.org, sridhar.rajagopal@intel.com,
	richard.gong@intel.com
Subject: Re: [PATCHv2 1/5] firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag
Date: Mon, 30 Nov 2020 12:55:44 -0600	[thread overview]
Message-ID: <771ba4f4-59e1-74b0-ba55-3f65914e2bc7@linux.intel.com> (raw)
In-Reply-To: <X7m6gy/B8DiafyYQ@archbook>

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