Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue

[John Ernberg <john.ernberg@xxxxxxxx>]

On 2/1/24 13:20, Julien Grall wrote:
> 
> 
> On 31/01/2024 15:32, John Ernberg wrote:
>> Hi Julien,
> 
> Hi John,
> 
>> On 1/31/24 13:22, Julien Grall wrote:
>>> Hi,
>>>
>>> On 31/01/2024 11:50, John Ernberg wrote:
>>>> When using Linux for dom0 there are a bunch of drivers that need to do
>>>> SMC
>>>> SIP calls into the PSCI provider to enable certain hardware bits 
>>>> like the
>>>> watchdog.
>>>
>>> Do you know which protocol this is under the hood. Is this SCMI?
>>
>> I think I confused myself here when I wrote the commit log.
>>
>> The EL3 code in our case is ATF, and it does not appear to be SCMI, nor
>> PSCI. The register usage of these SMC SIP calls are as follows:
>> a0 - service
>> a1 - function
>> a2-a7 - args
>>
>> In ATF the handler is declared as a runtime service.
>>
>> Would the appropriate commmit message here be something along the lines
>> of below?
>> """
>> When using Linux for dom0 there are a bunch of drivers that need to 
>> do   SMC
>> SIP calls into the firmware to enable certain hardware bits like the
>> watchdog.
>> """
> 
> It reads better thanks.
> 
> [...]
> 
>>> But even if we restrict to dom0, have you checked that none of the SMCs
>>> use buffers?
>> I haven't found any such instances in the Linux kernel where a buffer is
>> used. Adding a call filtering like suggested below additions of such
>> functions can be discovered and adapted for if they would show up later.
>>>
>>> Rather than providing a blanket forward, to me it sounds more like you
>>> want to provide an allowlist of the SMCs. This is more futureproof and
>>> avoid the risk to expose unsafe SMCs to any domain.
>>>
>>> For an example, you can have a look at the EEMI mediator for Xilinx.
>>
>> Ack. Do you prefer to see only on SMCCC service level or also on
>> function level? (a1 register, per description earlier)
> 
> I am not sure. It will depend on whether it is correct to expose *all* 
> the functions within a service level and they have the same format.
> 
> If you can't guarantee that, then you will most likely need to allowlist 
> at the function level.
> 
> Also, do you have a spec in hand that would help to understand which 
> service/function is implemented via those SMCs?

I don't have the spec unfortunately, but I will add a filter on both 
service and function for V2 and we'll take it from there.
> 
> Cheers,
> 

Thanks! // John Ernberg