RE: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt

2009-03-02 Thread Zhang, Yang
Isaku Yamahata wrote:
> You want to balloon the memory for vhpt from dom0 and don't want to
> allocate the memory by alloc_domheap_page() with explicit domain
> pointer. 
> Then, IA64_HVM_ImageHandler::getRequiredAvailableMemory() is wrong
> place to modify.
> Please see _initDomain() in XendDomainInfo.py and memory, maxmem
> and xc.domain_set_maxmem(), balloon.free().
> Only the argument to balloon.free() should be touched and
> the argument to xc.domain_se_maxmem() shouldn't be touched.

You are right. It should not be added into  the maxmem.

Best Regards
--yang
___
Xen-ia64-devel mailing list
Xen-ia64-devel@lists.xensource.com
http://lists.xensource.com/xen-ia64-devel


Re: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt

2009-03-02 Thread Isaku Yamahata
On Mon, Mar 02, 2009 at 07:21:28PM +0800, Zhang, Yang wrote:
> Isaku Yamahata wrote:
> > So you want to account vhpt memory.
> > Then, pass the domain pointer as argument to alloc_domheap_page(d,
> > ...) and leave the hunk in image.py.
> > Maybe the code path which frees domain memory needs twist.
> 
> it doesn't pass the domain pointer as argument. There are separate function 
> free_domain_vhpt() and free_domain_vtlb to frees the memory which is used as 
> vhpt.
> but i also free the saved memory which may not used by vhpt(sometimes this 
> will be true) 
> in vmx_relinquish_guest_resources().

You want to balloon the memory for vhpt from dom0 and don't want to allocate
the memory by alloc_domheap_page() with explicit domain pointer.
Then, IA64_HVM_ImageHandler::getRequiredAvailableMemory() is wrong
place to modify.
Please see _initDomain() in XendDomainInfo.py and memory, maxmem
and xc.domain_set_maxmem(), balloon.free().
Only the argument to balloon.free() should be touched and
the argument to xc.domain_se_maxmem() shouldn't be touched.
-- 
yamahata

___
Xen-ia64-devel mailing list
Xen-ia64-devel@lists.xensource.com
http://lists.xensource.com/xen-ia64-devel


RE: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt

2009-03-02 Thread Zhang, Yang
Isaku Yamahata wrote:
> So you want to account vhpt memory.
> Then, pass the domain pointer as argument to alloc_domheap_page(d,
> ...) and leave the hunk in image.py.
> Maybe the code path which frees domain memory needs twist.

it doesn't pass the domain pointer as argument. There are separate function 
free_domain_vhpt() and free_domain_vtlb to frees the memory which is used as 
vhpt.
but i also free the saved memory which may not used by vhpt(sometimes this will 
be true) 
in vmx_relinquish_guest_resources().
 
> The patch complicates the interface, thash_alloc().
> That makes it ugly and difficult to understand.
> Instead, introduce clean interface suitable for the purpose.

Ok, i will try to do it.

Best Regards
--yang
___
Xen-ia64-devel mailing list
Xen-ia64-devel@lists.xensource.com
http://lists.xensource.com/xen-ia64-devel


Re: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt

2009-03-02 Thread Isaku Yamahata
On Mon, Mar 02, 2009 at 05:38:22PM +0800, Zhang, Yang wrote:
> Isaku Yamahata wrote:
> > As you described above, you used alloc_domheap_page(NULL, ) which
> > means that pages for vhpt aren't accounted for a given domain.
> > So the hunk in tools/python/xen/xend/image.py doesn't make sense,
> > does it? Or have you found any issues without the modification to
> > image.py? 
> When using balloon, it does not count the memory for the vhpt in the previous 
> version.
> So when balloon memory from dom0, the guest always can not be launched.
> because there does not have the enough memory for vhpt. And the hunk add in 
> image.py is 
> to fix this issue. this is different from the bug guest can not get 
> continuous memory. 
> Maybe i should split this hunk as an new patch.

So you want to account vhpt memory.
Then, pass the domain pointer as argument to alloc_domheap_page(d, ...)
and leave the hunk in image.py.
Maybe the code path which frees domain memory needs twist.


> > Hmm, the patch makes the allocating/freeing vhpt interface convoluted.
> > The gut of your patch is to make the function, init_domain_vhpt(),
> > not to allocate pages by preallocation. So what should be done is
> > not only to add preallocate pages logic, but also to revise
> > the related functions.
> > Revise init_domain_vhpt() (and free_domain_vhpt()?). Maybe the
> > underlying functions, thash_alloc(), thash_free() need to be
> > refactored. 
> Sorry, i cannot understand your meaning well. what revision should be done?
> I feel that the preallocation just provides a method for init_domain_vhpt to 
> get continuous
> memory.  And we should not to revise this fuctions.

The patch complicates the interface, thash_alloc().
That makes it ugly and difficult to understand.
Instead, introduce clean interface suitable for the purpose.
-- 
yamahata

___
Xen-ia64-devel mailing list
Xen-ia64-devel@lists.xensource.com
http://lists.xensource.com/xen-ia64-devel


RE: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt

2009-03-02 Thread Zhang, Yang
Isaku Yamahata wrote:
> As you described above, you used alloc_domheap_page(NULL, ) which
> means that pages for vhpt aren't accounted for a given domain.
> So the hunk in tools/python/xen/xend/image.py doesn't make sense,
> does it? Or have you found any issues without the modification to
> image.py? 
When using balloon, it does not count the memory for the vhpt in the previous 
version.
So when balloon memory from dom0, the guest always can not be launched.
because there does not have the enough memory for vhpt. And the hunk add in 
image.py is 
to fix this issue. this is different from the bug guest can not get continuous 
memory. 
Maybe i should split this hunk as an new patch.

> Hmm, the patch makes the allocating/freeing vhpt interface convoluted.
> The gut of your patch is to make the function, init_domain_vhpt(),
> not to allocate pages by preallocation. So what should be done is
> not only to add preallocate pages logic, but also to revise
> the related functions.
> Revise init_domain_vhpt() (and free_domain_vhpt()?). Maybe the
> underlying functions, thash_alloc(), thash_free() need to be
> refactored. 
Sorry, i cannot understand your meaning well. what revision should be done?
I feel that the preallocation just provides a method for init_domain_vhpt to 
get continuous
memory.  And we should not to revise this fuctions.


Best Regards
--yang
___
Xen-ia64-devel mailing list
Xen-ia64-devel@lists.xensource.com
http://lists.xensource.com/xen-ia64-devel


Re: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt

2009-03-02 Thread Isaku Yamahata
On Sun, Mar 01, 2009 at 05:12:25PM +0800, Zhang, Yang wrote:
> >Isaku Yamahata wrote:
> >- The pages for vhpt is allocated by alloc_domheap_pages(NULL, ...),
> >  Thus those memory doesn't accounted to the domain.
> >  So adjusting domain memory size doesn't make sense. Just drop it.
> In the previous version, it allocates the pages for vhpt by 
> alloc_domheap_pages(NULL, ...) already . I do not change it. And I think that 
> the first arg with NULL represents that it doesn't belong to the domain.

As you described above, you used alloc_domheap_page(NULL, ) which means
that pages for vhpt aren't accounted for a given domain.
So the hunk in tools/python/xen/xend/image.py doesn't make sense, does it?
Or have you found any issues without the modification to image.py?


> >- Is vhpt_page[] really necessary?
> >  How about setting hcb->hash directly instead of temporary
> >  keeping it in vhpt_page[]?
> For the expansibility and the legible logic, I think it will be better to use 
> the vhpt_page[].

Hmm, the patch makes the allocating/freeing vhpt interface convoluted.
The gut of your patch is to make the function, init_domain_vhpt(),
not to allocate pages by preallocation. So what should be done is
not only to add preallocate pages logic, but also to revise
the related functions.
Revise init_domain_vhpt() (and free_domain_vhpt()?). Maybe the underlying
functions, thash_alloc(), thash_free() need to be refactored.

thanks,

> 
> Fix the issue that guest cannot be launched by using balloon driver.
> 
>  - In current version, when using balloon driver to balloon memory,it
> did not balloon the memory for vhpt.
> vhpt needs the continuous memory.But it allocates the memory for vhpt 
> after it finished allocating the memory for guest. So the vhpt usually 
> can not get the continuous memory and guest cannot be launched. Now
> i change the sequence.Before it allocates the memory for guest, i save
> the continuous memory for vhpt.when boot the vcpu, it allocate the saved
> memory for vhpt.
> 
> Signed-off-by: Yang Zhang 
> 
> diff -r b432c632ebe8 tools/python/xen/xend/image.py
> --- a/tools/python/xen/xend/image.py  Fri Feb 13 19:11:38 2009 +0900
> +++ b/tools/python/xen/xend/image.py  Sun Mar 01 03:28:28 2009 -0500
> @@ -870,6 +870,9 @@ class IA64_HVM_ImageHandler(HVMImageHand
>  extra_pages = 1024 + 5
>  mem_kb += extra_pages * page_kb
>  mem_kb += self.vramsize
> +# per-vcpu need 16M for max vhpt size
> +vhpt_kb = 16 * 1024
> +mem_kb += self.vm.getVCpuCount() * vhpt_kb
>  return mem_kb
>  
>  def getRequiredInitialReservation(self):
> diff -r b432c632ebe8 xen/arch/ia64/vmx/vmmu.c
> --- a/xen/arch/ia64/vmx/vmmu.cFri Feb 13 19:11:38 2009 +0900
> +++ b/xen/arch/ia64/vmx/vmmu.cSun Mar 01 03:28:28 2009 -0500
> @@ -62,7 +62,7 @@ static int init_domain_vhpt(struct vcpu 
>  else
>  size = canonicalize_vhpt_size(size);
>  
> -rc = thash_alloc(&(v->arch.vhpt), size, "vhpt");
> +rc = thash_alloc(v, size, "vhpt");
>  v->arch.arch_vmx.mpta = v->arch.vhpt.pta.val;
>  return rc;
>  }
> @@ -70,8 +70,10 @@ static int init_domain_vhpt(struct vcpu 
>  
>  static void free_domain_vhpt(struct vcpu *v)
>  {
> -if (v->arch.vhpt.hash)
> +if (v->arch.vhpt.hash) {
>  thash_free(&(v->arch.vhpt));
> +v->domain->arch.hvm_domain.vhpt_page[v->vcpu_id] = NULL;
> +}
>  }
>  
>  int init_domain_tlb(struct vcpu *v)
> @@ -82,7 +84,7 @@ int init_domain_tlb(struct vcpu *v)
>  if (rc)
>  return rc;
>  
> -rc = thash_alloc(&(v->arch.vtlb), default_vtlb_sz, "vtlb");
> +rc = thash_alloc(v, default_vtlb_sz, "vtlb");
>  if (rc) {
>  free_domain_vhpt(v);
>  return rc;
> diff -r b432c632ebe8 xen/arch/ia64/vmx/vmx_hypercall.c
> --- a/xen/arch/ia64/vmx/vmx_hypercall.c   Fri Feb 13 19:11:38 2009 +0900
> +++ b/xen/arch/ia64/vmx/vmx_hypercall.c   Sun Mar 01 03:28:28 2009 -0500
> @@ -147,6 +147,9 @@ do_hvm_op(unsigned long op, XEN_GUEST_HA
>  a.value = current->domain->domain_id;
>  rc = a.value ? -EINVAL : 0; /* no stub domain support */
>  break;
> +case HVM_PARAM_VHPT_SIZE:
> +rc = domain_pre_alloc_vhpt(d, a.value);
> +break;
>  default:
>  /* nothing */
>  break;
> diff -r b432c632ebe8 xen/arch/ia64/vmx/vmx_init.c
> --- a/xen/arch/ia64/vmx/vmx_init.cFri Feb 13 19:11:38 2009 +0900
> +++ b/xen/arch/ia64/vmx/vmx_init.cSun Mar 01 03:28:28 2009 -0500
> @@ -541,6 +541,7 @@ vmx_relinquish_guest_resources(struct do
>   for_each_vcpu(d, v)
>   vmx_release_assist_channel(v);
>  
> + domain_pre_free_vhpt(d);
>   vacpi_relinquish_resources(d);
>  
>   vmx_destroy_ioreq_page(d, &d->arch.vmx_platform.ioreq);
> @@ -661,3 +662,63 @@ void vmx_do_resume(struct vcpu *v)
>   }
>   }
>  }
> +
> +static inline int 

[Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt

2009-03-01 Thread Zhang, Yang
>Isaku Yamahata wrote:
>- The pages for vhpt is allocated by alloc_domheap_pages(NULL, ...),
>  Thus those memory doesn't accounted to the domain.
>  So adjusting domain memory size doesn't make sense. Just drop it.
In the previous version, it allocates the pages for vhpt by 
alloc_domheap_pages(NULL, ...) already . I do not change it. And I think that 
the first arg with NULL represents that it doesn't belong to the domain.

>- Is vhpt_page[] really necessary?
>  How about setting hcb->hash directly instead of temporary
>  keeping it in vhpt_page[]?
For the expansibility and the legible logic, I think it will be better to use 
the vhpt_page[].

Fix the issue that guest cannot be launched by using balloon driver.

 - In current version, when using balloon driver to balloon memory,it
did not balloon the memory for vhpt.
vhpt needs the continuous memory.But it allocates the memory for vhpt 
after it finished allocating the memory for guest. So the vhpt usually 
can not get the continuous memory and guest cannot be launched. Now
i change the sequence.Before it allocates the memory for guest, i save
the continuous memory for vhpt.when boot the vcpu, it allocate the saved
memory for vhpt.

Signed-off-by: Yang Zhang 

diff -r b432c632ebe8 tools/python/xen/xend/image.py
--- a/tools/python/xen/xend/image.pyFri Feb 13 19:11:38 2009 +0900
+++ b/tools/python/xen/xend/image.pySun Mar 01 03:28:28 2009 -0500
@@ -870,6 +870,9 @@ class IA64_HVM_ImageHandler(HVMImageHand
 extra_pages = 1024 + 5
 mem_kb += extra_pages * page_kb
 mem_kb += self.vramsize
+# per-vcpu need 16M for max vhpt size
+vhpt_kb = 16 * 1024
+mem_kb += self.vm.getVCpuCount() * vhpt_kb
 return mem_kb
 
 def getRequiredInitialReservation(self):
diff -r b432c632ebe8 xen/arch/ia64/vmx/vmmu.c
--- a/xen/arch/ia64/vmx/vmmu.c  Fri Feb 13 19:11:38 2009 +0900
+++ b/xen/arch/ia64/vmx/vmmu.c  Sun Mar 01 03:28:28 2009 -0500
@@ -62,7 +62,7 @@ static int init_domain_vhpt(struct vcpu 
 else
 size = canonicalize_vhpt_size(size);
 
-rc = thash_alloc(&(v->arch.vhpt), size, "vhpt");
+rc = thash_alloc(v, size, "vhpt");
 v->arch.arch_vmx.mpta = v->arch.vhpt.pta.val;
 return rc;
 }
@@ -70,8 +70,10 @@ static int init_domain_vhpt(struct vcpu 
 
 static void free_domain_vhpt(struct vcpu *v)
 {
-if (v->arch.vhpt.hash)
+if (v->arch.vhpt.hash) {
 thash_free(&(v->arch.vhpt));
+v->domain->arch.hvm_domain.vhpt_page[v->vcpu_id] = NULL;
+}
 }
 
 int init_domain_tlb(struct vcpu *v)
@@ -82,7 +84,7 @@ int init_domain_tlb(struct vcpu *v)
 if (rc)
 return rc;
 
-rc = thash_alloc(&(v->arch.vtlb), default_vtlb_sz, "vtlb");
+rc = thash_alloc(v, default_vtlb_sz, "vtlb");
 if (rc) {
 free_domain_vhpt(v);
 return rc;
diff -r b432c632ebe8 xen/arch/ia64/vmx/vmx_hypercall.c
--- a/xen/arch/ia64/vmx/vmx_hypercall.c Fri Feb 13 19:11:38 2009 +0900
+++ b/xen/arch/ia64/vmx/vmx_hypercall.c Sun Mar 01 03:28:28 2009 -0500
@@ -147,6 +147,9 @@ do_hvm_op(unsigned long op, XEN_GUEST_HA
 a.value = current->domain->domain_id;
 rc = a.value ? -EINVAL : 0; /* no stub domain support */
 break;
+case HVM_PARAM_VHPT_SIZE:
+rc = domain_pre_alloc_vhpt(d, a.value);
+break;
 default:
 /* nothing */
 break;
diff -r b432c632ebe8 xen/arch/ia64/vmx/vmx_init.c
--- a/xen/arch/ia64/vmx/vmx_init.c  Fri Feb 13 19:11:38 2009 +0900
+++ b/xen/arch/ia64/vmx/vmx_init.c  Sun Mar 01 03:28:28 2009 -0500
@@ -541,6 +541,7 @@ vmx_relinquish_guest_resources(struct do
for_each_vcpu(d, v)
vmx_release_assist_channel(v);
 
+   domain_pre_free_vhpt(d);
vacpi_relinquish_resources(d);
 
vmx_destroy_ioreq_page(d, &d->arch.vmx_platform.ioreq);
@@ -661,3 +662,63 @@ void vmx_do_resume(struct vcpu *v)
}
}
 }
+
+static inline int get_vhpt_size(unsigned long size)
+{
+switch (size) {
+case 0:
+size = DEFAULT_VHPT_SZ;
+break;
+case 1 ... 15:
+size = 15;
+break;
+case 16 ... (IA64_GRANULE_SHIFT - 1):
+break;
+default:
+size = IA64_GRANULE_SHIFT - 1;
+}
+return size;
+}
+int domain_pre_alloc_vhpt(struct domain *d, unsigned long size)
+{
+struct vcpu *v;
+struct page_info *pg = NULL;
+int rc = 0;
+
+size = get_vhpt_size(size);
+for_each_vcpu(d, v) {
+if (!v->is_initialised) {
+pg = alloc_domheap_pages(NULL, (size + 1 - PAGE_SHIFT), 0);
+if (pg == NULL) {
+printk("No enough contiguous memory(%ldKB) for alloc vhpt\n",
+size >> 9);
+rc = -ENOMEM;
+goto fail;
+}
+d->arch.hvm_domain.vhpt_page[v->vcpu_id] = pg;
+}
+}
+return rc;
+
+fail:
+domain_pre_fre

RE: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt

2009-02-18 Thread Zhang, Yang
Ok, I will modify it.
Thanks 
Best Regards
--yang

>-Original Message-
>From: Isaku Yamahata [mailto:yamah...@valinux.co.jp]
>Sent: 2009年2月19日 13:33
>To: Zhang, Yang
>Cc: xen-ia64-devel@lists.xensource.com
>Subject: Re: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous
>memory for vhpt
>
>On Thu, Feb 19, 2009 at 01:11:53PM +0800, Zhang, Yang wrote:
>> Hi
>>
>> >From: Isaku Yamahata [mailto:yamah...@valinux.co.jp]
>> >Sent: 2009年2月19日 11:12
>> >To: Zhang, Yang
>> >Cc: xen-ia64-devel@lists.xensource.com
>> >Subject: Re: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous
>> >memory for vhpt
>> >- It is a poor approach to introduce new HVM_PARAM_VHPT_SIZE.
>> >  This is inconsistent with XEN_DOMCTL_arch_setup by which libxc tells
>> >  VMM vhpt size of PV domain.
>> >  So the first idea which came into my mind is to call
>> >  XEN_DOMCTL_ARCH_setup before populating domain memory.
>> >  I haven't checked if it's possible or not, though. Maybe we can introduce
>> >  new XEN_DOMAINSETUP_xxx flag.
>>  I doesn't introduce HVM_PARAM_VHPT_SIZE. The hvm needs it to save the
>vhpt'size for allocating vhpt's memory when vcpu boot. I just add some code to
>save memory for vhpt beforehand when libxc tells VMM vhpt size of  hvm. And I
>don't think PV domain need to do this.
>
>Oh, sorry. I misread it. Okay, so the approach looks sane.
>
>- The functions newly added in arch/ia64/xen/domain.c are hvm domain
>  specific. So please move them under arch/ia64/xen/vmx/.
>
>- The pages for vhpt is allocated by alloc_domheap_pages(NULL, ...),
>  Thus those memory doesn't accounted to the domain.
>  So adjusting domain memory size doesn't make sense. Just drop it.
>
>- Is vhpt_page[] really necessary?
>  How about setting hcb->hash directly instead of temporary
>  keeping it in vhpt_page[]?
>  Probably adjusting codes of resource allocating/freeing might be required,
>  though.
>
>thanks,
>--
>yamahata
___
Xen-ia64-devel mailing list
Xen-ia64-devel@lists.xensource.com
http://lists.xensource.com/xen-ia64-devel

Re: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt

2009-02-18 Thread Isaku Yamahata
On Thu, Feb 19, 2009 at 01:11:53PM +0800, Zhang, Yang wrote:
> Hi
> 
> >From: Isaku Yamahata [mailto:yamah...@valinux.co.jp]
> >Sent: 2009年2月19日 11:12
> >To: Zhang, Yang
> >Cc: xen-ia64-devel@lists.xensource.com
> >Subject: Re: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous
> >memory for vhpt
> >- It is a poor approach to introduce new HVM_PARAM_VHPT_SIZE.
> >  This is inconsistent with XEN_DOMCTL_arch_setup by which libxc tells
> >  VMM vhpt size of PV domain.
> >  So the first idea which came into my mind is to call
> >  XEN_DOMCTL_ARCH_setup before populating domain memory.
> >  I haven't checked if it's possible or not, though. Maybe we can introduce
> >  new XEN_DOMAINSETUP_xxx flag.
>   I doesn't introduce HVM_PARAM_VHPT_SIZE. The hvm needs it to save the 
> vhpt'size for allocating vhpt's memory when vcpu boot. I just add some code 
> to save memory for vhpt beforehand when libxc tells VMM vhpt size of  hvm. 
> And I don't think PV domain need to do this.

Oh, sorry. I misread it. Okay, so the approach looks sane.

- The functions newly added in arch/ia64/xen/domain.c are hvm domain
  specific. So please move them under arch/ia64/xen/vmx/.

- The pages for vhpt is allocated by alloc_domheap_pages(NULL, ...),
  Thus those memory doesn't accounted to the domain.
  So adjusting domain memory size doesn't make sense. Just drop it.

- Is vhpt_page[] really necessary?
  How about setting hcb->hash directly instead of temporary
  keeping it in vhpt_page[]?
  Probably adjusting codes of resource allocating/freeing might be required,
  though.

thanks,
-- 
yamahata

___
Xen-ia64-devel mailing list
Xen-ia64-devel@lists.xensource.com
http://lists.xensource.com/xen-ia64-devel


RE: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt

2009-02-18 Thread Zhang, Yang
Hi

>From: Isaku Yamahata [mailto:yamah...@valinux.co.jp]
>Sent: 2009年2月19日 11:12
>To: Zhang, Yang
>Cc: xen-ia64-devel@lists.xensource.com
>Subject: Re: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous
>memory for vhpt
>- It is a poor approach to introduce new HVM_PARAM_VHPT_SIZE.
>  This is inconsistent with XEN_DOMCTL_arch_setup by which libxc tells
>  VMM vhpt size of PV domain.
>  So the first idea which came into my mind is to call
>  XEN_DOMCTL_ARCH_setup before populating domain memory.
>  I haven't checked if it's possible or not, though. Maybe we can introduce
>  new XEN_DOMAINSETUP_xxx flag.
I doesn't introduce HVM_PARAM_VHPT_SIZE. The hvm needs it to save the 
vhpt'size for allocating vhpt's memory when vcpu boot. I just add some code to 
save memory for vhpt beforehand when libxc tells VMM vhpt size of  hvm. And I 
don't think PV domain need to do this.



___
Xen-ia64-devel mailing list
Xen-ia64-devel@lists.xensource.com
http://lists.xensource.com/xen-ia64-devel

Re: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt

2009-02-18 Thread Isaku Yamahata
On Wed, Feb 18, 2009 at 11:20:03AM +0800, Zhang, Yang wrote:
> Fix the issue that guest cannot be launched by using balloon driver.
>  
> 
>  - In current version, when using balloon driver to balloon memory,it
> did not balloon the memory for vhpt.
> vhpt needs the continuous memory.But it allocates the memory for vhpt
> after it finished allocating the memory for guest. So the vhpt usually
> can not get the continuous memory and guest cannot be launched. Now
> i change the sequence.Before it allocates the memory for guest, i save
> the continuous memory for vhpt.when boot the vcpu, it allocate the saved
> memory for vhpt.

Hi. I agree that the possibility to allocate contiguous memory
is much higher before populating memory of a domain than after
the population. So it is a good idea to allocate vhpt early.

However the patch is a poor hack. Please reconsider the implementation
looking the whole picture.

- It is a poor approach to introduce new HVM_PARAM_VHPT_SIZE.
  This is inconsistent with XEN_DOMCTL_arch_setup by which libxc tells
  VMM vhpt size of PV domain.
  So the first idea which came into my mind is to call
  XEN_DOMCTL_ARCH_setup before populating domain memory.
  I haven't checked if it's possible or not, though. Maybe we can introduce
  new XEN_DOMAINSETUP_xxx flag.

- The same discussion applies to PV domain. So the approach should
  be applicable for both PV and HVM domain.


thanks,

> 
>  
> 
> Signed-off-by: Yang Zhang 
> 
>  
> 
> diff -r b432c632ebe8 tools/python/xen/xend/image.py
> 
> --- a/tools/python/xen/xend/image.py Fri Feb 13 19:11:38 2009 +0900
> 
> +++ b/tools/python/xen/xend/image.py  Tue Feb 17 20:36:43 2009 -0600
> 
> @@ -870,6 +870,9 @@ class IA64_HVM_ImageHandler(HVMImageHand
> 
>  extra_pages = 1024 + 5
> 
>  mem_kb += extra_pages * page_kb
> 
>  mem_kb += self.vramsize
> 
> +# per-vcpu need 16M for max vhpt size
> 
> +vhpt_kb = 16 * 1024
> 
> +mem_kb += self.vm.getVCpuCount() * vhpt_kb
> 
>  return mem_kb
> 
>  
> 
>  def getRequiredInitialReservation(self):
> 
> diff -r b432c632ebe8 xen/arch/ia64/vmx/vmmu.c
> 
> --- a/xen/arch/ia64/vmx/vmmu.cFri Feb 13 19:11:38 2009 +0900
> 
> +++ b/xen/arch/ia64/vmx/vmmu.c Tue Feb 17 20:35:39 2009 -0600
> 
> @@ -62,7 +62,7 @@ static int init_domain_vhpt(struct vcpu
> 
>  else
> 
>  size = canonicalize_vhpt_size(size);
> 
>  
> 
> -rc = thash_alloc(&(v->arch.vhpt), size, "vhpt");
> 
> +rc = thash_alloc(v, size, "vhpt");
> 
>  v->arch.arch_vmx.mpta = v->arch.vhpt.pta.val;
> 
>  return rc;
> 
>  }
> 
> @@ -70,8 +70,10 @@ static int init_domain_vhpt(struct vcpu
> 
>  
> 
>  static void free_domain_vhpt(struct vcpu *v)
> 
>  {
> 
> -if (v->arch.vhpt.hash)
> 
> +if (v->arch.vhpt.hash) {
> 
>  thash_free(&(v->arch.vhpt));
> 
> +v->domain->arch.hvm_domain.vhpt_page[v->vcpu_id] = NULL;
> 
> +}
> 
>  }
> 
>  
> 
>  int init_domain_tlb(struct vcpu *v)
> 
> @@ -82,7 +84,7 @@ int init_domain_tlb(struct vcpu *v)
> 
>  if (rc)
> 
>  return rc;
> 
>  
> 
> -rc = thash_alloc(&(v->arch.vtlb), default_vtlb_sz, "vtlb");
> 
> +rc = thash_alloc(v, default_vtlb_sz, "vtlb");
> 
>  if (rc) {
> 
>  free_domain_vhpt(v);
> 
>  return rc;
> 
> diff -r b432c632ebe8 xen/arch/ia64/vmx/vmx_hypercall.c
> 
> --- a/xen/arch/ia64/vmx/vmx_hypercall.c  Fri Feb 13 19:11:38 2009 +0900
> 
> +++ b/xen/arch/ia64/vmx/vmx_hypercall.c Tue Feb 17 20:35:39 2009 -0600
> 
> @@ -147,6 +147,9 @@ do_hvm_op(unsigned long op, XEN_GUEST_HA
> 
>  a.value = current->domain->domain_id;
> 
>  rc = a.value ? -EINVAL : 0; /* no stub domain support */
> 
>  break;
> 
> +case HVM_PARAM_VHPT_SIZE:
> 
> +rc = domain_pre_alloc_vhpt(d, a.value);
> 
> +break;
> 
>  default:
> 
>  /* nothing */
> 
>  break;
> 
> diff -r b432c632ebe8 xen/arch/ia64/vmx/vmx_init.c
> 
> --- a/xen/arch/ia64/vmx/vmx_init.cFri Feb 13 19:11:38 2009 +0900
> 
> +++ b/xen/arch/ia64/vmx/vmx_init.c Tue Feb 17 20:42:02 2009 -0600
> 
> @@ -541,6 +541,7 @@ vmx_relinquish_guest_resources(struct do
> 
> for_each_vcpu(d, v)
> 
>   vmx_release_assist_channel(v);
> 
>  
> 
> +   domain_pre_free_vhpt(d);
> 
> vacpi_relinquish_resources(d);
> 
>  
> 
> vmx_destroy_ioreq_page(d, &d->arch.vmx_platform.ioreq);
> 
> diff -r b432c632ebe8 xen/arch/ia64/vmx/vtlb.c
> 
> --- a/xen/arch/ia64/vmx/vtlb.cFri Feb 13 19:11:38 2009 +0900
> 
> +++ b/xen/arch/ia64/vmx/vtlb.c Tue Feb 17 20:38:23 2009 -0600
> 
> @@ -723,13 +723,30 @@ static void thash_init(thash_cb_t *hcb,
> 
>  hcb->cch_freelist = NULL;
> 
>  }
> 
>  
> 
> -int thash_alloc(thash_cb_t *hcb, u64 sz_log2, char *what)
> 
> +int thash_alloc(struct vcpu *v, u64 sz_log2, char *what)
> 
>  {
> 
>  struct page_info *page;