Re: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt
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 yang.zh...@intel.com
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 get_vhpt_size(unsigned long size)
+{
+switch (size) {
+case 0:
+size = DEFAULT_VHPT_SZ;
+
RE: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt
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
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
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
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
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
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 yang.zh...@intel.com
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;
void * vbase;
+thash_cb_t *hcb;
u64 sz = 1UL sz_log2;
-page = alloc_domheap_pages(NULL, (sz_log2 + 1 - PAGE_SHIFT), 0);
+if (!strcmp(what,
RE: [Xen-ia64-devel] [PATCH] Fix that guest can not get continuous memory for vhpt
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
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
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
