Excerpts from Paul Mackerras's message of April 1, 2021 3:32 pm: > On Tue, Mar 23, 2021 at 11:02:32AM +1000, Nicholas Piggin wrote: >> Move the GUEST_MODE_SKIP logic into KVM code. This is quite a KVM >> internal detail that has no real need to be in common handlers. >> >> Also add a comment explaining why this thing exists. > > [snip] > >> diff --git a/arch/powerpc/kvm/book3s_64_entry.S >> b/arch/powerpc/kvm/book3s_64_entry.S >> index 7a039ea78f15..a5412e24cc05 100644 >> --- a/arch/powerpc/kvm/book3s_64_entry.S >> +++ b/arch/powerpc/kvm/book3s_64_entry.S >> @@ -1,6 +1,7 @@ >> /* SPDX-License-Identifier: GPL-2.0-only */ >> #include <asm/asm-offsets.h> >> #include <asm/cache.h> >> +#include <asm/exception-64s.h> >> #include <asm/kvm_asm.h> >> #include <asm/kvm_book3s_asm.h> >> #include <asm/ppc_asm.h> >> @@ -20,9 +21,12 @@ kvmppc_interrupt: >> * guest R12 saved in shadow VCPU SCRATCH0 >> * guest R13 saved in SPRN_SCRATCH0 >> */ >> -#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE >> std r9,HSTATE_SCRATCH2(r13) >> lbz r9,HSTATE_IN_GUEST(r13) >> + cmpwi r9,KVM_GUEST_MODE_SKIP >> + beq- .Lmaybe_skip >> +.Lno_skip: >> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE >> cmpwi r9,KVM_GUEST_MODE_HOST_HV >> beq kvmppc_bad_host_intr >> #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE >> @@ -34,3 +38,48 @@ kvmppc_interrupt: >> #else >> b kvmppc_interrupt_pr >> #endif > > It's a bit hard to see without more context, but I think that in the > PR-only case (CONFIG_KVM_BOOK3S_HV_POSSIBLE undefined), this will > corrupt R9. You need to restore R9 before the unconditional branch to > kvmppc_interrupt_pr. (I realize this code gets modified further, but > I'd rather not break bisection.)
Very good catch, thanks. Thanks, Nick
