> -----Original Message-----
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Monday, July 15, 2013 8:27 PM
> To: Bhushan Bharat-R65777
> Cc: k...@vger.kernel.org; kvm-ppc@vger.kernel.org; Wood Scott-B07421; Yoder
> Stuart-B08248
> Subject: Re: [PATCH 2/5] booke: exit to guest userspace for unimplemented 
> hcalls
> in kvm
> 
> 
> On 15.07.2013, at 16:50, Bhushan Bharat-R65777 wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:ag...@suse.de]
> >> Sent: Monday, July 15, 2013 5:16 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: k...@vger.kernel.org; kvm-ppc@vger.kernel.org; Wood Scott-B07421;
> >> Yoder
> >> Stuart-B08248
> >> Subject: Re: [PATCH 2/5] booke: exit to guest userspace for
> >> unimplemented hcalls in kvm
> >>
> >>
> >> On 15.07.2013, at 13:38, Bhushan Bharat-R65777 wrote:
> >>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Alexander Graf [mailto:ag...@suse.de]
> >>>> Sent: Monday, July 15, 2013 5:02 PM
> >>>> To: Bhushan Bharat-R65777
> >>>> Cc: k...@vger.kernel.org; kvm-ppc@vger.kernel.org; Wood
> >>>> Scott-B07421; Yoder Stuart-B08248; Bhushan Bharat-R65777
> >>>> Subject: Re: [PATCH 2/5] booke: exit to guest userspace for
> >>>> unimplemented hcalls in kvm
> >>>>
> >>>>
> >>>> On 15.07.2013, at 13:11, Bharat Bhushan wrote:
> >>>>
> >>>>> Exit to guest user space if kvm does not implement the hcall.
> >>>>>
> >>>>> Signed-off-by: Bharat Bhushan <bharat.bhus...@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: {
> >>>>
> >>>> This is getting large. Please extract hcall handling into its own 
> >>>> function.
> >>>> Maybe you can merge the HV and non-HV case then too.
> >>>>
> >>>>> +               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);
> >>>>> +                       vcpu->arch.hcall_needed = 1;
> >>>>> +                       kvmppc_account_exit(vcpu, SYSCALL_EXITS);
> >>>>> +                       r = RESUME_HOST;
> >>>>>                 } else {
> >>>>>                         /*
> >>>>>                          * hcall from guest userspace -- send 
> >>>>> privileged @@ -1016,22
> >>>>> +1032,39 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> >>>>> +kvm_vcpu *vcpu,
> >>>>>                         kvmppc_core_queue_program(vcpu, ESR_PPR);
> >>>>>                 }
> >>>>>
> >>>>> -               r = RESUME_GUEST;
> >>>>> +               run->exit_reason = KVM_EXIT_EPAPR_HCALL;
> >>>
> >>>
> >>> Oops, what I have done, I wanted this to be
> >>> kvmppc_account_exit(vcpu, SYSCALL_EXITS);
> >>>
> >>> s/ run->exit_reason = KVM_EXIT_EPAPR_HCALL;/
> >>> kvmppc_account_exit(vcpu, SYSCALL_EXITS);
> >>>
> >>> -Bharat
> >>>
> >>>>
> >>>> This looks odd. Your exit reason only changes when you do the hcall
> >>>> exiting, right?
> >>>>
> >>>> You also need to guard user space hcall exits with an ENABLE_CAP.
> >>>> Otherwise older user space will break, as it doesn't know about the
> >>>> exit type
> >> yet.
> >>>
> >>> So the user space so make enable_cap also?
> >>
> >> User space needs to call enable_cap on this cap, yes. Otherwise a
> >> guest can confuse user space with an hcall exit it can't handle.
> >
> > We do not have enable_cap for book3s, any specific reason why ?
> 
> We do. If you enable PAPR, you get PAPR hcalls. If you enable OSI, you get OSI
> hcalls.

Oh, We check this on book3s_PR and book3s_HV.

> KVM hcalls on book3s don't return to user space.

It exits, is not it? "arch/powerpc/kvm/book3s_pr.c" exits with 
KVM_EXIT_PAPR_HCALL. And same in book3s_pv.

Btw, Adding this on booke is not a question. I am just understanding book3s.

-Bharat
 

> Which is something we
> probably want to change along with this patch set.
> 
> 
> Alex
> 


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to