Re: [PATCH v1 3/5] xen/arm: platforms: Add NXP S32CC platform code

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Andrei,

> On 11 Sep 2024, at 23:05, Andrei Cherechesu <andrei.cherechesu@xxxxxxxxxxx> 
> wrote:
> 
> Hi Julien, Stefano, 
> On 11/09/2024 08:19, Stefano Stabellini wrote:
>> On Tue, 10 Sep 2024, Julien Grall wrote:
>> 
>>> Hi,
>>> 
>>> On 10/09/2024 15:34, Andrei Cherechesu (OSS) wrote:
>>> 
>>>> From: Andrei Cherechesu <andrei.cherechesu@xxxxxxx>
>>>> 
>>>> Added support for NXP S32CC platforms (S32G2, S32G3, S32R45),
>>>> which need SCMI over SMC calls forwarded to the firmware
>>>> running at EL3 (TF-A). Linux relies on the SCMI Platform
>>>> for system services such as clocking, reset, etc.
>>>> 
>>> Is it SMCI as in the Arm spec? If so, this should not be platform specific.
> Yes, it is SCMI as defined by Arm. I agree it shouldn't be platform specific.
>> 
>>> 
>>> Furthermore, I was under the impression that we can't blindly forward
>>> everything from a domain to the firmware. While it might be okayish for 
>>> dom0,
>>> you also seem to give access to all the domains on the system is it 
>>> intended?
> In our case, only Dom0 has access to the SCMI mailbox. Hence, the other 
> unprivileged domains are not aware of SCMI and do not make any SCMI requests 
> to FW.
>> 
>>> 
>>> Anyway, there is a series on the ML to add a mediator for SCMI in Xen (see
>>> [1]). I think it would be preferable to focus on getting it merged as it 
>>> would
>>> 
>>> benefit everyone and increase the security posture (we could restrict 
>>> access).
> I also asked a few months ago on the ML in a virtio related thread if there 
> are any updates regarding 
> SCMI awareness for Xen and guests, then Stefano CCed Bertrand, but he did not 
> comment [0].
> I'm curious why the SCMI mediator patch series did not progress.
> [0] 
> https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2401111627360.3675@ubuntu-linux-20-04-desktop/

Sorry it seems i missed that one.

There are several initiatives ongoing to investigate the global problem of 
clock handling and more specifically
SCMI "sharing".
The SCMI protocol contains some features to have virtual channels and the 
question is how to make a transport
that is hypervisor agnostic to prevent to require the hypervisors to have to 
"decode" SCMI messages.

Virtio-scmi is not really used for clock management per say at the moment and 
more specifically I do not
think it is a good solution to manage clocks of non virtio devices.

In Soafee and in Linaro people are looking at using FF-A as a tansport for SCMI.
The idea would be that the hypervisor is configuring the virtual SCMI channels 
using FF-A as a transport
and then VMs are using FF-A to communicate with an SCMI server (probably 
sitting in secure world, at
least as a proxy). This is an investigation for now.

Requiring Xen to act as a mediator is also a solution but might require a lot 
of platform specific code
which i think we should prevent.

For now having a solution in Xen where SCMI calls through SMC are only allowed 
by Dom0 is the only
short term solution I think.

Cheers
Bertrand


>> 
>> Hi Andrei, Julien,
>> 
>> SCMI is very flexible and can be configured in a number of ways. In
>> general, Julien has a point that typically forwarding to firmware all
>> SCMI requests from Xen domains is not the desired behavior.
>> 
>> An an example, imagine the case where device1 is assigned to domain1 and
>> device2 is assigned to domain2. Now imagine that they both share a
>> clock. Domain1 and domain2 could fight over the clock frequency settings
>> using SCMI to change it, without being aware of each other activities.
>> It is likely that the system would malfunction.
> I completely agree and we are aware of the possible resource contention. 
> Another (simpler?) scenario where access control is needed, besides the one 
> you described, is when Domain1 would directly try to perform some requests 
> for some resources that affect Device2 (owned by Domain2). If Domain1 knows 
> the clock IDs used by Device2, for example, without any access control it 
> could perform a SCMI clock request affecting Device2's clocks, which should 
> be out of his control.
>> 
>> If this kind of situations can happen on NXP S32CC platforms, then this
>> patch might not be a good idea. As Julien suggested, you might want to
>> have a look at Oleksii's approach. We could probably allow Dom0 to make
>> all SCMI calls. If you think that is OK, you need to add a
>> (is_hardware_domain(d)) check.
>> On the other hand, if your SCMI server implementation has a way to
>> prevent possible harmful activities from happening, or maybe all clocks
>> are fixed-clocks so there are actually no SCMI operations to control the
>> clocks, then it could be possible that this patch might be fine. I admit
>> it is unlikely because there is a number of ways SCMI could be used by
>> one domain to hurt another domain.
>> 
>> Can you please give us a brief overview on how SCMI is expected to work
>> on NXP S32CC?
> Well, we normally rely on most SCMI protocols to access system-level 
> resources from agents: Base, Power Domain, System Power, Performance Domain, 
> Clock, Reset Domain. Linux jumps to EL3 via SMC carrying an SCMI message, and 
> FW running at EL3 decides how to handle it. Basically, Linux cannot directly 
> control most system-level resources.
> 
> With Xen, we currently don't allow unprivileged Domains to do SCMI requests. 
> The SMCs are of course trapped at EL2 and that's why we enabled forwarding to 
> EL3 without any access control, knowing it shouldn't break anything, and to 
> let everything function as normal. In some passthrough scenarios the 
> unprivileged domains rely on settings already made by firmware (for clocks, 
> pins, etc) that their assigned devices require, and in DT we replace them 
> with e.g. fixed-clock for clocks.
> 
> An "is_hardware_domain(d)" check should be enough for the moment to harden 
> the code, but I agree that this should not be something platform-specific in 
> the future, and the handling must be done in a generic way.
> So I would proceed with this approach for this patch series, if that's ok for 
> you, and I will also take a look at Oleksii's approach.
> 
> Regards,
> Andrei C
> 
> 
> 
>