Re: [PATCH v2 07/37] KVM: PPC: Book3S 64: Move GUEST_MODE_SKIP test into KVM
flat view
From: Daniel Axtens <hidden>
Date: 2021-03-05 05:54:43
Hi Nick,
- .if IKVM_SKIP -89: mtocrf 0x80,r9 - ld r10,IAREA+EX_CTR(r13) - mtctr r10 - ld r9,IAREA+EX_R9(r13) - ld r10,IAREA+EX_R10(r13) - ld r11,IAREA+EX_R11(r13) - ld r12,IAREA+EX_R12(r13) - .if IHSRR_IF_HVMODE - BEGIN_FTR_SECTION - b kvmppc_skip_Hinterrupt - FTR_SECTION_ELSE - b kvmppc_skip_interrupt - ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
I was initially concerned that you were pulling out the complexities around IHSRR_IF_HVMODE, but I checked exceptions-64s.S and the only exception handler that sets IHSRR_IF_HVMODE is hardware_interrupt and that does not set IKVM_SKIP, so we are indeed fine to not keep this complex little asm section.
quoted hunk ↗ jump to hunk
- .elseif IHSRR - b kvmppc_skip_Hinterrupt - .else - b kvmppc_skip_interrupt - .endif - .endif .endm
quoted hunk ↗ jump to hunk
+/* + * KVM uses a trick where it is running in MSR[HV]=1 mode in real-mode with the + * guest MMU context loaded, and it sets KVM_GUEST_MODE_SKIP and enables + * MSR[DR]=1 while leaving MSR[IR]=0, so it continues to fetch HV instructions + * but loads and stores will access the guest context. This is used to load + * the faulting instruction without walking page tables. + * + * However the guest context may not be able to translate, or it may cause a + * machine check or other issue, which will result in a fault in the host + * (even with KVM-HV). + * + * These faults are caught here and if the fault was (or was likely) due to + * that load, then we just return with the PC advanced +4 and skip the load, + * which then goes via the slow path. + */
This is a really helpful comment! Thanks!
quoted hunk ↗ jump to hunk
+.Lmaybe_skip: + cmpwi r12,BOOK3S_INTERRUPT_MACHINE_CHECK + beq 1f + cmpwi r12,BOOK3S_INTERRUPT_DATA_STORAGE + beq 1f + cmpwi r12,BOOK3S_INTERRUPT_DATA_SEGMENT + beq 1f +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE + /* HSRR interrupts have 2 added to trap vector */ + cmpwi r12,BOOK3S_INTERRUPT_H_DATA_STORAGE | 0x2
This took me by surprise for a while, but I checked how it works in exceptions-64s.S and indeed the GEN_KVM macro will add 0x2 to the IVEC if IHSRR is set, and it is set for h_data_storage. So this checks out to me. I have checked, to the best of my limited assembler capabilities that the logic before and after matches. It seems good to me. On that limited basis, Reviewed-by: Daniel Axtens <redacted> Kind regards, Daniel
+ beq 2f +#endif + b .Lno_skip +1: mfspr r9,SPRN_SRR0 + addi r9,r9,4 + mtspr SPRN_SRR0,r9 + ld r12,HSTATE_SCRATCH0(r13) + ld r9,HSTATE_SCRATCH2(r13) + GET_SCRATCH0(r13) + RFI_TO_KERNEL +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE +2: mfspr r9,SPRN_HSRR0 + addi r9,r9,4 + mtspr SPRN_HSRR0,r9 + ld r12,HSTATE_SCRATCH0(r13) + ld r9,HSTATE_SCRATCH2(r13) + GET_SCRATCH0(r13) + HRFI_TO_KERNEL +#endif -- 2.23.0