Re: [PATCH 1/2] KVM: PPC: BookE: Disable preemption before loading guest FP and Altivec

[Shrikanth Hegde <sshegde@linux.ibm.com>]

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();
> 
> 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.
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.