On 2025-04-25 16:49:19 [+0530], Shrikanth Hegde wrote:
> On 4/25/25 00:08, Sebastian Andrzej Siewior wrote:
> > On 2025-04-24 21:27:59 [+0530], Shrikanth Hegde wrote:
> > > > > diff --git a/arch/powerpc/kvm/book3s_hv.c
> > > > > b/arch/powerpc/kvm/book3s_hv.c
> > > > > index 19f4d298d..123539642 100644
> > > > > --- a/arch/powerpc/kvm/book3s_hv.c
> > > > > +++ b/arch/powerpc/kvm/book3s_hv.c
> > > > > @@ -4901,7 +4901,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu
> > > > > *vcpu, u64 time_limit,
> > > > > }
> > > > > if (need_resched())
> > > > > - cond_resched();
> > > > > + schedule();
> > > >
> > >
> > >
> > > > This looks unrelated and odd. I don't why but this should be a
> > > > cond_resched() so it can be optimized away on PREEMPT kernels.
> > >
> > > This is needed, otherwise KVM on powerVM setup gets stuck on
> > > preempt=full/lazy.
> >
> > But this makes no sense. On preempt=full the cond_resched() gets patched
> > out while schedule() doesn't. Okay, this explains the stuck.
>
> cond_resched works. What you said is right about schedule and preemption
> models.
> Initially I had some other code changes and they were causing it get stuck. i
> retested it.
so it is unrelated then ;)
> But looking at the semantics of usage of xfer_to_guest_mode_work
> I think using schedule is probably right over here.
> Correct me if i got it all wrong.
No, if you do xfer_to_guest_mode_work() then it will invoke schedule()
when appropriate. It just the thing in kvmhv_run_single_vcpu() looks odd
and might have been duct tape or an accident and could probably be
removed.
> on x86:
> kvm_arch_vcpu_ioctl_run
> vcpu_run
> for () {
> .. run guest..
> xfer_to_guest_mode_handle_work
> schedule
> }
>
>
> on Powerpc: ( taking book3s_hv flavour):
> kvm_arch_vcpu_ioctl_run
> kvmppc_vcpu_run_hv *1
> do while() {
> kvmhv_run_single_vcpu or kvmppc_run_vcpu
> -- checking for need_resched and signals and bails out
> *2
> }
>
>
> *1 - checks for need resched and signals before entering guest
I don't see the need_resched() check here.
> *2 - checks for need resched and signals while running the guest
>
>
> This patch is addressing only *1 but it needs to address *2 as well using
> generic framework.
> I think it is doable for books3s_hv atleast. (though might need rewrite)
>
> __kvmppc_vcpu_run is a block box to me yet. I think it first makes sense
> to move it C and then try use the xfer_to_guest_mode_handle_work.
> nick, vaibhav, any idea on __kvmppc_vcpu_run on how is it handling signal
> pending, and need_resched.
>
>
> So this is going to need more work specially on *2 and doing that is also key
> for preempt=lazy/full to work
> for kvm on powepc. will try to figure out.
Okay.
Sebastian
