Re: [PATCH 4/9] Implement hsave v5

[Anthony Liguori <anthony@codemonkey.ws>]

Alexander Graf wrote:
> Implement the hsave MSR, that gives the VCPU a GPA to save the
> old guest state in.
>
> v2 allows userspace to save/restore hsave
> v4 dummys out the hsave MSR, so we use a host page
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/x86/kvm/kvm_svm.h |    1 +
>  arch/x86/kvm/svm.c     |   12 ++++++++++++
>  2 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kvm/kvm_svm.h b/arch/x86/kvm/kvm_svm.h
> index 65ef0fc..40cb128 100644
> --- a/arch/x86/kvm/kvm_svm.h
> +++ b/arch/x86/kvm/kvm_svm.h
> @@ -41,6 +41,7 @@ struct vcpu_svm {
>  	unsigned long host_dr7;
>  
>  	u32 *msrpm;
> +	struct vmcb *hsave;
>  };
>  
>  #endif
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 62bfa2b..eb301fe 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -640,6 +640,7 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
>  	struct vcpu_svm *svm;
>  	struct page *page;
>  	struct page *msrpm_pages;
> +	struct page *hsave_page;
>  	int err;
>  
>  	svm = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
> @@ -665,6 +666,11 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
>  	svm->msrpm = page_address(msrpm_pages);
>  	svm_vcpu_init_msrpm(svm->msrpm);
>  
> +	hsave_page = alloc_page(GFP_KERNEL);
> +	if (!hsave_page)
> +		goto uninit;
> +	svm->hsave = page_address(hsave_page);
> +
>  	svm->vmcb = page_address(page);
>  	clear_page(svm->vmcb);
>  	svm->vmcb_pa = page_to_pfn(page) << PAGE_SHIFT;
> @@ -694,6 +700,7 @@ static void svm_free_vcpu(struct kvm_vcpu *vcpu)
>  
>  	__free_page(pfn_to_page(svm->vmcb_pa >> PAGE_SHIFT));
>  	__free_pages(virt_to_page(svm->msrpm), MSRPM_ALLOC_ORDER);
> +	__free_page(virt_to_page(svm->hsave));
>  	kvm_vcpu_uninit(vcpu);
>  	kmem_cache_free(kvm_vcpu_cache, svm);
>  }
> @@ -1376,6 +1383,9 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 *data)
>  	case MSR_IA32_LASTINTTOIP:
>  		*data = svm->vmcb->save.last_excp_to;
>  		break;
> +	case MSR_VM_HSAVE_PA:
> +		*data = 0;
> +		break;
>   

I'm confused as to why you aren't allowing userspace to set/get this 
MSR?  Wouldn't it be needed for save/restore?

Regards,

Anthony Liguori

>  	default:
>  		return kvm_get_msr_common(vcpu, ecx, data);
>  	}
> @@ -1470,6 +1480,8 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 data)
>  		pr_unimpl(vcpu, "unimplemented perfctr wrmsr: 0x%x data 0x%llx\n", ecx, data);
>  
>  		break;
> +	case MSR_VM_HSAVE_PA:
> +		break;
>  	default:
>  		return kvm_set_msr_common(vcpu, ecx, data);
>  	}
>