| Message ID | 1373886679-19581-3-git-send-email-Bharat.Bhushan@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
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@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; 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. Alex > break; > + } > #else > - case BOOKE_INTERRUPT_SYSCALL: > + case BOOKE_INTERRUPT_SYSCALL: { > + int i; > + r = RESUME_GUEST; > if (!(vcpu->arch.shared->msr & MSR_PR) && > (((u32)kvmppc_get_gpr(vcpu, 0)) == KVM_SC_MAGIC_R0)) { > /* KVM PV hypercalls */ > - kvmppc_set_gpr(vcpu, 3, kvmppc_kvm_pv(vcpu)); > - r = RESUME_GUEST; > + 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; > + run->exit_reason = KVM_EXIT_EPAPR_HCALL; > + r = RESUME_HOST; > } else { > /* Guest syscalls */ > kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SYSCALL); > } > kvmppc_account_exit(vcpu, SYSCALL_EXITS); > - r = RESUME_GUEST; > break; > + } > #endif > > case BOOKE_INTERRUPT_DTLB_MISS: { > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 4e05f8c..6c6199d 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -307,6 +307,7 @@ int kvm_dev_ioctl_check_extension(long ext) > case KVM_CAP_PPC_BOOKE_SREGS: > case KVM_CAP_PPC_BOOKE_WATCHDOG: > case KVM_CAP_PPC_EPR: > + case KVM_CAP_EXIT_EPAPR_HCALL: > #else > case KVM_CAP_PPC_SEGSTATE: > case KVM_CAP_PPC_HIOR: > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index 01ee50e..b5266e5 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -674,6 +674,7 @@ struct kvm_ppc_smmu_info { > #define KVM_CAP_PPC_RTAS 91 > #define KVM_CAP_IRQ_XICS 92 > #define KVM_CAP_ARM_EL1_32BIT 93 > +#define KVM_CAP_EXIT_EPAPR_HCALL 94 > > #ifdef KVM_CAP_IRQ_ROUTING > > -- > 1.7.0.4 > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Monday, July 15, 2013 5:02 PM > To: Bhushan Bharat-R65777 > Cc: kvm@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.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: { > > 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? -Bharat > > > Alex > > > break; > > + } > > #else > > - case BOOKE_INTERRUPT_SYSCALL: > > + case BOOKE_INTERRUPT_SYSCALL: { > > + int i; > > + r = RESUME_GUEST; > > if (!(vcpu->arch.shared->msr & MSR_PR) && > > (((u32)kvmppc_get_gpr(vcpu, 0)) == KVM_SC_MAGIC_R0)) { > > /* KVM PV hypercalls */ > > - kvmppc_set_gpr(vcpu, 3, kvmppc_kvm_pv(vcpu)); > > - r = RESUME_GUEST; > > + 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; > > + run->exit_reason = KVM_EXIT_EPAPR_HCALL; > > + r = RESUME_HOST; > > } else { > > /* Guest syscalls */ > > kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SYSCALL); > > } > > kvmppc_account_exit(vcpu, SYSCALL_EXITS); > > - r = RESUME_GUEST; > > break; > > + } > > #endif > > > > case BOOKE_INTERRUPT_DTLB_MISS: { > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > > index 4e05f8c..6c6199d 100644 > > --- a/arch/powerpc/kvm/powerpc.c > > +++ b/arch/powerpc/kvm/powerpc.c > > @@ -307,6 +307,7 @@ int kvm_dev_ioctl_check_extension(long ext) > > case KVM_CAP_PPC_BOOKE_SREGS: > > case KVM_CAP_PPC_BOOKE_WATCHDOG: > > case KVM_CAP_PPC_EPR: > > + case KVM_CAP_EXIT_EPAPR_HCALL: > > #else > > case KVM_CAP_PPC_SEGSTATE: > > case KVM_CAP_PPC_HIOR: > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index > > 01ee50e..b5266e5 100644 > > --- a/include/uapi/linux/kvm.h > > +++ b/include/uapi/linux/kvm.h > > @@ -674,6 +674,7 @@ struct kvm_ppc_smmu_info { #define > > KVM_CAP_PPC_RTAS 91 #define KVM_CAP_IRQ_XICS 92 #define > > KVM_CAP_ARM_EL1_32BIT 93 > > +#define KVM_CAP_EXIT_EPAPR_HCALL 94 > > > > #ifdef KVM_CAP_IRQ_ROUTING > > > > -- > > 1.7.0.4 > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > > the body of a message to majordomo@vger.kernel.org More majordomo info > > at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15.07.2013, at 13:38, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Monday, July 15, 2013 5:02 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm@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.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: { >> >> 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. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Monday, July 15, 2013 5:16 PM > To: Bhushan Bharat-R65777 > Cc: kvm@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:agraf@suse.de] > >> Sent: Monday, July 15, 2013 5:02 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm@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.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: { > >> > >> 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 ? -Bharat > > > Alex > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15.07.2013, at 16:50, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Monday, July 15, 2013 5:16 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm@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:agraf@suse.de] >>>> Sent: Monday, July 15, 2013 5:02 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm@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.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: { >>>> >>>> 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-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Monday, July 15, 2013 8:27 PM > To: Bhushan Bharat-R65777 > Cc: kvm@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:agraf@suse.de] > >> Sent: Monday, July 15, 2013 5:16 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm@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:agraf@suse.de] > >>>> Sent: Monday, July 15, 2013 5:02 PM > >>>> To: Bhushan Bharat-R65777 > >>>> Cc: kvm@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.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: { > >>>> > >>>> 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15.07.2013, at 17:13, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Monday, July 15, 2013 8:27 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm@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:agraf@suse.de] >>>> Sent: Monday, July 15, 2013 5:16 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm@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:agraf@suse.de] >>>>>> Sent: Monday, July 15, 2013 5:02 PM >>>>>> To: Bhushan Bharat-R65777 >>>>>> Cc: kvm@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.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: { >>>>>> >>>>>> 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 ;). Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Monday, July 15, 2013 8:59 PM > To: Bhushan Bharat-R65777 > Cc: kvm@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 17:13, Bhushan Bharat-R65777 wrote: > > > > > > >> -----Original Message----- > >> From: Alexander Graf [mailto:agraf@suse.de] > >> Sent: Monday, July 15, 2013 8:27 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm@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:agraf@suse.de] > >>>> Sent: Monday, July 15, 2013 5:16 PM > >>>> To: Bhushan Bharat-R65777 > >>>> Cc: kvm@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:agraf@suse.de] > >>>>>> Sent: Monday, July 15, 2013 5:02 PM > >>>>>> To: Bhushan Bharat-R65777 > >>>>>> Cc: kvm@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.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: { > >>>>>> > >>>>>> 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 :-) -Bharat > > > Alex > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15.07.2013, at 17:35, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Monday, July 15, 2013 8:59 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm@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 17:13, Bhushan Bharat-R65777 wrote: >> >>> >>> >>>> -----Original Message----- >>>> From: Alexander Graf [mailto:agraf@suse.de] >>>> Sent: Monday, July 15, 2013 8:27 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm@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:agraf@suse.de] >>>>>> Sent: Monday, July 15, 2013 5:16 PM >>>>>> To: Bhushan Bharat-R65777 >>>>>> Cc: kvm@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:agraf@suse.de] >>>>>>>> Sent: Monday, July 15, 2013 5:02 PM >>>>>>>> To: Bhushan Bharat-R65777 >>>>>>>> Cc: kvm@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.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: { >>>>>>>> >>>>>>>> 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-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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 -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); + 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; break; + } #else - case BOOKE_INTERRUPT_SYSCALL: + case BOOKE_INTERRUPT_SYSCALL: { + int i; + r = RESUME_GUEST; if (!(vcpu->arch.shared->msr & MSR_PR) && (((u32)kvmppc_get_gpr(vcpu, 0)) == KVM_SC_MAGIC_R0)) { /* KVM PV hypercalls */ - kvmppc_set_gpr(vcpu, 3, kvmppc_kvm_pv(vcpu)); - r = RESUME_GUEST; + 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; + run->exit_reason = KVM_EXIT_EPAPR_HCALL; + r = RESUME_HOST; } else { /* Guest syscalls */ kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SYSCALL); } kvmppc_account_exit(vcpu, SYSCALL_EXITS); - r = RESUME_GUEST; break; + } #endif case BOOKE_INTERRUPT_DTLB_MISS: { diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 4e05f8c..6c6199d 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -307,6 +307,7 @@ int kvm_dev_ioctl_check_extension(long ext) case KVM_CAP_PPC_BOOKE_SREGS: case KVM_CAP_PPC_BOOKE_WATCHDOG: case KVM_CAP_PPC_EPR: + case KVM_CAP_EXIT_EPAPR_HCALL: #else case KVM_CAP_PPC_SEGSTATE: case KVM_CAP_PPC_HIOR: diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 01ee50e..b5266e5 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -674,6 +674,7 @@ struct kvm_ppc_smmu_info { #define KVM_CAP_PPC_RTAS 91 #define KVM_CAP_IRQ_XICS 92 #define KVM_CAP_ARM_EL1_32BIT 93 +#define KVM_CAP_EXIT_EPAPR_HCALL 94 #ifdef KVM_CAP_IRQ_ROUTING
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(-)