Hi, On 12/13/19 6:04 PM, Andrew Jones wrote: > On Thu, Nov 28, 2019 at 06:04:10PM +0000, Alexandru Elisei wrote: >> When a guest tries to execute code from MMIO memory, KVM injects an >> external abort into that guest. We have now fixed the psci test to not >> fetch instructions from the I/O region, and it's not that often that a >> guest misbehaves in such a way. Let's expand our coverage by adding a >> proper test targetting this corner case. >> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx> >> --- >> lib/arm64/asm/esr.h | 3 ++ >> arm/selftest.c | 112 ++++++++++++++++++++++++++++++++++++++++++-- >> 2 files changed, 112 insertions(+), 3 deletions(-) >> >> diff --git a/lib/arm64/asm/esr.h b/lib/arm64/asm/esr.h >> index 8e5af4d90767..8c351631b0a0 100644 >> --- a/lib/arm64/asm/esr.h >> +++ b/lib/arm64/asm/esr.h >> @@ -44,4 +44,7 @@ >> #define ESR_EL1_EC_BKPT32 (0x38) >> #define ESR_EL1_EC_BRK64 (0x3C) >> >> +#define ESR_EL1_FSC_MASK (0x3F) >> +#define ESR_EL1_FSC_EXTABT (0x10) >> + >> #endif /* _ASMARM64_ESR_H_ */ >> diff --git a/arm/selftest.c b/arm/selftest.c >> index e9dc5c0cab28..2512e011034f 100644 >> --- a/arm/selftest.c >> +++ b/arm/selftest.c >> @@ -16,6 +16,8 @@ >> #include <asm/psci.h> >> #include <asm/smp.h> >> #include <asm/barrier.h> >> +#include <asm/mmu.h> >> +#include <asm/pgtable.h> >> >> static cpumask_t ready, valid; >> >> @@ -68,6 +70,7 @@ static void check_setup(int argc, char **argv) >> static struct pt_regs expected_regs; >> static bool und_works; >> static bool svc_works; >> +static bool pabt_works; >> #if defined(__arm__) >> /* >> * Capture the current register state and execute an instruction >> @@ -91,7 +94,7 @@ static bool svc_works; >> "str r1, [r0, #" xstr(S_PC) "]\n" \ >> excptn_insn "\n" \ >> post_insns "\n" \ >> - :: "r" (&expected_regs) : "r0", "r1") >> + :: "r" (&expected_regs) : "r0", "r1", "r2") >> >> static bool check_regs(struct pt_regs *regs) >> { >> @@ -171,6 +174,54 @@ static void user_psci_system_off(struct pt_regs *regs) >> { >> __user_psci_system_off(); >> } >> + >> +static void check_pabt_exit(void) >> +{ >> + install_exception_handler(EXCPTN_PABT, NULL); >> + >> + report("pabt", pabt_works); >> + exit(report_summary()); >> +} >> + >> +#define PABT_ADDR ((3ul << 30) - PAGE_SIZE) >> +static void pabt_handler(struct pt_regs *regs) >> +{ >> + expected_regs.ARM_pc = PABT_ADDR; >> + pabt_works = check_regs(regs); >> + >> + regs->ARM_pc = (unsigned long)&check_pabt_exit; >> +} >> + >> +static void check_pabt(void) >> +{ >> + unsigned long sctlr; >> + >> + if (PABT_ADDR < __phys_end) { >> + report_skip("pabt: physical memory overlap"); >> + return; >> + } >> + >> + mmu_set_range_ptes(current_thread_info()->pgtable, PABT_ADDR, >> + PABT_ADDR, PABT_ADDR + PAGE_SIZE, __pgprot(PTE_WBWA)); >> + >> + /* Make sure we can actually execute from a writable region */ >> + asm volatile("mrc p15, 0, %0, c1, c0, 0": "=r" (sctlr)); >> + if (sctlr & CR_ST) { >> + sctlr &= ~CR_ST; >> + asm volatile("mcr p15, 0, %0, c1, c0, 0" :: "r" (sctlr)); >> + isb(); >> + /* >> + * Required according to the sequence in ARM DDI 0406C.d, page >> + * B3-1358. >> + */ >> + flush_tlb_all(); >> + } >> + >> + install_exception_handler(EXCPTN_PABT, pabt_handler); >> + >> + test_exception("ldr r2, =" xstr(PABT_ADDR), "bx r2", ""); >> + __builtin_unreachable(); >> +} >> #elif defined(__aarch64__) >> >> /* >> @@ -212,7 +263,7 @@ static void user_psci_system_off(struct pt_regs *regs) >> "stp x0, x1, [x1]\n" \ >> "1:" excptn_insn "\n" \ >> post_insns "\n" \ >> - :: "r" (&expected_regs) : "x0", "x1") >> + :: "r" (&expected_regs) : "x0", "x1", "x2") >> >> static bool check_regs(struct pt_regs *regs) >> { >> @@ -288,6 +339,59 @@ static bool check_svc(void) >> return svc_works; >> } >> >> +static void check_pabt_exit(void) >> +{ >> + install_exception_handler(EL1H_SYNC, ESR_EL1_EC_IABT_EL1, NULL); >> + >> + report("pabt", pabt_works); >> + exit(report_summary()); >> +} >> + >> +#define PABT_ADDR ((1ul << 38) - PAGE_SIZE) >> +static void pabt_handler(struct pt_regs *regs, unsigned int esr) >> +{ >> + bool is_extabt; >> + >> + expected_regs.pc = PABT_ADDR; >> + is_extabt = (esr & ESR_EL1_FSC_MASK) == ESR_EL1_FSC_EXTABT; >> + pabt_works = check_regs(regs) && is_extabt; >> + >> + regs->pc = (u64)&check_pabt_exit; >> +} >> + >> +static void check_pabt(void) >> +{ >> + enum vector v = check_vector_prep(); >> + unsigned long sctlr; >> + >> + if (PABT_ADDR < __phys_end) { >> + report_skip("pabt: physical memory overlap"); >> + return; >> + } >> + >> + /* >> + * According to ARM DDI 0487E.a, table D5-33, footnote c, all regions >> + * writable at EL0 are treated as PXN. Map the page without the user bit >> + * set. >> + */ >> + mmu_set_range_ptes(current_thread_info()->pgtable, PABT_ADDR, >> + PABT_ADDR, PABT_ADDR + PAGE_SIZE, __pgprot(PTE_WBWA)); >> + >> + /* Make sure we can actually execute from a writable region */ >> + sctlr = read_sysreg(sctlr_el1); >> + if (sctlr & SCTLR_EL1_WXN) { >> + write_sysreg(sctlr & ~SCTLR_EL1_WXN, sctlr_el1); >> + isb(); >> + /* SCTLR_EL1.WXN is permitted to be cached in a TLB. */ >> + flush_tlb_all(); >> + } >> + >> + install_exception_handler(v, ESR_EL1_EC_IABT_EL1, pabt_handler); >> + >> + test_exception("ldr x2, =" xstr(PABT_ADDR), "br x2", ""); > This line fails to compile with gcc 4.8.5[*]. I get > > Error: unexpected characters following instruction at operand 2 -- `ldr x2,=((1ul<<38)-((1UL)<<16))' > > But setting PABT_ADDR to 0x3fffff0000 (and arm's PABT_ADDR to 0xbfff0000) works. I'll use the hex value for the addresses. And I'll add a comment explaining why I chose them. Thanks for the testing! Thanks, Alex > [*] gcc version 4.8.5 20150623 (Red Hat 4.8.5-36) (GCC) > > Thanks, > drew > > >> + __builtin_unreachable(); >> +} >> + >> static void user_psci_system_off(struct pt_regs *regs, unsigned int esr) >> { >> __user_psci_system_off(); >> @@ -298,7 +402,9 @@ static void check_vectors(void *arg __unused) >> { >> report("und", check_und()); >> report("svc", check_svc()); >> - if (is_user()) { >> + if (!is_user()) { >> + check_pabt(); >> + } else { >> #ifdef __arm__ >> install_exception_handler(EXCPTN_UND, user_psci_system_off); >> #else >> -- >> 2.20.1 >>