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

[Andrei Cherechesu <andrei.cherechesu@xxxxxxxxxxx>]

    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/
    
> 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