[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v6 01/12] xen/arm: enable SVE extension for Xen
> On 22 May 2023, at 10:30, Julien Grall <julien@xxxxxxx> wrote: > > Hi, > > On 22/05/2023 09:43, Luca Fancellu wrote: >>> On 22 May 2023, at 08:50, Jan Beulich <jbeulich@xxxxxxxx> wrote: >>> >>> On 19.05.2023 16:46, Julien Grall wrote: >>>> On 19/05/2023 15:26, Luca Fancellu wrote: >>>>>> On 18 May 2023, at 10:35, Julien Grall <julien@xxxxxxx> wrote: >>>>>>> +/* >>>>>>> + * Arm SVE feature code >>>>>>> + * >>>>>>> + * Copyright (C) 2022 ARM Ltd. >>>>>>> + */ >>>>>>> + >>>>>>> +#include <xen/types.h> >>>>>>> +#include <asm/arm64/sve.h> >>>>>>> +#include <asm/arm64/sysregs.h> >>>>>>> +#include <asm/processor.h> >>>>>>> +#include <asm/system.h> >>>>>>> + >>>>>>> +extern unsigned int sve_get_hw_vl(void); >>>>>>> + >>>>>>> +register_t compute_max_zcr(void) >>>>>>> +{ >>>>>>> + register_t cptr_bits = get_default_cptr_flags(); >>>>>>> + register_t zcr = vl_to_zcr(SVE_VL_MAX_BITS); >>>>>>> + unsigned int hw_vl; >>>>>>> + >>>>>>> + /* Remove trap for SVE resources */ >>>>>>> + WRITE_SYSREG(cptr_bits & ~HCPTR_CP(8), CPTR_EL2); >>>>>>> + isb(); >>>>>>> + >>>>>>> + /* >>>>>>> + * Set the maximum SVE vector length, doing that we will know the >>>>>>> VL >>>>>>> + * supported by the platform, calling sve_get_hw_vl() >>>>>>> + */ >>>>>>> + WRITE_SYSREG(zcr, ZCR_EL2); >>>>>> >>>>>> From my reading of the Arm (D19-6331, ARM DDI 0487J.a), a direct write >>>>>> to a system register would need to be followed by an context >>>>>> synchronization event (e.g. isb()) before the software can rely on the >>>>>> value. >>>>>> >>>>>> In this situation, AFAICT, the instruciton in sve_get_hw_vl() will use >>>>>> the content of ZCR_EL2. So don't we need an ISB() here? >>>>> >>>>> From what I’ve read in the manual for ZCR_ELx: >>>>> >>>>> An indirect read of ZCR_EL2.LEN appears to occur in program order >>>>> relative to a direct write of >>>>> the same register, without the need for explicit synchronization >>>>> >>>>> I’ve interpreted it as “there is no need to sync before write” and I’ve >>>>> looked into Linux and it does not >>>>> Appear any synchronisation mechanism after a write to that register, but >>>>> if I am wrong I can for sure >>>>> add an isb if you prefer. >>>> >>>> Ah, I was reading the generic section about synchronization and didn't >>>> realize there was a paragraph in the ZCR_EL2 section as well. >>>> >>>> Reading the new section, I agree with your understanding. The isb() is >>>> not necessary. >>> >>> And RDVL counts as an "indirect read"? I'm pretty sure "normal" SVE insn >>> use is falling in that category, but RDVL might also be viewed as more >>> similar to MRS in this regard? While the construct CurrentVL is used in >>> either case, I'm still not sure this goes without saying. >> Hi Jan, >> Looking into the Linux code, in function vec_probe_vqs(...) in >> arch/arm64/kernel/fpsimd.c, >> ZCR_EL1 is written, without synchronisation, and afterwards RDVL is used. > > You are making the assumption that the Linux code is correct. It is mostly > likely the case, but in general it is best to justify barriers based on the > Arm Arm because it is authoritative. > > In this case, the Arm Arm is pretty clear on the difference between indirect > read and direct read (see D19-63333 ARM DDI 0487J.A). The latter only refers > to use of the instruction of MRS. RDVL is its own instruction and therefore > this is an indirect read. Yes you are right > > Cheers, > > -- > Julien Grall
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |