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

[Bertrand Marquis <Bertrand.Marquis@arm.com>]

Hi Luca,

> On 20 Apr 2023, at 09:58, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
> 
>>> 
>>> Hi Bertrand,
>>> 
>>> These are the changes I’m doing to this patch to address your comment, are you ok with them?
>>> 
>>> diff --git a/xen/arch/arm/arm64/sve.c b/xen/arch/arm/arm64/sve.c
>>> index f0eab18dc384..1fef466ba0aa 100644
>>> --- a/xen/arch/arm/arm64/sve.c
>>> +++ b/xen/arch/arm/arm64/sve.c
>>> @@ -91,35 +91,35 @@ int sve_context_init(struct vcpu *v)
>>>   if ( !ctx )
>>>       return -ENOMEM;
>>> 
>>> -    v->arch.vfp.sve_context = ctx;
>>> +    /* Point to the end of Z0-Z31 memory, just before FFR memory */
>>> +    v->arch.vfp.sve_zreg_ctx_end = ctx +
>>> +        (sve_zreg_ctx_size(sve_vl_bits) / sizeof(uint64_t));
>>> 
>>>   return 0;
>>> }
>>> 
>>> void sve_context_free(struct vcpu *v)
>>> {
>>> -    XFREE(v->arch.vfp.sve_context);
>>> +    unsigned int sve_vl_bits = sve_decode_vl(v->domain->arch.sve_vl);
>>> +
>>> +    /* Point back to the beginning of Z0-Z31 + FFR memory */
>>> +    v->arch.vfp.sve_zreg_ctx_end = v->arch.vfp.sve_zreg_ctx_end -
>>> +        (sve_zreg_ctx_size(sve_vl_bits) / sizeof(uint64_t));
>> 
>> Please use a local variable here instead.
>> 
>> For the rest all good yes, it makes the save/restore code simpler :-)
>> 
> 
> I did at the beginning, but then I realised XFREE would set to NULL the local variable instead,
> I could open code it and call xfree on the local variable and put v->arch.vfp.sve_zreg_ctx_end
> to NULL afterwards, but Julien asked me to use XFREE.
> 
> What is your preference here?
> 
I did not realised XFREE was actually also setting it to NULL.

Then i would keep your code but use -= instead.

Cheers
Bertrand