On Thursday 24 April 2008 15:15:30 Avi Kivity wrote:
> 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 <[EMAIL PROTECTED]>
> >>> 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.
Oops... I forgot that...
> > +
> > + 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.
OK.
> > +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(¤t->mm->mmap_sem);
> > + kvm->arch.ept_identity_pagetable = gfn_to_page(kvm,
> > + VMX_EPT_IDENTITY_PAGETABLE_ADDR >> PAGE_SHIFT);
> > + up_read(¤t->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?
Agree.
> > +
> > 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?
ept_identity_pagetable_done was used to indicate if the pagetable was setted
up, and ept_identity_pagetable was used to indicate if the page used for
pagetable was allocated... I don't want to run alloc_identity_pagetable()
again and again and again... Another method is read several bits at the front
of page to tell if the pagetable was setted up, but somehow tricky...
-------------------------------------------------------------------------
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
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
