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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 16 Sep 2024, Julien Grall wrote:
> On 13/09/2024 08:43, Julien Grall wrote:
> > On 12/09/2024 23:44, Stefano Stabellini wrote:
> > > On Thu, 12 Sep 2024, Bertrand Marquis wrote:
> > > > 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.
> > > 
> > > +1
> > I am open to go this way. But I would like the commit message to contain
> > some details on whether this will always work fine for dom0 (this would tell
> > whether the feature can be supported or needs to be in experimental/tech
> > preview).
> > 
> > At least my main concern is anything to do with RAM. On Matrix, Bertrand
> > suggested that none of the messages should contain addresses. What about the
> > transport? Is it using buffer? If so, are they fixed?
> > 
> > Brief looking at Linux, there are multiple DT compatible. IIUC, we would
> > only support "arm,scmi-smc" with the implementation. If so, maybe we should
> > check the DT.
> 
> Some clarifications as I was asked on Matrix. The issue described in this
> patch is not specific to SCMI. So I think the SCMI trapping & forward should
> be part of common code. If it we are not sure whether it is safe, then it
> should be protected by CONFIG_EXPERT or CONFIG_UNSUPPORTED.
> 
> I hope this clarifies what I would like. Not sure if the others agree though.

I think it is a good idea to make the SCMI trapping and forwarding in
common code. I am fine with that. I think it is OK to enable it by
default without CONFIG_EXPERT. I am OK with or without using
CONFIG_UNSUPPORTED. I am happy either way. Thanks Julien, good
suggestion.