From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH] libxc/arm: align to page size the base address of the device tree Date: Wed, 20 Nov 2013 16:16:51 +0000 Message-ID: <528CE073.2040808@linaro.org> References: <1384887144-7229-1-git-send-email-julien.grall@linaro.org> <1384940907.28441.11.camel@kazak.uk.xensource.com> <528CB359.4060200@linaro.org> <1384953217.6071.25.camel@kazak.uk.xensource.com> <528CB686.3000907@linaro.org> <1384954226.6071.27.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VjASV-00062q-Tl for xen-devel@lists.xenproject.org; Wed, 20 Nov 2013 16:16:56 +0000 Received: by mail-wg0-f46.google.com with SMTP id x12so9186367wgg.1 for ; Wed, 20 Nov 2013 08:16:54 -0800 (PST) In-Reply-To: <1384954226.6071.27.camel@kazak.uk.xensource.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: Ian Campbell Cc: xen-devel@lists.xenproject.org, tim@xen.org, ian.jackson@eu.citrix.com, stefano.stabellini@citrix.com, patches@linaro.org List-Id: xen-devel@lists.xenproject.org On 11/20/2013 01:30 PM, Ian Campbell wrote: > On Wed, 2013-11-20 at 13:17 +0000, Julien Grall wrote: >> >> On 11/20/2013 01:13 PM, Ian Campbell wrote: >>> On Wed, 2013-11-20 at 13:04 +0000, Julien Grall wrote: >>>> >>>> On 11/20/2013 09:48 AM, Ian Campbell wrote: >>>>> On Tue, 2013-11-19 at 18:52 +0000, Julien Grall wrote: >>>>>> xc_dom_alloc_segment requires start address to be page align. >>>>> >>>>> I wonder why I didn't see this? It seems very unlikely that my dtb would >>>>> be exactly page sized, yet it works... Ah, I have >128M of RAM in my >>>>> guest so the base would be 128M. Well spotted. >>>>> >>>>> I think it would be slightly preferable to round dtbsize up to a page. >>>>> e.g. the following. What do you think? >>>>> >>>> >>>> Sounds good to me. I wrote my patch in the other way because I though >>>> RAM size can be non-page aligned. >>> >>> That would be concern except the sizes are defined immediately above as >>> shifts based on page numbers. >>> >>>> >>>>> 8>------------------ >>>>> >>>>> From 079a2815d86567de1cbb541541d08a0b9ac3d18d Mon Sep 17 00:00:00 2001 >>>>> From: Ian Campbell >>>>> Date: Wed, 20 Nov 2013 09:45:32 +0000 >>>>> Subject: [PATCH] libxl: arm: ensure DTB is page aligned >>>>> >>>>> xc_dom_alloc_segment requires this. Since rambase and ramend are both page >>>>> aligned, rounding up the DTB is sufficient. >>>>> >>>>> Reported-by: Julien Grall >>>>> Signed-off-by: Ian Campbell >>>>> --- >>>>> 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 ffe575b..a40e04d 100644 >>>>> --- a/tools/libxc/xc_dom_arm.c >>>>> +++ b/tools/libxc/xc_dom_arm.c >>>>> @@ -282,7 +282,7 @@ int arch_setup_meminit(struct xc_dom_image *dom) >>>>> { >>>>> const uint64_t rambase = dom->rambase_pfn << XC_PAGE_SHIFT; >>>>> const uint64_t ramend = rambase + ( dom->total_pages << XC_PAGE_SHIFT ); >>>>> - const uint64_t dtbsize = ( dom->devicetree_size + 3 ) & ~0x3; >>>>> + const uint64_t dtbsize = ROUNDUP(dom->devicetree_size, XC_PAGE_SHIFT); >>>> >>>> XC_PAGE_SHIFT only contains the shift. You should use XC_PAGE_SIZE. >>> >>> That's not how ROUNDUP is defined in libxc: >>> #define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1)) >>> >>> so it expects _w (width?) to be the shift. >> >> Oh right, I though it was defined as in xen. In any case, I think you >> should use XC_DOM_PAGE_SHIFT(dom). > > Those macros are almost unused in libxc. I suspect they are leftovers > from ia64 support, that being the only platform which we've supported > which had the possibility of non-4k pages. Sorry, I have only checked xc_dom_core.c For your patch: Acked-by: Julien Grall -- Julien Grall