Xen devel - Jan 2005 - [PATCH] X server crashes Xen on xeno-unstable.bk

[My platform is a VMX machine, but this is a generic problem in
xeno-unstable.bk. I use FC3 as a base and some onboard graphics
controller chipset]

Starting an X server on xeno-unstable.bk causes Xen to crash with
a page fault generated in Xen. It turns out that the fault is
generated by put_page_from_l1e().

The culprit is the following dereference:

     struct domain   *e = page->u.inuse.domain;

Where the value of page is undefined if the condition

     if ( !(l1v & _PAGE_PRESENT) || !pfn_is_ram(pfn) )

does hold. Unfortunately, this condition is checked after the dereference.

The fix is straightforward, move the dereference after the condition test.

	Leendert

--- xeno-unstable.bk/xen/arch/x86/memory.c.orig	2005-01-06 14:19:33.000000000
-0500
+++ xeno-unstable.bk/xen/arch/x86/memory.c	2005-01-06 15:02:54.000000000 -0500
@@ -467,11 +467,12 @@
     unsigned long    l1v  = l1_pgentry_val(l1e);
     unsigned long    pfn  = l1_pgentry_to_pagenr(l1e);
     struct pfn_info *page = &frame_table[pfn];
-    struct domain   *e = page->u.inuse.domain;
+    struct domain   *e;
 
     if ( !(l1v & _PAGE_PRESENT) || !pfn_is_ram(pfn) )
         return;
 
+    e = page->u.inuse.domain;
     if ( unlikely(e != d) )
     {
         /*


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It''s fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/xen-devel

# > The culprit is the following dereference:
# >
# >      struct domain   *e = page->u.inuse.domain;
# >
# > Where the value of page is undefined if the condition
# >
# >      if ( !(l1v & _PAGE_PRESENT) || !pfn_is_ram(pfn) )
# >
# > does hold. Unfortunately, this condition is checked after the
# > dereference.
# 
# Interesting. I''m guessing the reason we haven''t seen this is
that we
# don''t build on FC3. The newer version of gcc has obviously spotted
the
# opportunity to hoist the load.

The compiler I use (3.4.2) did hoist it, but not far enough. It moved
the dereference after the (l1v & _PAGE_PRESENT) condition but before
pfn_is_ram(pfn). pfn_is_ram(pfn) failed because this was video memory.

Looking through memory.c, this was the only case where page was derefenced
before pfn_is_ram() is called.

	Leendert


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It''s fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/xen-devel

> Starting an X server on xeno-unstable.bk causes Xen to crash with
> a page fault generated in Xen. It turns out that the fault is
> generated by put_page_from_l1e().
> 
> The culprit is the following dereference:
> 
>      struct domain   *e = page->u.inuse.domain;
> 
> Where the value of page is undefined if the condition
> 
>      if ( !(l1v & _PAGE_PRESENT) || !pfn_is_ram(pfn) )
> 
> does hold. Unfortunately, this condition is checked after the 
> dereference.
Interesting. I''m guessing the reason we haven''t seen this is
that we
don''t build on FC3. The newer version of gcc has obviously spotted the
opportunity to hoist the load.

I''ll checkin the fix.

Thanks,
Ian


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It''s fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/xen-devel

> > Interesting. I''m guessing the reason we haven''t seen
this is that we
> > don''t build on FC3. The newer version of gcc has obviously 
> spotted the
> > opportunity to hoist the load.
> 
> of curiosity what test platforms do you use anyway? for example that 
> "idle_loop" bug happens on Suse 9.1.
We build on a variety of distributions.

The nightly builds still happen on RH9, which is what I generally use (I
have a trusting relationship with gcc 3.2.2-5, and I fear change ;-)

Keir uses FC2, and Christian uses Debain Sarge from time to time too.

Testing gets done on a variety of installed distros that we have, adding
RH7.2 and FC3 to the above. 

We''re planning to start doing builds and tests on RHEL and SLES soon
too. (We''ve previously been too cheap ;-)


Ian


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It''s fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/xen-devel

> Interesting. I''m guessing the reason we haven''t seen this
is that we
> don''t build on FC3. The newer version of gcc has obviously spotted
the
> opportunity to hoist the load.
of curiosity what test platforms do you use anyway? for example that 
"idle_loop" bug happens on Suse 9.1.


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It''s fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/xen-devel