[PATCH] Allow 32bit PAE guests to change their L3 entries

[PATCH] Allow 32bit PAE guests to change their L3 entries

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]


Hi!

adjust_guest_l3e() adds (_PAGE_USER|_PAGE_RW) to the entry, and then
get_page_from_l3e() checks that _PAGE_USER or _PAGE_RW are not present
for PAE guests.  get_page_from_l3e() will always fail at this point
for PAE guests.

Attached patch fixes this. This bug has been uncovered with NetBSD 32bit PAE 
guests.

Patch from Manuel Bouyer <bouyer@netbsd.org> for xen-3.1-testing,
ported to xen-unstable by me.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


Keir: Please apply this patch also to xen-3.2-testing and xen-3.1-testing.



-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

[-- Attachment #2: xen_mm.diff --]
[-- Type: text/plain, Size: 734 bytes --]

diff -r 819399d0bdb1 xen/arch/x86/mm.c
--- a/xen/arch/x86/mm.c	Fri Feb 15 14:16:42 2008 +0000
+++ b/xen/arch/x86/mm.c	Mon Feb 18 10:55:18 2008 +0100
@@ -1567,14 +1567,14 @@ static int mod_l3_entry(l3_pgentry_t *pl
             return 0;
         }
 
-        adjust_guest_l3e(nl3e, d);
-
         /* Fast path for identical mapping and presence. */
         if (!l3e_has_changed(ol3e, nl3e, _PAGE_PRESENT))
             return UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, pfn, curr, preserve_ad);
 
         if ( unlikely(!get_page_from_l3e(nl3e, pfn, d)) )
             return 0;
+
+        adjust_guest_l3e(nl3e, d);
 
         if ( unlikely(!UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, pfn, curr,
                                     preserve_ad)) )

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] Allow 32bit PAE guests to change their L3 entries

[Keir Fraser]

On 18/2/08 12:32, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> adjust_guest_l3e() adds (_PAGE_USER|_PAGE_RW) to the entry, and then
> get_page_from_l3e() checks that _PAGE_USER or _PAGE_RW are not present
> for PAE guests.  get_page_from_l3e() will always fail at this point
> for PAE guests.
> 
> Attached patch fixes this. This bug has been uncovered with NetBSD 32bit PAE
> guests.

It breaks the fast update path, and also all mod_lN_entry() functions should
probably be reorganised for consistency. I'll make these changes and apply
it.

 -- Keir

Re: [PATCH] Allow 32bit PAE guests to change their L3 entries

[Christoph Egger]

On Monday 18 February 2008 14:44:32 Keir Fraser wrote:
> On 18/2/08 12:32, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > adjust_guest_l3e() adds (_PAGE_USER|_PAGE_RW) to the entry, and then
> > get_page_from_l3e() checks that _PAGE_USER or _PAGE_RW are not present
> > for PAE guests.  get_page_from_l3e() will always fail at this point
> > for PAE guests.
> >
> > Attached patch fixes this. This bug has been uncovered with NetBSD 32bit
> > PAE guests.
>
> It breaks the fast update path, and also all mod_lN_entry() functions
> should probably be reorganised for consistency. I'll make these changes and
> apply it.

Thank you very much. But shouldn't have c/s 17061 gone into xen-staging first
and then into xen-stable ?

Christoph



-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH] Allow 32bit PAE guests to change their L3 entries

[Keir Fraser]

On 18/2/08 13:57, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> It breaks the fast update path, and also all mod_lN_entry() functions
>> should probably be reorganised for consistency. I'll make these changes and
>> apply it.
> 
> Thank you very much. But shouldn't have c/s 17061 gone into xen-staging first
> and then into xen-stable ?

It's in both. Also now in 3.2 and 3.1 staging trees.

 -- Keir

Re: [PATCH] Allow 32bit PAE guests to change their L3 entries

[Christoph Egger]

On Monday 18 February 2008 15:35:28 Keir Fraser wrote:
> On 18/2/08 13:57, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> >> It breaks the fast update path, and also all mod_lN_entry() functions
> >> should probably be reorganised for consistency. I'll make these changes
> >> and apply it.
> >
> > Thank you very much. But shouldn't have c/s 17061 gone into xen-staging
> > first and then into xen-unstable ?
>
> It's in both. Also now in 3.2 and 3.1 staging trees.

Thanks.  I just was surprised seeing the patch in xen-unstable before in 
xen-staging. Maybe your automated regression system had a kind of a "race"
window :)

Christoph


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH] Allow 32bit PAE guests to change their L3 entries

[Keir Fraser]

On 18/2/08 14:41, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>>> Thank you very much. But shouldn't have c/s 17061 gone into xen-staging
>>> first and then into xen-unstable ?
>> 
>> It's in both. Also now in 3.2 and 3.1 staging trees.
> 
> Thanks.  I just was surprised seeing the patch in xen-unstable before in
> xen-staging. Maybe your automated regression system had a kind of a "race"
> window :)

I pushed it through manually as the test harness has failed (apparently)
spuriously a couple of times in a row.

 -- Keir