Re: [RFC PATCH v5 02/10] xen/arm: scmi-smc: update to be used under sci subsystem

[Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx>]

On 27/08/2025 22:39, Stefano Stabellini wrote:
> On Wed, 27 Aug 2025, Oleksii Moisieiev wrote:
>> On 23/08/2025 03:26, Stefano Stabellini wrote:
>>> On Tue, 22 Jul 2025, Oleksii Moisieiev wrote:
>>>> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>>>>
>>>> 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.
>>>>
>>>> There are no functional changes in general, the driver is just adopted
>>>> to the SCI interface.
>>>>
>>>> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>>>> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@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 fc7918c7fc..bbf88fbb9a 100644
>>>> --- a/xen/arch/arm/firmware/Kconfig
>>>> +++ b/xen/arch/arm/firmware/Kconfig
>>>> @@ -8,9 +8,18 @@ config ARM_SCI
>>>>    
>>>>    menu "Firmware Drivers"
>>>>    
>>>> +choice
>>>> +  prompt "ARM SCI driver type"
>>>> +  default SCMI_SMC
>>>> +  help
>>>> +  Choose which ARM SCI driver to enable.
>>>> +
>>>> +config ARM_SCI_NONE
>>>> +  bool "none"
>>>> +
>>>>    config SCMI_SMC
>>>>            bool "Forward SCMI over SMC calls from hwdom to EL3 firmware"
>>>> -  default y
>>>> +  select ARM_SCI
>>>>            help
>>>>              This option enables basic awareness for SCMI calls using SMC 
>>>> as
>>>>              doorbell mechanism and Shared Memory for transport 
>>>> ("arm,scmi-smc"
>>>> @@ -18,4 +27,6 @@ config SCMI_SMC
>>>>              firmware node is used to trap and forward corresponding SCMI 
>>>> SMCs
>>>>              to firmware running at EL3, for calls coming from the 
>>>> hardware domain.
>>>>    
>>>> +endchoice
>>>> +
>>>>    endmenu
>>>> diff --git a/xen/arch/arm/firmware/scmi-smc.c 
>>>> b/xen/arch/arm/firmware/scmi-smc.c
>>>> index 33473c04b1..13d1137592 100644
>>>> --- a/xen/arch/arm/firmware/scmi-smc.c
>>>> +++ b/xen/arch/arm/firmware/scmi-smc.c
>>>> @@ -9,6 +9,7 @@
>>>>     * Copyright 2024 NXP
>>>>     */
>>>>    
>>>> +#include <asm/device.h>
>>>>    #include <xen/acpi.h>
>>>>    #include <xen/device_tree.h>
>>>>    #include <xen/errno.h>
>>>> @@ -16,12 +17,11 @@
>>>>    #include <xen/sched.h>
>>>>    #include <xen/types.h>
>>>>    
>>>> +#include <asm/firmware/sci.h>
>>>>    #include <asm/smccc.h>
>>>> -#include <asm/firmware/scmi-smc.h>
>>>>    
>>>>    #define SCMI_SMC_ID_PROP   "arm,smc-id"
>>>>    
>>>> -static bool __ro_after_init scmi_enabled;
>>>>    static uint32_t __ro_after_init scmi_smc_id;
>>>>    
>>>>    /*
>>>> @@ -41,14 +41,11 @@ static bool scmi_is_valid_smc_id(uint32_t fid)
>>>>     *
>>>>     * Returns true if SMC was handled (regardless of response), false 
>>>> otherwise.
>>>>     */
>>>> -bool scmi_handle_smc(struct cpu_user_regs *regs)
>>>> +static bool scmi_handle_smc(struct cpu_user_regs *regs)
>>>>    {
>>>>        uint32_t fid = (uint32_t)get_user_reg(regs, 0);
>>>>        struct arm_smccc_res res;
>>>>    
>>>> -    if ( !scmi_enabled )
>>>> -        return false;
>>>> -
>>>>        if ( !scmi_is_valid_smc_id(fid) )
>>>>            return false;
>>>>    
>>>> @@ -78,49 +75,45 @@ bool scmi_handle_smc(struct cpu_user_regs *regs)
>>>>        return true;
>>>>    }
>>>>    
>>>> -static int __init scmi_check_smccc_ver(void)
>>>> +static int scmi_smc_domain_init(struct domain *d,
>>>> +                                struct xen_domctl_createdomain *config)
>>>>    {
>>>> -    if ( smccc_ver < ARM_SMCCC_VERSION_1_1 )
>>>> -    {
>>>> -        printk(XENLOG_WARNING
>>>> -               "SCMI: No SMCCC 1.1 support, SCMI calls forwarding 
>>>> disabled\n");
>>>> -        return -ENOSYS;
>>>> -    }
>>>> +    if ( !is_hardware_domain(d) )
>>>> +        return 0;
>>> Should we also check for config->arch.sci_type ==
>>> XEN_DOMCTL_CONFIG_ARM_SCI_SCMI_SMC ?
>>>
>> Current changes assume that only SCMI_SMC is present. This check was
>> added in the subsequent patch when multiagent was introduced.
> In theory ARM_SCI_NONE is possible?
>
It is. When "scmi-smc-passthrough" parameter was provided then xen will 
skip "firmware/scmi" node for the dom0.

--
Oleksii