Re: [PATCH v5 01/12] xen/arm: enable SVE extension for Xen

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Luca,

> On 14 Apr 2023, at 15:28, Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:
> 
> 
> 
>> On 13 Apr 2023, at 13:47, Bertrand Marquis <Bertrand.Marquis@xxxxxxx> wrote:
>> 
>> Hi Luca,
>> 
>>> On 12 Apr 2023, at 11:49, Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:
>>> 
>>> Enable Xen to handle the SVE extension, add code in cpufeature module
>>> to handle ZCR SVE register, disable trapping SVE feature on system
>>> boot only when SVE resources are accessed.
>>> While there, correct coding style for the comment on coprocessor
>>> trapping.
>>> 
>>> Now cptr_el2 is part of the domain context and it will be restored
>>> on context switch, this is a preparation for saving the SVE context
>>> which will be part of VFP operations, so restore it before the call
>>> to save VFP registers.
>>> To save an additional isb barrier, restore cptr_el2 before an
>>> existing isb barrier and move the call for saving VFP context after
>>> that barrier.
>>> 
>>> Change the KConfig entry to make ARM64_SVE symbol selectable, by
>>> default it will be not selected.
>>> 
>>> Create sve module and sve_asm.S that contains assembly routines for
>>> the SVE feature, this code is inspired from linux and it uses
>>> instruction encoding to be compatible with compilers that does not
>>> support SVE.
>>> 
>>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>>> ---
>>> Changes from v4:
>>> - don't use fixed types in vl_to_zcr, forgot to address that in
>>> v3, by mistake I changed that in patch 2, fixing now (Jan)
>>> Changes from v3:
>>> - no changes
>>> Changes from v2:
>>> - renamed sve_asm.S in sve-asm.S, new files should not contain
>>> underscore in the name (Jan)
>>> Changes from v1:
>>> - Add assert to vl_to_zcr, it is never called with vl==0, but just
>>> to be sure it won't in the future.
>>> Changes from RFC:
>>> - Moved restoring of cptr before an existing barrier (Julien)
>>> - Marked the feature as unsupported for now (Julien)
>>> - Trap and un-trap only when using SVE resources in
>>> compute_max_zcr() (Julien)
>>> ---
>>> xen/arch/arm/Kconfig                     | 10 +++--
>>> xen/arch/arm/arm64/Makefile              |  1 +
>>> xen/arch/arm/arm64/cpufeature.c          |  7 ++--
>>> xen/arch/arm/arm64/sve-asm.S             | 48 +++++++++++++++++++++++
>>> xen/arch/arm/arm64/sve.c                 | 50 ++++++++++++++++++++++++
>>> xen/arch/arm/cpufeature.c                |  6 ++-
>>> xen/arch/arm/domain.c                    |  9 +++--
>>> xen/arch/arm/include/asm/arm64/sve.h     | 43 ++++++++++++++++++++
>>> xen/arch/arm/include/asm/arm64/sysregs.h |  1 +
>>> xen/arch/arm/include/asm/cpufeature.h    | 14 +++++++
>>> xen/arch/arm/include/asm/domain.h        |  1 +
>>> xen/arch/arm/include/asm/processor.h     |  2 +
>>> xen/arch/arm/setup.c                     |  5 ++-
>>> xen/arch/arm/traps.c                     | 28 +++++++------
>>> 14 files changed, 201 insertions(+), 24 deletions(-)
>>> create mode 100644 xen/arch/arm/arm64/sve-asm.S
>>> create mode 100644 xen/arch/arm/arm64/sve.c
>>> create mode 100644 xen/arch/arm/include/asm/arm64/sve.h
>>> 
>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>> index 239d3aed3c7f..41f45d8d1203 100644
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -112,11 +112,15 @@ config ARM64_PTR_AUTH
>>> This feature is not supported in Xen.
>>> 
>>> config ARM64_SVE
>>> - def_bool n
>>> + bool "Enable Scalar Vector Extension support (UNSUPPORTED)" if UNSUPPORTED
>>> depends on ARM_64
>>> help
>>> -  Scalar Vector Extension support.
>>> -  This feature is not supported in Xen.
>>> +  Scalar Vector Extension (SVE/SVE2) support for guests.
>> 
>> I would prevent to mention SVE2 here unless both versions of SVE are 
>> supported with this config.
>> Is it the case ?
> 
> Hi Bertrand,
> 
> Yes both versions of SVE are supported with this config, SVE2 is a superset 
> of SVE that includes new
> instructions, but the work done in this serie for registers settings and 
> context switch will apply to both
> versions.

Good so this is ok then.

You can add my:
Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand


> 
>> 
>> Cheers
>> Bertrand