Re: [PATCH v5 07/12] xen: enable Dom0 to use SVE feature

[Jan Beulich <jbeulich@suse.com>]

On 18.04.2023 14:47, Bertrand Marquis wrote:
>> On 12 Apr 2023, at 11:49, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
>> @@ -118,3 +121,21 @@ void sve_restore_state(struct vcpu *v)
>>
>>     sve_load_ctx(sve_ctx_zreg_end, v->arch.vfp.fpregs, 1);
>> }
>> +
>> +int __init sve_sanitize_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 and not bigger than the maximum value
>> +     * 2048
>> +     */
>> +    if ( val < 0 )
>> +        *out = get_sys_vl_len();
>> +    else if ( ((val % SVE_VL_MULTIPLE_VAL) == 0) && (val <= SVE_VL_MAX_BITS) )
>> +        *out = val;
> 
> Shouldn't you also check if it is not greater than the maximum vector length ?

Perhaps not "also" but "instead", because the supported length shouldn't be
larger than SVE_VL_MAX_BITS (or if there was a risk that it might be, that
should be taken care of elsewhere, e.g. in get_sys_vl_len(), not here).

>> @@ -61,6 +62,21 @@ custom_param("dom0_mem", parse_dom0_mem);
>>
>> int __init parse_arch_dom0_param(const char *s, const char *e)
>> {
>> +    long long val;
>> +
>> +    if ( !parse_signed_integer("sve", s, e, &val) )
>> +    {
>> +#ifdef CONFIG_ARM64_SVE
>> +        if ( (val >= INT_MIN) && (val <= INT_MAX) )
>> +            opt_dom0_sve = val;
>> +        else
>> +            printk(XENLOG_INFO "'sve=%lld' value out of range!\n", val);
>> +#else
>> +        no_config_param("ARM64_SVE", "sve", s, e);
>> +#endif
> 
> Correct me if my understanding is wrong but here you just ignore the sve
> parameter if SVE is not supported by Xen ?
> 
> I am a bit wondering if we should not just refuse it here as the user might
> wrongly think that his parameter had some effect.
> 
> Or is it a usual way to handle this case ?

It is, or else we'd need to alter what no_config_param() does. Plus ignoring
the argument when !ARM64_SVE is no different from passing the same argument
to an older Xen version, or from passing any unknown one.

Jan