From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2 3/8] xen/x86: Construct the {l2, l3}_bootmap at compile time Date: Wed, 24 Feb 2016 12:07:21 +0000 Message-ID: <56CD9CF9.40404@citrix.com> References: <1456245085-2302-1-git-send-email-andrew.cooper3@citrix.com> <1456245085-2302-4-git-send-email-andrew.cooper3@citrix.com> <56CDA36102000078000D5953@prv-mh.provo.novell.com> <56CD9699.9070500@citrix.com> <56CDA6FA02000078000D5991@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: <56CDA6FA02000078000D5991@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: Xen-devel List-Id: xen-devel@lists.xenproject.org On 24/02/16 11:50, Jan Beulich wrote: >>>> On 24.02.16 at 12:40, wrote: >> On 24/02/16 11:34, Jan Beulich wrote: >>>>>> On 23.02.16 at 17:31, wrote: >>>> --- >>>> xen/arch/x86/boot/head.S | 18 +++++------------- >>>> xen/arch/x86/boot/x86_64.S | 19 +++++++++++++++++++ >>>> xen/arch/x86/x86_64/mm.c | 4 ---- >>>> 3 files changed, 24 insertions(+), 17 deletions(-) >>> Is this intentionally leaving the EFI equivalent untouched? >> Yes. >> >>>> --- a/xen/arch/x86/boot/x86_64.S >>>> +++ b/xen/arch/x86/boot/x86_64.S >>>> @@ -184,3 +184,22 @@ GLOBAL(idle_pg_table) >>>> .size idle_pg_table, . - idle_pg_table >>>> >>>> GLOBAL(__page_tables_end) >>>> + >>>> +/* Init pagetables. Enough page directories to map into the bottom 1GB. */ >>>> + .section .init.data, "a", @progbits >>>> + .align PAGE_SIZE, 0 >>>> + >>>> +GLOBAL(l2_bootmap) >>>> + .quad sym_phys(l1_identmap) + __PAGE_HYPERVISOR >>> The relocation needed for this and ... >>> >>>> + idx = 1 >>>> + .rept 7 >>>> + .quad (idx << L2_PAGETABLE_SHIFT) | __PAGE_HYPERVISOR | _PAGE_PSE >>>> + idx = idx + 1 >>>> + .endr >>>> + .fill L2_PAGETABLE_ENTRIES - 8, 8, 0 >>>> + .size l2_bootmap, . - l2_bootmap >>>> + >>>> +GLOBAL(l3_bootmap) >>>> + .quad sym_phys(l2_bootmap) + __PAGE_HYPERVISOR >>> ... this won't get properly adjusted by efi_arch_relocate_image(), >>> due to living outside of [__page_tables_start, __page_tables_end). >> Deliberately so. >> >> The EFI needs to relocate the tables anyway. It currently (re)writes >> them fresh at the relocated address, and leaving this be is the more >> simple option. > Okay, I guess you mean to say that both together continue to > work, which without it being said explicitly (allowing the implication > that this also has got tested) I was rather unsure about. Hence > I'd like to at least ask for an explicit respective statement in the > commit message. Ok. The other reason why leaving the EFI side along is that these tables can't both be in .init.data and covered by [__page_tables_start, __page_tables_end). ~Andrew