[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.