Re: [Xen-devel][PATCH] unshadow the page table page which are used as data page

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

Hi,

At 10:48 +0800 on 05 Dec (1196851700), Xin, Xiaohui wrote:
> The patch will check if the data guest writes to the page table contains
> valid mfn or not, if not, we believe it is a data page now and then
> unshadow the page.

Good plan, thanks.

> +static inline void
> +check_for_data_page_unshadow(struct vcpu *v, void *addr,mfn_t mfn)
> +{
> +    gfn_t gfn;
> +    p2m_type_t p2m_type;
> +
> +#if SHADOW_OPTIMIZATIONS & SHOPT_EARLY_UNSHADOW
> +    if ( mfn_to_page(mfn)->shadow_flags & SHF_L2_32 )
> +    {
> +        gfn = _gfn((paddr_t)(((*(u32*)addr) & (PADDR_MASK&PAGE_MASK))) >>
> +                 PAGE_SHIFT);

Shouldn't you also check for the _PAGE_PRESENT bit?  Otherwise I think
normal shadow performance could be hurt.

> +        if ( gfn_to_mfn(v->domain, gfn, &p2m_type) == INVALID_MFN )
> +            sh_remove_shadows(v, mfn, 1, 0);
> +    }
> +    else if (mfn_to_page(mfn)->shadow_flags &
> +                (SHF_L4_64|SHF_L2_PAE|SHF_L2H_PAE))

Why not SHF_L3_64, SHF_l2_64 and SHF_L2H_64?

> +    {
> +        gfn = _gfn((paddr_t)(((*(u64*)addr) & (PADDR_MASK&PAGE_MASK))) >>
> +                 PAGE_SHIFT);
> +        if ( gfn_to_mfn(v->domain, gfn, &p2m_type) == INVALID_MFN )
> +            sh_remove_shadows(v, mfn, 1, 0);
> +    }
> +#endif
> +}
>  
>  int
>  sh_x86_emulate_write(struct vcpu *v, unsigned long vaddr, void *src,
> @@ -4068,7 +4092,8 @@ sh_x86_emulate_write(struct vcpu *v, uns
>          check_for_early_unshadow(v, mfn);
>      else
>          reset_early_unshadow(v);
> -    
> +
> +    check_for_data_page_unshadow(v, addr, mfn);
>      paging_mark_dirty(v->domain, mfn_x(mfn));

This, and its equivalent in sh_x86_emulate_cmpxchg, need to be gated on
bytes() being greater than the size of the read you'll be doing in
check_for_data_page_unshadow() or you'll get #PF when a guest writes to
the top word of a page.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Citrix Systems.
[Company #5334508: XenSource UK Ltd, reg'd c/o EC2Y 5EB, UK.]

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel