From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2] xen/arm: Allow balooning working with 1:1 memory mapping Date: Mon, 16 Dec 2013 14:52:49 +0000 Message-ID: <52AF13C1.9020506@linaro.org> References: <1386965881-12151-1-git-send-email-julien.grall@linaro.org> <52AED12C020000780010D832@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VsZXR-0005Ze-E0 for xen-devel@lists.xenproject.org; Mon, 16 Dec 2013 14:52:53 +0000 Received: by mail-we0-f181.google.com with SMTP id x55so4782495wes.12 for ; Mon, 16 Dec 2013 06:52:51 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini , Jan Beulich Cc: Keir Fraser , ian.campbell@citrix.com, patches@linaro.org, tim@xen.org, stefano.stabellini@citrix.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 12/16/2013 12:11 PM, Stefano Stabellini wrote: > On Mon, 16 Dec 2013, Jan Beulich wrote: >>>>> On 13.12.13 at 21:18, Julien Grall wrote: >>> --- a/xen/common/memory.c >>> +++ b/xen/common/memory.c >>> @@ -90,7 +90,7 @@ static void increase_reservation(struct memop_args *a) >>> >>> static void populate_physmap(struct memop_args *a) >>> { >>> - struct page_info *page; >>> + struct page_info *page = NULL; >> >> Why? >> >>> @@ -122,7 +122,29 @@ static void populate_physmap(struct memop_args *a) >>> } >>> else >>> { >>> - page = alloc_domheap_pages(d, a->extent_order, a->memflags); >>> + if ( d == dom0 && is_dom0_mapped_11() ) >>> + { >>> + mfn = gpfn; >>> + if (!mfn_valid(mfn)) >> >> Coding style. >> >>> + { >>> + gdprintk(XENLOG_INFO, "Invalid mfn 0x%"PRI_xen_pfn"\n", >>> + mfn); >>> + goto out; >>> + } >>> + >>> + page = mfn_to_page(mfn); >>> + if ( !get_page(page, d) ) >>> + { >>> + gdprintk(XENLOG_INFO, >>> + "mfn 0x%"PRI_xen_pfn" doesn't belong to dom0\n", >>> + mfn); >>> + goto out; >>> + } >>> + put_page(page); >>> + } >> >> I think this hack doesn't belong into a common file. Rather than >> having the (anyway oddly named) is_dom0_mapped_11(), it >> would seem more clean to have this implemented by an inline >> function for ARM, returning the struct page_info * (and getting >> #define-d to NULL for all other cases, perhaps not even in an >> x86 header, but right in the source file). > > Considering that having a 1:1 mapping for dom0 is conceivable even on > x86 (if you want PVH dom0 but your platform doesn't come with an IOMMU), > I would prefer the approach taken here. However I find your alternative > suggestion acceptable too. I will stay on your solution, with is_dom0_mapped_11 modify to is_domain_direct_mapped(). -- Julien Grall