| Message ID | 20190128173940.25813-5-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | HWCAP_CPUID registers for aarch64 | expand |
Alex Bennée <alex.bennee@linaro.org> writes: > This tests a bunch of registers that the kernel allows userspace to > read including the CPUID registers. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> I'll also merge the following fix: modified tests/tcg/aarch64/sysregs.c @@ -11,6 +11,10 @@ #include <string.h> #include <stdbool.h> +#ifndef HWCAP_CPUID +#define HWCAP_CPUID (1 << 11) +#endif + int failed_mask_count; #define get_cpu_reg(id) ({ \ > > --- > v4 > - also test for extra bits that shouldn't be exposed > --- > tests/tcg/aarch64/Makefile.target | 2 +- > tests/tcg/aarch64/sysregs.c | 120 ++++++++++++++++++++++++++++++ > 2 files changed, 121 insertions(+), 1 deletion(-) > create mode 100644 tests/tcg/aarch64/sysregs.c > > diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target > index 08c45b8470..cc1a7eb486 100644 > --- a/tests/tcg/aarch64/Makefile.target > +++ b/tests/tcg/aarch64/Makefile.target > @@ -7,7 +7,7 @@ VPATH += $(AARCH64_SRC) > > # we don't build any of the ARM tests > AARCH64_TESTS=$(filter-out $(ARM_TESTS), $(TESTS)) > -AARCH64_TESTS+=fcvt > +AARCH64_TESTS+=fcvt sysregs > TESTS:=$(AARCH64_TESTS) > > fcvt: LDFLAGS+=-lm > diff --git a/tests/tcg/aarch64/sysregs.c b/tests/tcg/aarch64/sysregs.c > new file mode 100644 > index 0000000000..8e11288ee3 > --- /dev/null > +++ b/tests/tcg/aarch64/sysregs.c > @@ -0,0 +1,120 @@ > +/* > + * Check emulated system register access for linux-user mode. > + * > + * See: https://www.kernel.org/doc/Documentation/arm64/cpu-feature-registers.txt > + */ > + > +#include <asm/hwcap.h> > +#include <stdio.h> > +#include <sys/auxv.h> > +#include <signal.h> > +#include <string.h> > +#include <stdbool.h> > + > +int failed_mask_count; > + > +#define get_cpu_reg(id) ({ \ > + unsigned long __val = 0xdeadbeef; \ > + asm("mrs %0, "#id : "=r" (__val)); \ > + printf("%-20s: 0x%016lx\n", #id, __val); \ > + __val; \ > + }) > + > +#define get_cpu_reg_check_mask(id, mask) ({ \ > + unsigned long __cval = get_cpu_reg(id); \ > + unsigned long __extra = __cval & ~mask; \ > + if (__extra) { \ > + printf("%-20s: 0x%016lx\n", " !!extra bits!!", __extra); \ > + failed_mask_count++; \ > + } \ > +}) > + > +bool should_fail; > +int should_fail_count; > +int should_not_fail_count; > +uintptr_t failed_pc[10]; > + > +void sigill_handler(int signo, siginfo_t *si, void *data) > +{ > + ucontext_t *uc = (ucontext_t *)data; > + > + if (should_fail) { > + should_fail_count++; > + } else { > + uintptr_t pc = (uintptr_t) uc->uc_mcontext.pc; > + failed_pc[should_not_fail_count++] = pc; > + } > + uc->uc_mcontext.pc += 4; > +} > + > +int main(void) > +{ > + struct sigaction sa; > + > + /* Hook in a SIGILL handler */ > + memset(&sa, 0, sizeof(struct sigaction)); > + sa.sa_flags = SA_SIGINFO; > + sa.sa_sigaction = &sigill_handler; > + sigemptyset(&sa.sa_mask); > + > + if (sigaction(SIGILL, &sa, 0) != 0) { > + perror("sigaction"); > + return 1; > + } > + > + /* since 4.12 */ > + printf("Checking CNT registers\n"); > + > + get_cpu_reg(ctr_el0); > + get_cpu_reg(cntvct_el0); > + get_cpu_reg(cntfrq_el0); > + > + /* when (getauxval(AT_HWCAP) & HWCAP_CPUID), since 4.11*/ > + if (!(getauxval(AT_HWCAP) & HWCAP_CPUID)) { > + printf("CPUID registers unavailable\n"); > + return 1; > + } else { > + printf("Checking CPUID registers\n"); > + } > + > + /* > + * Some registers only expose some bits to user-space. Anything > + * that is IMDEF is exported as 0 to user-space. > + */ > + get_cpu_reg_check_mask(id_aa64isar0_el1, 0x000fffffff0ffff0ULL); > + get_cpu_reg_check_mask(id_aa64isar1_el1, 0x00000000ffffffffULL); > + get_cpu_reg(id_aa64mmfr0_el1); > + get_cpu_reg(id_aa64mmfr1_el1); > + get_cpu_reg_check_mask(id_aa64pfr0_el1, 0x000f000f0ff0000ULL); > + get_cpu_reg(id_aa64pfr1_el1); > + get_cpu_reg(id_aa64dfr0_el1); > + get_cpu_reg(id_aa64dfr1_el1); > + > + get_cpu_reg_check_mask(midr_el1, 0x00000000ffffffffULL); > + get_cpu_reg(mpidr_el1); > + /* REVIDR is all IMPDEF so should be all zeros to user-space */ > + get_cpu_reg_check_mask(revidr_el1, 0x0); > + > + printf("Remaining registers should fail\n"); > + should_fail = true; > + > + /* Unexposed register access causes SIGILL */ > + get_cpu_reg(id_mmfr0_el1); > + > + if (should_not_fail_count > 0) { > + int i; > + for (i = 0; i < should_not_fail_count; i++) { > + uintptr_t pc = failed_pc[i]; > + uint32_t insn = *(uint32_t *) pc; > + printf("insn %#x @ %#lx unexpected FAIL\n", insn, pc); > + } > + return 1; > + } > + > + if (failed_mask_count > 0) { > + printf("Extra information leaked to user-space!\n"); > + return 1; > + } > + > + return should_fail_count == 1 ? 0 : 1; > +} -- Alex Bennée
On Mon, 28 Jan 2019 at 17:39, Alex Bennée <alex.bennee@linaro.org> wrote: > > This tests a bunch of registers that the kernel allows userspace to > read including the CPUID registers. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > > --- > v4 > - also test for extra bits that shouldn't be exposed > --- > tests/tcg/aarch64/Makefile.target | 2 +- > tests/tcg/aarch64/sysregs.c | 120 ++++++++++++++++++++++++++++++ > 2 files changed, 121 insertions(+), 1 deletion(-) > create mode 100644 tests/tcg/aarch64/sysregs.c > > diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target > index 08c45b8470..cc1a7eb486 100644 > --- a/tests/tcg/aarch64/Makefile.target > +++ b/tests/tcg/aarch64/Makefile.target > @@ -7,7 +7,7 @@ VPATH += $(AARCH64_SRC) > > # we don't build any of the ARM tests > AARCH64_TESTS=$(filter-out $(ARM_TESTS), $(TESTS)) > -AARCH64_TESTS+=fcvt > +AARCH64_TESTS+=fcvt sysregs > TESTS:=$(AARCH64_TESTS) > > fcvt: LDFLAGS+=-lm > diff --git a/tests/tcg/aarch64/sysregs.c b/tests/tcg/aarch64/sysregs.c > new file mode 100644 > index 0000000000..8e11288ee3 > --- /dev/null > +++ b/tests/tcg/aarch64/sysregs.c > @@ -0,0 +1,120 @@ > +/* > + * Check emulated system register access for linux-user mode. > + * > + * See: https://www.kernel.org/doc/Documentation/arm64/cpu-feature-registers.txt > + */ This needs the usual copyright line and license statement. > + > +#include <asm/hwcap.h> > +#include <stdio.h> > +#include <sys/auxv.h> > +#include <signal.h> > +#include <string.h> > +#include <stdbool.h> > + > +int failed_mask_count; > + > +#define get_cpu_reg(id) ({ \ > + unsigned long __val = 0xdeadbeef; \ > + asm("mrs %0, "#id : "=r" (__val)); \ > + printf("%-20s: 0x%016lx\n", #id, __val); \ > + __val; \ > + }) > + > +#define get_cpu_reg_check_mask(id, mask) ({ \ > + unsigned long __cval = get_cpu_reg(id); \ > + unsigned long __extra = __cval & ~mask; \ > + if (__extra) { \ > + printf("%-20s: 0x%016lx\n", " !!extra bits!!", __extra); \ > + failed_mask_count++; \ > + } \ > +}) A brief comment describing what the arguments to these macros do would be helpful if we ever have to add more registers in future. > + > +bool should_fail; > +int should_fail_count; > +int should_not_fail_count; > +uintptr_t failed_pc[10]; > + > +void sigill_handler(int signo, siginfo_t *si, void *data) > +{ > + ucontext_t *uc = (ucontext_t *)data; > + > + if (should_fail) { > + should_fail_count++; > + } else { > + uintptr_t pc = (uintptr_t) uc->uc_mcontext.pc; > + failed_pc[should_not_fail_count++] = pc; > + } > + uc->uc_mcontext.pc += 4; > +} > + > +int main(void) > +{ > + struct sigaction sa; > + > + /* Hook in a SIGILL handler */ > + memset(&sa, 0, sizeof(struct sigaction)); > + sa.sa_flags = SA_SIGINFO; > + sa.sa_sigaction = &sigill_handler; > + sigemptyset(&sa.sa_mask); > + > + if (sigaction(SIGILL, &sa, 0) != 0) { > + perror("sigaction"); > + return 1; > + } > + > + /* since 4.12 */ This is a rather cryptic comment. > + printf("Checking CNT registers\n"); > + > + get_cpu_reg(ctr_el0); > + get_cpu_reg(cntvct_el0); > + get_cpu_reg(cntfrq_el0); > + > + /* when (getauxval(AT_HWCAP) & HWCAP_CPUID), since 4.11*/ Missing space before "*/". > + if (!(getauxval(AT_HWCAP) & HWCAP_CPUID)) { > + printf("CPUID registers unavailable\n"); > + return 1; > + } else { > + printf("Checking CPUID registers\n"); > + } > + > + /* > + * Some registers only expose some bits to user-space. Anything > + * that is IMDEF is exported as 0 to user-space. IMPDEF > + */ > + get_cpu_reg_check_mask(id_aa64isar0_el1, 0x000fffffff0ffff0ULL); I can't make sense of this mask value. Presumably it's supposed to be 1s where the value is visible, but the docs say that the visible bits for ID_AA64ISAR0_EL1 are [23..4] and [55..28], whereas the mask value here has 1s in [51..24] and [20..4], unless I've miscounted. > + get_cpu_reg_check_mask(id_aa64isar1_el1, 0x00000000ffffffffULL); > + get_cpu_reg(id_aa64mmfr0_el1); > + get_cpu_reg(id_aa64mmfr1_el1); > + get_cpu_reg_check_mask(id_aa64pfr0_el1, 0x000f000f0ff0000ULL); This seems to have [31..28] set but not [35..32]. The bits that should be [51..48] are also in the wrong place, and the whole number has one hex digit too few to be 64 bits. > + get_cpu_reg(id_aa64pfr1_el1); > + get_cpu_reg(id_aa64dfr0_el1); > + get_cpu_reg(id_aa64dfr1_el1); > + > + get_cpu_reg_check_mask(midr_el1, 0x00000000ffffffffULL); > + get_cpu_reg(mpidr_el1); > + /* REVIDR is all IMPDEF so should be all zeros to user-space */ > + get_cpu_reg_check_mask(revidr_el1, 0x0); Missing check of the mask value for ID_AA64MMFR2_EL1 ? Should have bits [35..32] visible. Missing checks for: ID_AA64ZFR0_EL1 ID_AA64AFR0_EL1 ID_AA64AFR1_EL1 Missing checks for all the parts of the ID space that are currently unallocated (and which should be exposed as RAZ/WI). Missing checks for registers which are exposed but with all features hidden, eg ID_PFR0_EL1 and the other aarch32 ID regs. The kernel returns some value, ie does not fault, all the architecturally defined registers in Op0=3, Op1=0, CRn=0, CRm=0,4,5,6,7. For CRm = 0 only op2 = {0,5,6} [MIDR_EL1, MPIDR_EL1, REVIDR_EL1] are valid, but for CRm = {4,5,6,7} the whole set of op2 = {0..7} are defined (and RAZ if not yet given some meaning). thanks -- PMM
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target index 08c45b8470..cc1a7eb486 100644 --- a/tests/tcg/aarch64/Makefile.target +++ b/tests/tcg/aarch64/Makefile.target @@ -7,7 +7,7 @@ VPATH += $(AARCH64_SRC) # we don't build any of the ARM tests AARCH64_TESTS=$(filter-out $(ARM_TESTS), $(TESTS)) -AARCH64_TESTS+=fcvt +AARCH64_TESTS+=fcvt sysregs TESTS:=$(AARCH64_TESTS) fcvt: LDFLAGS+=-lm diff --git a/tests/tcg/aarch64/sysregs.c b/tests/tcg/aarch64/sysregs.c new file mode 100644 index 0000000000..8e11288ee3 --- /dev/null +++ b/tests/tcg/aarch64/sysregs.c @@ -0,0 +1,120 @@ +/* + * Check emulated system register access for linux-user mode. + * + * See: https://www.kernel.org/doc/Documentation/arm64/cpu-feature-registers.txt + */ + +#include <asm/hwcap.h> +#include <stdio.h> +#include <sys/auxv.h> +#include <signal.h> +#include <string.h> +#include <stdbool.h> + +int failed_mask_count; + +#define get_cpu_reg(id) ({ \ + unsigned long __val = 0xdeadbeef; \ + asm("mrs %0, "#id : "=r" (__val)); \ + printf("%-20s: 0x%016lx\n", #id, __val); \ + __val; \ + }) + +#define get_cpu_reg_check_mask(id, mask) ({ \ + unsigned long __cval = get_cpu_reg(id); \ + unsigned long __extra = __cval & ~mask; \ + if (__extra) { \ + printf("%-20s: 0x%016lx\n", " !!extra bits!!", __extra); \ + failed_mask_count++; \ + } \ +}) + +bool should_fail; +int should_fail_count; +int should_not_fail_count; +uintptr_t failed_pc[10]; + +void sigill_handler(int signo, siginfo_t *si, void *data) +{ + ucontext_t *uc = (ucontext_t *)data; + + if (should_fail) { + should_fail_count++; + } else { + uintptr_t pc = (uintptr_t) uc->uc_mcontext.pc; + failed_pc[should_not_fail_count++] = pc; + } + uc->uc_mcontext.pc += 4; +} + +int main(void) +{ + struct sigaction sa; + + /* Hook in a SIGILL handler */ + memset(&sa, 0, sizeof(struct sigaction)); + sa.sa_flags = SA_SIGINFO; + sa.sa_sigaction = &sigill_handler; + sigemptyset(&sa.sa_mask); + + if (sigaction(SIGILL, &sa, 0) != 0) { + perror("sigaction"); + return 1; + } + + /* since 4.12 */ + printf("Checking CNT registers\n"); + + get_cpu_reg(ctr_el0); + get_cpu_reg(cntvct_el0); + get_cpu_reg(cntfrq_el0); + + /* when (getauxval(AT_HWCAP) & HWCAP_CPUID), since 4.11*/ + if (!(getauxval(AT_HWCAP) & HWCAP_CPUID)) { + printf("CPUID registers unavailable\n"); + return 1; + } else { + printf("Checking CPUID registers\n"); + } + + /* + * Some registers only expose some bits to user-space. Anything + * that is IMDEF is exported as 0 to user-space. + */ + get_cpu_reg_check_mask(id_aa64isar0_el1, 0x000fffffff0ffff0ULL); + get_cpu_reg_check_mask(id_aa64isar1_el1, 0x00000000ffffffffULL); + get_cpu_reg(id_aa64mmfr0_el1); + get_cpu_reg(id_aa64mmfr1_el1); + get_cpu_reg_check_mask(id_aa64pfr0_el1, 0x000f000f0ff0000ULL); + get_cpu_reg(id_aa64pfr1_el1); + get_cpu_reg(id_aa64dfr0_el1); + get_cpu_reg(id_aa64dfr1_el1); + + get_cpu_reg_check_mask(midr_el1, 0x00000000ffffffffULL); + get_cpu_reg(mpidr_el1); + /* REVIDR is all IMPDEF so should be all zeros to user-space */ + get_cpu_reg_check_mask(revidr_el1, 0x0); + + printf("Remaining registers should fail\n"); + should_fail = true; + + /* Unexposed register access causes SIGILL */ + get_cpu_reg(id_mmfr0_el1); + + if (should_not_fail_count > 0) { + int i; + for (i = 0; i < should_not_fail_count; i++) { + uintptr_t pc = failed_pc[i]; + uint32_t insn = *(uint32_t *) pc; + printf("insn %#x @ %#lx unexpected FAIL\n", insn, pc); + } + return 1; + } + + if (failed_mask_count > 0) { + printf("Extra information leaked to user-space!\n"); + return 1; + } + + return should_fail_count == 1 ? 0 : 1; +}
This tests a bunch of registers that the kernel allows userspace to read including the CPUID registers. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- v4 - also test for extra bits that shouldn't be exposed --- tests/tcg/aarch64/Makefile.target | 2 +- tests/tcg/aarch64/sysregs.c | 120 ++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/aarch64/sysregs.c -- 2.17.1