From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Roger_Pau_Monn=E9?= Subject: Re: [V10 PATCH 0/4] pvh dom0 patches... Date: Thu, 8 May 2014 12:27:48 +0200 Message-ID: <536B5C24.6060102@citrix.com> References: <1398820008-9005-1-git-send-email-mukesh.rathor@oracle.com> <5361049B.7040409@citrix.com> <20140430111216.2bef8e60@mantra.us.oracle.com> <20140430181923.68d75467@mantra.us.oracle.com> <53637BF3.2000502@citrix.com> <20140502170114.7ec2a9e6@mantra.us.oracle.com> <53675166.30400@citrix.com> <20140505172858.4c6c3f0a@mantra.us.oracle.com> <53688B84.8090607@citrix.com> <20140506180053.7c6da000@mantra.us.oracle.com> <536A01D8020000780000FB15@mail.emea.novell.com> <536A0188.9090109@citrix.com> <536A365D020000780000FE17@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WiLYQ-0007Ry-QE for xen-devel@lists.xenproject.org; Thu, 08 May 2014 10:27:55 +0000 In-Reply-To: <536A365D020000780000FE17@mail.emea.novell.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: Jan Beulich , Mukesh Rathor Cc: George.Dunlap@eu.citrix.com, xen-devel@lists.xenproject.org, keir.xen@gmail.com, tim@xen.org List-Id: xen-devel@lists.xenproject.org On 07/05/14 13:34, Jan Beulich wrote: >>>> On 07.05.14 at 11:48, wrote: >> I've expanded my patch that added the memory removed form the holes to = >> the end of the memory map to also create an e820 map for Dom0 that = >> reflects the reality of the underlying p2m. This way PVH guests (either = >> DomU or Dom0) should only use XENMEM_memory_map to get the correct e820 = >> map (and no need for clamping in the Dom0 case). > = > Looks generally good (except that it doesn't seem to deal with the > DomU case yet), but there are a couple of possible issues: > = >> --- a/xen/arch/x86/domain_build.c >> +++ b/xen/arch/x86/domain_build.c >> @@ -327,11 +327,13 @@ static __init void pvh_add_mem_mapping(struct doma= in = >> *d, unsigned long gfn, >> * pvh fixme: The following doesn't map MMIO ranges when they sit above= the >> * highest E820 covered address. >> */ >> -static __init void pvh_map_all_iomem(struct domain *d) >> +static __init void pvh_map_all_iomem(struct domain *d, unsigned long nr= _pages) >> { >> unsigned long start_pfn, end_pfn, end =3D 0, start =3D 0; >> const struct e820entry *entry; >> - unsigned int i, nump; >> + unsigned int i, j, nump, navail, nmap, nr_holes =3D 0; > = > With nr_pages above being unsigned long, I think some of these > variables should be too. > = >> @@ -369,6 +374,89 @@ static __init void pvh_map_all_iomem(struct domain = *d) >> nump =3D end_pfn - start_pfn; >> pvh_add_mem_mapping(d, start_pfn, start_pfn, nump); >> } >> + >> + /* >> + * Add the memory removed by the holes at the end of the >> + * memory map. >> + */ >> + for ( i =3D 0, entry =3D e820.map; i < e820.nr_map; i++, entry++ ) >> + { >> + if ( entry->type !=3D E820_RAM ) >> + continue; >> + >> + end_pfn =3D PFN_UP(entry->addr + entry->size); >> + if ( end_pfn <=3D nr_pages ) >> + continue; >> + >> + navail =3D end_pfn - nr_pages; >> + nmap =3D navail > nr_holes ? nr_holes : navail; >> + start_pfn =3D PFN_DOWN(entry->addr) < nr_pages ? >> + nr_pages : PFN_DOWN(entry->addr); >> + page =3D alloc_domheap_pages(d, get_order_from_pages(nmap), 0); >> + if ( !page ) >> + panic("Not enough RAM for domain 0"); >> + for ( j =3D 0; j < nmap; j++ ) >> + { >> + rc =3D guest_physmap_add_page(d, start_pfn + j, page_to_mfn= (page), 0); > = > I realize that this interface isn't the most flexible one in terms of what > page orders it allows to be passed in, but could you at least use order > 9 here when the allocation order above is 9 or higher? I've changed it to be: guest_physmap_add_page(d, start_pfn, page_to_mfn(page), order); and removed the loop. > = >> +static __init void pvh_setup_e820(struct domain *d, unsigned long nr_pa= ges) >> +{ >> + struct e820entry *entry; >> + unsigned int i; >> + unsigned long pages, cur_pages =3D 0; >> + >> + /* >> + * Craft the e820 memory map for Dom0 based on the hardware e820 ma= p. >> + */ >> + d->arch.e820 =3D xzalloc_array(struct e820entry, e820.nr_map); >> + if ( !d->arch.e820 ) >> + panic("Unable to allocate memory for Dom0 e820 map"); >> + >> + memcpy(d->arch.e820, e820.map, sizeof(struct e820entry) * e820.nr_m= ap); >> + d->arch.nr_e820 =3D e820.nr_map; >> + >> + /* Clamp e820 memory map to match the memory assigned to Dom0 */ >> + for ( i =3D 0, entry =3D d->arch.e820; i < d->arch.nr_e820; i++, en= try++ ) >> + { >> + if ( entry->type !=3D E820_RAM ) >> + continue; >> + >> + if ( nr_pages =3D=3D cur_pages ) >> + { >> + /* >> + * We already have all the assigned memory, >> + * mark this region as reserved. >> + */ >> + entry->type =3D E820_RESERVED; > = > That seems undesirable, as it will prevent Linux from using that space > for MMIO purposes. Plus keeping the region here is consistent with > simply shrinking the range below (yielding the remainder uncovered > by any E820 entry). I think you ought to zap the entry here. I don't think this regions can be used for MMIO, the gpfns in the guest p2m are not set as p2m_mmio_direct. Here is the updated patch: --- commit b84f2ae87de562a3df69599c4b3734262e106445 Author: Roger Pau Monne Date: Thu May 8 09:21:49 2014 +0200 xen: fix setup of PVH Dom0 memory map = This patch adds the holes removed by MMIO regions to the end of the memory map for PVH Dom0, so the guest OS doesn't have to manually populate this memory. = Also, provide a suitable e820 memory map for PVH Dom0, that matches the underlying p2m map. This means that PVH guests should always use XENMEM_memory_map in order to obtain the e820, even when running as Dom0. = Signed-off-by: Roger Pau Monn=E9 diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c index 38ed9f6..3cbd43c 100644 --- a/xen/arch/x86/domain_build.c +++ b/xen/arch/x86/domain_build.c @@ -327,11 +327,14 @@ static __init void pvh_add_mem_mapping(struct domain = *d, unsigned long gfn, * pvh fixme: The following doesn't map MMIO ranges when they sit above the * highest E820 covered address. */ -static __init void pvh_map_all_iomem(struct domain *d) +static __init void pvh_map_all_iomem(struct domain *d, unsigned long nr_pa= ges) { unsigned long start_pfn, end_pfn, end =3D 0, start =3D 0; const struct e820entry *entry; - unsigned int i, nump; + unsigned long nump, nmap, navail, nr_holes =3D 0; + unsigned int i, order; + struct page_info *page; + int rc; = for ( i =3D 0, entry =3D e820.map; i < e820.nr_map; i++, entry++ ) { @@ -353,6 +356,9 @@ static __init void pvh_map_all_iomem(struct domain *d) nump =3D end_pfn - start_pfn; /* Add pages to the mapping */ pvh_add_mem_mapping(d, start_pfn, start_pfn, nump); + if ( start_pfn <=3D nr_pages ) + nr_holes +=3D (end_pfn < nr_pages) ? + nump : (nr_pages - start_pfn); } start =3D end; } @@ -369,6 +375,84 @@ static __init void pvh_map_all_iomem(struct domain *d) nump =3D end_pfn - start_pfn; pvh_add_mem_mapping(d, start_pfn, start_pfn, nump); } + + /* + * Add the memory removed by the holes at the end of the + * memory map. + */ + for ( i =3D 0, entry =3D e820.map; i < e820.nr_map && nr_holes > 0; + i++, entry++ ) + { + if ( entry->type !=3D E820_RAM ) + continue; + + end_pfn =3D PFN_UP(entry->addr + entry->size); + if ( end_pfn <=3D nr_pages ) + continue; + + navail =3D end_pfn - nr_pages; + nmap =3D navail > nr_holes ? nr_holes : navail; + start_pfn =3D PFN_DOWN(entry->addr) < nr_pages ? + nr_pages : PFN_DOWN(entry->addr); + order =3D get_order_from_pages(nmap); + page =3D alloc_domheap_pages(d, order, 0); + if ( !page ) + panic("Not enough RAM for domain 0"); + rc =3D guest_physmap_add_page(d, start_pfn, page_to_mfn(page), ord= er); + if ( rc !=3D 0 ) + panic("Unable to add gpfn %#lx mfn %#lx order: %u to Dom0 phys= map", + start_pfn, page_to_mfn(page), order); + nr_holes -=3D nmap; + } + + ASSERT(nr_holes =3D=3D 0); +} + +static __init void pvh_setup_e820(struct domain *d, unsigned long nr_pages) +{ + struct e820entry *entry; + unsigned int i; + unsigned long pages, cur_pages =3D 0; + + /* + * Craft the e820 memory map for Dom0 based on the hardware e820 map. + */ + d->arch.e820 =3D xzalloc_array(struct e820entry, e820.nr_map); + if ( !d->arch.e820 ) + panic("Unable to allocate memory for Dom0 e820 map"); + + memcpy(d->arch.e820, e820.map, sizeof(struct e820entry) * e820.nr_map); + d->arch.nr_e820 =3D e820.nr_map; + + /* Clamp e820 memory map to match the memory assigned to Dom0 */ + for ( i =3D 0, entry =3D d->arch.e820; i < d->arch.nr_e820; i++, entry= ++ ) + { + if ( entry->type !=3D E820_RAM ) + continue; + + if ( nr_pages =3D=3D cur_pages ) + { + /* + * We already have all the assigned memory, + * set this region length to 0. + */ + entry->size =3D 0; + continue; + } + + pages =3D entry->size >> PAGE_SHIFT; + if ( (cur_pages + pages) > nr_pages ) + { + /* Truncate region */ + entry->size =3D (nr_pages - cur_pages) << PAGE_SHIFT; + cur_pages =3D nr_pages; + } + else + { + cur_pages +=3D pages; + } + } + ASSERT(cur_pages =3D=3D nr_pages); } = static __init void dom0_update_physmap(struct domain *d, unsigned long pfn, @@ -1391,7 +1475,8 @@ int __init construct_dom0( pfn =3D shared_info_paddr >> PAGE_SHIFT; dom0_update_physmap(d, pfn, mfn, 0); = - pvh_map_all_iomem(d); + pvh_map_all_iomem(d, nr_pages); + pvh_setup_e820(d, nr_pages); } = if ( d->domain_id =3D=3D hardware_domid )