[Qemu-devel] [PATCH] ppc: Fix bug in handling of PAPR hypercall exits

[Qemu-devel] [PATCH] ppc: Fix bug in handling of PAPR hypercall exits

[David Gibson]

Currently for powerpc, kvm_arch_handle_exit() always returns 1, meaning
that its caller - kvm_cpu_exec() - will always exit immediately afterwards
to the loop in qemu_kvm_cpu_thread_fn().

There's no need to do this.  Once we've handled the hypercall there's no
reason we can't go straight around and KVM_RUN again, which is what ret = 0
will signal.  The only exception might be for hypercalls which affect the
state of cpu_can_run(), however the only one that might do this is H_CEDE
and for kvm that is always handled in the kernel, not qemu.

Furtherm setting ret = 0 means that when exit_requested is set from a
hypercall, we will enter KVM_RUN once more with a signal which lets the
the kernel do its internal logic to complete the hypercall with out
actually executing any more guest code.  This is important if our hypercall
also triggered a reset, which previously would re-initialize everything
without completing the hypercall.  This caused the kernel to get confused
because it thought the guest was still in the middle of a hypercall when
it has actually been reset.

This patch therefore changes to ret = 0, which is both a bugfix and a small
optimization.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/kvm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 829e180..a31d278 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -766,7 +766,7 @@ int kvm_arch_handle_exit(CPUPPCState *env, struct kvm_run *run)
         dprintf("handle PAPR hypercall\n");
         run->papr_hcall.ret = spapr_hypercall(env, run->papr_hcall.nr,
                                               run->papr_hcall.args);
-        ret = 1;
+        ret = 0;
         break;
 #endif
     default:
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] ppc: Fix bug in handling of PAPR hypercall exits

[Alexander Graf]

On 07.08.2012, at 06:44, David Gibson wrote:

> Currently for powerpc, kvm_arch_handle_exit() always returns 1, meaning
> that its caller - kvm_cpu_exec() - will always exit immediately afterwards
> to the loop in qemu_kvm_cpu_thread_fn().
> 
> There's no need to do this.  Once we've handled the hypercall there's no
> reason we can't go straight around and KVM_RUN again, which is what ret = 0
> will signal.  The only exception might be for hypercalls which affect the
> state of cpu_can_run(), however the only one that might do this is H_CEDE
> and for kvm that is always handled in the kernel, not qemu.
> 
> Furtherm setting ret = 0 means that when exit_requested is set from a
> hypercall, we will enter KVM_RUN once more with a signal which lets the
> the kernel do its internal logic to complete the hypercall with out
> actually executing any more guest code.  This is important if our hypercall
> also triggered a reset, which previously would re-initialize everything
> without completing the hypercall.  This caused the kernel to get confused
> because it thought the guest was still in the middle of a hypercall when
> it has actually been reset.
> 
> This patch therefore changes to ret = 0, which is both a bugfix and a small
> optimization.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Thanks, applied to ppc-next.

Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc: Fix bug in handling of PAPR hypercall exits

[Benjamin Herrenschmidt]

On Tue, 2012-08-07 at 11:05 +0200, Alexander Graf wrote:

> > This patch therefore changes to ret = 0, which is both a bugfix and a small
> > optimization.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Thanks, applied to ppc-next.

When do you plan to send your queue to Anthony ? Some of that stuff
should be going in ASAP.

Cheers,
Ben.

Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc: Fix bug in handling of PAPR hypercall exits

[Alexander Graf]

On 07.08.2012, at 23:03, Benjamin Herrenschmidt wrote:

> On Tue, 2012-08-07 at 11:05 +0200, Alexander Graf wrote:
> 
>>> This patch therefore changes to ret = 0, which is both a bugfix and a small
>>> optimization.
>>> 
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> 
>> Thanks, applied to ppc-next.
> 
> When do you plan to send your queue to Anthony ? Some of that stuff
> should be going in ASAP.

Before the hard freeze.


Alex