* [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank
@ 2015-09-17 17:36 Julien Grall
2015-09-17 17:42 ` Julien Grall
0 siblings, 1 reply; 7+ messages in thread
From: Julien Grall @ 2015-09-17 17:36 UTC (permalink / raw)
To: xen-devel
Cc: Julien Grall, Wei Liu, Ian Jackson, stefano.stabellini,
ian.campbell
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".
---
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
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,
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank
2015-09-17 17:36 [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank Julien Grall
@ 2015-09-17 17:42 ` Julien Grall
2015-09-18 8:52 ` Ian Campbell
0 siblings, 1 reply; 7+ messages in thread
From: Julien Grall @ 2015-09-17 17:42 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Wei.Liu2, stefano.stabellini, ian.campbell
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 <julien.grall@citrix.com>
> ---
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank
2015-09-17 17:42 ` Julien Grall
@ 2015-09-18 8:52 ` Ian Campbell
2015-09-18 10:30 ` Wei Liu
0 siblings, 1 reply; 7+ messages in thread
From: Ian Campbell @ 2015-09-18 8:52 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Wei.Liu2, stefano.stabellini, Ian Jackson
On Thu, 2015-09-17 at 18:42 +0100, Julien Grall wrote:
> 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 <julien.grall@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
This is a no brainer for 4.6 (and further) IMHO and with Wei not being
around I shall plan to apply later today unless there are objections.
Ian.
>
> > ---
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> >
> > 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,
> >
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank
2015-09-18 8:52 ` Ian Campbell
@ 2015-09-18 10:30 ` Wei Liu
2015-09-21 11:54 ` Ian Campbell
0 siblings, 1 reply; 7+ messages in thread
From: Wei Liu @ 2015-09-18 10:30 UTC (permalink / raw)
To: Ian Campbell
Cc: Julien Grall, xen-devel, Ian Jackson, stefano.stabellini,
Wei.Liu2
On Fri, Sep 18, 2015 at 09:52:44AM +0100, Ian Campbell wrote:
> On Thu, 2015-09-17 at 18:42 +0100, Julien Grall wrote:
> > 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 <julien.grall@citrix.com>
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> This is a no brainer for 4.6 (and further) IMHO and with Wei not being
> around I shall plan to apply later today unless there are objections.
>
No objection from me of course.
Wei.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank
2015-09-18 10:30 ` Wei Liu
@ 2015-09-21 11:54 ` Ian Campbell
2015-09-25 12:26 ` Ian Campbell
0 siblings, 1 reply; 7+ messages in thread
From: Ian Campbell @ 2015-09-21 11:54 UTC (permalink / raw)
To: Wei Liu; +Cc: Julien Grall, xen-devel, stefano.stabellini, Ian Jackson
On Fri, 2015-09-18 at 11:30 +0100, Wei Liu wrote:
> > > Hmmmm I forgot my Signed-off-by :(.
> > >
> > > Signed-off-by: Julien Grall <julien.grall@citrix.com>
> >
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >
> > This is a no brainer for 4.6 (and further) IMHO and with Wei not being
> > around I shall plan to apply later today unless there are objections.
> >
>
> No objection from me of course.
Applied to staging and staging-4.6.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank
2015-09-21 11:54 ` Ian Campbell
@ 2015-09-25 12:26 ` Ian Campbell
2015-09-28 15:41 ` Ian Campbell
0 siblings, 1 reply; 7+ messages in thread
From: Ian Campbell @ 2015-09-25 12:26 UTC (permalink / raw)
To: Wei Liu; +Cc: Julien Grall, xen-devel, stefano.stabellini, Ian Jackson
On Mon, 2015-09-21 at 12:54 +0100, Ian Campbell wrote:
> On Fri, 2015-09-18 at 11:30 +0100, Wei Liu wrote:
>
> > > > Hmmmm I forgot my Signed-off-by :(.
> > > >
> > > > Signed-off-by: Julien Grall <julien.grall@citrix.com>
> > >
> > > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > >
> > > This is a no brainer for 4.6 (and further) IMHO and with Wei not
> > > being
> > > around I shall plan to apply later today unless there are objections.
> > >
> >
> > No objection from me of course.
>
> Applied to staging and staging-4.6.
This turned out to be a fib, I'd only applied it to staging.
I've made a note to apply to 4.6 once the current moratorium is over.
Ian, this should go into 4.5 too.
Ian.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank
2015-09-25 12:26 ` Ian Campbell
@ 2015-09-28 15:41 ` Ian Campbell
0 siblings, 0 replies; 7+ messages in thread
From: Ian Campbell @ 2015-09-28 15:41 UTC (permalink / raw)
To: Wei Liu; +Cc: Julien Grall, xen-devel, stefano.stabellini, Ian Jackson
On Fri, 2015-09-25 at 13:26 +0100, Ian Campbell wrote:
> On Mon, 2015-09-21 at 12:54 +0100, Ian Campbell wrote:
> > On Fri, 2015-09-18 at 11:30 +0100, Wei Liu wrote:
> >
> > > > > Hmmmm I forgot my Signed-off-by :(.
> > > > >
> > > > > Signed-off-by: Julien Grall <julien.grall@citrix.com>
> > > >
> > > > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > > >
> > > > This is a no brainer for 4.6 (and further) IMHO and with Wei not
> > > > being
> > > > around I shall plan to apply later today unless there are
> > > > objections.
> > > >
> > >
> > > No objection from me of course.
> >
> > Applied to staging and staging-4.6.
>
> This turned out to be a fib, I'd only applied it to staging.
>
> I've made a note to apply to 4.6 once the current moratorium is over.
Done.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-09-28 15:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-17 17:36 [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank Julien Grall
2015-09-17 17:42 ` Julien Grall
2015-09-18 8:52 ` Ian Campbell
2015-09-18 10:30 ` Wei Liu
2015-09-21 11:54 ` Ian Campbell
2015-09-25 12:26 ` Ian Campbell
2015-09-28 15:41 ` Ian Campbell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).