diff mbox series

[v3,06/10] xen/arm: enable Dom0 to use SVE feature

Message ID 20230317131949.4031014-7-luca.fancellu@arm.com (mailing list archive)
State Superseded
Headers show
Series SVE feature for arm guests | expand

Commit Message

Luca Fancellu March 17, 2023, 1:19 p.m. UTC
Add a command line parameter to allow Dom0 the use of SVE resources,
the command line parameter dom0_sve controls the feature on this
domain and sets the maximum SVE vector length for Dom0.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
Changes from v2:
 - xen_domctl_createdomain field has changed into sve_vl and its
   value now is the VL / 128, create an helper function for that.
Changes from v1:
 - No changes
Changes from RFC:
 - Changed docs to explain that the domain won't be created if the
   requested vector length is above the supported one from the
   platform.
---
 docs/misc/xen-command-line.pandoc    | 13 +++++++++++++
 xen/arch/arm/arm64/sve.c             |  5 +++++
 xen/arch/arm/domain_build.c          |  4 ++++
 xen/arch/arm/include/asm/arm64/sve.h |  9 +++++++++
 4 files changed, 31 insertions(+)

Comments

Jan Beulich March 20, 2023, 9:13 a.m. UTC | #1
On 17.03.2023 14:19, Luca Fancellu wrote:
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1005,6 +1005,19 @@ restrictions set up here. Note that the values to be specified here are
>  ACPI PXM ones, not Xen internal node numbers. `relaxed` sets up vCPU
>  affinities to prefer but be not limited to the specified node(s).
>  
> +### dom0_sve (arm)
> +> `= <integer>`
> +
> +> Default: `0`
> +
> +Enable arm SVE usage for Dom0 domain and sets the maximum SVE vector length.
> +Values above 0 means feature is enabled for Dom0, otherwise feature is disabled.
> +Possible values are from 0 to maximum 2048, being multiple of 128, that will be
> +the maximum vector length.
> +Please note that the platform can supports a lower value, if the requested value
> +is above the supported one, the domain creation will fail and the system will
> +stop.
> +
>  ### dom0_vcpus_pin
>  > `= <boolean>`

I'd like to raise the question of proliferation of top-level command
line options controlling Dom0. In x86 we've specifically started to use
"dom0=" as the one top-level option where almost all new controls should
be added as sub-options.

_If_ a top-level option is indeed preferred, then please avoid the use
of an underscore in its name, when a dash does fine.

Jan
Luca Fancellu March 22, 2023, 7:32 a.m. UTC | #2
> On 20 Mar 2023, at 09:13, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 17.03.2023 14:19, Luca Fancellu wrote:
>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -1005,6 +1005,19 @@ restrictions set up here. Note that the values to be specified here are
>> ACPI PXM ones, not Xen internal node numbers. `relaxed` sets up vCPU
>> affinities to prefer but be not limited to the specified node(s).
>> 
>> +### dom0_sve (arm)
>> +> `= <integer>`
>> +
>> +> Default: `0`
>> +
>> +Enable arm SVE usage for Dom0 domain and sets the maximum SVE vector length.
>> +Values above 0 means feature is enabled for Dom0, otherwise feature is disabled.
>> +Possible values are from 0 to maximum 2048, being multiple of 128, that will be
>> +the maximum vector length.
>> +Please note that the platform can supports a lower value, if the requested value
>> +is above the supported one, the domain creation will fail and the system will
>> +stop.
>> +
>> ### dom0_vcpus_pin
>>> `= <boolean>`
> 
> I'd like to raise the question of proliferation of top-level command
> line options controlling Dom0. In x86 we've specifically started to use
> "dom0=" as the one top-level option where almost all new controls should
> be added as sub-options.
> 
> _If_ a top-level option is indeed preferred, then please avoid the use
> of an underscore in its name, when a dash does fine.

Yes, maybe something like dom0=sve=<VL>,[...] is nice for new possible incoming features,
While I was browsing the code, I spotted a possible bug in x86/dom0_build.c, in
parse_dom0_param(...) if an option is not recognised we set rc = -EINVAL, but we don’t
stop the loop and we continue to parse.
Is it the intended behaviour?

> 
> Jan
Jan Beulich March 22, 2023, 7:51 a.m. UTC | #3
On 22.03.2023 08:32, Luca Fancellu wrote:
> While I was browsing the code, I spotted a possible bug in x86/dom0_build.c, in
> parse_dom0_param(...) if an option is not recognised we set rc = -EINVAL, but we don’t
> stop the loop and we continue to parse.
> Is it the intended behaviour?

My view: Yes. But I can see how one might look at this differently.
Yet iirc the same behavior can be found elsewhere as well, e.g. for
"spec-ctrl=". The goal is to parse everything we recognize, covering
for people using options on older hypervisors which are supported
only by newer ones.

Jan
diff mbox series

Patch

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index e0b89b7d3319..595e0d17183c 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1005,6 +1005,19 @@  restrictions set up here. Note that the values to be specified here are
 ACPI PXM ones, not Xen internal node numbers. `relaxed` sets up vCPU
 affinities to prefer but be not limited to the specified node(s).
 
+### dom0_sve (arm)
+> `= <integer>`
+
+> Default: `0`
+
+Enable arm SVE usage for Dom0 domain and sets the maximum SVE vector length.
+Values above 0 means feature is enabled for Dom0, otherwise feature is disabled.
+Possible values are from 0 to maximum 2048, being multiple of 128, that will be
+the maximum vector length.
+Please note that the platform can supports a lower value, if the requested value
+is above the supported one, the domain creation will fail and the system will
+stop.
+
 ### dom0_vcpus_pin
 > `= <boolean>`
 
diff --git a/xen/arch/arm/arm64/sve.c b/xen/arch/arm/arm64/sve.c
index 1b95a3cbadd1..6593b59b58e8 100644
--- a/xen/arch/arm/arm64/sve.c
+++ b/xen/arch/arm/arm64/sve.c
@@ -5,10 +5,15 @@ 
  * Copyright (C) 2022 ARM Ltd.
  */
 
+#include <xen/param.h>
 #include <xen/sched.h>
 #include <xen/sizes.h>
 #include <asm/arm64/sve.h>
 
+/* opt_dom0_sve: allow Dom0 to use SVE and set maximum vector length. */
+unsigned int __initdata opt_dom0_sve;
+integer_param("dom0_sve", opt_dom0_sve);
+
 extern unsigned int sve_get_hw_vl(void);
 extern void sve_save_ctx(uint64_t *sve_ctx, uint64_t *pregs, int save_ffr);
 extern void sve_load_ctx(uint64_t const *sve_ctx, uint64_t const *pregs,
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9707eb7b1bb1..aa5ff52b90c2 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -26,6 +26,7 @@ 
 #include <asm/platform.h>
 #include <asm/psci.h>
 #include <asm/setup.h>
+#include <asm/arm64/sve.h>
 #include <asm/cpufeature.h>
 #include <asm/domain_build.h>
 #include <xen/event.h>
@@ -4084,6 +4085,9 @@  void __init create_dom0(void)
     if ( iommu_enabled )
         dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
 
+    if ( opt_dom0_sve > 0 )
+        dom0_cfg.arch.sve_vl = domainconfig_encode_vl(opt_dom0_sve);
+
     dom0 = domain_create(0, &dom0_cfg, CDF_privileged | CDF_directmap);
     if ( IS_ERR(dom0) )
         panic("Error creating domain 0 (rc = %ld)\n", PTR_ERR(dom0));
diff --git a/xen/arch/arm/include/asm/arm64/sve.h b/xen/arch/arm/include/asm/arm64/sve.h
index ced010f42dad..8654b0fac4bc 100644
--- a/xen/arch/arm/include/asm/arm64/sve.h
+++ b/xen/arch/arm/include/asm/arm64/sve.h
@@ -25,8 +25,15 @@  static inline uint16_t domainconfig_decode_vl(uint8_t sve_vl)
     return sve_vl * SVE_VL_MULTIPLE_VAL;
 }
 
+static inline uint8_t domainconfig_encode_vl(uint16_t sve_vl_bits)
+{
+    return sve_vl_bits / SVE_VL_MULTIPLE_VAL;
+}
+
 #ifdef CONFIG_ARM64_SVE
 
+extern unsigned int opt_dom0_sve;
+
 register_t compute_max_zcr(void);
 register_t vl_to_zcr(uint16_t vl);
 uint16_t get_sys_vl_len(void);
@@ -37,6 +44,8 @@  void sve_restore_state(struct vcpu *v);
 
 #else /* !CONFIG_ARM64_SVE */
 
+#define opt_dom0_sve (0)
+
 static inline register_t compute_max_zcr(void)
 {
     return 0;