RE: [PATCH 2/5] booke: exit to guest userspace for unimplemented hcalls in kvm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----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 :-)

-Bharat

> 
> 
> 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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux