All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ian Campbell <Ian.Campbell@citrix.com>
To: Frediano Ziglio <freddy77@gmail.com>
Cc: Tim Deegan <tim@xen.org>, Julien Grall <julien.grall@linaro.org>,
	Stefano Stabellini <stefano.stabellini@citrix.com>,
	xen-devel@lists.xen.org
Subject: Re: [PATCH v3] xen/arm: Fix crash if last memory section is bigger than 1gb
Date: Fri, 3 Oct 2014 10:09:56 +0100	[thread overview]
Message-ID: <1412327396.423.17.camel@citrix.com> (raw)
In-Reply-To: <CAHt6W4dtWyPf-OspLsyPs6ZZWvj6rfor0ePwcE7iWy9KNzHLZQ@mail.gmail.com>

On Thu, 2014-10-02 at 16:16 +0100, Frediano Ziglio wrote:
> setup_xenheap_mappings setup head memory on Arm 32 has a limit of 1gb.
> On system with large memory is possible that there are no blocks of memory
> smaller than 1gb leading xenheap_pages to be more than 1gb.
> This cause memory errors trying to access heap after the 1gb limit.

As I mentioned in a previous round of review this explanation doesn't
really fit.

I think better would be:

        On arm32 the xenheap has a maximum size of 1GB. On systems with
        more than 8GB (so 1/8 total RAM is greater than 1GB) there is no
        point in searching for a region with 1/8 of the total RAM when
        only 1GB will be used. Therefore limit the maximum size to 1GB
        before searching.

If you agree I will make this change upon commit.

> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> Reviewed-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/setup.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 446de8a..c43c776 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -500,7 +500,7 @@ static void __init setup_mm(unsigned long
> dtb_paddr, size_t dtb_size)
>       *
>       *  - must be 32 MiB aligned
>       *  - must not include Xen itself or the boot modules
> -     *  - must be at most 1/8 the total RAM in the system
> +     *  - must be at most 1GB or 1/8 the total RAM in the system if less
>       *  - must be at least 128M
>       *
>       * We try to allocate the largest xenheap possible within these
> @@ -509,6 +509,7 @@ static void __init setup_mm(unsigned long
> dtb_paddr, size_t dtb_size)
>      heap_pages = ram_pages;
>      xenheap_pages = (heap_pages/8 + 0x1fffUL) & ~0x1fffUL;
>      xenheap_pages = max(xenheap_pages, 128UL<<(20-PAGE_SHIFT));
> +    xenheap_pages = min(xenheap_pages, 1UL<<(30-PAGE_SHIFT));
> 
>      do
>      {

  reply	other threads:[~2014-10-03  9:09 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-02 15:16 [PATCH v3] xen/arm: Fix crash if last memory section is bigger than 1gb Frediano Ziglio
2014-10-03  9:09 ` Ian Campbell [this message]
2014-10-03 10:15   ` Frediano Ziglio
2014-10-03 14:06     ` Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1412327396.423.17.camel@citrix.com \
    --to=ian.campbell@citrix.com \
    --cc=freddy77@gmail.com \
    --cc=julien.grall@linaro.org \
    --cc=stefano.stabellini@citrix.com \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.