From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH] libxc: fix claim mode when creating HVM guest Date: Thu, 30 Jan 2014 14:38:26 +0000 Message-ID: <52EA63E2.9020603@eu.citrix.com> References: <1390845218-823-1-git-send-email-wei.liu2@citrix.com> <20140129183349.GA14312@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140129183349.GA14312@phenom.dumpdata.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: Konrad Rzeszutek Wilk , Wei Liu Cc: Ian Jackson , Ian Campbell , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 01/29/2014 06:33 PM, Konrad Rzeszutek Wilk wrote: > On Mon, Jan 27, 2014 at 05:53:38PM +0000, Wei Liu wrote: >> The original code is wrong because: >> * claim mode wants to know the total number of pages needed while >> original code provides the additional number of pages needed. >> * if pod is enabled memory will already be allocated by the time we try >> to claim memory. >> >> So the fix would be: >> * move claim mode before actual memory allocation. >> * pass the right number of pages to hypervisor. >> >> The "right number of pages" should be number of pages of target memory >> minus VGA_HOLE_SIZE, regardless of whether PoD is enabled. >> >> This fixes bug #32. >> >> Signed-off-by: Wei Liu >> Cc: Konrad Wilk > > And also 'Tested-by: Konrad Rzeszutek Wilk ' Right, well nobody has shouted, so: Release-acked-by: George Dunlap > > Thank you! >> Cc: George Dunlap >> Cc: Ian Campbell >> Cc: Ian Jackson >> --- >> WRT 4.4 release: this patch should be accpeted, otherwise PoD + claim >> mode is complete broken. If this patch is deemed too complicated, we >> should flip the switch to disable claim mode by default for 4.4. >> --- >> tools/libxc/xc_hvm_build_x86.c | 36 +++++++++++++++++++++++------------- >> 1 file changed, 23 insertions(+), 13 deletions(-) >> >> diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c >> index 77bd365..dd3b522 100644 >> --- a/tools/libxc/xc_hvm_build_x86.c >> +++ b/tools/libxc/xc_hvm_build_x86.c >> @@ -49,6 +49,8 @@ >> #define NR_SPECIAL_PAGES 8 >> #define special_pfn(x) (0xff000u - NR_SPECIAL_PAGES + (x)) >> >> +#define VGA_HOLE_SIZE (0x20) >> + >> static int modules_init(struct xc_hvm_build_args *args, >> uint64_t vend, struct elf_binary *elf, >> uint64_t *mstart_out, uint64_t *mend_out) >> @@ -302,14 +304,31 @@ static int setup_guest(xc_interface *xch, >> for ( i = mmio_start >> PAGE_SHIFT; i < nr_pages; i++ ) >> page_array[i] += mmio_size >> PAGE_SHIFT; >> >> + /* >> + * Try to claim pages for early warning of insufficient memory available. >> + * This should go before xc_domain_set_pod_target, becuase that function >> + * actually allocates memory for the guest. Claiming after memory has been >> + * allocated is pointless. >> + */ >> + if ( claim_enabled ) { >> + rc = xc_domain_claim_pages(xch, dom, target_pages - VGA_HOLE_SIZE); >> + if ( rc != 0 ) >> + { >> + PERROR("Could not allocate memory for HVM guest as we cannot claim memory!"); >> + goto error_out; >> + } >> + } >> + >> if ( pod_mode ) >> { >> /* >> - * Subtract 0x20 from target_pages for the VGA "hole". Xen will >> - * adjust the PoD cache size so that domain tot_pages will be >> - * target_pages - 0x20 after this call. >> + * Subtract VGA_HOLE_SIZE from target_pages for the VGA >> + * "hole". Xen will adjust the PoD cache size so that domain >> + * tot_pages will be target_pages - VGA_HOLE_SIZE after >> + * this call. >> */ >> - rc = xc_domain_set_pod_target(xch, dom, target_pages - 0x20, >> + rc = xc_domain_set_pod_target(xch, dom, >> + target_pages - VGA_HOLE_SIZE, >> NULL, NULL, NULL); >> if ( rc != 0 ) >> { >> @@ -333,15 +352,6 @@ static int setup_guest(xc_interface *xch, >> cur_pages = 0xc0; >> stat_normal_pages = 0xc0; >> >> - /* try to claim pages for early warning of insufficient memory available */ >> - if ( claim_enabled ) { >> - rc = xc_domain_claim_pages(xch, dom, nr_pages - cur_pages); >> - if ( rc != 0 ) >> - { >> - PERROR("Could not allocate memory for HVM guest as we cannot claim memory!"); >> - goto error_out; >> - } >> - } >> while ( (rc == 0) && (nr_pages > cur_pages) ) >> { >> /* Clip count to maximum 1GB extent. */ >> -- >> 1.7.10.4 >>