Re: [PATCH v7 11/12] xen/arm: add sve property for dom0less domUs

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Luca,

> On 23 May 2023, at 09:43, Luca Fancellu <Luca.Fancellu@xxxxxxx> 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@xxxxxxx>

Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

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
>