Re: [PATCH 6/6] kvm: qemu: Enable EPT support for real mode

[Avi Kivity <avi@qumranet.com>]

Yang, Sheng wrote:
> On Tuesday 22 April 2008 18:16:41 Avi Kivity wrote:
>   
>> Yang, Sheng wrote:
>>     
>>> From 73c33765f3d879001818cd0719038c78a0c65561 Mon Sep 17 00:00:00 2001
>>> From: Sheng Yang <sheng.yang@intel.com>
>>> Date: Fri, 18 Apr 2008 17:15:39 +0800
>>> Subject: [PATCH] kvm: qemu: Enable EPT support for real mode
>>>
>>> This patch build a identity page table on the last page of VGA bios, and
>>> use it as the guest page table in nonpaging mode for EPT.
>>>       
>> Doing this in qemu means older versions of qemu can't work with an
>> ept-enabled kernel.  Also, placing the table in the vga bios might
>> conflict with video card assignment to a guest.
>>
>> Suggest placing this near the realmode tss (see vmx.c:init_rmode_tss())
>> which serves a similar function.
>>     
>
> Something like this? (along with one page reserved in e820 table)
>
> I put the page it into 0xfffbc000 now. But I think the following implement is 
> not very elegant... Too complex compared to the qemu one.
>
> BTW: The S/R and live migration problem was fixed.
>
>   

Ah, good.

>
> +static int init_rmode_identity_map(struct kvm *kvm)
> +{
> +	int i, r, ret;
> +	pfn_t identity_map_pfn;
> +	u32 table[PT32_ENT_PER_PAGE];
>   

That's 4KB.  On i386 with 4K stacks, this may cause a stack overflow.  
Even with 8K stacks you're on thin ice here, with the temperature 
rapidly rising.

> +
> +	if (kvm->arch.ept_identity_pagetable_done)
> +		return 1;
> +	ret = 0;
> +	identity_map_pfn = VMX_EPT_IDENTITY_PAGETABLE_ADDR >> PAGE_SHIFT;
> +	r = kvm_clear_guest_page(kvm, identity_map_pfn, 0, PAGE_SIZE);
> +	if (r < 0)
> +		goto out;
> +	/*
> +	 * Set up identity-mapping pagetable for EPT in real mode, also verify
> +	 * the contain of page
>   

s/contain/contents/

> +	 * 0xe7 = _PAGE_PRESENT | _PAGE_RW | _PAGE_USER | _PAGE_ACCESSED |
> +	 *	  _PAGE_DIRTY   | _PAGE_PSE
> +	 */
> +	for (i = 0; i < PT32_ENT_PER_PAGE; i++)
> +		table[i] = (i << 22) + 0xe7;
>   

Instead of the comment, you can put the identifiers into the code 
instead of 0xe7.  And, to avoid the stack overflow, simply use 
kvm_write_guest() here.

> +static int alloc_identity_pagetable(struct kvm *kvm)
> +{
> +	struct kvm_userspace_memory_region kvm_userspace_mem;
> +	int r = 0;
> +
> +	down_write(&kvm->slots_lock);
> +	if (kvm->arch.ept_identity_pagetable)
> +		goto out;
> +	kvm_userspace_mem.slot = IDENTITY_PAGETABLE_PRIVATE_MEMSLOT;
> +	kvm_userspace_mem.flags = 0;
> +	kvm_userspace_mem.guest_phys_addr = VMX_EPT_IDENTITY_PAGETABLE_ADDR;
> +	kvm_userspace_mem.memory_size = PAGE_SIZE;
> +	r = __kvm_set_memory_region(kvm, &kvm_userspace_mem, 0);
> +	if (r)
> +		goto out;
> +
> +	down_read(&current->mm->mmap_sem);
> +	kvm->arch.ept_identity_pagetable = gfn_to_page(kvm,
> +			VMX_EPT_IDENTITY_PAGETABLE_ADDR >> PAGE_SHIFT);
> +	up_read(&current->mm->mmap_sem);
> +out:
> +	up_write(&kvm->slots_lock);
> +	return r;
> +}
>   

There's already a memslot for the tss, no?  Why not expand it by a page?

> +
>  static void allocate_vpid(struct vcpu_vmx *vmx)
>  {
>  	int vpid;
> @@ -1904,6 +1960,15 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
>  	return 0;
>  }
>
> +static int init_rmode(struct kvm *kvm)
> +{
> +	if (!init_rmode_tss(kvm))
> +		return 0;
> +	if (!init_rmode_identity_map(kvm))
> +		return 0;
> +	return 1;
> +}
> +
>  static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> @@ -1911,7 +1976,7 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  	int ret;
>
>  	down_read(&vcpu->kvm->slots_lock);
> -	if (!init_rmode_tss(vmx->vcpu.kvm)) {
> +	if (!init_rmode(vmx->vcpu.kvm)) {
>  		ret = -ENOMEM;
>  		goto out;
>  	}
> @@ -2967,6 +3032,10 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm 
> *kvm, unsigned int id)
>  		if (alloc_apic_access_page(kvm) != 0)
>  			goto free_vmcs;
>
> +	if (vm_need_ept())
> +		if (alloc_identity_pagetable(kvm) != 0)
> +			goto free_vmcs;
> +
>  	return &vmx->vcpu;
>
>  free_vmcs:
> diff --git a/arch/x86/kvm/vmx.h b/arch/x86/kvm/vmx.h
> index 8f662e3..469a107 100644
> --- a/arch/x86/kvm/vmx.h
> +++ b/arch/x86/kvm/vmx.h
> @@ -340,6 +340,7 @@ enum vmcs_field {
>  #define MSR_IA32_FEATURE_CONTROL_VMXON_ENABLED  0x4
>
>  #define APIC_ACCESS_PAGE_PRIVATE_MEMSLOT	9
> +#define IDENTITY_PAGETABLE_PRIVATE_MEMSLOT	10
>
>  #define VMX_NR_VPIDS				(1 << 16)
>  #define VMX_VPID_EXTENT_SINGLE_CONTEXT		1
> @@ -362,4 +363,6 @@ enum vmcs_field {
>  #define VMX_EPT_FAKE_ACCESSED_MASK		(1ul << 62)
>  #define VMX_EPT_FAKE_DIRTY_MASK			(1ul << 63)
>
> +#define VMX_EPT_IDENTITY_PAGETABLE_ADDR		0xfffbc000ul
> +
>  #endif
> diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
> index 003bc0e..69afbab 100644
> --- a/include/asm-x86/kvm_host.h
> +++ b/include/asm-x86/kvm_host.h
> @@ -314,6 +314,9 @@ struct kvm_arch{
>  	struct page *apic_access_page;
>
>  	gpa_t wall_clock;
> +
> +	struct page *ept_identity_pagetable;
> +	bool ept_identity_pagetable_done;

Why not use ept_identity_pagetable != NULL to encode 
ept_identity_pagetable_done?


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


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone