On 15.07.2013, at 17:35, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@xxxxxxx] >> Sent: Monday, July 15, 2013 8:59 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 17:13, Bhushan Bharat-R65777 wrote: >> >>> >>> >>>> -----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. >> >> It doesn't even start handling the hcall if papr_enabled isn't set ;). > > On PR, not HV :-) Book3S HV doesn't support non-PAPR ;). 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