> On 22 May 2023, at 10:30, Julien Grall <jul...@xen.org> wrote:
>
> Hi,
>
> On 22/05/2023 09:43, Luca Fancellu wrote:
>>> On 22 May 2023, at 08:50, Jan Beulich <jbeul...@suse.com> 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 <jul...@xen.org> 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
