Re: bug, bad page free with hvm guest after commits about AMD IOMMU

[Tim Deegan <Tim.Deegan@citrix.com>]

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

CC'ing John and George, who reported similar-sounding failures. 

At 14:56 +0100 on 05 May (1304607375), Anthony Perard wrote:
> On Thu, May 5, 2011 at 14:40, Tim Deegan <Tim.Deegan@citrix.com> wrote:
> > At 14:31 +0100 on 05 May (1304605909), Anthony PERARD wrote:
> >> Hi,
> >>
> >> With the last xen-unstable, when I try to run a HVM guest, the vgabios
> >> did not display anything on the screen (it is only white), and Xen
> >> print this:
> >
> > Interesting.  I get those messages but the BIOS continues OK so I hadn't
> > been in a hurry to investigate.  I'll take a look.
> 
> Yes, the guest works fine after that, but on linux, there is no vga
> console, but a X server work fine.

I suspect it's the use of type 0 to mean RAM: it means that an entry of
all zeroes no longer gives type mmio so something's not going to qemu
correctly.

Does the attached patch fix it for you?  I'm still seeing intermittent
failures with it but they may be unrelated. 

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2134 bytes --]

diff -r 39f2942fe56b xen/arch/x86/mm/hap/p2m-ept.c
--- a/xen/arch/x86/mm/hap/p2m-ept.c	Wed May 04 14:46:32 2011 +0100
+++ b/xen/arch/x86/mm/hap/p2m-ept.c	Thu May 05 15:43:27 2011 +0100
@@ -579,8 +579,9 @@ static mfn_t ept_get_entry(struct p2m_do
             goto out;
     }
 
-
-    if ( ept_entry->sa_p2mt != p2m_invalid )
+    /* Need to check for all-zeroes because typecode 0 is p2m_ram and an
+     * entirely empty entry shouldn't have RAM type. */
+    if ( ept_entry->epte != 0 && ept_entry->sa_p2mt != p2m_invalid )
     {
         *t = ept_entry->sa_p2mt;
         *a = ept_entry->access;
diff -r 39f2942fe56b xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c	Wed May 04 14:46:32 2011 +0100
+++ b/xen/arch/x86/mm/p2m.c	Thu May 05 15:43:27 2011 +0100
@@ -1883,9 +1883,6 @@ static mfn_t p2m_gfn_to_mfn_current(stru
             p2mt = p2m_flags_to_type(l1e_get_flags(l1e));
             ASSERT(l1e_get_pfn(l1e) != INVALID_MFN || !p2m_is_ram(p2mt));
 
-            if ( l1e.l1 == 0 )
-                p2mt = p2m_invalid;
-
             if ( p2m_flags_to_type(l1e_get_flags(l1e))
                  == p2m_populate_on_demand )
             {
diff -r 39f2942fe56b xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h	Wed May 04 14:46:32 2011 +0100
+++ b/xen/include/asm-x86/p2m.h	Thu May 05 15:43:27 2011 +0100
@@ -381,12 +381,12 @@ static inline p2m_type_t p2m_flags_to_ty
 {
     /* Type is stored in the "available" bits */
 #ifdef __x86_64__
-    /*
-     * AMD IOMMU: When we share p2m table with iommu, bit 9 - bit 11 will be
-     * used for iommu hardware to encode next io page level. Bit 59 - bit 62
-     * are used for iommu flags, We could not use these bits to store p2m types.
-     */
-
+    /* For AMD IOMMUs we need to use type 0 for plain RAM, but we need
+     * to make sure that an entirely empty PTE doesn't have RAM type */
+    if ( flags == 0 ) 
+        return p2m_invalid;
+    /* AMD IOMMUs use bits 9-11 to encode next io page level and bits
+     * 59-62 for iommu flags so we can't use them to store p2m type info. */
     return (flags >> 12) & 0x7f;
 #else
     return (flags >> 9) & 0x7;

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

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