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

[Xen-devel] Re: [PATCH 5/6] Handler m2p table and frametable fault in pa

To: "Yunhong Jiang" <yunhong.jiang@xxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH 5/6] Handler m2p table and frametable fault in page faulthandler
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Mon, 29 Jun 2009 10:32:18 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Tim Deegan <Tim.Deegan@xxxxxxxxxx>, Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Delivery-date: Mon, 29 Jun 2009 02:32:49 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <E2263E4A5B2284449EEBD0AAB751098402CCF824D2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
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/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <E2263E4A5B2284449EEBD0AAB751098402CCF824D2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>>> "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx> 28.06.09 11:27 >>>
>...
>+int handle_memadd_fault(unsigned long addr, struct cpu_user_regs *regs)
>+{
>+    l3_pgentry_t  *pl3e;
>+    l3_pgentry_t l3e;
>+    l2_pgentry_t *pl2e;
>+    l2_pgentry_t l2e, idle_l2e;
>+    unsigned long mfn, cr3;
>+
>+    idle_l2e = idle_pg_table_l2[l2_linear_offset(addr)];
>+    if (!(l2e_get_flags(idle_l2e) & _PAGE_PRESENT))
>+        return 0;
>+
>+    cr3 = read_cr3();
>+    mfn = cr3 >> PAGE_SHIFT;
>+
>+    /*
>+     * No need get page type here and validate checking for xen mapping
>+     */
>+    pl3e = map_domain_page(mfn);
>+    pl3e += (cr3 & 0xFE0UL) >> 3;
>+    l3e = pl3e[3];
>+
>+    if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
>+        return 0;
>+
>+    mfn = l3e_get_pfn(l3e);
>+    pl2e = map_domain_page(mfn);
>+
>+    l2e = pl2e[l2_table_offset(addr)];
>+
>+    if (l2e_get_flags(l2e) & _PAGE_PRESENT)
>+        return 0;

You'd also need to check that idle_page_table_l2's entry has _PAGE_PRESENT
set here, otherwise you'd risk live locking the domain on an out-of-current-
bounds M2P access.

Furthermore, I'm unsure forwarding the page fault in case you found the
domain's L2 entry to already have _PAGE_PRESENT set is a good way to
handle things: A racing access on another vCPU of the same guest may just
have managed to install that entry.

Bottom line is, you probably need to exclusively check idle_page_table_l2
here.

>+
>+    memcpy(&pl2e[l2_table_offset(addr)],
>+            &idle_pg_table_l2[l2_linear_offset(addr)],
>+            sizeof(l2_pgentry_t));

Using memcpy for a single page table entry seems odd - why not use a direct
assignment? However, perhaps using memcpy() here is the right thing - to
avoid future faults, you could update the full M2P related part of the L2
directory in a single step. This ought to be safe, since the M2P table can only
be extended, but never shrunk.

>+
>+    return EXCRET_fault_fixed;
>+}

You're leaking map_domain_page()-es here (and at the earlier return points).

(All the comments likewise apply to the subsequent 64-bit variant.)

Jan


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

<Prev in Thread] Current Thread [Next in Thread>