ping....
在 2022年6月13日星期一，Dongjiu Geng <[email protected]> 写道： > Jan Kiszka <[email protected]> 于2022年6月10日周五 13:20写道： > > > > On 09.06.22 17:27, Dongjiu Geng wrote: > > > ping，sorry for the noise > > > > > > > No need to be sorry, I unfortunately didn't find the time to fully > > understand this. > > Thanks for your time. > > > > > > > > > Dongjiu Geng <[email protected]> 于2022年6月3日周五 21:11写道： > > >> > > >> The HPFAR can be invalid if the stage 2 fault did not happen during a > > >> stage 1 page table walk (the ESR_EL2.S1PTW bit is clear) and one of > the > > >> two following cases are true: > > >> 1). The fault was due to a permission fault > > >> 2). The processor carries errata 834220 > > >> > > >> Therefore, for all non S1PTW faults where we either have a permission > > >> fault or the errata workaround is enabled, we resolve the IPA using > the > > >> AT instruction. > > >> > > >> Signed-off-by: Dongjiu Geng <[email protected]> > > >> --- > > >> hypervisor/arch/arm64/include/asm/paging.h | 8 ++ > > >> hypervisor/arch/arm64/include/asm/sysregs.h | 8 ++ > > >> hypervisor/arch/arm64/include/asm/traps.h | 2 + > > >> hypervisor/arch/arm64/mmio.c | 7 +- > > >> hypervisor/arch/arm64/traps.c | 89 > +++++++++++++++++++-- > > >> 5 files changed, 105 insertions(+), 9 deletions(-) > > >> > > >> diff --git a/hypervisor/arch/arm64/include/asm/paging.h > b/hypervisor/arch/arm64/include/asm/paging.h > > >> index e600cf58..4f0cb81c 100644 > > >> --- a/hypervisor/arch/arm64/include/asm/paging.h > > >> +++ b/hypervisor/arch/arm64/include/asm/paging.h > > >> @@ -198,6 +198,14 @@ unsigned int get_cpu_parange(void); > > >> | (cpu_parange_encoded << > TCR_PS_SHIFT) \ > > >> | VTCR_RES1) > > >> > > >> +/* Flags for get fault ipa from gva */ > > >> +#define GV2M_READ (0u<<0) > > >> +#define GV2M_WRITE (1u<<0) > > >> + > > >> +/* Indicates address translation aborted */ > > >> +#define PAR_F (1UL) > > >> +#define PADDR_MASK ((1UL << 48) - 1UL) > > >> + > > >> int arm_paging_cell_init(struct cell *cell); > > >> void arm_paging_cell_destroy(struct cell *cell); > > >> > > >> diff --git a/hypervisor/arch/arm64/include/asm/sysregs.h > b/hypervisor/arch/arm64/include/asm/sysregs.h > > >> index 868ef887..2c683832 100644 > > >> --- a/hypervisor/arch/arm64/include/asm/sysregs.h > > >> +++ b/hypervisor/arch/arm64/include/asm/sysregs.h > > >> @@ -117,6 +117,14 @@ > > >> #define ESR_IL(esr) GET_FIELD((esr), 25, 25) > > >> /* Instruction specific syndrome */ > > >> #define ESR_ISS(esr) GET_FIELD((esr), 24, 0) > > >> + > > >> +/* Fault status code of instruction specific syndrome */ > > >> +#define ESR_ISS_FSC(esr) GET_FIELD((esr), 5, 0) > > >> + > > >> +/* Shared ISS fault status code(IFSC/DFSC) for Data/Instruction > aborts */ > > >> +#define ESR_ISS_FSC_TYPE (0x3C) > > >> +#define ESR_ISS_FSC_PERM (0x0C) > > >> + > > >> /* Exception classes values */ > > >> #define ESR_EC_UNKNOWN 0x00 > > >> #define ESR_EC_WFx 0x01 > > >> diff --git a/hypervisor/arch/arm64/include/asm/traps.h > b/hypervisor/arch/arm64/include/asm/traps.h > > >> index a7c07624..0efedef1 100644 > > >> --- a/hypervisor/arch/arm64/include/asm/traps.h > > >> +++ b/hypervisor/arch/arm64/include/asm/traps.h > > >> @@ -25,6 +25,8 @@ struct trap_context { > > >> > > >> void arch_handle_trap(union registers *guest_regs); > > >> void arch_el2_abt(union registers *regs); > > >> +bool arch_get_fault_ipa(struct trap_context *ctx, unsigned long *ipa, > > >> + unsigned int flag); > > >> > > >> /* now include from arm-common */ > > >> #include_next <asm/traps.h> > > >> diff --git a/hypervisor/arch/arm64/mmio.c > b/hypervisor/arch/arm64/mmio.c > > >> index 7fbfef75..70301ab3 100644 > > >> --- a/hypervisor/arch/arm64/mmio.c > > >> +++ b/hypervisor/arch/arm64/mmio.c > > >> @@ -43,7 +43,6 @@ enum trap_return arch_handle_dabt(struct > trap_context *ctx) > > >> { > > >> enum mmio_result mmio_result; > > >> struct mmio_access mmio; > > >> - unsigned long hpfar; > > >> unsigned long hdfar; > > >> /* Decode the syndrome fields */ > > >> u32 iss = ESR_ISS(ctx->esr); > > >> @@ -57,10 +56,10 @@ enum trap_return arch_handle_dabt(struct > trap_context *ctx) > > >> u32 is_write = iss >> 6 & 0x1; > > >> u32 size = 1 << sas; > > >> > > >> - arm_read_sysreg(HPFAR_EL2, hpfar); > > >> arm_read_sysreg(FAR_EL2, hdfar); > > >> - mmio.address = hpfar << 8; > > >> - mmio.address |= hdfar & 0xfff; > > >> + > > >> + if (!arch_get_fault_ipa(ctx, &mmio.address, GV2M_READ)) > > >> + return TRAP_HANDLED; /* Try again */ > > > > So, this means we need to make the guest trap again, and then HPFAR is > > valid?? > > Maybe this invalidation is due to someone playing with the Stage-2 > page table of the domain, and can not get the right IPA, so we try > again. > > > > > > >> > > >> this_cpu_public()->stats[JAILHOUSE_CPU_STAT_VMEXITS_MMIO]++; > > >> > > >> diff --git a/hypervisor/arch/arm64/traps.c > b/hypervisor/arch/arm64/traps.c > > >> index 488dd7f8..10441b4b 100644 > > >> --- a/hypervisor/arch/arm64/traps.c > > >> +++ b/hypervisor/arch/arm64/traps.c > > >> @@ -33,6 +33,85 @@ void arch_skip_instruction(struct trap_context > *ctx) > > >> arm_write_sysreg(ELR_EL2, pc); > > >> } > > >> > > >> +static bool check_workaround_834220(void) > > >> +{ > > >> + unsigned long midr; > > >> + unsigned int variant, revision, part; > > >> + > > >> + arm_read_sysreg(MIDR_EL1, midr); > > >> + > > >> + variant = (midr >> 20) & 0xf; > > >> + revision = midr & 0xf; > > >> + part = (midr >> 4) & 0xfff; > > >> + > > >> + /* Cortex-A57 r0p0 - r1p2 */ > > >> + if (part == 0xD07 && variant <= 1 && revision <= 2) > > >> + return true; > > >> + > > >> + return false; > > >> +} > > >> + > > >> +static bool hpfar_is_not_valid(bool s1ptw, u8 fsc) > > >> +{ > > >> + /* > > >> + * The HPFAR can be invalid if the stage 2 fault did not > > >> + * happen during a stage 1 page table walk (the ESR_EL2.S1PTW > > >> + * bit is clear) and one of the two following cases are true: > > >> + * 1. The fault was due to a permission fault > > >> + * 2. The processor carries errata 834220 > > >> + * > > >> + */ > > >> + return (s1ptw == 0U) && (((fsc & ESR_ISS_FSC_TYPE) == > ESR_ISS_FSC_PERM) || check_workaround_834220()); > > >> +} > > >> + > > >> +bool arch_get_fault_ipa(struct trap_context *ctx, unsigned long > *ipa, unsigned int flags) > > >> +{ > > >> + unsigned long hpfar, hdfar, par, tmp; > > >> + > > >> + u32 s1ptw = ESR_ISS(ctx->esr) >> 7 & 0x1; > > >> + u8 fsc = ESR_ISS_FSC(ctx->esr); > > >> + > > >> + arm_read_sysreg(FAR_EL2, hdfar); > > >> + > > >> + if (hpfar_is_not_valid(s1ptw, fsc)) { > > >> + > > >> + /* Save current PAR_EL1 */ > > >> + arm_read_sysreg(PAR_EL1, tmp); > > >> + > > >> + /* > > >> + * Performs stage 1 address translation, with > permissions as if > > >> + * writing to or reading from the given virtual > address from EL1 > > >> + */ > > >> + if ( (flags & GV2M_WRITE) == GV2M_WRITE ) { > > >> + asm volatile ("at s1e1w, %0;" : : "r" > (hdfar)); > > >> + } else { > > >> + asm volatile ("at s1e1r, %0;" : : "r" > (hdfar)); > > >> + } > > >> + > > >> + isb(); > > >> + > > >> + /* The resulting address can be read from the PAR_EL1 > */ > > >> + arm_read_sysreg(PAR_EL1, par); > > >> + > > >> + /* Recover current PAR_EL1 */ > > >> + arm_write_sysreg(PAR_EL1, tmp); > > >> + > > >> + /* If PAR_EL1.F = 1, address translation aborted */ > > >> + if ((par & PAR_F) == PAR_F) { > > >> + printk("Failed to ipa!\n"); > > >> + return false; > > >> + } else { > > >> + *ipa = (par & PADDR_MASK & PAGE_MASK) | > (hdfar & (~PAGE_MASK)); > > >> + } > > >> + } else { > > >> + arm_read_sysreg(HPFAR_EL2, hpfar); > > >> + *ipa = hpfar << 8; > > >> + *ipa |= hdfar & 0xfff; > > >> + } > > >> + > > >> + return true; > > >> +} > > >> + > > >> static enum trap_return handle_hvc(struct trap_context *ctx) > > >> { > > >> unsigned long *regs = ctx->regs; > > >> @@ -71,7 +150,7 @@ static enum trap_return handle_sysreg(struct > trap_context *ctx) > > >> > > >> static enum trap_return handle_iabt(struct trap_context *ctx) > > >> { > > >> - unsigned long hpfar, hdfar; > > >> + unsigned long hpfar; > > >> > > >> if (this_cpu_data()->sdei_event) { > > >> this_cpu_data()->sdei_event = false; > > >> @@ -83,11 +162,11 @@ static enum trap_return handle_iabt(struct > trap_context *ctx) > > >> return TRAP_HANDLED; > > >> } > > >> > > >> - arm_read_sysreg(HPFAR_EL2, hpfar); > > >> - arm_read_sysreg(FAR_EL2, hdfar); > > >> + if (arch_get_fault_ipa(ctx, &hpfar, GV2M_READ)) > > >> + panic_printk("FATAL: instruction abort at 0x%lx\n", > hpfar); > > >> + else > > >> + panic_printk("FATAL: instruction abort and can not > get ipa\n"); > > >> > > >> - panic_printk("FATAL: instruction abort at 0x%lx\n", > > >> - (hpfar << 8) | (hdfar & 0xfff)); > > >> return TRAP_FORBIDDEN; > > >> } > > >> > > >> -- > > >> 2.25.1 > > >> > > > > How common is this erratum? Is it already affecting real boards? Just to > > get a feeling for the urgency. > > Yes, it can affect the real boards. > For example, if we configure a buffer's stage2 page table to readonly > for VM, > this VM's process writes this buffer, then it will trigger stage2 > permission fault. > In this case, we can not get the right IPA without this patch. > > > > > > Thanks, > > Jan > > > > -- > > Siemens AG, Technology > > Competence Center Embedded Linux > -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/CABSBigQTreqdPXV6y3hH%2BAerOK5hr3Y6C2bq1T9CdVeC27z6uQ%40mail.gmail.com.
