RE: [PATCH 2/5] booke: exit to guest userspace for unimplemented hcalls in kvm

Bhushan Bharat-R65777
2013-07-16T04:46:45Z

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Monday, July 15, 2013 11:38 PM
> To: Bhushan Bharat-R65777
> Cc: kvm@vger.kernel.org; kvm-ppc@vger.kernel.org; agraf@suse.de; Yoder Stuart-
> B08248; Bhushan Bharat-R65777; Bhushan Bharat-R65777
> Subject: Re: [PATCH 2/5] booke: exit to guest userspace for unimplemented hcalls
> in kvm
>
> On 07/15/2013 06:11:16 AM, Bharat Bhushan wrote:
> > Exit to guest user space if kvm does not implement the hcall.
> >
> > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> > ---
> > arch/powerpc/kvm/booke.c | 47
> > +++++++++++++++++++++++++++++++++++++------
> > arch/powerpc/kvm/powerpc.c | 1 +
> > include/uapi/linux/kvm.h | 1 +
> > 3 files changed, 42 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
> > 17722d8..c8b41b4 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -1005,9 +1005,25 @@ int kvmppc_handle_exit(struct kvm_run *run,
> > struct kvm_vcpu *vcpu,
> > break;
> >
> > #ifdef CONFIG_KVM_BOOKE_HV
> > - case BOOKE_INTERRUPT_HV_SYSCALL:
> > + case BOOKE_INTERRUPT_HV_SYSCALL: {
> > + int i;
> > if (!(vcpu->arch.shared->msr & MSR_PR)) {
> > - kvmppc_set_gpr(vcpu, 3, kvmppc_kvm_pv(vcpu));
> > + r = kvmppc_kvm_pv(vcpu);
> > + if (r != EV_UNIMPLEMENTED) {
> > + /* except unimplemented return to guest
> > */
> > + kvmppc_set_gpr(vcpu, 3, r);
> > + kvmppc_account_exit(vcpu,
> > SYSCALL_EXITS);
> > + r = RESUME_GUEST;
> > + break;
> > + }
> > + /* Exit to userspace for unimplemented hcalls
> > in kvm */
> > + run->epapr_hcall.nr = kvmppc_get_gpr(vcpu, 11);
> > + run->epapr_hcall.ret = 0;
> > + for (i = 0; i < 8; i++)
> > + run->epapr_hcall.args[i] =
> > kvmppc_get_gpr(vcpu, 3 + i);
>
> You need to clear the upper half of each register if CONFIG_PPC64=y and MSR_CM
> is not set.
>
> > + vcpu->arch.hcall_needed = 1;
>
> The existing code for hcall_needed restores 9 return arguments, rather than the
> 8 that are defined for this interface. Thus, you'll be restoring one word of
> padding into the guest -- which could be arbitrary userspace data that shouldn't
> be leaked. r12 is volatile in the ePAPR hcall ABI so simply clobbering it isn't
> a problem, though.

Oops; Not just that, currently this uses struct type "papr_hcall" while on booke we should use epapr_hcall. I will make a function which will be defined in book3s.c and booke.c to setup hcall return registers accordingly.

-Bharat

>
> -Scott