Received-SPF: pass (google.com: domain of linux-kernel+bounces-76278-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <8af59b01-53cf-4fc4-9946-6c630fb7b38e@quicinc.com>
Date: Thu, 22 Feb 2024 15:25:11 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 0/7] Introduction of a remoteproc tee to load signed
 firmware
Content-Language: en-US
To: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>,
        Bjorn Andersson
	<andersson@kernel.org>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        Jens
 Wiklander <jens.wiklander@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley
	<conor+dt@kernel.org>
CC: <linux-stm32@st-md-mailman.stormreply.com>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-remoteproc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <op-tee@lists.trustedfirmware.org>, <devicetree@vger.kernel.org>
References: <20240214172127.1022199-1-arnaud.pouliquen@foss.st.com>
 <fe4ec29f-7521-4369-a382-bae50dbf0ee5@quicinc.com>
 <adcf0c0d-f452-4285-8651-c147fcca001b@foss.st.com>
From: Naman Jain <quic_namajain@quicinc.com>
In-Reply-To: <adcf0c0d-f452-4285-8651-c147fcca001b@foss.st.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit

On 2/22/2024 2:17 PM, Arnaud POULIQUEN wrote:
> Hello Naman,
> 
> On 2/22/24 06:43, Naman Jain wrote:
>> On 2/14/2024 10:51 PM, Arnaud Pouliquen wrote:
>>> Updates from the previous version [1]:
>>>
>>> This version proposes another approach based on an alternate load and boot
>>> of the coprocessor. Therefore, the constraint introduced by tee_remoteproc
>>> is that the firmware has to be authenticated and loaded before the resource
>>> table can be obtained.
>>>
>>> The existing boot sequence is: >
>>>     1) Get the resource table and store it in a cache,
>>>        calling rproc->ops->parse_fw().
>>>     2) Parse the resource table and handle resources,
>>>        calling rproc_handle_resources.
>>>     3) Load the firmware, calling rproc->ops->load().
>>>     4) Start the firmware, calling rproc->ops->start().
>>>    => Steps 1 and 2 are executed in rproc_fw_boot(), while steps 3 and 4 are
>>>      executed in rproc_start().
>>> => the use of rproc->ops->load() ops is mandatory
>>>
>>> The boot sequence needed for TEE boot is:
>>>
>>>     1) Load the firmware.
>>>     2) Get the loaded resource, no cache.
>>>     3) Parse the resource table and handle resources.
>>>     4) Start the firmware.
>>
>> Hi,
>> What problem are we really addressing here by reordering load, parse of
>> FW resources?
> 
> The feature introduced in TEE is the signature of the firmware images. That
> means that before getting the resource table, we need to first authenticate the
> firmware images.
> Authenticating a firmware image means that we have to copy the firmware into
> protected memory that cannot be corrupted by the non-secure and then verify the
> signature.
> The strategy implemented in OP-TEE is to load the firmware into destination
> memory and then authenticate it.
> This strategy avoids having a temporary copy of the whole images in a secure memory.
> This strategy imposes loading the firmware images before retrieving the resource
> table.
> 
>> Basically, what are the limitations of the current design you are referring to?
>> I understood that TEE is designed that way.
> 
> The limitation of the current design is that we obtain the resource table before
> loading the firmware. Following the current design would impose constraints in
> TEE that are not straightforward. Step 1 (getting the resource table and storing
> it in a cache) would require having a copy of the resource table in TEE after
> authenticating the images. However, authenticating the firmware, as explained
> before, depends on the strategy implemented. In TEE implementation, we load the
> firmware to authenticate it in the destination memory.
> 
> Regards,
> Arnaud


Hello Arnaud,
I think now I got your point. In TEE, you don't want to do anything(read
resource table) with FW images, until its loaded and authenticated.
Since current design was not allowing you to do it, you had to 
reorganize the code so that this can be achieved.

Generally speaking, in current design, if authentication fails for some
reason later, one can handle it, but it depends on the implementation of
parse_fw op if the damage is already done.

Please correct me if this is wrong assumption.
Patch looks good to me.

Regards,
Naman Jain