Re: [PATCH v5 05/12] arm/sve: save/restore SVE context switch

[Julien Grall <julien@xen.org>]

Hi Luca,

On 13/04/2023 15:35, Luca Fancellu wrote:
> 
> 
>> On 13 Apr 2023, at 14:11, Julien Grall <julien@xen.org> wrote:
>>
>>
>>
>> On 12/04/2023 10:49, Luca Fancellu wrote:
>>> Save/restore context switch for SVE, allocate memory to contain
>>> the Z0-31 registers whose length is maximum 2048 bits each and
>>> FFR who can be maximum 256 bits, the allocated memory depends on
>>> how many bits is the vector length for the domain and how many bits
>>> are supported by the platform.
>>> Save P0-15 whose length is maximum 256 bits each, in this case the
>>> memory used is from the fpregs field in struct vfp_state,
>>> because V0-31 are part of Z0-31 and this space would have been
>>> unused for SVE domain otherwise.
>>> Create zcr_el{1,2} fields in arch_vcpu, initialise zcr_el2 on vcpu
>>> creation given the requested vector length and restore it on
>>> context switch, save/restore ZCR_EL1 value as well.
>>> Remove headers from sve.c that are already included using
>>> xen/sched.h.
>> I dislike this because ...
>>
>>> diff --git a/xen/arch/arm/arm64/sve.c b/xen/arch/arm/arm64/sve.c
>>> index 78f7482619da..5485648850a0 100644
>>> --- a/xen/arch/arm/arm64/sve.c
>>> +++ b/xen/arch/arm/arm64/sve.c
>>> @@ -5,14 +5,29 @@
>>>    * Copyright (C) 2022 ARM Ltd.
>>>    */
>>>   -#include <xen/types.h>
>>> -#include <asm/cpufeature.h>
>>
>> ... it is not entirely obvious that sched.h will import asm/cpufeatures.h. This could easily change in the future and would only require us to re-add those includes.
> 
> Ok I will reintroduce #include <asm/cpufeature.h>, do I understand correctly that is the only header you would like me to retain?

My remark was for all the headers you removed. It is not obvious that 
any of them will be included by sched.h.

Cheers,

-- 
Julien Grall