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]

 



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.


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