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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.05.2023 13:57, Luca Fancellu wrote:
>> On 23 May 2023, at 12:53, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 23.05.2023 13:50, Luca Fancellu wrote:
>>>> On 23 May 2023, at 11:31, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>> On 23.05.2023 12:21, Luca Fancellu wrote:
>>>>>> On 23 May 2023, at 11:02, Jan Beulich <jbeulich@xxxxxxxx> 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