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

From: Julien Grall <julien@xen.org>
To: John Ernberg <john.ernberg@actia.se>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Michal Orzel <michal.orzel@amd.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Peng Fan <peng.fan@nxp.com>
Cc: Jonas Blixt <jonas.blixt@actia.se>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue
Date: Thu, 1 Feb 2024 12:20:06 +0000	[thread overview]
Message-ID: <167f0c7a-e037-446c-82f8-2584e35a7af1@xen.org> (raw)
In-Reply-To: <494d4961-ad8a-4d1d-aaa6-d1bfb9d6a137@actia.se>

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?

Cheers,

-- 
Julien Grall