| Message ID | 20230523074326.3035745-12-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SVE feature for arm guests | expand |
Hi Luca, > On 23 May 2023, at 09:43, Luca Fancellu <Luca.Fancellu@arm.com> wrote: > > Add a device tree property in the dom0less domU configuration > to enable the guest to use SVE. > > Update documentation. > > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Cheers Bertrand > --- > Changes from v6: > - Use ifdef in create_domUs and fail if 'sve' is used on systems > with CONFIG_ARM64_SVE not selected (Bertrand, Julien, Jan) > Changes from v5: > - Stop the domain creation if SVE not supported or SVE VL > errors (Julien, Bertrand) > - now sve_sanitize_vl_param is renamed to sve_domctl_vl_param > and returns a boolean, change the affected code. > - Reworded documentation. > Changes from v4: > - Now it is possible to specify the property "sve" for dom0less > device tree node without any value, that means the platform > supported VL will be used. > Changes from v3: > - Now domainconfig_encode_vl is named sve_encode_vl > Changes from v2: > - xen_domctl_createdomain field name has changed into sve_vl > and its value is the VL/128, use domainconfig_encode_vl > to encode a plain VL in bits. > Changes from v1: > - No changes > Changes from RFC: > - Changed documentation > --- > docs/misc/arm/device-tree/booting.txt | 16 +++++++++++++++ > xen/arch/arm/domain_build.c | 28 +++++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt > index 3879340b5e0a..32a0e508c471 100644 > --- a/docs/misc/arm/device-tree/booting.txt > +++ b/docs/misc/arm/device-tree/booting.txt > @@ -193,6 +193,22 @@ with the following properties: > Optional. Handle to a xen,cpupool device tree node that identifies the > cpupool where the guest will be started at boot. > > +- sve > + > + Optional. The `sve` property enables Arm SVE usage for the 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. > + Specifying this property with no value, means that the SVE vector length > + will be set equal to the maximum vector length supported by the platform. > + Values above 0 explicitly set the maximum SVE vector length for the domain, > + 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. > + > - xen,enhanced > > A string property. Possible property values are: > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 9202a96d9c28..ba4fe9e165ee 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -4008,6 +4008,34 @@ void __init create_domUs(void) > d_cfg.max_maptrack_frames = val; > } > > + if ( dt_get_property(node, "sve", &val) ) > + { > +#ifdef CONFIG_ARM64_SVE > + unsigned int sve_vl_bits; > + bool ret = false; > + > + if ( !val ) > + { > + /* Property found with no value, means max HW VL supported */ > + ret = sve_domctl_vl_param(-1, &sve_vl_bits); > + } > + else > + { > + if ( dt_property_read_u32(node, "sve", &val) ) > + ret = sve_domctl_vl_param(val, &sve_vl_bits); > + else > + panic("Error reading 'sve' property"); > + } > + > + if ( ret ) > + d_cfg.arch.sve_vl = sve_encode_vl(sve_vl_bits); > + else > + panic("SVE vector length error\n"); > +#else > + panic("'sve' property found, but CONFIG_ARM64_SVE not selected"); > +#endif > + } > + > /* > * The variable max_init_domid is initialized with zero, so here it's > * very important to use the pre-increment operator to call > -- > 2.34.1 >
Hi Luca, Sorry for jumping into this but I just wanted to read the dt binding doc and spotted one thing by accident. On 24/05/2023 17:20, Bertrand Marquis wrote: > > > Hi Luca, > >> On 23 May 2023, at 09:43, Luca Fancellu <Luca.Fancellu@arm.com> wrote: >> >> Add a device tree property in the dom0less domU configuration >> to enable the guest to use SVE. >> >> Update documentation. >> >> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > > Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> (...) >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 9202a96d9c28..ba4fe9e165ee 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -4008,6 +4008,34 @@ void __init create_domUs(void) >> d_cfg.max_maptrack_frames = val; >> } >> >> + if ( dt_get_property(node, "sve", &val) ) >> + { >> +#ifdef CONFIG_ARM64_SVE >> + unsigned int sve_vl_bits; >> + bool ret = false; >> + >> + if ( !val ) >> + { >> + /* Property found with no value, means max HW VL supported */ >> + ret = sve_domctl_vl_param(-1, &sve_vl_bits); >> + } >> + else >> + { >> + if ( dt_property_read_u32(node, "sve", &val) ) >> + ret = sve_domctl_vl_param(val, &sve_vl_bits); >> + else >> + panic("Error reading 'sve' property"); Both here and ... >> + } >> + >> + if ( ret ) >> + d_cfg.arch.sve_vl = sve_encode_vl(sve_vl_bits); >> + else >> + panic("SVE vector length error\n"); >> +#else >> + panic("'sve' property found, but CONFIG_ARM64_SVE not selected"); here you are missing \n at the end of string. If you take a look at panic() implementation, new line char is not added so in your case it would result in an ugly formatted panic message. ~Michal
> On 25 May 2023, at 09:52, Michal Orzel <michal.orzel@amd.com> wrote: > > Hi Luca, > > Sorry for jumping into this but I just wanted to read the dt binding doc and spotted one thing by accident. > > On 24/05/2023 17:20, Bertrand Marquis wrote: >> >> >> Hi Luca, >> >>> On 23 May 2023, at 09:43, Luca Fancellu <Luca.Fancellu@arm.com> wrote: >>> >>> Add a device tree property in the dom0less domU configuration >>> to enable the guest to use SVE. >>> >>> Update documentation. >>> >>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >> >> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> > > (...) >>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >>> index 9202a96d9c28..ba4fe9e165ee 100644 >>> --- a/xen/arch/arm/domain_build.c >>> +++ b/xen/arch/arm/domain_build.c >>> @@ -4008,6 +4008,34 @@ void __init create_domUs(void) >>> d_cfg.max_maptrack_frames = val; >>> } >>> >>> + if ( dt_get_property(node, "sve", &val) ) >>> + { >>> +#ifdef CONFIG_ARM64_SVE >>> + unsigned int sve_vl_bits; >>> + bool ret = false; >>> + >>> + if ( !val ) >>> + { >>> + /* Property found with no value, means max HW VL supported */ >>> + ret = sve_domctl_vl_param(-1, &sve_vl_bits); >>> + } >>> + else >>> + { >>> + if ( dt_property_read_u32(node, "sve", &val) ) >>> + ret = sve_domctl_vl_param(val, &sve_vl_bits); >>> + else >>> + panic("Error reading 'sve' property"); > Both here and ... > >>> + } >>> + >>> + if ( ret ) >>> + d_cfg.arch.sve_vl = sve_encode_vl(sve_vl_bits); >>> + else >>> + panic("SVE vector length error\n"); >>> +#else >>> + panic("'sve' property found, but CONFIG_ARM64_SVE not selected"); > here you are missing \n at the end of string. If you take a look at panic() implementation, > new line char is not added so in your case it would result in an ugly formatted panic message. Hi Michal, Thank you for pointing that out! Indeed there might be some issues, I will fix in the next push. @Bertrand, can I retain your R-by with this fix? > > ~Michal
Hi Luca, > On 25 May 2023, at 10:55, Luca Fancellu <Luca.Fancellu@arm.com> wrote: > > > >> On 25 May 2023, at 09:52, Michal Orzel <michal.orzel@amd.com> wrote: >> >> Hi Luca, >> >> Sorry for jumping into this but I just wanted to read the dt binding doc and spotted one thing by accident. >> >> On 24/05/2023 17:20, Bertrand Marquis wrote: >>> >>> >>> Hi Luca, >>> >>>> On 23 May 2023, at 09:43, Luca Fancellu <Luca.Fancellu@arm.com> wrote: >>>> >>>> Add a device tree property in the dom0less domU configuration >>>> to enable the guest to use SVE. >>>> >>>> Update documentation. >>>> >>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >>> >>> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> >> >> (...) >>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >>>> index 9202a96d9c28..ba4fe9e165ee 100644 >>>> --- a/xen/arch/arm/domain_build.c >>>> +++ b/xen/arch/arm/domain_build.c >>>> @@ -4008,6 +4008,34 @@ void __init create_domUs(void) >>>> d_cfg.max_maptrack_frames = val; >>>> } >>>> >>>> + if ( dt_get_property(node, "sve", &val) ) >>>> + { >>>> +#ifdef CONFIG_ARM64_SVE >>>> + unsigned int sve_vl_bits; >>>> + bool ret = false; >>>> + >>>> + if ( !val ) >>>> + { >>>> + /* Property found with no value, means max HW VL supported */ >>>> + ret = sve_domctl_vl_param(-1, &sve_vl_bits); >>>> + } >>>> + else >>>> + { >>>> + if ( dt_property_read_u32(node, "sve", &val) ) >>>> + ret = sve_domctl_vl_param(val, &sve_vl_bits); >>>> + else >>>> + panic("Error reading 'sve' property"); >> Both here and ... >> >>>> + } >>>> + >>>> + if ( ret ) >>>> + d_cfg.arch.sve_vl = sve_encode_vl(sve_vl_bits); >>>> + else >>>> + panic("SVE vector length error\n"); >>>> +#else >>>> + panic("'sve' property found, but CONFIG_ARM64_SVE not selected"); >> here you are missing \n at the end of string. If you take a look at panic() implementation, >> new line char is not added so in your case it would result in an ugly formatted panic message. > > Hi Michal, > > Thank you for pointing that out! Indeed there might be some issues, I will fix in the next push. > > @Bertrand, can I retain your R-by with this fix? Yes Bertrand > >> >> ~Michal
On 25/05/2023 10:55, Luca Fancellu wrote: > > >> On 25 May 2023, at 09:52, Michal Orzel <michal.orzel@amd.com> wrote: >> >> Hi Luca, >> >> Sorry for jumping into this but I just wanted to read the dt binding doc and spotted one thing by accident. >> >> On 24/05/2023 17:20, Bertrand Marquis wrote: >>> >>> >>> Hi Luca, >>> >>>> On 23 May 2023, at 09:43, Luca Fancellu <Luca.Fancellu@arm.com> wrote: >>>> >>>> Add a device tree property in the dom0less domU configuration >>>> to enable the guest to use SVE. >>>> >>>> Update documentation. >>>> >>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >>> >>> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> >> >> (...) >>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >>>> index 9202a96d9c28..ba4fe9e165ee 100644 >>>> --- a/xen/arch/arm/domain_build.c >>>> +++ b/xen/arch/arm/domain_build.c >>>> @@ -4008,6 +4008,34 @@ void __init create_domUs(void) >>>> d_cfg.max_maptrack_frames = val; >>>> } >>>> >>>> + if ( dt_get_property(node, "sve", &val) ) >>>> + { >>>> +#ifdef CONFIG_ARM64_SVE >>>> + unsigned int sve_vl_bits; >>>> + bool ret = false; >>>> + >>>> + if ( !val ) >>>> + { >>>> + /* Property found with no value, means max HW VL supported */ >>>> + ret = sve_domctl_vl_param(-1, &sve_vl_bits); >>>> + } >>>> + else >>>> + { >>>> + if ( dt_property_read_u32(node, "sve", &val) ) >>>> + ret = sve_domctl_vl_param(val, &sve_vl_bits); >>>> + else >>>> + panic("Error reading 'sve' property"); >> Both here and ... >> >>>> + } >>>> + >>>> + if ( ret ) >>>> + d_cfg.arch.sve_vl = sve_encode_vl(sve_vl_bits); >>>> + else >>>> + panic("SVE vector length error\n"); >>>> +#else >>>> + panic("'sve' property found, but CONFIG_ARM64_SVE not selected"); >> here you are missing \n at the end of string. If you take a look at panic() implementation, >> new line char is not added so in your case it would result in an ugly formatted panic message. > > Hi Michal, > > Thank you for pointing that out! Indeed there might be some issues, I will fix in the next push. With that fixed, Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
Hi, On 23/05/2023 08:43, Luca Fancellu wrote: > Add a device tree property in the dom0less domU configuration > to enable the guest to use SVE. > > Update documentation. > > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > --- > Changes from v6: > - Use ifdef in create_domUs and fail if 'sve' is used on systems > with CONFIG_ARM64_SVE not selected (Bertrand, Julien, Jan) > Changes from v5: > - Stop the domain creation if SVE not supported or SVE VL > errors (Julien, Bertrand) > - now sve_sanitize_vl_param is renamed to sve_domctl_vl_param > and returns a boolean, change the affected code. > - Reworded documentation. > Changes from v4: > - Now it is possible to specify the property "sve" for dom0less > device tree node without any value, that means the platform > supported VL will be used. > Changes from v3: > - Now domainconfig_encode_vl is named sve_encode_vl > Changes from v2: > - xen_domctl_createdomain field name has changed into sve_vl > and its value is the VL/128, use domainconfig_encode_vl > to encode a plain VL in bits. > Changes from v1: > - No changes > Changes from RFC: > - Changed documentation > --- > docs/misc/arm/device-tree/booting.txt | 16 +++++++++++++++ > xen/arch/arm/domain_build.c | 28 +++++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt > index 3879340b5e0a..32a0e508c471 100644 > --- a/docs/misc/arm/device-tree/booting.txt > +++ b/docs/misc/arm/device-tree/booting.txt > @@ -193,6 +193,22 @@ with the following properties: > Optional. Handle to a xen,cpupool device tree node that identifies the > cpupool where the guest will be started at boot. > > +- sve > + > + Optional. The `sve` property enables Arm SVE usage for the domain and sets > + the maximum SVE vector length, the option is applicable only to AArch64 Depending on the discussion on the other patch, s/aarch64/arm64/. With the other comments addressed: Acked-by: Julien Grall <jgrall@amazon.com> > + guests. > + A value equal to 0 disables the feature, this is the default value. > + Specifying this property with no value, means that the SVE vector length > + will be set equal to the maximum vector length supported by the platform. > + Values above 0 explicitly set the maximum SVE vector length for the domain, > + 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. > + > - xen,enhanced > > A string property. Possible property values are: > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 9202a96d9c28..ba4fe9e165ee 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -4008,6 +4008,34 @@ void __init create_domUs(void) > d_cfg.max_maptrack_frames = val; > } > > + if ( dt_get_property(node, "sve", &val) ) > + { > +#ifdef CONFIG_ARM64_SVE > + unsigned int sve_vl_bits; > + bool ret = false; > + > + if ( !val ) > + { > + /* Property found with no value, means max HW VL supported */ > + ret = sve_domctl_vl_param(-1, &sve_vl_bits); > + } > + else > + { > + if ( dt_property_read_u32(node, "sve", &val) ) > + ret = sve_domctl_vl_param(val, &sve_vl_bits); > + else > + panic("Error reading 'sve' property"); > + } > + > + if ( ret ) > + d_cfg.arch.sve_vl = sve_encode_vl(sve_vl_bits); > + else > + panic("SVE vector length error\n"); > +#else > + panic("'sve' property found, but CONFIG_ARM64_SVE not selected"); > +#endif > + } > + > /* > * The variable max_init_domid is initialized with zero, so here it's > * very important to use the pre-increment operator to call
diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt index 3879340b5e0a..32a0e508c471 100644 --- a/docs/misc/arm/device-tree/booting.txt +++ b/docs/misc/arm/device-tree/booting.txt @@ -193,6 +193,22 @@ with the following properties: Optional. Handle to a xen,cpupool device tree node that identifies the cpupool where the guest will be started at boot. +- sve + + Optional. The `sve` property enables Arm SVE usage for the 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. + Specifying this property with no value, means that the SVE vector length + will be set equal to the maximum vector length supported by the platform. + Values above 0 explicitly set the maximum SVE vector length for the domain, + 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. + - xen,enhanced A string property. Possible property values are: diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 9202a96d9c28..ba4fe9e165ee 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -4008,6 +4008,34 @@ void __init create_domUs(void) d_cfg.max_maptrack_frames = val; } + if ( dt_get_property(node, "sve", &val) ) + { +#ifdef CONFIG_ARM64_SVE + unsigned int sve_vl_bits; + bool ret = false; + + if ( !val ) + { + /* Property found with no value, means max HW VL supported */ + ret = sve_domctl_vl_param(-1, &sve_vl_bits); + } + else + { + if ( dt_property_read_u32(node, "sve", &val) ) + ret = sve_domctl_vl_param(val, &sve_vl_bits); + else + panic("Error reading 'sve' property"); + } + + if ( ret ) + d_cfg.arch.sve_vl = sve_encode_vl(sve_vl_bits); + else + panic("SVE vector length error\n"); +#else + panic("'sve' property found, but CONFIG_ARM64_SVE not selected"); +#endif + } + /* * The variable max_init_domid is initialized with zero, so here it's * very important to use the pre-increment operator to call
Add a device tree property in the dom0less domU configuration to enable the guest to use SVE. Update documentation. Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> --- Changes from v6: - Use ifdef in create_domUs and fail if 'sve' is used on systems with CONFIG_ARM64_SVE not selected (Bertrand, Julien, Jan) Changes from v5: - Stop the domain creation if SVE not supported or SVE VL errors (Julien, Bertrand) - now sve_sanitize_vl_param is renamed to sve_domctl_vl_param and returns a boolean, change the affected code. - Reworded documentation. Changes from v4: - Now it is possible to specify the property "sve" for dom0less device tree node without any value, that means the platform supported VL will be used. Changes from v3: - Now domainconfig_encode_vl is named sve_encode_vl Changes from v2: - xen_domctl_createdomain field name has changed into sve_vl and its value is the VL/128, use domainconfig_encode_vl to encode a plain VL in bits. Changes from v1: - No changes Changes from RFC: - Changed documentation --- docs/misc/arm/device-tree/booting.txt | 16 +++++++++++++++ xen/arch/arm/domain_build.c | 28 +++++++++++++++++++++++++++ 2 files changed, 44 insertions(+)