Re: [RFC PATCH v3 2/7] xen/arm: scmi-smc: update to be used under sci subsystem

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

Hi Stefano,

On 14.03.25 01:48, Stefano Stabellini wrote:
> On Tue, 11 Mar 2025, Grygorii Strashko wrote:
> > The introduced SCI (System Control Interface) subsystem provides unified
> > interface to integrate in Xen SCI drivers which adds support for ARM
> > firmware (EL3, SCP) based software interfaces (like SCMI) that are used in
> > system management. The SCI subsystem allows to add drivers for different FW
> > interfaces or have different drivers for the same FW interface (for example,
> > SCMI with different transports).
> >
> > This patch updates SCMI over SMC calls handling layer, introduced by
> > commit 3e322bef8bc0 ("xen/arm: firmware: Add SCMI over SMC calls handling
> > layer"), to be SCI driver:
> > - convert to DT device;
> > - convert to SCI Xen interface.
> >
> > Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> > ---
> >   xen/arch/arm/firmware/Kconfig                | 13 ++-
> >   xen/arch/arm/firmware/scmi-smc.c             | 93 +++++++++++---------
> >   xen/arch/arm/include/asm/firmware/scmi-smc.h | 41 ---------
> >   xen/arch/arm/vsmc.c                          |  5 +-
> >   xen/include/public/arch-arm.h                |  1 +
> >   5 files changed, 64 insertions(+), 89 deletions(-)
> >   delete mode 100644 xen/arch/arm/include/asm/firmware/scmi-smc.h
> >
> > diff --git a/xen/arch/arm/firmware/Kconfig b/xen/arch/arm/firmware/Kconfig
> > index fc7918c7fc56..02d7b600317f 100644
[...]

> >   
> >   /*
> >    * Local variables:
> > diff --git a/xen/arch/arm/include/asm/firmware/scmi-smc.h 
> > b/xen/arch/arm/include/asm/firmware/scmi-smc.h
> > deleted file mode 100644
> > index 6b1a164a400e..000000000000
> > --- a/xen/arch/arm/include/asm/firmware/scmi-smc.h
> > +++ /dev/null
> > @@ -1,41 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0-only */
> > -/*
> > - * xen/arch/arm/include/asm/firmware/scmi-smc.h
> > - *
> > - * ARM System Control and Management Interface (SCMI) over SMC
> > - * Generic handling layer
> > - *
> > - * Andrei Cherechesu <andrei.cherechesu@xxxxxxx>
> > - * Copyright 2024 NXP
> > - */
> > -
> > -#ifndef __ASM_SCMI_SMC_H__
> > -#define __ASM_SCMI_SMC_H__
> > -
> > -#include <xen/types.h>
> > -
> > -struct cpu_user_regs;
> > -
> > -#ifdef CONFIG_SCMI_SMC
> > -
> > -bool scmi_handle_smc(struct cpu_user_regs *regs);
> > -
> > -#else
> > -
> > -static inline bool scmi_handle_smc(struct cpu_user_regs *regs)
> > -{
> > -    return false;
> > -}
> > -
> > -#endif /* CONFIG_SCMI_SMC */
> > -
> > -#endif /* __ASM_SCMI_H__ */
> > -
> > -/*
> > - * Local variables:
> > - * mode: C
> > - * c-file-style: "BSD"
> > - * c-basic-offset: 4
> > - * indent-tabs-mode: nil
> > - * End:
> > - */
> > diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> > index 51b3c0297314..b33c69a1c22a 100644
> > --- a/xen/arch/arm/vsmc.c
> > +++ b/xen/arch/arm/vsmc.c
> > @@ -21,7 +21,6 @@
> >   #include <asm/traps.h>
> >   #include <asm/vpsci.h>
> >   #include <asm/platform.h>
> > -#include <asm/firmware/scmi-smc.h>
> >   
> >   /* Number of functions currently supported by Hypervisor Service. */
> >   #define XEN_SMCCC_FUNCTION_COUNT 3
> > @@ -233,7 +232,7 @@ static bool handle_sip(struct cpu_user_regs *regs)
> >       if ( platform_smc(regs) )
> >           return true;
> >   
> > -    return scmi_handle_smc(regs);
> > +    return sci_handle_call(regs);
> Is this change...
>
>
> >   }
> >   
> >   /*
> > @@ -301,8 +300,6 @@ static bool vsmccc_handle_call(struct cpu_user_regs *regs)
> >               break;
> >           case ARM_SMCCC_OWNER_SIP:
> >               handled = handle_sip(regs);
> > -            if ( !handled )
> > -                handled = sci_handle_call(regs);
> the one allowing this to be removed? 
Yes, it's just switch from API introduced by commit 3e322bef8bc0 ("xen/arm:
firmware: Add SCMI over SMC calls handling layer") to more generic SCI API.

> Are we sure all possible SCIs are SIPs?
Yes, as I see it, for sure for SCMI.
SCI is platform specific and so SiP.

> >               break;
> >           case ARM_SMCCC_OWNER_TRUSTED_APP ... ARM_SMCCC_OWNER_TRUSTED_APP_END:
> >           case ARM_SMCCC_OWNER_TRUSTED_OS ... ARM_SMCCC_OWNER_TRUSTED_OS_END:
> > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> > index 55eed9992c9d..095b1a23e30c 100644
> > --- a/xen/include/public/arch-arm.h
> > +++ b/xen/include/public/arch-arm.h
> > @@ -328,6 +328,7 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
> >   #define XEN_DOMCTL_CONFIG_TEE_FFA       2
> >   
> >   #define XEN_DOMCTL_CONFIG_ARM_SCI_NONE      0
> > +#define XEN_DOMCTL_CONFIG_ARM_SCI_SCMI_SMC  1
> >   
> >   struct xen_arch_domainconfig {
> >       /* IN/OUT */
> > --
> > 2.34.1
--
Best regards,
-grygorii