From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 6/8] tools: arm: refactor code to setup guest p2m and fill it with RAM Date: Fri, 25 Apr 2014 13:51:45 +0100 Message-ID: <535A5A61.1070706@linaro.org> References: <1398424945.18537.424.camel@kazak.uk.xensource.com> <1398424967-9306-6-git-send-email-ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1398424967-9306-6-git-send-email-ian.campbell@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: Ian Campbell , xen-devel@lists.xen.org Cc: ian.jackson@eu.citrix.com, tim@xen.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org Hi Ian, On 25/04/14 12:22, Ian Campbell wrote: > +static int populate_guest_memory(struct xc_dom_image *dom, > + xen_pfn_t base_pfn, xen_pfn_t nr_pfns) > +{ > + int rc; > + xen_pfn_t allocsz, pfn; > + > + if (!nr_pfns) > + return 0; > + > + DOMPRINTF("%s: populating RAM @ %016"PRIx64"-%016"PRIx64" (%"PRId64"MB)", > + __FUNCTION__, > + (uint64_t)base_pfn << XC_PAGE_SHIFT, > + (uint64_t)(base_pfn + nr_pfns) << XC_PAGE_SHIFT, > + (uint64_t)nr_pfns >> (20-XC_PAGE_SHIFT)); > + > + for ( pfn = 0; pfn < nr_pfns; pfn++ ) > + dom->p2m_host[pfn] = base_pfn + pfn; > + > + for ( pfn = rc = allocsz = 0; (pfn < nr_pfns) && !rc; pfn += allocsz ) May I ask for a bit of clean up here? - allocsz doesn't need to be initialized. It will be unconditionally set at the beginning of the loop - rc can be set 0 at the beginning of the function. > + { > + allocsz = nr_pfns - pfn; > + if ( allocsz > 1024*1024 ) > + allocsz = 1024*1024; > + > + rc = xc_domain_populate_physmap_exact( > + dom->xch, dom->guest_domid, allocsz, > + 0, 0, &dom->p2m_host[pfn]); > + } > + > + return rc; > +} > + > int arch_setup_meminit(struct xc_dom_image *dom) > { > int rc; > - xen_pfn_t pfn, allocsz, i; > + xen_pfn_t pfn; > uint64_t modbase; > > /* Convenient */ > @@ -259,6 +291,8 @@ int arch_setup_meminit(struct xc_dom_image *dom) > const uint64_t ram0size = ramsize; > const uint64_t ram0end = GUEST_RAM0_BASE + ram0size; > > + const xen_pfn_t p2m_size = (ram0end - GUEST_RAM0_BASE) >> XC_PAGE_SHIFT; > + > const uint64_t kernbase = dom->kernel_seg.vstart; > const uint64_t kernend = ROUNDUP(dom->kernel_seg.vend, 21/*2MB*/); > const uint64_t kernsize = kernend - kernbase; > @@ -292,27 +326,17 @@ int arch_setup_meminit(struct xc_dom_image *dom) > > dom->shadow_enabled = 1; > > - dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages); > + dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * p2m_size); > if ( dom->p2m_host == NULL ) > return -EINVAL; > + for ( pfn = 0; pfn < p2m_size; pfn++ ) > + dom->p2m_host[pfn] = INVALID_MFN; With this solution, you will loop 262244 times for nothing (the hole between the 2 banks). Also when the guess will have lots of RAM, it will be slow because we loop nearly twice the array (one here, the other in populate_guest_memory). It think we can avoid looping twice by making the two banks contiguous in the memory (i.e starting the second bank at 4GB instead of 8GB). Regards, -- Julien Grall