Re: [Xen-devel] [PATCH v2 4/6] x86/NPT: don't walk entire page tables when changing types on a range

[Tim Deegan <tim@xxxxxxx>]

At 13:30 +0100 on 22 Apr (1398169846), Jan Beulich wrote:
> This builds on the fact that in order for no NPF VM exit to occur,
> _PAGE_USER must always be set. I.e. by clearing the flag we can force a
> VM exit allowing us to do similar lazy type changes as on EPT.
> 
> That way, the generic entry-wise code can go away, and we could remove
> the range restriction in enforced on HVMOP_track_dirty_vram for XSA-27.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2557,7 +2557,16 @@ void svm_vmexit_handler(struct cpu_user_
>          perfc_incra(svmexits, VMEXIT_NPF_PERFC);
>          if ( cpu_has_svm_decode )
>              v->arch.hvm_svm.cached_insn_len = vmcb->guest_ins_len & 0xf;
> -        svm_do_nested_pgfault(v, regs, vmcb->exitinfo1, vmcb->exitinfo2);
> +        rc = p2m_npt_fault(vmcb->exitinfo2);

Can we limit the classes of fault that we need to check for
recalc on?  e.g. if bit 0 of the error code is clear, then the page is
not mapped at all. 

I'm unsure about having two different fixup paths here anyway -- one
for log-dirty and one for everything else (PoD, sharing &c).  This
call should probably go _inside_ svm_do_nested_pgfault() at least.

Also, maybe it wants a more descriptive name than p2m_npt_fault().
p2m_pt_hnadle_misconfig(), to match the EPT equivalent?

> +static int p2m_pt_change_entry_type_range(struct p2m_domain *p2m,
> +                                          p2m_type_t ot, p2m_type_t nt,
> +                                          unsigned long first_gfn,
> +                                          unsigned long last_gfn)
> +{
> +    unsigned long mask = (1 << PAGETABLE_ORDER) - 1;
> +    unsigned int i;
> +    int err = 0;
> +
> +    ASSERT(hap_enabled(p2m->domain));
> +
> +    for ( i = 1; i <= 4; )
> +    {
> +        if ( first_gfn & mask )
> +        {
> +            unsigned long end_gfn = min(first_gfn | mask, last_gfn);
> +
> +            err = p2m_pt_set_recalc_range(p2m, i, first_gfn, end_gfn);
> +            if ( err || end_gfn >= last_gfn )
> +                break;
> +            first_gfn = end_gfn + 1;
> +        }
> +        else if ( (last_gfn & mask) != mask )
> +        {
> +            unsigned long start_gfn = max(first_gfn, last_gfn & ~mask);
> +
> +            err = p2m_pt_set_recalc_range(p2m, i, start_gfn, last_gfn);
> +            if ( err || start_gfn <= first_gfn )
> +                break;
> +            last_gfn = start_gfn - 1;
> +        }
> +        else
> +        {
> +            ++i;
> +            mask |= mask << PAGETABLE_ORDER;
> +        }
> +    }
> +
> +    return err;
> +}

This looks like it could be merged with the EPT equivalent.  Or would
that be too unwieldy?

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel