From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank Date: Thu, 17 Sep 2015 18:42:53 +0100 Message-ID: <55FAFB9D.7090107@citrix.com> References: <1442511396-21344-1-git-send-email-julien.grall@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZcdEB-0003Nh-Bv for xen-devel@lists.xenproject.org; Thu, 17 Sep 2015 17:44:11 +0000 In-Reply-To: <1442511396-21344-1-git-send-email-julien.grall@citrix.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: xen-devel@lists.xenproject.org Cc: Ian Jackson , Wei.Liu2@citrix.com, stefano.stabellini@citrix.com, ian.campbell@citrix.com List-Id: xen-devel@lists.xenproject.org On 17/09/15 18:36, Julien Grall wrote: > When creating a guest with more than 3GB of memory, the 2 banks will be > used and the loop with overrunning. The code will fail later on because > Xen will deny to populate the region: > > domainbuilder: detail: xc_dom_devicetree_mem: called > domainbuilder: detail: xc_dom_mem_init: mem 3096 MB, pages 0xc1800 pages, 4k each > domainbuilder: detail: xc_dom_mem_init: 0xc1800 pages > domainbuilder: detail: xc_dom_boot_mem_init: called > domainbuilder: detail: set_mode: guest xen-3.0-aarch64, address size 64 > domainbuilder: detail: xc_dom_malloc : 14384 kB > domainbuilder: detail: populate_guest_memory: populating RAM @0000000040000000-0000000100000000 (3072MB) > domainbuilder: detail: populate_one_size: populated 0x3/0x3 entries with shift 18 > domainbuilder: detail: populate_guest_memory: populating RAM @0000000200000000-0000000201800000 (24MB) > domainbuilder: detail: populate_one_size: populated 0xc/0xc entries with shift 9 > domainbuilder: detail: populate_guest_memory: populating RAM @0000007fad41c000-0007fb39dd42c000 (2141954816MB) > domainbuilder: detail: populate_one_size: populated 0x100/0x1e4 entries with shift 0 > domainbuilder: detail: populate_guest_memory: Not enough RAM > > This is because we are currently accessing the bank before checking the > validity of the index. AFAICT, on Debian Jessie, the compiler (gcc 4.9.2) is > assuming that it's not necessary to verify the index because it's used > before. This is a valid assumption because the operand of && are > execute from from left to right. > > Re-order the checks to verify the validity of the index before accessing > the bank. > > The problem has been present since the introduction of the multi-bank > feature in commit 45d9867837f099e9eed4189dac5ed39d1fe2ed49 " tools: arm: > prepare domain builder for multiple banks of guest RAM". Hmmmm I forgot my Signed-off-by :(. Signed-off-by: Julien Grall > --- > Cc: Wei Liu > Cc: Ian Jackson > > This patch is a candidate for Xen 4.6 and backport to Xen 4.5. Without > it, it's impossible to boot guest using more than 3GB (limit after which > the memory bank is used). > --- > tools/libxc/xc_dom_arm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c > index b00d667..aeaba54 100644 > --- a/tools/libxc/xc_dom_arm.c > +++ b/tools/libxc/xc_dom_arm.c > @@ -460,7 +460,7 @@ int arch_setup_meminit(struct xc_dom_image *dom) > dom->p2m_host[pfn] = INVALID_P2M_ENTRY; > > /* setup initial p2m and allocate guest memory */ > - for ( i = 0; dom->rambank_size[i] && i < GUEST_RAM_BANKS; i++ ) > + for ( i = 0; i < GUEST_RAM_BANKS && dom->rambank_size[i]; i++ ) > { > if ((rc = populate_guest_memory(dom, > bankbase[i] >> XC_PAGE_SHIFT, > -- Julien Grall