| Submitter | David Gibson |
|---|---|
| Date | Aug. 7, 2012, 4:44 a.m. |
| Message ID | <1344314685-9868-1-git-send-email-david@gibson.dropbear.id.au> |
| Download | mbox | patch |
| Permalink | /patch/175503/ |
| State | New |
| Headers | show |
Comments
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
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.
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
Patch
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:
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(-)