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. > - .elseif IHSRR > - b kvmppc_skip_Hinterrupt > - .else > - b kvmppc_skip_interrupt > - .endif > - .endif > .endm > +/* > + * 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! > +.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 <dja@xxxxxxxxxx> 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
