| |
|
 |
|
 |
|
|
|
| |
|
|
xen-devel
[Xen-devel] Re: [PATCH][XEN-PAGING][2/2] Enable hardware assisted paging
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@xxxxxxxxxxxxx>, XenSource UK Limited
Registered office c/o EC2Y 5EB, UK; company number 05334508
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
|
|
| |
|
Home