[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH v3 4/7] xen/arm: scmi: introduce SCI SCMI SMC multi-agent driver
On Tue, Mar 11, 2025 at 01:16:15PM +0200, Grygorii Strashko wrote: > diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in > index 7edf272386e3..fc6041724a13 100644 > --- a/docs/man/xl.cfg.5.pod.in > +++ b/docs/man/xl.cfg.5.pod.in > @@ -3126,6 +3126,21 @@ single SCMI OSPM agent support. > Should be used together with B<dom0_scmi_smc_passthrough> Xen command line > option. > > +=item B<scmi_smc_multiagent> > + > +Enables ARM SCMI SMC multi-agent support for the guest by enabling SCMI over > +SMC calls forwarding from domain to the EL3 firmware (like Trusted > Firmware-A) > +with a multi SCMI OSPM agent support. The SCMI B<agent_id> should be > +specified for the guest. > + > +=back This new =back shouldn't exist in this patch, because there's no =over been added. This just fix a bug present in the previous patch. > + > +=item B<agent_id=NUMBER> > + > +Specifies a non-zero ARM SCI agent id for the guest. This option is mandatory > +if the SCMI SMC support is enabled for the guest. The agent ids of domains > +existing on a single host must be unique. Are they other restriction on what agent_id value can be? I mean from the description -4242 is a valid value. > =back > > =back > diff --git a/docs/misc/xen-command-line.pandoc > b/docs/misc/xen-command-line.pandoc > index 8e50f6b7c7ac..bc3c64d6ec90 100644 > --- a/docs/misc/xen-command-line.pandoc > +++ b/docs/misc/xen-command-line.pandoc > @@ -1091,6 +1091,15 @@ 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). > > +### dom0_scmi_agent_id (ARM) > +> `= <integer>` > + > +The option is available when `CONFIG_SCMI_SMC_MA` is compiled in, and allows > to > +enable SCMI functionality for Dom0 by specifying a non-zero ARM SCMI agent > id. > +The SCMI will be disabled for Dom0 if this option is not specified > +(for example, thin Dom0 or dom0less use-cases). > +The agent ids of domains existing on a single host must be unique. > + > ### dtuart (ARM) > > `= path [:options]` > > diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c > index d41adea1cefd..cdf5edb299af 100644 > --- a/tools/libs/light/libxl_arm.c > +++ b/tools/libs/light/libxl_arm.c > @@ -229,6 +229,10 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, > case LIBXL_ARM_SCI_TYPE_SCMI_SMC: > config->arch.arm_sci_type = XEN_DOMCTL_CONFIG_ARM_SCI_SCMI_SMC; > break; > + case LIBXL_ARM_SCI_TYPE_SCMI_SMC_MULTIAGENT: > + config->arch.arm_sci_type = XEN_DOMCTL_CONFIG_ARM_SCI_SCMI_SMC_MA; > + config->arch.arm_sci_agent_id = d_config->b_info.arm_sci.agent_id; > + break; > default: > LOG(ERROR, "Unknown ARM_SCI type %d", > d_config->b_info.arm_sci.type); > diff --git a/tools/libs/light/libxl_types.idl > b/tools/libs/light/libxl_types.idl > index ea0d30654cdd..e6707c7ca9e7 100644 > --- a/tools/libs/light/libxl_types.idl > +++ b/tools/libs/light/libxl_types.idl > @@ -553,11 +553,13 @@ libxl_sve_type = Enumeration("sve_type", [ > > libxl_arm_sci_type = Enumeration("arm_sci_type", [ > (0, "none"), > - (1, "scmi_smc") > + (1, "scmi_smc"), > + (2, "scmi_smc_multiagent") > ], init_val = "LIBXL_ARM_SCI_TYPE_NONE") > > libxl_arm_sci = Struct("arm_sci", [ > ("type", libxl_arm_sci_type), > + ("agent_id", uint8) Is it necessary to limit this value to a 8-bit value here? Skimming through the Xen code, it seems that that value can be 32-bits at times, but just restricted to 8-bits in the hypercall. > ]) > > libxl_rdm_reserve = Struct("rdm_reserve", [ > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c > index ac9bf0b25c5a..011222ec55b9 100644 > --- a/tools/xl/xl_parse.c > +++ b/tools/xl/xl_parse.c > @@ -1324,11 +1327,24 @@ static int parse_arm_sci_config(XLU_Config *cfg, > libxl_arm_sci *arm_sci, > tok = ptr + 1; > } > break; > + case STATE_AGENT_ID: > + if (*ptr == ',' || *ptr == '\0') { > + state = *ptr == ',' ? STATE_OPTION : STATE_TERMINAL; > + *ptr = '\0'; > + arm_sci->agent_id = strtoul(tok, NULL, 0); You should check that the value returned by strtoul() is actually valid and does fit in `agent_id`. > + tok = ptr + 1; > + } > default: > break; > } > } > > + if (arm_sci->type == LIBXL_ARM_SCI_TYPE_SCMI_SMC_MULTIAGENT && > + arm_sci->agent_id == 0) { > + fprintf(stderr, "A non-zero ARM_SCI agent_id must be specified\n"); > + goto parse_error; > + } > + > if (tok != ptr || state != STATE_TERMINAL) > goto parse_error; > > @@ -3042,6 +3058,7 @@ skip_usbdev: > libxl_arm_sci arm_sci = { 0 }; > if (!parse_arm_sci_config(config, &arm_sci, buf)) { > b_info->arm_sci.type = arm_sci.type; > + b_info->arm_sci.agent_id = arm_sci.agent_id; I just realise that it's probably enough to call parse_arm_sci_config(.., &b_info->arm_sci) instead of declaring another local `arm_sci` variable. Or is it necessary to have a different variable somehow? > } else { > exit(EXIT_FAILURE); > } Thanks, -- Anthony Perard | Vates XCP-ng Developer XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |