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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.03.2023 13:48, Luca Fancellu wrote:
>> On 28 Mar 2023, at 11:08, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 27.03.2023 12:59, Luca Fancellu wrote:
>>> @@ -838,6 +838,18 @@ 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.
>>> +    Values above 0 means feature is enabled for Dom0, otherwise feature is
>>> +    disabled.
>>
>> Nit: "above" suggests negative values may also enable the feature, which
>> I'm sure isn't intended. You may want to consider using negative values
>> to signal "use length supported by hardware".
> 
> This is a very good suggestion, do you think I should restrict only to one 
> negative value,
> for example -1, instead of every negative value?

That highly depends on whether there's any foreseeable use for other negative
values. I can't imagine such, so I'm inclined to say that "just negative" is
all that matters.

>>> +    Please note that the platform can supports a lower value, if the 
>>> requested
>>
>> Maybe better "... may only support ..."?
> 
> ok
> 
>>
>>> +    value is above the supported one, the domain creation will fail and the
>>> +    system will stop.
>>
>> Such behavior may be acceptable for DomU-s which aren't essential for the
>> system (i.e. possibly excluding ones in dom0less scenarios), but I don't
>> think that's very nice for Dom0. I'd rather suggest falling back to no
>> SVE in such an event.
> 
> I guess that with the introduction of a negative value meaning max supported
> VL, it is ok to stop the system if the user provides a custom VL value that is
> not OK. I remember Julien asked to stop the creation of Dom0 in such a case on
> the RFC serie.

Oh, okay. I don't mean to override a maintainer's view. I don't see the
connection to assigning meaning to negative values though - preventing
successful (even if functionally restricted) boot is imo never a good
thing, when it can easily be avoided.

Jan