Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
On 24/05/2023 11:05, Bertrand Marquis wrote: Hi Luca, Hi, On 23 May 2023, at 09:43, Luca Fancellu wrote: Add a command line parameter to allow Dom0 the use of SVE resources, the command line parameter sve=, sub argument of dom0=, controls the feature on this domain and sets the maximum SVE vector length for Dom0. Add a new function, parse_signed_integer(), to parse an integer command line argument. Signed-off-by: Luca Fancellu Reviewed-by: Bertrand Marquis with ... --- Changes from v6: - Fixed case for e==NULL in parse_signed_integer, drop parenthesis from if conditions, delete inline sve_domctl_vl_param and rely on DCE from the compiler (Jan) - Drop parenthesis from opt_dom0_sve (Julien) - Do not continue if 'sve' is in command line args but CONFIG_ARM64_SVE is not selected: https://lore.kernel.org/all/7614ae25-f59d-430a-9c3e-30b1ce0e1...@arm.com/ Changes from v5: - stop the domain if VL error occurs (Julien, Bertrand) - update the documentation - Rename sve_sanitize_vl_param to sve_domctl_vl_param to mark the fact that we are sanitizing a parameter coming from the user before encoding it into sve_vl in domctl structure. (suggestion from Bertrand in a separate discussion) - update comment in parse_signed_integer, return boolean in sve_domctl_vl_param (Jan). Changes from v4: - Negative values as user param means max supported HW VL (Jan) - update documentation, make use of no_config_param(), rename parse_integer into parse_signed_integer and take long long *, also put a comment on the -2 return condition, update declaration comment to reflect the modifications (Jan) Changes from v3: - Don't use fixed len types when not needed (Jan) - renamed domainconfig_encode_vl to sve_encode_vl - Use a sub argument of dom0= to enable the feature (Jan) - Add parse_integer() function 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| 20 ++-- xen/arch/arm/arm64/sve.c | 20 xen/arch/arm/domain_build.c | 26 ++ xen/arch/arm/include/asm/arm64/sve.h | 10 ++ xen/common/kernel.c | 28 xen/include/xen/lib.h| 10 ++ 6 files changed, 112 insertions(+), 2 deletions(-) diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index e0b89b7d3319..47e5b4eb6199 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -777,9 +777,9 @@ Specify the bit width of the DMA heap. ### dom0 = List of [ pv | pvh, shadow=, verbose=, -cpuid-faulting=, msr-relaxed= ] +cpuid-faulting=, msr-relaxed= ] (x86) -Applicability: x86 += List of [ sve= ] (Arm) Controls for how dom0 is constructed on x86 systems. @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. If using this option is necessary to fix an issue, please report a bug. +Enables features on dom0 on Arm systems. + +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and sets NIT: "Domain" is bit redundant here. +the maximum SVE vector length, the option is applicable only to AArch64 +guests. Here i would just remove "guests", just AArch64 is enough. I am ok if you choose to use "AArch64 Dom0 kernels" So far we have no use of AArch64 in our documentation. We have a few use of Arm64 (with various uppercase). In the code base, we seem to have a mix of AArch64 and Arm64. At the moment, I am not going to ask for consistency in the code. But we should aim to not introduce inconsistency in the documentation. I don't have a strong opinion whether we should use aarch64 or arm64. My only request is to be consistent. Cheers, -- Julien Grall
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
Hi Luca, > On 23 May 2023, at 09:43, Luca Fancellu wrote: > > Add a command line parameter to allow Dom0 the use of SVE resources, > the command line parameter sve=, sub argument of dom0=, > controls the feature on this domain and sets the maximum SVE vector > length for Dom0. > > Add a new function, parse_signed_integer(), to parse an integer > command line argument. > > Signed-off-by: Luca Fancellu Reviewed-by: Bertrand Marquis with ... > --- > Changes from v6: > - Fixed case for e==NULL in parse_signed_integer, drop parenthesis > from if conditions, delete inline sve_domctl_vl_param and rely on > DCE from the compiler (Jan) > - Drop parenthesis from opt_dom0_sve (Julien) > - Do not continue if 'sve' is in command line args but > CONFIG_ARM64_SVE is not selected: > https://lore.kernel.org/all/7614ae25-f59d-430a-9c3e-30b1ce0e1...@arm.com/ > Changes from v5: > - stop the domain if VL error occurs (Julien, Bertrand) > - update the documentation > - Rename sve_sanitize_vl_param to sve_domctl_vl_param to > mark the fact that we are sanitizing a parameter coming from > the user before encoding it into sve_vl in domctl structure. > (suggestion from Bertrand in a separate discussion) > - update comment in parse_signed_integer, return boolean in > sve_domctl_vl_param (Jan). > Changes from v4: > - Negative values as user param means max supported HW VL (Jan) > - update documentation, make use of no_config_param(), rename > parse_integer into parse_signed_integer and take long long *, > also put a comment on the -2 return condition, update > declaration comment to reflect the modifications (Jan) > Changes from v3: > - Don't use fixed len types when not needed (Jan) > - renamed domainconfig_encode_vl to sve_encode_vl > - Use a sub argument of dom0= to enable the feature (Jan) > - Add parse_integer() function > 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| 20 ++-- > xen/arch/arm/arm64/sve.c | 20 > xen/arch/arm/domain_build.c | 26 ++ > xen/arch/arm/include/asm/arm64/sve.h | 10 ++ > xen/common/kernel.c | 28 > xen/include/xen/lib.h| 10 ++ > 6 files changed, 112 insertions(+), 2 deletions(-) > > diff --git a/docs/misc/xen-command-line.pandoc > b/docs/misc/xen-command-line.pandoc > index e0b89b7d3319..47e5b4eb6199 100644 > --- a/docs/misc/xen-command-line.pandoc > +++ b/docs/misc/xen-command-line.pandoc > @@ -777,9 +777,9 @@ Specify the bit width of the DMA heap. > > ### dom0 > = List of [ pv | pvh, shadow=, verbose=, > -cpuid-faulting=, msr-relaxed= ] > +cpuid-faulting=, msr-relaxed= ] (x86) > > -Applicability: x86 > += List of [ sve= ] (Arm) > > Controls for how dom0 is constructed on x86 systems. > > @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. > > If using this option is necessary to fix an issue, please report a bug. > > +Enables features on dom0 on Arm systems. > + > +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and > sets > +the maximum SVE vector length, the option is applicable only to AArch64 > +guests. Here i would just remove "guests", just AArch64 is enough. I am ok if you choose to use "AArch64 Dom0 kernels" > +A value equal to 0 disables the feature, this is the default value. > +Values below 0 means the feature uses the maximum SVE vector length > +supported by hardware, if SVE is supported. > +Values above 0 explicitly set the maximum SVE vector length for Dom0, > +allowed values are from 128 to maximum 2048, being multiple of 128. > +Please note that when the user explicitly specifies the value, if that > value > +is above the hardware supported maximum SVE vector length, the domain > +creation will fail and the system will stop, the same will occur if the > +option is provided with a non zero value, but the platform doesn't > support > +SVE. > + I agree on the discussion with Jan here so you can keep my R-b if modified as discussed. Cheers Bertrand > ### dom0-cpuid > = List of comma separated booleans > > diff --git a/xen/arch/arm/arm64/sve.c b/xen/arch/arm/arm64/sve.c > index 84a6dedc1fd7..feaca2cf647d 100644 > --- a/xen/arch/arm/arm64/sve.c > +++ b/xen/arch/arm/arm64/sve.c > @@ -13,6 +13,9 @@ > #include > #include > > +/* opt_dom0_sve: allow Dom0 to use SVE and set maximum vector length. */ > +int __initdata opt_dom0_sve; > + > extern unsigned int sve_get_hw_vl(void); > > /* > @@ -152,6 +155,23
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
On 23.05.2023 13:57, Luca Fancellu wrote: >> On 23 May 2023, at 12:53, Jan Beulich wrote: >> On 23.05.2023 13:50, Luca Fancellu wrote: On 23 May 2023, at 11:31, Jan Beulich wrote: On 23.05.2023 12:21, Luca Fancellu wrote: >> On 23 May 2023, at 11:02, Jan Beulich wrote: >> On 23.05.2023 09:43, Luca Fancellu wrote: >>> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 >>> systems. >>> >>> If using this option is necessary to fix an issue, please report a >>> bug. >>> >>> +Enables features on dom0 on Arm systems. >>> + >>> +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain >>> and sets >>> +the maximum SVE vector length, the option is applicable only to >>> AArch64 >>> +guests. >> >> Why "guests"? Does the option affect more than Dom0? > > I used “guests” because in my mind I was referring to all the aarch64 OS > that can be used > as control domain, I can change it if it sounds bad. If you means OSes then better also say OSes. But maybe this doesn't need specifically expressing, by saying e.g. "..., the option is applicable only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen? >>> >>> I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say >>> “... AArch64 kernel guests.”? >> >> I'd recommend to avoid the term "guest" when you talk about Dom0 alone. >> Commonly "guest" means ordinary domains only, i.e. in particular excluding >> Dom0. What's wrong with "AArch64 Dom0 kernels"? > > Ok works for me, I will use “AArch64 Dom0 kernels", I thought “guests” were a > generic category > and then we have “privileged guests”, for example Dom0 or driver domain, and > “unprivileged guests” > like DomUs. Well, yes - "commonly" doesn't mean "always". >>> +A value equal to 0 disables the feature, this is the default value. >>> +Values below 0 means the feature uses the maximum SVE vector length >>> +supported by hardware, if SVE is supported. >>> +Values above 0 explicitly set the maximum SVE vector length for >>> Dom0, >>> +allowed values are from 128 to maximum 2048, being multiple of 128. >>> +Please note that when the user explicitly specifies the value, if >>> that value >>> +is above the hardware supported maximum SVE vector length, the >>> domain >>> +creation will fail and the system will stop, the same will occur >>> if the >>> +option is provided with a non zero value, but the platform doesn't >>> support >>> +SVE. >> >> Assuming this also covers the -1 case, I wonder if that isn't a little >> too >> strict. "Maximum supported" imo can very well be 0. > > Maximum supported, when platforms uses SVE, can be at minimum 128 by arm > specs. When there is SVE - sure. But when there's no SVE, 0 is kind of the implied length. And I'd view a command line option value of -1 quite okay in that case: They've asked for the maximum supported, so they'll get 0. No reason to crash the system during boot. >>> >>> Ok I see what you mean, for example when Kconfig SVE is enabled, but the >>> platform doesn’t >>> have SVE feature, requesting sve=-1 will keep the value to 0, and no system >>> will be stopped. >>> >>> Maybe I can say: >>> >>> “... the same will occur if the option is provided with a positive non zero >>> value, >>> but the platform doesn't support SVE." >> >> Right, provided that matches the implementation. > > Ok I will do the changes, can I retain your R-by? I suppose it covers also > documentation right? I guess whether doc is covered is fuzzy. Since the doc part is Arm- specific, I'd probably consider it not covered with the "!arm" that I appended. But whichever way you look at it, you can keep the tag in place. Jan
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
> On 23 May 2023, at 12:53, Jan Beulich wrote: > > On 23.05.2023 13:50, Luca Fancellu wrote: >>> On 23 May 2023, at 11:31, Jan Beulich wrote: >>> On 23.05.2023 12:21, Luca Fancellu wrote: > On 23 May 2023, at 11:02, Jan Beulich wrote: > On 23.05.2023 09:43, Luca Fancellu wrote: >> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. >> >> If using this option is necessary to fix an issue, please report a bug. >> >> +Enables features on dom0 on Arm systems. >> + >> +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain >> and sets >> +the maximum SVE vector length, the option is applicable only to >> AArch64 >> +guests. > > Why "guests"? Does the option affect more than Dom0? I used “guests” because in my mind I was referring to all the aarch64 OS that can be used as control domain, I can change it if it sounds bad. >>> >>> If you means OSes then better also say OSes. But maybe this doesn't need >>> specifically expressing, by saying e.g. "..., the option is applicable >>> only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen? >> >> I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say >> “... AArch64 kernel guests.”? > > I'd recommend to avoid the term "guest" when you talk about Dom0 alone. > Commonly "guest" means ordinary domains only, i.e. in particular excluding > Dom0. What's wrong with "AArch64 Dom0 kernels"? Ok works for me, I will use “AArch64 Dom0 kernels", I thought “guests” were a generic category and then we have “privilegedguests”, for example Dom0 or driver domain, and “unprivileged guests” like DomUs. > >> +A value equal to 0 disables the feature, this is the default value. >> +Values below 0 means the feature uses the maximum SVE vector length >> +supported by hardware, if SVE is supported. >> +Values above 0 explicitly set the maximum SVE vector length for >> Dom0, >> +allowed values are from 128 to maximum 2048, being multiple of 128. >> +Please note that when the user explicitly specifies the value, if >> that value >> +is above the hardware supported maximum SVE vector length, the >> domain >> +creation will fail and the system will stop, the same will occur if >> the >> +option is provided with a non zero value, but the platform doesn't >> support >> +SVE. > > Assuming this also covers the -1 case, I wonder if that isn't a little too > strict. "Maximum supported" imo can very well be 0. Maximum supported, when platforms uses SVE, can be at minimum 128 by arm specs. >>> >>> When there is SVE - sure. But when there's no SVE, 0 is kind of the implied >>> length. And I'd view a command line option value of -1 quite okay in that >>> case: They've asked for the maximum supported, so they'll get 0. No reason >>> to crash the system during boot. >> >> Ok I see what you mean, for example when Kconfig SVE is enabled, but the >> platform doesn’t >> have SVE feature, requesting sve=-1 will keep the value to 0, and no system >> will be stopped. >> >> Maybe I can say: >> >> “... the same will occur if the option is provided with a positive non zero >> value, >> but the platform doesn't support SVE." > > Right, provided that matches the implementation. Ok I will do the changes, can I retain your R-by? I suppose it covers also documentation right? > > Jan
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
On 23.05.2023 13:50, Luca Fancellu wrote: >> On 23 May 2023, at 11:31, Jan Beulich wrote: >> On 23.05.2023 12:21, Luca Fancellu wrote: On 23 May 2023, at 11:02, Jan Beulich wrote: On 23.05.2023 09:43, Luca Fancellu wrote: > @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. > >If using this option is necessary to fix an issue, please report a bug. > > +Enables features on dom0 on Arm systems. > + > +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain > and sets > +the maximum SVE vector length, the option is applicable only to > AArch64 > +guests. Why "guests"? Does the option affect more than Dom0? >>> >>> I used “guests” because in my mind I was referring to all the aarch64 OS >>> that can be used >>> as control domain, I can change it if it sounds bad. >> >> If you means OSes then better also say OSes. But maybe this doesn't need >> specifically expressing, by saying e.g. "..., the option is applicable >> only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen? > > I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say > “... AArch64 kernel guests.”? I'd recommend to avoid the term "guest" when you talk about Dom0 alone. Commonly "guest" means ordinary domains only, i.e. in particular excluding Dom0. What's wrong with "AArch64 Dom0 kernels"? > +A value equal to 0 disables the feature, this is the default value. > +Values below 0 means the feature uses the maximum SVE vector length > +supported by hardware, if SVE is supported. > +Values above 0 explicitly set the maximum SVE vector length for Dom0, > +allowed values are from 128 to maximum 2048, being multiple of 128. > +Please note that when the user explicitly specifies the value, if > that value > +is above the hardware supported maximum SVE vector length, the domain > +creation will fail and the system will stop, the same will occur if > the > +option is provided with a non zero value, but the platform doesn't > support > +SVE. Assuming this also covers the -1 case, I wonder if that isn't a little too strict. "Maximum supported" imo can very well be 0. >>> >>> Maximum supported, when platforms uses SVE, can be at minimum 128 by arm >>> specs. >> >> When there is SVE - sure. But when there's no SVE, 0 is kind of the implied >> length. And I'd view a command line option value of -1 quite okay in that >> case: They've asked for the maximum supported, so they'll get 0. No reason >> to crash the system during boot. > > Ok I see what you mean, for example when Kconfig SVE is enabled, but the > platform doesn’t > have SVE feature, requesting sve=-1 will keep the value to 0, and no system > will be stopped. > > Maybe I can say: > > “... the same will occur if the option is provided with a positive non zero > value, > but the platform doesn't support SVE." Right, provided that matches the implementation. Jan
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
> On 23 May 2023, at 11:31, Jan Beulich wrote: > > On 23.05.2023 12:21, Luca Fancellu wrote: >>> On 23 May 2023, at 11:02, Jan Beulich wrote: >>> On 23.05.2023 09:43, Luca Fancellu wrote: @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. If using this option is necessary to fix an issue, please report a bug. +Enables features on dom0 on Arm systems. + +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and sets +the maximum SVE vector length, the option is applicable only to AArch64 +guests. >>> >>> Why "guests"? Does the option affect more than Dom0? >> >> I used “guests” because in my mind I was referring to all the aarch64 OS >> that can be used >> as control domain, I can change it if it sounds bad. > > If you means OSes then better also say OSes. But maybe this doesn't need > specifically expressing, by saying e.g. "..., the option is applicable > only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen? I think there is no limitation so Dom0 can be 32 bit or 64. Maybe I can say “... AArch64 kernel guests.”? > +A value equal to 0 disables the feature, this is the default value. +Values below 0 means the feature uses the maximum SVE vector length +supported by hardware, if SVE is supported. +Values above 0 explicitly set the maximum SVE vector length for Dom0, +allowed values are from 128 to maximum 2048, being multiple of 128. +Please note that when the user explicitly specifies the value, if that value +is above the hardware supported maximum SVE vector length, the domain +creation will fail and the system will stop, the same will occur if the +option is provided with a non zero value, but the platform doesn't support +SVE. >>> >>> Assuming this also covers the -1 case, I wonder if that isn't a little too >>> strict. "Maximum supported" imo can very well be 0. >> >> Maximum supported, when platforms uses SVE, can be at minimum 128 by arm >> specs. > > When there is SVE - sure. But when there's no SVE, 0 is kind of the implied > length. And I'd view a command line option value of -1 quite okay in that > case: They've asked for the maximum supported, so they'll get 0. No reason > to crash the system during boot. Ok I see what you mean, for example when Kconfig SVE is enabled, but the platform doesn’t have SVE feature, requesting sve=-1 will keep the value to 0, and no system will be stopped. Maybe I can say: “... the same will occur if the option is provided with a positive non zero value, but the platform doesn't support SVE." > > Jan
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
On 23.05.2023 12:21, Luca Fancellu wrote: >> On 23 May 2023, at 11:02, Jan Beulich wrote: >> On 23.05.2023 09:43, Luca Fancellu wrote: >>> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. >>> >>> If using this option is necessary to fix an issue, please report a bug. >>> >>> +Enables features on dom0 on Arm systems. >>> + >>> +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and >>> sets >>> +the maximum SVE vector length, the option is applicable only to AArch64 >>> +guests. >> >> Why "guests"? Does the option affect more than Dom0? > > I used “guests” because in my mind I was referring to all the aarch64 OS that > can be used > as control domain, I can change it if it sounds bad. If you means OSes then better also say OSes. But maybe this doesn't need specifically expressing, by saying e.g. "..., the option is applicable only on AArch64"? Or can a Dom0 be 32-bit on Arm64 Xen? >>> +A value equal to 0 disables the feature, this is the default value. >>> +Values below 0 means the feature uses the maximum SVE vector length >>> +supported by hardware, if SVE is supported. >>> +Values above 0 explicitly set the maximum SVE vector length for Dom0, >>> +allowed values are from 128 to maximum 2048, being multiple of 128. >>> +Please note that when the user explicitly specifies the value, if that >>> value >>> +is above the hardware supported maximum SVE vector length, the domain >>> +creation will fail and the system will stop, the same will occur if the >>> +option is provided with a non zero value, but the platform doesn't >>> support >>> +SVE. >> >> Assuming this also covers the -1 case, I wonder if that isn't a little too >> strict. "Maximum supported" imo can very well be 0. > > Maximum supported, when platforms uses SVE, can be at minimum 128 by arm > specs. When there is SVE - sure. But when there's no SVE, 0 is kind of the implied length. And I'd view a command line option value of -1 quite okay in that case: They've asked for the maximum supported, so they'll get 0. No reason to crash the system during boot. Jan
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
> On 23 May 2023, at 11:02, Jan Beulich wrote: > > On 23.05.2023 09:43, Luca Fancellu wrote: >> Add a command line parameter to allow Dom0 the use of SVE resources, >> the command line parameter sve=, sub argument of dom0=, >> controls the feature on this domain and sets the maximum SVE vector >> length for Dom0. >> >> Add a new function, parse_signed_integer(), to parse an integer >> command line argument. >> >> Signed-off-by: Luca Fancellu > > Reviewed-by: Jan Beulich # !arm > >> --- a/docs/misc/xen-command-line.pandoc >> +++ b/docs/misc/xen-command-line.pandoc >> @@ -777,9 +777,9 @@ Specify the bit width of the DMA heap. >> >> ### dom0 >> = List of [ pv | pvh, shadow=, verbose=, >> -cpuid-faulting=, msr-relaxed= ] >> +cpuid-faulting=, msr-relaxed= ] (x86) >> >> -Applicability: x86 >> += List of [ sve= ] (Arm) > > While in the text below you mention this is Arm64 only, I think the tag > here would better express this as well. Ok I can use Arm64 instead if there is no opposition from others > >> @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. >> >> If using this option is necessary to fix an issue, please report a bug. >> >> +Enables features on dom0 on Arm systems. >> + >> +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and >> sets >> +the maximum SVE vector length, the option is applicable only to AArch64 >> +guests. > > Why "guests"? Does the option affect more than Dom0? I used “guests” because in my mind I was referring to all the aarch64 OS that can be used as control domain, I can change it if it sounds bad. > >> +A value equal to 0 disables the feature, this is the default value. >> +Values below 0 means the feature uses the maximum SVE vector length >> +supported by hardware, if SVE is supported. >> +Values above 0 explicitly set the maximum SVE vector length for Dom0, >> +allowed values are from 128 to maximum 2048, being multiple of 128. >> +Please note that when the user explicitly specifies the value, if that >> value >> +is above the hardware supported maximum SVE vector length, the domain >> +creation will fail and the system will stop, the same will occur if the >> +option is provided with a non zero value, but the platform doesn't >> support >> +SVE. > > Assuming this also covers the -1 case, I wonder if that isn't a little too > strict. "Maximum supported" imo can very well be 0. Maximum supported, when platforms uses SVE, can be at minimum 128 by arm specs. > > Jan
Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature
On 23.05.2023 09:43, Luca Fancellu wrote: > Add a command line parameter to allow Dom0 the use of SVE resources, > the command line parameter sve=, sub argument of dom0=, > controls the feature on this domain and sets the maximum SVE vector > length for Dom0. > > Add a new function, parse_signed_integer(), to parse an integer > command line argument. > > Signed-off-by: Luca Fancellu Reviewed-by: Jan Beulich # !arm > --- a/docs/misc/xen-command-line.pandoc > +++ b/docs/misc/xen-command-line.pandoc > @@ -777,9 +777,9 @@ Specify the bit width of the DMA heap. > > ### dom0 > = List of [ pv | pvh, shadow=, verbose=, > -cpuid-faulting=, msr-relaxed= ] > +cpuid-faulting=, msr-relaxed= ] (x86) > > -Applicability: x86 > += List of [ sve= ] (Arm) While in the text below you mention this is Arm64 only, I think the tag here would better express this as well. > @@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems. > > If using this option is necessary to fix an issue, please report a bug. > > +Enables features on dom0 on Arm systems. > + > +* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and > sets > +the maximum SVE vector length, the option is applicable only to AArch64 > +guests. Why "guests"? Does the option affect more than Dom0? > +A value equal to 0 disables the feature, this is the default value. > +Values below 0 means the feature uses the maximum SVE vector length > +supported by hardware, if SVE is supported. > +Values above 0 explicitly set the maximum SVE vector length for Dom0, > +allowed values are from 128 to maximum 2048, being multiple of 128. > +Please note that when the user explicitly specifies the value, if that > value > +is above the hardware supported maximum SVE vector length, the domain > +creation will fail and the system will stop, the same will occur if the > +option is provided with a non zero value, but the platform doesn't > support > +SVE. Assuming this also covers the -1 case, I wonder if that isn't a little too strict. "Maximum supported" imo can very well be 0. Jan
[PATCH v7 07/12] xen: enable Dom0 to use SVE feature
Add a command line parameter to allow Dom0 the use of SVE resources,
the command line parameter sve=, sub argument of dom0=,
controls the feature on this domain and sets the maximum SVE vector
length for Dom0.
Add a new function, parse_signed_integer(), to parse an integer
command line argument.
Signed-off-by: Luca Fancellu
---
Changes from v6:
- Fixed case for e==NULL in parse_signed_integer, drop parenthesis
from if conditions, delete inline sve_domctl_vl_param and rely on
DCE from the compiler (Jan)
- Drop parenthesis from opt_dom0_sve (Julien)
- Do not continue if 'sve' is in command line args but
CONFIG_ARM64_SVE is not selected:
https://lore.kernel.org/all/7614ae25-f59d-430a-9c3e-30b1ce0e1...@arm.com/
Changes from v5:
- stop the domain if VL error occurs (Julien, Bertrand)
- update the documentation
- Rename sve_sanitize_vl_param to sve_domctl_vl_param to
mark the fact that we are sanitizing a parameter coming from
the user before encoding it into sve_vl in domctl structure.
(suggestion from Bertrand in a separate discussion)
- update comment in parse_signed_integer, return boolean in
sve_domctl_vl_param (Jan).
Changes from v4:
- Negative values as user param means max supported HW VL (Jan)
- update documentation, make use of no_config_param(), rename
parse_integer into parse_signed_integer and take long long *,
also put a comment on the -2 return condition, update
declaration comment to reflect the modifications (Jan)
Changes from v3:
- Don't use fixed len types when not needed (Jan)
- renamed domainconfig_encode_vl to sve_encode_vl
- Use a sub argument of dom0= to enable the feature (Jan)
- Add parse_integer() function
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| 20 ++--
xen/arch/arm/arm64/sve.c | 20
xen/arch/arm/domain_build.c | 26 ++
xen/arch/arm/include/asm/arm64/sve.h | 10 ++
xen/common/kernel.c | 28
xen/include/xen/lib.h| 10 ++
6 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/docs/misc/xen-command-line.pandoc
b/docs/misc/xen-command-line.pandoc
index e0b89b7d3319..47e5b4eb6199 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -777,9 +777,9 @@ Specify the bit width of the DMA heap.
### dom0
= List of [ pv | pvh, shadow=, verbose=,
-cpuid-faulting=, msr-relaxed= ]
+cpuid-faulting=, msr-relaxed= ] (x86)
-Applicability: x86
+= List of [ sve= ] (Arm)
Controls for how dom0 is constructed on x86 systems.
@@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems.
If using this option is necessary to fix an issue, please report a bug.
+Enables features on dom0 on Arm systems.
+
+* The `sve` integer parameter enables Arm SVE usage for Dom0 domain and sets
+the maximum SVE vector length, the option is applicable only to AArch64
+guests.
+A value equal to 0 disables the feature, this is the default value.
+Values below 0 means the feature uses the maximum SVE vector length
+supported by hardware, if SVE is supported.
+Values above 0 explicitly set the maximum SVE vector length for Dom0,
+allowed values are from 128 to maximum 2048, being multiple of 128.
+Please note that when the user explicitly specifies the value, if that
value
+is above the hardware supported maximum SVE vector length, the domain
+creation will fail and the system will stop, the same will occur if the
+option is provided with a non zero value, but the platform doesn't support
+SVE.
+
### dom0-cpuid
= List of comma separated booleans
diff --git a/xen/arch/arm/arm64/sve.c b/xen/arch/arm/arm64/sve.c
index 84a6dedc1fd7..feaca2cf647d 100644
--- a/xen/arch/arm/arm64/sve.c
+++ b/xen/arch/arm/arm64/sve.c
@@ -13,6 +13,9 @@
#include
#include
+/* opt_dom0_sve: allow Dom0 to use SVE and set maximum vector length. */
+int __initdata opt_dom0_sve;
+
extern unsigned int sve_get_hw_vl(void);
/*
@@ -152,6 +155,23 @@ void sve_restore_state(struct vcpu *v)
sve_load_ctx(v->arch.vfp.sve_zreg_ctx_end, v->arch.vfp.fpregs, 1);
}
+bool __init sve_domctl_vl_param(int val, unsigned int *out)
+{
+/*
+ * Negative SVE parameter value means to use the maximum supported
+ * vector length, otherwise if a positive value is provided, check if the
+ * vector length is a multiple of 128
+ */
+if ( val < 0 )
+*out = get_sys_vl_len();
+else if ( (val % SVE_VL_MULTIPLE_VAL) == 0 )
+
