[Xen-devel] [PATCH] Re: xen/x86: Fix compilation issue with clang 3.2

[Tim Deegan <tim@xxxxxxx>]

At 14:20 +0100 on 01 May (1367418011), Julien Grall wrote:
> HYPERVISOR_VIRT_START is an unsigned long constant. In shadow code, Xen
> compares this value with a int which is wrong.

Thanks for the report.

In fact the comparison is never going to be true, even with the wider
type, since we're comparing 8796025913344 to a number that's never
larger than 2047.  This is a hangover from the 32-bit build, so I'll
just remove it:

commit aa2f0f18c7068127520c97b3b2ad9acf039ee7e9
Author: Tim Deegan <tim@xxxxxxx>
Date:   Thu May 2 11:37:56 2013 +0100

    x86/mm/shadow: remove dead code for avoiding Xen entries on 32-bit tables.
    
    All non-external-mode (==PV) guests have 4-level pagetables now that
    the PAE build of Xen is gone.
    
    This patch should have no effect, since the condition it removes could
    never be true anyway: the l2 offset of HYPERVISOR_VIRT_START on 64-bit
    Xen is much higher than any l2 offset we could have seen in the
    tables (and indeed bigger than the 'int' type, which clang was
    complaining about).  Actual compat PV guest xen entries are handled by
    the equivalent test in the 64-bit SHADOW_FOREACH_L2E() below.
    
    Reported-by: Julien Grall <julien.grall@xxxxxxxxxx>
    Signed-off-by: Tim Deegan <tim@xxxxxxx>

diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index a8ef75e..e216063 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -1305,26 +1305,23 @@ do {                                                    
                \
 
 #if GUEST_PAGING_LEVELS == 2
 
-/* 32-bit l2 on PAE/64: four pages, touch every second entry, and avoid Xen */
+/* 32-bit l2 on PAE/64: four pages, touch every second entry */
 #define SHADOW_FOREACH_L2E(_sl2mfn, _sl2e, _gl2p, _done, _dom, _code)     \
 do {                                                                      \
     int _i, _j, __done = 0;                                               \
-    int _xen = !shadow_mode_external(_dom);                               \
-    ASSERT(mfn_to_page(_sl2mfn)->u.sh.type == SH_type_l2_32_shadow);\
+    ASSERT(shadow_mode_external(_dom));                                   \
+    ASSERT(mfn_to_page(_sl2mfn)->u.sh.type == SH_type_l2_32_shadow);      \
     for ( _j = 0; _j < 4 && !__done; _j++ )                               \
     {                                                                     \
         shadow_l2e_t *_sp = sh_map_domain_page(_sl2mfn);                  \
         for ( _i = 0; _i < SHADOW_L2_PAGETABLE_ENTRIES; _i += 2 )         \
-            if ( (!(_xen))                                                \
-                 || ((_j * SHADOW_L2_PAGETABLE_ENTRIES) + _i)             \
-                 < (HYPERVISOR_VIRT_START >> SHADOW_L2_PAGETABLE_SHIFT) ) \
-            {                                                             \
-                (_sl2e) = _sp + _i;                                       \
-                if ( shadow_l2e_get_flags(*(_sl2e)) & _PAGE_PRESENT )     \
-                    {_code}                                               \
-                if ( (__done = (_done)) ) break;                          \
-                increment_ptr_to_guest_entry(_gl2p);                      \
-            }                                                             \
+        {                                                                 \
+            (_sl2e) = _sp + _i;                                           \
+            if ( shadow_l2e_get_flags(*(_sl2e)) & _PAGE_PRESENT )         \
+                {_code}                                                   \
+            if ( (__done = (_done)) ) break;                              \
+            increment_ptr_to_guest_entry(_gl2p);                          \
+        }                                                                 \
         sh_unmap_domain_page(_sp);                                        \
         if ( _j < 3 ) _sl2mfn = sh_next_page(_sl2mfn);                    \
     }                                                                     \
@@ -1332,26 +1329,22 @@ do {                                                    
                  \
 
 #elif GUEST_PAGING_LEVELS == 3
 
-/* PAE: if it's an l2h, don't touch Xen mappings */
+/* PAE: touch all entries */
 #define SHADOW_FOREACH_L2E(_sl2mfn, _sl2e, _gl2p, _done, _dom, _code)      \
 do {                                                                       \
     int _i;                                                                \
-    int _xen = !shadow_mode_external(_dom);                                \
     shadow_l2e_t *_sp = sh_map_domain_page((_sl2mfn));                     \
-    ASSERT(mfn_to_page(_sl2mfn)->u.sh.type == SH_type_l2_pae_shadow \
-           || mfn_to_page(_sl2mfn)->u.sh.type == SH_type_l2h_pae_shadow);\
+    ASSERT(shadow_mode_external(_dom));                                    \
+    ASSERT(mfn_to_page(_sl2mfn)->u.sh.type == SH_type_l2_pae_shadow        \
+           || mfn_to_page(_sl2mfn)->u.sh.type == SH_type_l2h_pae_shadow);  \
     for ( _i = 0; _i < SHADOW_L2_PAGETABLE_ENTRIES; _i++ )                 \
-        if ( (!(_xen))                                                     \
-             || mfn_to_page(_sl2mfn)->u.sh.type != SH_type_l2h_pae_shadow\
-             || ((_i + (3 * SHADOW_L2_PAGETABLE_ENTRIES))                  \
-                 < (HYPERVISOR_VIRT_START >> SHADOW_L2_PAGETABLE_SHIFT)) ) \
-        {                                                                  \
-            (_sl2e) = _sp + _i;                                            \
-            if ( shadow_l2e_get_flags(*(_sl2e)) & _PAGE_PRESENT )          \
-                {_code}                                                    \
-            if ( _done ) break;                                            \
-            increment_ptr_to_guest_entry(_gl2p);                           \
-        }                                                                  \
+    {                                                                      \
+        (_sl2e) = _sp + _i;                                                \
+        if ( shadow_l2e_get_flags(*(_sl2e)) & _PAGE_PRESENT )              \
+            {_code}                                                        \
+        if ( _done ) break;                                                \
+        increment_ptr_to_guest_entry(_gl2p);                               \
+    }                                                                      \
     sh_unmap_domain_page(_sp);                                             \
 } while (0)
 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel