Shrikanth Hegde <[email protected]> writes: > Hi Narayana. > > On 10/1/26 4:56 PM, Narayana Murty N wrote: >> Hi Shrikanth, >> Thanks for the fixes. I had one question on patch 1. > > See response below. > >> >>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c >>> index 13ad4cf5fa71..5b9118eefe1d 100644 >>> --- a/arch/powerpc/kvm/booke.c >>> +++ b/arch/powerpc/kvm/booke.c >>> @@ -1404,10 +1404,17 @@ int kvmppc_handle_exit(struct kvm_vcpu *vcpu, >>> unsigned int exit_nr) >>> if (s <= 0) >>> r = (s << 2) | RESUME_HOST | (r & RESUME_FLAG_NV); >>> else { >>> - /* interrupts now hard-disabled */ >>> + /* >>> + * kvmppc_fix_ee_before_entry() marks the software >>> + * IRQ state enabled while interrupts are still >>> + * hard-disabled. So disable preemption while loading >>> + * guest FP and Altivec. >>> + */ >>> kvmppc_fix_ee_before_entry(); >>> + preempt_disable(); >>> kvmppc_load_guest_fp(vcpu); >>> kvmppc_load_guest_altivec(vcpu); >>> + preempt_enable_no_resched(); >> Would it be simpler to move kvmppc_fix_ee_before_entry() after the >> FP/Altivec loads instead? >> >> The normal kvmppc_vcpu_run() entry path already loads the guest >> FP/Altivec state while interrupts are still disabled and calls >> kvmppc_fix_ee_before_entry() immediately before entering the guest. >> >> So could this path follow the same ordering: >> >> kvmppc_load_guest_fp(vcpu); >> kvmppc_load_guest_altivec(vcpu); >> kvmppc_fix_ee_before_entry(); >>
Thanks Narayana, that actually make more sense to me. >> That would avoid making the software IRQ state enabled before loading >> the guest FP/Altivec state, and also avoid the additional >> preempt_disable()/preempt_enable_no_resched() pair. >> >> Thanks, >> Narayana Murty. >>> } >>> } >> > > I thought I had put that for discussion after ---, but looks like I forgot. > > I don't mind the above too. but I didn't have a way to test it. Actually I agree with Narayan's reasoning here. Sorry for being so late to the game. As you know we have been occupied with other things internally. > So kept it as is based on what Christian said works for him. > If you have a way to test the patches, please let me know. > We can try that too. Let's ask Christian directly :) @Christian - Once we have v2, would you kindly help us test that please? We think the other approach which Narayana suggested might be a better fix for this and it can avoid preempt_enable/disable() dance. However, looks like we may not have the board where we can reproduce this. -ritesh
