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. KVM hcalls on book3s don't return to user space. 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
