> -----Original Message----- > From: Alexander Graf [mailto:agraf@xxxxxxx] > Sent: Monday, July 15, 2013 8:27 PM > To: Bhushan Bharat-R65777 > Cc: kvm@xxxxxxxxxxxxxxx; kvm-ppc@xxxxxxxxxxxxxxx; 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:agraf@xxxxxxx] > >> Sent: Monday, July 15, 2013 5:16 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm@xxxxxxxxxxxxxxx; kvm-ppc@xxxxxxxxxxxxxxx; 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:agraf@xxxxxxx] > >>>> Sent: Monday, July 15, 2013 5:02 PM > >>>> To: Bhushan Bharat-R65777 > >>>> Cc: kvm@xxxxxxxxxxxxxxx; kvm-ppc@xxxxxxxxxxxxxxx; 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.bhushan@xxxxxxxxxxxxx> > >>>>> --- > >>>>> 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" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html