Dave Martin [off-list ref] writes:

On Mon, Aug 21, 2017 at 10:33:55AM +0100, Alex Benn?e wrote:

quoted

Dave Martin [off-list ref] writes:

quoted

The SVE architecture adds some system registers, ID register fields
and a dedicated ESR exception class.

This patch adds the appropriate definitions that will be needed by
the kernel.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/asm/esr.h     |  3 ++-
 arch/arm64/include/asm/kvm_arm.h |  1 +
 arch/arm64/include/asm/sysreg.h  | 16 ++++++++++++++++
 arch/arm64/kernel/traps.c        |  1 +
 4 files changed, 20 insertions(+), 1 deletion(-)

[...]

quoted

quoted

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 248339e..2d259e8 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h

[...]

quoted

quoted

+#define SYS_ZCR_EL1			sys_reg(3, 0, 1, 2, 0)
+

I'll have to take these on trust. They are mentioned in both the ARM ARM
and the SVE supplement but I can't see any actual definitions of them.

[I note from subsequent replies you've now found this in the
accompanying HTML]

[...]

quoted

quoted

+#define CPACR_EL1_ZEN_EL1EN	(1 << 16)
+#define CPACR_EL1_ZEN_EL0EN	(1 << 17)
+#define CPACR_EL1_ZEN		(CPACR_EL1_ZEN_EL1EN |
CPACR_EL1_ZEN_EL0EN)

This is a little weird as it is a 2 bit field in which 00 and 11 are not
simply the sum of their bits. If the code wrote CPACR_EL1_ZEN_EL0EN |
CPACR_EL1_ZEN_EL1EN to the CPACR_EL1 you wouldn't get the expected behaviour.

This seemed natural-ish if you believe that disabling at EL1 implies
disabling at EL0.  This is consistent with the way the traps at EL2/3
work, and lack of this property would be a sort of privilege inversion.

The meanings of the bits are not orthogonal, but it's still useful to be
able to twiddle EL0EN by itself when EL1EN is set (since we wan't to
control access for EL0 but leave EL1 access enabled).

Maybe comments would be sufficient:

#define CPACR_EL1_ZEN_EL1EN ... /* enable EL1 access */
#define CPACR_EL1_ZEN_EL0EN ... /* enable EL0 access, if EL1EN is also
set */

Certainly it would help. I'll see as I go through the rest of the code
but it looks like a potential bear trap we should at least mitigate.

I'm not sure how to make the names both reasonably terse and self-
escribing, but I'm open to ideas.

Sorry no decent ideas. Naming things is hard as they say.

BTW see the follow-up mail for the other things I found....

--
Alex Benn?e