WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

RE: [Xen-devel][PATCH] unshadow the page table page which are used as da

To: "Tim Deegan" <Tim.Deegan@xxxxxxxxxx>
Subject: RE: [Xen-devel][PATCH] unshadow the page table page which are used as data page
From: "Xin, Xiaohui" <xiaohui.xin@xxxxxxxxx>
Date: Fri, 7 Dec 2007 21:12:05 +0800
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, "Kay, Allen M" <allen.m.kay@xxxxxxxxx>
Delivery-date: Fri, 07 Dec 2007 05:13:55 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20071205101016.GA7121@xxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <9A1462408D6D394C8A7A812E98F00A4D0251FEBC@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20071205101016.GA7121@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acg3JxcvJy7cdH7FTNOZ4gVnPZtvdwBpzR7Q
Thread-topic: [Xen-devel][PATCH] unshadow the page table page which are used as data page
Tim,
Attached is the updated patch which based on some part of your suggestion and 
some part of our new thoughts about it. We have re-checked the code path of the 
guest write emulate, found that in some extent(not all) the code checks the 
valid mfn for the guest written data. But maybe for the optimization, the code 
just check valide mfn when PRESENT bit exists. Maybe it can cover most of the 
cases, but not all, that's what we have found in the vt-d iperf test.

To minimize the hurt to other performance of shadow, the patch tries to use the 
valid mfn check in the original code, please have a review. I'm not sure about 
the cost of the gfn_to_mfn(), and not sure whether we may get some trade-off. 
If you have good ideas, please let us know.

Thanks
Xiaohui
-----Original Message-----
From: Tim Deegan [mailto:Tim.Deegan@xxxxxxxxxx] 
Sent: 2007年12月5日 18:10
To: Xin, Xiaohui
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx; Kay, Allen M
Subject: Re: [Xen-devel][PATCH] unshadow the page table page which are used as 
data page

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.]

Attachment: perf3.diff
Description: perf3.diff

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