Re: Xen PV PTE ABI (or lack thereof)

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 21/01/16 13:55, Jan Beulich wrote:

>>>> I was intending to have CONFIG_PV_PTE_DEBUG as an EXPERT option,
>>>> disabled by default even in debug builds.
>>>>
>>>> There should not be an ABI difference between release and "normal" debug
>>>> builds.
>>> Well, I see your point, but as said above I'm not convinced
>>> disabling all that code is the right solution. In fact, what you
>>> propose is not far away from removing that code altogether.
>> The two bits are only used for specialised debugging.  They should be
>> relegated to people doing specific debugging, and not interfere with the
>> overwhelming majority of cases where Xen doesn't need to use any
>> software available PTE bits.
> Your repeated claim that _PAGE_GUEST_KERNEL is purely debugging only
> makes we wonder how you would mean to adjust adjust_guest_l1e() with
> that flag gone (most notably the last of its if()-s).

diff --git a/xen/arch/x86/mm.c
b/xen/arch/x86/mm.c                                                                                                                                      

index b81d1fd..46ef5ce
100644                                                                                                                                                           

---
a/xen/arch/x86/mm.c                                                                                                                                                                 

+++
b/xen/arch/x86/mm.c                                                                                                                                                                 

@@ -1060,10 +1060,11 @@
get_page_from_l4e(                                                                                                                                              

                  == (_PAGE_GUEST_KERNEL|_PAGE_GLOBAL)
)                     
\                                                                                                         

                 MEM_LOG("Global bit is set to kernel page
%lx",             
\                                                                                                         

                        
l1e_get_pfn((pl1e)));                               
\                                                                                                         

-            if ( !(l1e_get_flags((pl1e)) & _PAGE_USER)
)                    
\                                                                                                         

-                l1e_add_flags((pl1e),
(_PAGE_GUEST_KERNEL|_PAGE_USER));     
\                                                                                                         

-            if ( !(l1e_get_flags((pl1e)) & _PAGE_GUEST_KERNEL)
)            
\                                                                                                         

-                l1e_add_flags((pl1e),
(_PAGE_GLOBAL|_PAGE_USER));           
\                                                                                                         

+            if ( l1e_get_flags((pl1e)) & _PAGE_USER
)                       
\                                                                                                         

+                l1e_add_flags((pl1e),
_PAGE_GLOBAL);                        
\                                                                                                         

+           
else                                                            
\                                                                                                         

+                l1e_remove_flags((pl1e),
_PAGE_GLOBAL);                     
\                                                                                                         

+            l1e_add_flags((pl1e),
_PAGE_USER);                              
\                                                                                                         

        
}                                                                   
\                                                                                                         

     } while ( 0 )


_PAGE_GUEST_KERNEL isn't in the ABI, which means that the 2nd if() is
the only piece of code which validly sets it.

Read-modify-write operations already don't function correctly as
_PAGE_GUEST_KERNEL is a hidden saturating bit from the guests point of view.

~Andrew