|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH v4 3/8] xen/arm: scmi-smc: passthrough SCMI SMC to domain, single agent
On Mon, 19 May 2025, Oleksii Moisieiev wrote:
> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>
> The commit 3e322bef8bc0 ("xen/arm: firmware: Add SCMI over SMC calls
> handling layer") introduces simple driver which forwards SCMI over SMC
> calls from hwdom/dom0 to EL3 firmware (TF-A) with a single SCMI OSPM agent
> support. While it is working gracefully for hwdom/dom0 use case it doesn't
> cover "thin Dom0 with guest domain, which serves as Driver domain"
> use-case. In this case HW need to be enable in Driver domain and dom0 is
> performing only control functions.
>
> The EL3 SCMI firmware (TF-A) with a single SCMI OSPM agent support is
> pretty generic case for the default vendors SDK and new platforms.
>
> This patch enables passthrough of SCMI SMC single agent interface to the
> one guest domain serving as Driver domain.
>
> Configure Dom0 to enable SCMI passthrough:
>
> - dom0: add dom0_scmi_smc_passthrough to the Xen Command Line
>
> Enabled SCMI passthrough for guest using toolstack in the following way:
>
> - domD: xl.cfg add "arm_sci" option as below
>
> arm_sci = "type=scmi_smc"
>
> - domD: xl.cfg enable access to the "arm,scmi-shmem"
>
> iomem = [
> "47ff0,1@22001",
> ]
>
> - domD: add SCMI nodes to the Driver domain partial device tree as in the
> below example:
>
> passthrough {
> scmi_shm_0: sram@22001000 {
> compatible = "arm,scmi-shmem";
> reg = <0x0 0x22001000 0x0 0x1000>;
> };
>
> firmware {
> compatible = "simple-bus";
> scmi: scmi {
> compatible = "arm,scmi-smc";
> shmem = <&scmi_shm_0>;
> ...
> }
> }
> }
>
> dom0less case configuration:
>
> - add "xen,sci_type" property for required DomU ("xen,domain") node
>
> xen,sci_type="scmi_smc"
>
> - add scmi nodes to the Driver domain partial device tree the same way
> as above and enable access to the "arm,scmi-shmem" according to
> dom0less documentation. For example:
>
> scmi_shm_0: sram@22001000 {
> compatible = "arm,scmi-shmem";
> reg = <0x00 0x22001000 0x00 0x1000>;
> -> xen,reg = <0x0 0x47ff0000 0x0 0x1000 0x0 0x22001000>;
> -> xen,force-assign-without-iommu;
> };
>
> The SCMI SMC single agent interface can be enabled for one and only one
> domain. In general, the configuration is similar to any other HW
> passthrough, except explicitly enabling SCMI with "arm_sci" xl.cfg option.
>
> Note that "arm,scmi-smc" and "arm,scmi-shmem" nodes will be removed from
> dom0/hwdom DT in case of
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@xxxxxxxx>
> ---
>
> Changes in v4:
> - xl.cfg doc
> - fix comments from Stefano Stabellini
> - fix toolstack code as sugested by Anthony PERARD
> - use MATCH_OPTION()
> - move arm_sci struct and cfg params in "arch_arm"
> - add SCMI passthrough for dom0less case
>
> docs/man/xl.cfg.5.pod.in | 34 ++++++++
> docs/misc/arm/device-tree/booting.txt | 15 ++++
> docs/misc/xen-command-line.pandoc | 9 +++
> tools/include/libxl.h | 5 ++
> tools/libs/light/libxl_arm.c | 14 ++++
> tools/libs/light/libxl_types.idl | 10 +++
> tools/xl/xl_parse.c | 36 +++++++++
> xen/arch/arm/dom0less-build.c | 33 +++++++-
> xen/arch/arm/firmware/Kconfig | 4 +-
> xen/arch/arm/firmware/scmi-smc.c | 112 +++++++++++++++++++++++++-
> 10 files changed, 267 insertions(+), 5 deletions(-)
>
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index 8e1422104e..1ccf50b8ea 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -3092,6 +3092,40 @@ Otherwise, the value specified by the `nr_spis`
> parameter will be used.
> The number of SPIs should match the highest interrupt ID that will be
> assigned to the domain.
>
> +=item B<arm_sci="ARM_SCI_STRING">
> +
> +Set ARM_SCI specific options for the guest. ARM SCI is System
> +Control Protocol allows domain to manage various functions that are provided
> +by HW platform firmware.
> +
> +B<ARM_SCI_STRING> is a comma separated list of C<KEY=VALUE> settings,
> +from the following list:
> +
> +=over 4
> +
> +=item B<type=STRING>
> +
> +Specifies an ARM SCI type for the guest.
> +
> +=over 4
> +
> +=item B<none>
> +
> +Don't allow guest to use ARM SCI if present on the platform. This is the
> +default value.
> +
> +=item B<scmi_smc>
> +
> +Enables ARM SCMI SMC support for the guest by enabling SCMI over SMC calls
> +forwarding from domain to the EL3 firmware (like Trusted Firmware-A) with a
> +single SCMI OSPM agent support.
> +Should be used together with B<dom0_scmi_smc_passthrough> Xen command line
> +option.
> +
> +=back
> +
> +=back
> +
> =back
>
> =head3 x86
> diff --git a/docs/misc/arm/device-tree/booting.txt
> b/docs/misc/arm/device-tree/booting.txt
> index 9c881baccc..8943c04173 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -281,6 +281,21 @@ with the following properties:
> passed through. This option is the default if this property is missing
> and the user does not provide the device partial device tree for the
> domain.
>
> +- xen,sci_type
> +
> + A string property specifying an ARM SCI type for the guest.
> +
> + - "none"
> + Don't allow guest to use ARM SCI if present on the platform. This is the
> + default value.
> +
> + - "scmi_smc"
> + Enables ARM SCMI SMC support for the guest by enabling SCMI over SMC
> calls
> + forwarding from domain to the EL3 firmware (like Trusted Firmware-A)
> with a
> + single SCMI OSPM agent support.
> + Should be used together with dom0_scmi_smc_passthrough Xen command line
> + option.
> +
> Under the "xen,domain" compatible node, one or more sub-nodes are present
> for the DomU kernel and ramdisk.
>
> diff --git a/docs/misc/xen-command-line.pandoc
> b/docs/misc/xen-command-line.pandoc
> index 9bbd00baef..8e50f6b7c7 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1082,6 +1082,15 @@ affinities to prefer but be not limited to the
> specified node(s).
>
> Pin dom0 vcpus to their respective pcpus
>
> +### dom0_scmi_smc_passthrough (ARM)
Please rename this option because written like this is really confusing:
it makes me think we are passing through SCMI to Dom0, while actually it
is the opposite.
I would call this option something like "dom0_hide_scmi_smc" because the
only effect is to remove SCMI_SMC from Dom0. Other names might also be
OK, such as "scmi_smc_passthrough".
> +> `= <boolean>`
> +
> +The option is available when `CONFIG_SCMI_SMC` is compiled in, and allows to
> +enable SCMI SMC single agent interface for any, but only one guest domain,
> +which serves as Driver domain. The SCMI will be disabled for Dom0/hwdom and
> +SCMI nodes removed from Dom0/hwdom device tree.
> +(for example, thin Dom0 with Driver domain use-case).
> +
> ### dtuart (ARM)
> > `= path [:options]`
>
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index f8fe4afd7d..5fa43637ab 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -313,6 +313,11 @@
> */
> #define LIBXL_HAVE_BUILDINFO_ARCH_NR_SPIS 1
>
> +/*
> + * libxl_domain_build_info has the arch_arm.sci* fields.
> + */
> +#define LIBXL_HAVE_BUILDINFO_ARCH_ARM_SCI 1
> +
> /*
> * LIBXL_HAVE_SOFT_RESET indicates that libxl supports performing
> * 'soft reset' for domains and there is 'soft_reset' shutdown reason
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 28cea1f643..28ba9eb787 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -222,6 +222,20 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> config->arch.sve_vl = d_config->b_info.arch_arm.sve_vl / 128U;
> }
>
> + switch (d_config->b_info.arch_arm.arm_sci.type) {
> + case LIBXL_ARM_SCI_TYPE_NONE:
> + config->arch.arm_sci_type = XEN_DOMCTL_CONFIG_ARM_SCI_NONE;
> + break;
> + case LIBXL_ARM_SCI_TYPE_SCMI_SMC:
> + config->arch.arm_sci_type = XEN_DOMCTL_CONFIG_ARM_SCI_SCMI_SMC;
> + break;
> + default:
> + LOG(ERROR, "Unknown ARM_SCI type %d",
> + d_config->b_info.arch_arm.arm_sci.type);
> + return ERROR_FAIL;
> + }
> + LOG(DEBUG, " - SCI type=%u", config->arch.arm_sci_type);
> +
> return 0;
> }
>
> diff --git a/tools/libs/light/libxl_types.idl
> b/tools/libs/light/libxl_types.idl
> index 33c9cfc1a2..aa2190ab5b 100644
> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -551,6 +551,15 @@ libxl_sve_type = Enumeration("sve_type", [
> (2048, "2048")
> ], init_val = "LIBXL_SVE_TYPE_DISABLED")
>
> +libxl_arm_sci_type = Enumeration("arm_sci_type", [
> + (0, "none"),
> + (1, "scmi_smc")
> + ], init_val = "LIBXL_ARM_SCI_TYPE_NONE")
> +
> +libxl_arm_sci = Struct("arm_sci", [
> + ("type", libxl_arm_sci_type),
> + ])
> +
> libxl_rdm_reserve = Struct("rdm_reserve", [
> ("strategy", libxl_rdm_reserve_strategy),
> ("policy", libxl_rdm_reserve_policy),
> @@ -725,6 +734,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> ("vuart", libxl_vuart_type),
> ("sve_vl", libxl_sve_type),
> ("nr_spis", uint32),
> + ("arm_sci", libxl_arm_sci),
> ])),
> ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
> ])),
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 9a3679c023..bd22be9d33 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -1284,6 +1284,36 @@ out:
> if (rc) exit(EXIT_FAILURE);
> }
>
> +static int parse_arm_sci_config(XLU_Config *cfg, libxl_arm_sci *arm_sci,
> + const char *str)
> +{
> + int ret = 0;
> + char *buf2, *ptr;
> + char *oparg;
> +
> + if (NULL == (buf2 = ptr = strdup(str)))
> + return ERROR_NOMEM;
> +
> + ptr = strtok(buf2, ",");
> + while (ptr != NULL)
> + {
> + if (MATCH_OPTION("type", ptr, oparg)) {
> + ret = libxl_arm_sci_type_from_string(oparg, &arm_sci->type);
> + if (ret) {
> + fprintf(stderr, "Unknown ARM_SCI type: %s\n", oparg);
> + ret = ERROR_INVAL;
> + goto parse_error;
> + }
> + }
> +
> + ptr = strtok(NULL, ",");
> + }
> +
> +parse_error:
> + free(buf2);
> + return ret;
> +}
> +
> void parse_config_data(const char *config_source,
> const char *config_data,
> int config_len,
> @@ -2981,6 +3011,12 @@ skip_usbdev:
> if (!xlu_cfg_get_long (config, "nr_spis", &l, 0))
> b_info->arch_arm.nr_spis = l;
>
> + if (!xlu_cfg_get_string(config, "arm_sci", &buf, 1)) {
> + if (parse_arm_sci_config(config, &b_info->arch_arm.arm_sci, buf)) {
> + exit(EXIT_FAILURE);
> + }
> + }
> +
> parse_vkb_list(config, d_config);
>
> d_config->virtios = NULL;
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index a09c4c4bd7..0a00f03a25 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -815,6 +815,36 @@ static int __init construct_domU(struct domain *d,
> return rc;
> }
>
> +int __init domu_dt_sci_parse(struct dt_device_node *node,
> + struct xen_domctl_createdomain *d_cfg)
> +{
> + const char *sci_type = NULL;
> + int ret;
> +
> + d_cfg->arch.arm_sci_type = XEN_DOMCTL_CONFIG_ARM_SCI_NONE;
> +
> + if ( !IS_ENABLED(CONFIG_ARM_SCI) ||
> + !dt_property_read_bool(node, "xen,sci_type") )
> + return 0;
> +
> + ret = dt_property_read_string(node, "xen,sci_type", &sci_type);
> + if ( ret )
> + return ret;
> +
> + if ( !strcmp(sci_type, "none") )
> + d_cfg->arch.arm_sci_type = XEN_DOMCTL_CONFIG_ARM_SCI_NONE;
> + else if ( !strcmp(sci_type, "scmi_smc") )
> + d_cfg->arch.arm_sci_type = XEN_DOMCTL_CONFIG_ARM_SCI_SCMI_SMC;
> + else
> + {
> + printk(XENLOG_ERR "xen,sci_type in not valid (%s) for domain %s\n",
> + sci_type, dt_node_name(node));
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> void __init create_domUs(void)
> {
> struct dt_device_node *node;
> @@ -975,7 +1005,8 @@ void __init create_domUs(void)
> if ( !llc_coloring_enabled && llc_colors_str )
> panic("'llc-colors' found, but LLC coloring is disabled\n");
>
> - d_cfg.arch.arm_sci_type = XEN_DOMCTL_CONFIG_ARM_SCI_NONE;
> + if ( domu_dt_sci_parse(node, &d_cfg) )
> + panic("Error getting SCI configuration\n");
>
> /*
> * The variable max_init_domid is initialized with zero, so here it's
> diff --git a/xen/arch/arm/firmware/Kconfig b/xen/arch/arm/firmware/Kconfig
> index bbf88fbb9a..5c5f0880c4 100644
> --- a/xen/arch/arm/firmware/Kconfig
> +++ b/xen/arch/arm/firmware/Kconfig
> @@ -25,7 +25,9 @@ config SCMI_SMC
> doorbell mechanism and Shared Memory for transport ("arm,scmi-smc"
> compatible only). The value of "arm,smc-id" DT property from SCMI
> firmware node is used to trap and forward corresponding SCMI SMCs
> - to firmware running at EL3, for calls coming from the hardware domain.
> + to firmware running at EL3, for calls coming from the hardware domain
> or
> + driver domain.
> + Use with EL3 firmware which supports only single SCMI OSPM agent.
>
> endchoice
>
> diff --git a/xen/arch/arm/firmware/scmi-smc.c
> b/xen/arch/arm/firmware/scmi-smc.c
> index 13d1137592..7470a21505 100644
> --- a/xen/arch/arm/firmware/scmi-smc.c
> +++ b/xen/arch/arm/firmware/scmi-smc.c
> @@ -14,6 +14,8 @@
> #include <xen/device_tree.h>
> #include <xen/errno.h>
> #include <xen/init.h>
> +#include <xen/iocap.h>
> +#include <xen/param.h>
> #include <xen/sched.h>
> #include <xen/types.h>
>
> @@ -22,7 +24,11 @@
>
> #define SCMI_SMC_ID_PROP "arm,smc-id"
>
> +static bool __ro_after_init opt_dom0_scmi_smc_passthrough = false;
> +boolean_param("dom0_scmi_smc_passthrough", opt_dom0_scmi_smc_passthrough);
> +
> static uint32_t __ro_after_init scmi_smc_id;
> +static struct domain __read_mostly *scmi_dom;
>
> /*
> * Check if provided SMC Function Identifier matches the one known by the
> SCMI
> @@ -50,7 +56,7 @@ static bool scmi_handle_smc(struct cpu_user_regs *regs)
> return false;
>
> /* Only the hardware domain should use SCMI calls */
> - if ( !is_hardware_domain(current->domain) )
> + if ( scmi_dom != current->domain )
> {
> gdprintk(XENLOG_WARNING, "SCMI: Unprivileged access attempt\n");
> return false;
> @@ -75,12 +81,45 @@ static bool scmi_handle_smc(struct cpu_user_regs *regs)
> return true;
> }
>
> +static int
> +scmi_smc_domain_sanitise_config(struct xen_domctl_createdomain *config)
> +{
> + if ( config->arch.arm_sci_type != XEN_DOMCTL_CONFIG_ARM_SCI_NONE &&
> + config->arch.arm_sci_type != XEN_DOMCTL_CONFIG_ARM_SCI_SCMI_SMC )
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int scmi_smc_domain_init(struct domain *d,
> struct xen_domctl_createdomain *config)
> {
> - if ( !is_hardware_domain(d) )
> + /*
> + * scmi_passthrough is not enabled:
> + * - proceed only for hw_domain
> + * - fail if guest domain has SCMI enabled.
> + */
> + if ( !opt_dom0_scmi_smc_passthrough && !is_hardware_domain(d) )
> + {
> + if ( config->arch.arm_sci_type == XEN_DOMCTL_CONFIG_ARM_SCI_SCMI_SMC
> )
> + return -EINVAL;
> + else
> + return 0;
> + }
> + /*
> + * scmi_passthrough is enabled:
> + * - ignore hw_domain
> + * - proceed only for domain with SCMI enabled.
> + */
> + if ( opt_dom0_scmi_smc_passthrough &&
> + (config->arch.arm_sci_type == XEN_DOMCTL_CONFIG_ARM_SCI_NONE ||
> + is_hardware_domain(d)) )
> return 0;
>
> + if ( scmi_dom )
> + return -EEXIST;
> +
> + scmi_dom = d;
> d->arch.sci_enabled = true;
> printk(XENLOG_DEBUG "SCMI: %pd init\n", d);
> return 0;
> @@ -88,12 +127,77 @@ static int scmi_smc_domain_init(struct domain *d,
>
> static void scmi_smc_domain_destroy(struct domain *d)
> {
> - if ( !is_hardware_domain(d) )
> + if ( scmi_dom && scmi_dom != d )
> return;
>
> + scmi_dom = NULL;
> + d->arch.sci_enabled = false;
> printk(XENLOG_DEBUG "SCMI: %pd destroy\n", d);
> }
>
> +/*
> + * Handle Dom0 SCMI SMC specific DT nodes
> + *
> + * if dom0_scmi_smc_passthrough=false:
> + * - Copy SCMI nodes into Dom0 device tree.
> + * if dom0_scmi_smc_passthrough=true:
> + * - skip SCMI nodes from Dom0 DT
> + * - give dom0 control access to SCMI shmem MMIO, so SCMI can be passed
> + * through to guest.
Is this so that the xl toolstack can be used? Because I don't think it
would be needed in the dom0less case?
> + */
> +static bool scmi_smc_dt_handle_node(struct domain *d,
> + struct dt_device_node *node)
> +{
> + static const struct dt_device_match shmem_matches[] __initconst = {
> + DT_MATCH_COMPATIBLE("arm,scmi-shmem"),
> + { /* sentinel */ },
> + };
> + static const struct dt_device_match scmi_matches[] __initconst = {
> + DT_MATCH_PATH("/firmware/scmi"),
> + { /* sentinel */ },
> + };
> +
> + /* skip scmi shmem node for dom0 if scmi not enabled */
> + if ( dt_match_node(shmem_matches, node) && !sci_domain_is_enabled(d) )
> + {
> + dt_dprintk("Skip scmi shmem node\n");
> + return true;
> + }
> +
> + /*
> + * skip scmi node for dom0 if scmi not enabled, but give dom0 control
> + * access to SCMI shmem
> + */
> + if ( dt_match_node(scmi_matches, node) && !sci_domain_is_enabled(d) )
> + {
> + struct dt_device_node *shmem_node;
> + const __be32 *prop;
> + u64 paddr, size;
> + int ret;
> +
> + /* give dom0 control access to SCMI shmem */
> + prop = dt_get_property(node, "shmem", NULL);
> + if ( !prop )
> + return true;
> +
> + shmem_node = dt_find_node_by_phandle(be32_to_cpup(prop));
> + if ( !shmem_node )
> + return true;
> +
> + ret = dt_device_get_address(shmem_node, 0, &paddr, &size);
> + if ( ret )
> + return true;
> +
> + ret = iomem_permit_access(d, paddr_to_pfn(paddr),
> + paddr_to_pfn(paddr + size - 1));
> +
> + dt_dprintk("Skip scmi node\n");
> + return true;
> + }
> +
> + return false;
> +}
> +
> static int __init scmi_check_smccc_ver(void)
> {
> if ( smccc_ver < ARM_SMCCC_VERSION_1_1 )
> @@ -108,8 +212,10 @@ static int __init scmi_check_smccc_ver(void)
>
> static const struct sci_mediator_ops scmi_smc_ops = {
> .handle_call = scmi_handle_smc,
> + .domain_sanitise_config = scmi_smc_domain_sanitise_config,
> .domain_init = scmi_smc_domain_init,
> .domain_destroy = scmi_smc_domain_destroy,
> + .dom0_dt_handle_node = scmi_smc_dt_handle_node,
> };
>
> /* Initialize the SCMI layer based on SMCs and Device-tree */
> --
> 2.34.1
>
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |