From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: Xen PV PTE ABI (or lack thereof) Date: Thu, 21 Jan 2016 15:10:32 +0000 Message-ID: <56A0F4E8.2060402@citrix.com> References: <569FE999.2080404@citrix.com> <56A0CA0902000078000C9899@prv-mh.provo.novell.com> <56A0BDF2.8030308@citrix.com> <56A0E42702000078000C9970@prv-mh.provo.novell.com> <56A0DA69.7090002@citrix.com> <56A0F17A02000078000C99EC@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56A0F17A02000078000C99EC@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: George Dunlap , Huaitong Han , TimDeegan , Xen-devel List List-Id: xen-devel@lists.xenproject.org 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