Re: [PATCH 7/9] Add VMLOAD and VMSAVE handlers

[Avi Kivity <avi@qumranet.com>]

Alexander Graf wrote:
> This implements the VMLOAD and VMSAVE instructions, that usually surround
> the VMRUN instructions. Both instructions load / restore the same elements,
> so we only need to implement them once.
>
>  
> +static int nested_svm_vmloadsave(struct vmcb *from_vmcb, struct vmcb *to_vmcb)
> +{
> +	memcpy(&to_vmcb->save.fs, &from_vmcb->save.fs, sizeof(struct vmcb_seg));
> +	memcpy(&to_vmcb->save.gs, &from_vmcb->save.gs, sizeof(struct vmcb_seg));
> +	memcpy(&to_vmcb->save.tr, &from_vmcb->save.tr, sizeof(struct vmcb_seg));
> +	memcpy(&to_vmcb->save.ldtr, &from_vmcb->save.ldtr,
> +	       sizeof(struct vmcb_seg));
>   

You can use simple assignment here.

> +static int vmload_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
> +{
> +	svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
> +	skip_emulated_instruction(&svm->vcpu);
> +
> +	if (svm->vmcb->save.cpl) {
> +		printk(KERN_ERR "%s: invalid cpl 0x%x at ip 0x%lx\n",
> +		       __func__, svm->vmcb->save.cpl,
> +		       kvm_rip_read(&svm->vcpu));
> +		kvm_queue_exception(&svm->vcpu, GP_VECTOR);
> +		return 1;
> +	}
>   

Check before skip.

Don't we need to check that svm is enabled in the guest as well (and 
inject #UD if not)?

> +
> +static int vmsave_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
> +{
> +	svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
> +	skip_emulated_instruction(&svm->vcpu);
> +
> +	if (svm->vmcb->save.cpl) {
> +		printk(KERN_ERR "%s: invalid cpl 0x%x at ip 0x%lx\n",
> +		       __func__, svm->vmcb->save.cpl, kvm_rip_read(&svm->vcpu));
> +		kvm_queue_exception(&svm->vcpu, GP_VECTOR);
> +		return 1;
> +	}
> +
> +	nested_svm_do(svm, svm->vmcb->save.rax, 0, NULL, nested_svm_vmsave);
> +
>   

Ditto.

> +	return 1;
> +}
>   

Blank line.

-- 
error compiling committee.c: too many arguments to function