This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 06/27] arm64/sve: System register and exception syndrome definitions
- From: Alex Bennée <alex dot bennee at linaro dot org>
- To: Dave Martin <Dave dot Martin at arm dot com>
- Cc: linux-arch at vger dot kernel dot org, libc-alpha at sourceware dot org, Ard Biesheuvel <ard dot biesheuvel at linaro dot org>, Szabolcs Nagy <szabolcs dot nagy at arm dot com>, Catalin Marinas <catalin dot marinas at arm dot com>, Will Deacon <will dot deacon at arm dot com>, Richard Sandiford <richard dot sandiford at arm dot com>, kvmarm at lists dot cs dot columbia dot edu, linux-arm-kernel at lists dot infradead dot org
- Date: Mon, 21 Aug 2017 15:36:37 +0100
- Subject: Re: [PATCH 06/27] arm64/sve: System register and exception syndrome definitions
- Authentication-results: sourceware.org; auth=none
- References: <1502280338-23002-1-git-send-email-Dave.Martin@arm.com> <1502280338-23002-7-git-send-email-Dave.Martin@arm.com> <87a82t5kos.fsf@linaro.org> <20170821135654.GP6321@e103592.cambridge.arm.com>
Dave Martin <Dave.Martin@arm.com> writes:
> On Mon, Aug 21, 2017 at 10:33:55AM +0100, Alex Bennée wrote:
>>
>> Dave Martin <Dave.Martin@arm.com> writes:
>>
>> > 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(-)
>
> [...]
>
>> > 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
>
> [...]
>
>> > +#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]
>
> [...]
>
>> > +#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