From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aWdV6-0006Rz-OR for mharc-grub-devel@gnu.org; Fri, 19 Feb 2016 00:21:08 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45873) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWdV4-0006Rn-7A for grub-devel@gnu.org; Fri, 19 Feb 2016 00:21:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aWdUz-0007jM-50 for grub-devel@gnu.org; Fri, 19 Feb 2016 00:21:06 -0500 Received: from mx2.suse.de ([195.135.220.15]:39550) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWdUy-0007iZ-UY for grub-devel@gnu.org; Fri, 19 Feb 2016 00:21:01 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 5E28AAAB4; Fri, 19 Feb 2016 05:20:59 +0000 (UTC) Subject: Re: [PATCH v3 09/10] xen: modify page table construction To: Daniel Kiper References: <1455729577-23702-1-git-send-email-jgross@suse.com> <1455729577-23702-10-git-send-email-jgross@suse.com> <20160218164001.GE3482@olila.local.net-space.pl> From: Juergen Gross X-Enigmail-Draft-Status: N1110 Message-ID: <56C6A63A.9060200@suse.com> Date: Fri, 19 Feb 2016 06:20:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160218164001.GE3482@olila.local.net-space.pl> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] X-Received-From: 195.135.220.15 Cc: grub-devel@gnu.org, phcoder@gmail.com, mchang@suse.com, xen-devel@lists.xen.org X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Feb 2016 05:21:07 -0000 On 18/02/16 17:40, Daniel Kiper wrote: > On Wed, Feb 17, 2016 at 06:19:36PM +0100, Juergen Gross wrote: >> Modify the page table construction to allow multiple virtual regions >> to be mapped. This is done as preparation for removing the p2m list >> from the initial kernel mapping in order to support huge pv domains. >> >> This allows a cleaner approach for mapping the relocator page by >> using this capability. >> >> The interface to the assembler level of the relocator has to be changed >> in order to be able to process multiple page table areas. >> >> Signed-off-by: Juergen Gross >> --- >> V3: use constants instead of numbers as requested by Daniel Kiper >> add lots of comments to assembly code as requested by Daniel Kiper ... >> 1: >> + movq 0(%r8), %r12 /* Get start pfn of the current area */ >> + movq GRUB_TARGET_SIZEOF_LONG(%r8), %rcx /* Get # of pg tables */ > > Use %r9 here and... > >> + testq %rcx, %rcx /* 0 -> last area reached */ >> + jz 3f >> +2: >> movq %r12, %rdi >> - movq %rsi, %rbx >> - movq 0(%rsi), %rsi >> - shlq $12, %rsi >> - orq $5, %rsi >> - movq $2, %rdx >> - movq %rcx, %r9 >> + shlq $PAGE_SHIFT, %rdi /* virtual address (1:1 mapping) */ >> + movq (%rbx, %r12, 8), %rsi /* mfn */ >> + shlq $PAGE_SHIFT, %rsi >> + orq $(GRUB_PAGE_PRESENT | GRUB_PAGE_USER), %rsi /* Build pte */ >> + movq $UVMF_INVLPG, %rdx >> + movq %rcx, %r9 /* %rcx clobbered by hypercall */ > > ... you can avoid this... > >> movq $__HYPERVISOR_update_va_mapping, %rax >> syscall >> >> movq %r9, %rcx > > and this... > >> - addq $8, %rbx >> - addq $4096, %r12 >> - movq %rbx, %rsi >> + incq %r12 /* next pfn */ >> >> - loop 1b >> + loop 2b >> This would require to open code the loop statement here with %r9 as count register. ... >> diff --git a/grub-core/lib/xen/relocator.c b/grub-core/lib/xen/relocator.c >> index 8f427d3..250fbd4 100644 >> --- a/grub-core/lib/xen/relocator.c >> +++ b/grub-core/lib/xen/relocator.c >> @@ -36,15 +36,18 @@ extern grub_uint8_t grub_relocator_xen_remap_end; >> extern grub_xen_reg_t grub_relocator_xen_stack; >> extern grub_xen_reg_t grub_relocator_xen_start_info; >> extern grub_xen_reg_t grub_relocator_xen_entry_point; >> -extern grub_xen_reg_t grub_relocator_xen_paging_start; >> -extern grub_xen_reg_t grub_relocator_xen_paging_size; >> extern grub_xen_reg_t grub_relocator_xen_remapper_virt; >> extern grub_xen_reg_t grub_relocator_xen_remapper_virt2; >> extern grub_xen_reg_t grub_relocator_xen_remapper_map; >> extern grub_xen_reg_t grub_relocator_xen_mfn_list; >> +extern struct { >> + grub_xen_reg_t start; >> + grub_xen_reg_t size; >> +} grub_relocator_xen_paging_areas[XEN_MAX_MAPPINGS]; > > Should not you add GRUB_PACKED here? Could you define type > earlier and use it here? Why packed? Aah, I think I should align the variable in the assembly source instead. Regarding type: sure I could. Juergen