Huang2, Wei
2007-Feb-27 05:58 UTC
[Xen-devel] [PATCH][XEN-PAGING][2/2] Enable hardware assisted paging and AMD-V nested paging support
This patch supports hardware assisted paging based on new paging interface. Additionally, it also enables AMD-V nested paging. Please apply to xen-unstable tree. Signed-off-by: Wei Huang <wei.huang2@amd.com> b/xen/arch/x86/mm/hap/Makefile | 2 b/xen/arch/x86/mm/hap/hap.c | 735 ++++++++++++++++++++++++++++++++++++++++ b/xen/arch/x86/mm/hap/private.h | 109 +++++ b/xen/arch/x86/mm/hap/support.c | 327 +++++++++++++++++ b/xen/include/asm-x86/hap.h | 127 ++++++ xen/arch/x86/hvm/hvm.c | 3 xen/arch/x86/hvm/svm/svm.c | 159 ++++++++ xen/arch/x86/hvm/svm/vmcb.c | 10 xen/arch/x86/mm/Makefile | 1 xen/arch/x86/mm/paging.c | 37 +- xen/include/asm-x86/domain.h | 16 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2007-Feb-27 11:31 UTC
[Xen-devel] Re: [PATCH][XEN-PAGING][2/2] Enable hardware assisted paging and AMD-V nested paging support
Hi, Thanks for your patch. Brief comments inline: Content-Description: hap_npt_patch.txt> +static int svm_do_nested_pgfault(unsigned long va, struct cpu_user_regs *regs) > +{ > + unsigned long gpa = va; > + if (mmio_space(gpa)) { > + handle_mmio(gpa); > + return 1; > + } > + > + /* P2M table needs to be fixed. */ > + return paging_fault(va, regs); > +} > +#VMEXIT(NPF) returns a guest physical address, which (a) might be longer than an unsigned long (PAE guest on PAE host) and (b) probably shouldn''t be called "va" in the prototype. :) You''ll either need to extend paging_fault() to take a wider type or (since your current implementation calls domain_crash() unconditionally) just not call it at all.> void paging_domain_init(struct domain *d) > { > p2m_init(d); > - shadow_domain_init(d); > + > + if ( opt_hap_enabled && hap_capable_system ) > + hap_domain_init(d); > + else > + shadow_domain_init(d); > }shadow_domain_init() should be called regardless of whether the domain will be shadowed; it just sets up some locks and list_heads. ( opt_hap_enabled && hap_capable_system ) is not enough to detect whether to use HAP; you need to test is_hvm_domain(d) as well.> /* vcpu paging struct initialization goes here */ > void paging_vcpu_init(struct vcpu *v) > { > - shadow_vcpu_init(v); > + if ( opt_hap_enabled && hap_capable_system ) > + hap_vcpu_init(v); > + else > + shadow_vcpu_init(v); > }... && is_hvm_vcpu(v). Likewise in the rest of the file.> + if ( unlikely(d == current->domain) ) { > + gdprintk(XENLOG_INFO, "Don''t try to do a npt op on yourself!\n"); > + return -EINVAL; > + }ITYM "a HAP op". :)> +void hap_update_paging_modes(struct vcpu *v) > +{ > + struct domain *d; > + > + HERE_I_AM; > + > + d = v->domain; > + hap_lock(d); > + > + /* update paging mode based on guest state */ > + npt_update_guest_paging_mode(v);This is in an architecture-independent file: there needs to be another layer of indirection here. Plumb through as a hvm_ function?> +/************************************************/ > +/* AMD NESTED PAGING FEATURES */ > +/************************************************/ > +void npt_detect(void) > +{ > + u32 eax, ebx, ecx, edx; > + > + /* check CPUID for nested paging support */ > + cpuid(0x8000000A, &eax, &ebx, &ecx, &edx); > + if ( edx & 0x01 ) { /* nested paging */ > + hap_capable_system = 1; > + } > + else if ( opt_hap_enabled ) { > + printk(" nested paging is not supported by this CPU.\n"); > + hap_capable_system = 0; /* no nested paging, we disable flag. */ > + } > +} > + > +/* update paging mode pointer for AMD nested paging */ > +void npt_update_guest_paging_mode(struct vcpu *v) > +{ > + struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; > + u64 cr0_value = vmcb->cr0; > + u64 cr4_value = vmcb->cr4; > + u64 efer_value = vmcb->efer; > + > + if ( (cr0_value & X86_CR0_PE) && (cr0_value & X86_CR0_PG)) { > + if ( (efer_value & EFER_LME) && (cr4_value & X86_CR4_PAE) ) > + v->arch.paging.mode = &hap_paging_long_mode; > + else if ( cr4_value & X86_CR4_PAE) > + v->arch.paging.mode = &hap_paging_pae_mode; > + else > + v->arch.paging.mode = &hap_paging_protected_mode; > + } > + else { > + v->arch.paging.mode = &hap_paging_real_mode; > + } > +}These should be in their own file, or in arch/x86/hvm/svm/ somewhere.> +#define NPT_GUEST_CR3_SHIFT_NON_PAE 12 /* both legacy mode and long mode */ > +#define NPT_GUEST_CR3_SHIFT_PAE 5 /* PAE mode */Call these HAP_GUEST_CR3_SHIFT_*? Or, since you don''t use them, leave them out.> +#include "../shadow/page-guest32.h"Move this file into arch/x86/mm/ if it''s going to be shared. Cheers, Tim. P.S. Another quick question: have you looked at virtualizing gate A20? -- Tim Deegan <Tim.Deegan@xensource.com>, XenSource UK Limited Registered office c/o EC2Y 5EB, UK; company number 05334508 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Huang2, Wei
2007-Feb-28 05:56 UTC
[Xen-devel] RE: [PATCH][XEN-PAGING][2/2] Enable hardware assisted paging and AMD-V nested paging support
Tim, I fixed the patch based on your comments. The changes are listed as following: 1. It calls is_hvm_domain()/is_hvm_vcpu() for checking before hap is turned on. 2. It always calls shadow_vcpu_init(). 3. It moves npt_detect() to svm.c. 4. Instead of calling npt_update_paging_mode(), it uses hvm_paging_enabled(), hvm_pae_enabled(), hvm_long_mode_enabled() to update guest''s mode. These functions are not architecture dependent. 5. It moves page-guest32.h to upper directory. This file is shared by both shadow code and hap code. 6. I also cleaned up the code to fix minor issues. I have tested the code and it did not break shadow code. Thanks, -Wei -----Original Message----- From: Tim Deegan [mailto:Tim.Deegan@xensource.com] Sent: Tuesday, February 27, 2007 5:31 AM To: Huang2, Wei Cc: Xen Devel; Tim Deegan Subject: Re: [PATCH][XEN-PAGING][2/2] Enable hardware assisted paging and AMD-V nested paging support Hi, Thanks for your patch. Brief comments inline: Content-Description: hap_npt_patch.txt> +static int svm_do_nested_pgfault(unsigned long va, structcpu_user_regs *regs)> +{ > + unsigned long gpa = va; > + if (mmio_space(gpa)) { > + handle_mmio(gpa); > + return 1; > + } > + > + /* P2M table needs to be fixed. */ > + return paging_fault(va, regs); > +} > +#VMEXIT(NPF) returns a guest physical address, which (a) might be longer than an unsigned long (PAE guest on PAE host) and (b) probably shouldn''t be called "va" in the prototype. :) You''ll either need to extend paging_fault() to take a wider type or (since your current implementation calls domain_crash() unconditionally) just not call it at all.> void paging_domain_init(struct domain *d) > { > p2m_init(d); > - shadow_domain_init(d); > + > + if ( opt_hap_enabled && hap_capable_system ) > + hap_domain_init(d); > + else > + shadow_domain_init(d); > }shadow_domain_init() should be called regardless of whether the domain will be shadowed; it just sets up some locks and list_heads. ( opt_hap_enabled && hap_capable_system ) is not enough to detect whether to use HAP; you need to test is_hvm_domain(d) as well.> /* vcpu paging struct initialization goes here */ > void paging_vcpu_init(struct vcpu *v) > { > - shadow_vcpu_init(v); > + if ( opt_hap_enabled && hap_capable_system ) > + hap_vcpu_init(v); > + else > + shadow_vcpu_init(v); > }... && is_hvm_vcpu(v). Likewise in the rest of the file.> + if ( unlikely(d == current->domain) ) { > + gdprintk(XENLOG_INFO, "Don''t try to do a npt op onyourself!\n");> + return -EINVAL; > + }ITYM "a HAP op". :)> +void hap_update_paging_modes(struct vcpu *v) > +{ > + struct domain *d; > + > + HERE_I_AM; > + > + d = v->domain; > + hap_lock(d); > + > + /* update paging mode based on guest state */ > + npt_update_guest_paging_mode(v);This is in an architecture-independent file: there needs to be another layer of indirection here. Plumb through as a hvm_ function?> +/************************************************/ > +/* AMD NESTED PAGING FEATURES */ > +/************************************************/ > +void npt_detect(void) > +{ > + u32 eax, ebx, ecx, edx; > + > + /* check CPUID for nested paging support */ > + cpuid(0x8000000A, &eax, &ebx, &ecx, &edx); > + if ( edx & 0x01 ) { /* nested paging */ > + hap_capable_system = 1; > + } > + else if ( opt_hap_enabled ) { > + printk(" nested paging is not supported by this CPU.\n"); > + hap_capable_system = 0; /* no nested paging, we disable flag.*/> + } > +} > + > +/* update paging mode pointer for AMD nested paging */ > +void npt_update_guest_paging_mode(struct vcpu *v) > +{ > + struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; > + u64 cr0_value = vmcb->cr0; > + u64 cr4_value = vmcb->cr4; > + u64 efer_value = vmcb->efer; > + > + if ( (cr0_value & X86_CR0_PE) && (cr0_value & X86_CR0_PG)) { > + if ( (efer_value & EFER_LME) && (cr4_value & X86_CR4_PAE) ) > + v->arch.paging.mode = &hap_paging_long_mode; > + else if ( cr4_value & X86_CR4_PAE) > + v->arch.paging.mode = &hap_paging_pae_mode; > + else > + v->arch.paging.mode = &hap_paging_protected_mode; > + } > + else { > + v->arch.paging.mode = &hap_paging_real_mode; > + } > +}These should be in their own file, or in arch/x86/hvm/svm/ somewhere.> +#define NPT_GUEST_CR3_SHIFT_NON_PAE 12 /* both legacy mode and longmode */> +#define NPT_GUEST_CR3_SHIFT_PAE 5 /* PAE mode */Call these HAP_GUEST_CR3_SHIFT_*? Or, since you don''t use them, leave them out.> +#include "../shadow/page-guest32.h"Move this file into arch/x86/mm/ if it''s going to be shared. Cheers, Tim. P.S. Another quick question: have you looked at virtualizing gate A20? -- Tim Deegan <Tim.Deegan@xensource.com>, XenSource UK Limited Registered office c/o EC2Y 5EB, UK; company number 05334508 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel