xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 6/8]: PVH ballooning and grant changes
@ 2012-09-21 19:19 Mukesh Rathor
  2012-09-24 13:55 ` Stefano Stabellini
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Mukesh Rathor @ 2012-09-21 19:19 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Xen-devel@lists.xensource.com,
	Ian Campbell, stefano.stabellini@eu.citrix.com


---
 drivers/xen/balloon.c     |   35 ++++++++++++++++++++++++++++-------
 drivers/xen/gntdev.c      |    3 ++-
 drivers/xen/grant-table.c |   25 +++++++++++++++++++++----
 3 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 31ab82f..85a6917 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -358,10 +358,21 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
 		BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) &&
 		       phys_to_machine_mapping_valid(pfn));
 
-		set_phys_to_machine(pfn, frame_list[i]);
+		if (xen_pv_domain() && 
+		    xen_feature(XENFEAT_auto_translated_physmap)) {
+			/* PVH: we just need to update native page table */
+			pte_t *ptep;
+			unsigned int level;
+			void *addr = __va(pfn << PAGE_SHIFT);
+			ptep = lookup_address((unsigned long)addr, &level);
+			set_pte(ptep, pfn_pte(pfn, PAGE_KERNEL));
+		} else
+			set_phys_to_machine(pfn, frame_list[i]);
 
 		/* Link back into the page tables if not highmem. */
-		if (xen_pv_domain() && !PageHighMem(page)) {
+		if (xen_pv_domain() && !PageHighMem(page) && 
+		    !xen_feature(XENFEAT_auto_translated_physmap)) {
+
 			int ret;
 			ret = HYPERVISOR_update_va_mapping(
 				(unsigned long)__va(pfn << PAGE_SHIFT),
@@ -418,12 +429,21 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 		scrub_page(page);
 
 		if (xen_pv_domain() && !PageHighMem(page)) {
-			ret = HYPERVISOR_update_va_mapping(
-				(unsigned long)__va(pfn << PAGE_SHIFT),
-				__pte_ma(0), 0);
-			BUG_ON(ret);
+			if (xen_feature(XENFEAT_auto_translated_physmap)) {
+				unsigned int level;
+				pte_t *ptep;
+				void *addr = __va(pfn << PAGE_SHIFT);
+				ptep = lookup_address((unsigned long)addr, 
+							&level);
+				set_pte(ptep, __pte(0));
+
+			} else {
+				ret = HYPERVISOR_update_va_mapping(
+					(unsigned long)__va(pfn << PAGE_SHIFT),
+					__pte_ma(0), 0);
+				BUG_ON(ret);
+			}
 		}
-
 	}
 
 	/* Ensure that ballooned highmem pages don't have kmaps. */
@@ -433,6 +453,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 	/* No more mappings: invalidate P2M and add to balloon. */
 	for (i = 0; i < nr_pages; i++) {
 		pfn = mfn_to_pfn(frame_list[i]);
+		/* PVH note: following will noop for auto translated */
 		__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
 		balloon_append(pfn_to_page(pfn));
 	}
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 1ffd03b..35d1e3c 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -802,7 +802,8 @@ static int __init gntdev_init(void)
 	if (!xen_domain())
 		return -ENODEV;
 
-	use_ptemod = xen_pv_domain();
+	use_ptemod = xen_pv_domain() && 
+		     !xen_feature(XENFEAT_auto_translated_physmap);
 
 	err = misc_register(&gntdev_miscdev);
 	if (err != 0) {
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 0bfc1ef..d831d52 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -974,14 +974,19 @@ static void gnttab_unmap_frames_v2(void)
 static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
 {
 	struct gnttab_setup_table setup;
-	unsigned long *frames;
+	unsigned long *frames, start_gpfn;
 	unsigned int nr_gframes = end_idx + 1;
 	int rc;
 
-	if (xen_hvm_domain()) {
+	if (xen_hvm_domain() || xen_feature(XENFEAT_auto_translated_physmap)) {
 		struct xen_add_to_physmap xatp;
 		unsigned int i = end_idx;
 		rc = 0;
+
+		if (xen_hvm_domain())
+			start_gpfn = xen_hvm_resume_frames >> PAGE_SHIFT;
+		else
+			start_gpfn = virt_to_pfn(gnttab_shared.addr);
 		/*
 		 * Loop backwards, so that the first hypercall has the largest
 		 * index, ensuring that the table will grow only once.
@@ -990,7 +995,7 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
 			xatp.domid = DOMID_SELF;
 			xatp.idx = i;
 			xatp.space = XENMAPSPACE_grant_table;
-			xatp.gpfn = (xen_hvm_resume_frames >> PAGE_SHIFT) + i;
+			xatp.gpfn = start_gpfn + i;
 			rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
 			if (rc != 0) {
 				printk(KERN_WARNING
@@ -1053,7 +1058,7 @@ static void gnttab_request_version(void)
 	int rc;
 	struct gnttab_set_version gsv;
 
-	if (xen_hvm_domain())
+	if (xen_hvm_domain() || xen_feature(XENFEAT_auto_translated_physmap))
 		gsv.version = 1;
 	else
 		gsv.version = 2;
@@ -1081,12 +1086,24 @@ static void gnttab_request_version(void)
 int gnttab_resume(void)
 {
 	unsigned int max_nr_gframes;
+	char *kmsg="Failed to kmalloc pages for pv in hvm grant frames\n";
 
 	gnttab_request_version();
 	max_nr_gframes = gnttab_max_grant_frames();
 	if (max_nr_gframes < nr_grant_frames)
 		return -ENOSYS;
 
+	/* PVH note: xen will free existing kmalloc'd mfn in 
+	 * XENMEM_add_to_physmap */
+	if (xen_pv_domain() && xen_feature(XENFEAT_auto_translated_physmap) && 
+	    !gnttab_shared.addr) {
+		gnttab_shared.addr =
+			kmalloc(max_nr_gframes * PAGE_SIZE, GFP_KERNEL);
+		if ( !gnttab_shared.addr ) {
+			printk(KERN_WARNING "%s", kmsg);
+			return -ENOMEM;
+		}
+	}
 	if (xen_pv_domain())
 		return gnttab_map(0, nr_grant_frames - 1);
 
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v1 6/8]: PVH ballooning and grant changes
  2012-09-21 19:19 [PATCH v1 6/8]: PVH ballooning and grant changes Mukesh Rathor
@ 2012-09-24 13:55 ` Stefano Stabellini
  2012-09-26  1:22   ` Mukesh Rathor
  2012-09-25 13:39 ` Ian Campbell
  2012-10-02 10:52 ` Stefano Stabellini
  2 siblings, 1 reply; 5+ messages in thread
From: Stefano Stabellini @ 2012-09-24 13:55 UTC (permalink / raw)
  To: Mukesh Rathor
  Cc: Xen-devel@lists.xensource.com, Ian Campbell, Stefano Stabellini,
	Konrad Rzeszutek Wilk

On Fri, 21 Sep 2012, Mukesh Rathor wrote:
> ---
>  drivers/xen/balloon.c     |   35 ++++++++++++++++++++++++++++-------
>  drivers/xen/gntdev.c      |    3 ++-
>  drivers/xen/grant-table.c |   25 +++++++++++++++++++++----
>  3 files changed, 51 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 31ab82f..85a6917 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -358,10 +358,21 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
>  		BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) &&
>  		       phys_to_machine_mapping_valid(pfn));
>  
> -		set_phys_to_machine(pfn, frame_list[i]);
> +		if (xen_pv_domain() && 
> +		    xen_feature(XENFEAT_auto_translated_physmap)) {
> +			/* PVH: we just need to update native page table */
> +			pte_t *ptep;
> +			unsigned int level;
> +			void *addr = __va(pfn << PAGE_SHIFT);
> +			ptep = lookup_address((unsigned long)addr, &level);
> +			set_pte(ptep, pfn_pte(pfn, PAGE_KERNEL));
> +		} else
> +			set_phys_to_machine(pfn, frame_list[i]);



>  		/* Link back into the page tables if not highmem. */
> -		if (xen_pv_domain() && !PageHighMem(page)) {
> +		if (xen_pv_domain() && !PageHighMem(page) && 
> +		    !xen_feature(XENFEAT_auto_translated_physmap)) {
> +
>  			int ret;
>  			ret = HYPERVISOR_update_va_mapping(
>  				(unsigned long)__va(pfn << PAGE_SHIFT),
> @@ -418,12 +429,21 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  		scrub_page(page);
>  
>  		if (xen_pv_domain() && !PageHighMem(page)) {
> -			ret = HYPERVISOR_update_va_mapping(
> -				(unsigned long)__va(pfn << PAGE_SHIFT),
> -				__pte_ma(0), 0);
> -			BUG_ON(ret);
> +			if (xen_feature(XENFEAT_auto_translated_physmap)) {
> +				unsigned int level;
> +				pte_t *ptep;
> +				void *addr = __va(pfn << PAGE_SHIFT);
> +				ptep = lookup_address((unsigned long)addr, 
> +							&level);
> +				set_pte(ptep, __pte(0));
> +
> +			} else {
> +				ret = HYPERVISOR_update_va_mapping(
> +					(unsigned long)__va(pfn << PAGE_SHIFT),
> +					__pte_ma(0), 0);
> +				BUG_ON(ret);
> +			}
>  		}
> -
>  	}
>  
>  	/* Ensure that ballooned highmem pages don't have kmaps. */
> @@ -433,6 +453,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  	/* No more mappings: invalidate P2M and add to balloon. */
>  	for (i = 0; i < nr_pages; i++) {
>  		pfn = mfn_to_pfn(frame_list[i]);
> +		/* PVH note: following will noop for auto translated */
>  		__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
>  		balloon_append(pfn_to_page(pfn));
>  	}

Maybe we need a function "update_mapping" internal to balloon.c that
would be set to HYPERVISOR_update_va_mapping for pv guests, set_pte for
pvh guests and nothing for pv on hvm guests.

Speaking of which, why do pvh guests need a set_pte while pv on hvm
guests don't? I would think that their behaviour should be the same
regarding ballooning.


> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 1ffd03b..35d1e3c 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -802,7 +802,8 @@ static int __init gntdev_init(void)
>  	if (!xen_domain())
>  		return -ENODEV;
>  
> -	use_ptemod = xen_pv_domain();
> +	use_ptemod = xen_pv_domain() && 
> +		     !xen_feature(XENFEAT_auto_translated_physmap);
>  
>  	err = misc_register(&gntdev_miscdev);
>  	if (err != 0) {
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 0bfc1ef..d831d52 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -974,14 +974,19 @@ static void gnttab_unmap_frames_v2(void)
>  static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
>  {
>  	struct gnttab_setup_table setup;
> -	unsigned long *frames;
> +	unsigned long *frames, start_gpfn;
>  	unsigned int nr_gframes = end_idx + 1;
>  	int rc;
>  
> -	if (xen_hvm_domain()) {
> +	if (xen_hvm_domain() || xen_feature(XENFEAT_auto_translated_physmap)) {
>  		struct xen_add_to_physmap xatp;
>  		unsigned int i = end_idx;
>  		rc = 0;
> +
> +		if (xen_hvm_domain())
> +			start_gpfn = xen_hvm_resume_frames >> PAGE_SHIFT;
> +		else
> +			start_gpfn = virt_to_pfn(gnttab_shared.addr);
>  		/*
>  		 * Loop backwards, so that the first hypercall has the largest
>  		 * index, ensuring that the table will grow only once.
> @@ -990,7 +995,7 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
>  			xatp.domid = DOMID_SELF;
>  			xatp.idx = i;
>  			xatp.space = XENMAPSPACE_grant_table;
> -			xatp.gpfn = (xen_hvm_resume_frames >> PAGE_SHIFT) + i;
> +			xatp.gpfn = start_gpfn + i;
>  			rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
>  			if (rc != 0) {
>  				printk(KERN_WARNING
> @@ -1053,7 +1058,7 @@ static void gnttab_request_version(void)
>  	int rc;
>  	struct gnttab_set_version gsv;
>  
> -	if (xen_hvm_domain())
> +	if (xen_hvm_domain() || xen_feature(XENFEAT_auto_translated_physmap))
>  		gsv.version = 1;
>  	else
>  		gsv.version = 2;
> @@ -1081,12 +1086,24 @@ static void gnttab_request_version(void)
>  int gnttab_resume(void)
>  {
>  	unsigned int max_nr_gframes;
> +	char *kmsg="Failed to kmalloc pages for pv in hvm grant frames\n";
>  
>  	gnttab_request_version();
>  	max_nr_gframes = gnttab_max_grant_frames();
>  	if (max_nr_gframes < nr_grant_frames)
>  		return -ENOSYS;
>  
> +	/* PVH note: xen will free existing kmalloc'd mfn in 
> +	 * XENMEM_add_to_physmap */
> +	if (xen_pv_domain() && xen_feature(XENFEAT_auto_translated_physmap) && 
> +	    !gnttab_shared.addr) {
> +		gnttab_shared.addr =
> +			kmalloc(max_nr_gframes * PAGE_SIZE, GFP_KERNEL);
> +		if ( !gnttab_shared.addr ) {
> +			printk(KERN_WARNING "%s", kmsg);
> +			return -ENOMEM;
> +		}
> +	}

the way PVH guests use gnttab_shared.addr is very similar to the way PV
on HVM guests use xen_hvm_resume_frames, except that
xen_hvm_resume_frames is allocated using alloc_xen_mmio instead of
kmalloc.

I wonder if it would produce better code switching PV on HVM guests to
gnttab_shared.addr too (or PVH guests to xen_hvm_resume_frames, but then
xen_hvm_resume_frames would need to be renamed).

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1 6/8]: PVH ballooning and grant changes
  2012-09-21 19:19 [PATCH v1 6/8]: PVH ballooning and grant changes Mukesh Rathor
  2012-09-24 13:55 ` Stefano Stabellini
@ 2012-09-25 13:39 ` Ian Campbell
  2012-10-02 10:52 ` Stefano Stabellini
  2 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2012-09-25 13:39 UTC (permalink / raw)
  To: Mukesh Rathor
  Cc: Xen-devel@lists.xensource.com, Stefano Stabellini,
	Konrad Rzeszutek Wilk

On Fri, 2012-09-21 at 20:19 +0100, Mukesh Rathor wrote:
> ---
>  drivers/xen/balloon.c     |   35 ++++++++++++++++++++++++++++-------
>  drivers/xen/gntdev.c      |    3 ++-
>  drivers/xen/grant-table.c |   25 +++++++++++++++++++++----
>  3 files changed, 51 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 31ab82f..85a6917 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -358,10 +358,21 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
>  		BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) &&
>  		       phys_to_machine_mapping_valid(pfn));
>  
> -		set_phys_to_machine(pfn, frame_list[i]);
> +		if (xen_pv_domain() && 
> +		    xen_feature(XENFEAT_auto_translated_physmap)) {
> +			/* PVH: we just need to update native page table */

Do you not need a PageHighMem check here somewhere? (although this is
moot if you follow the suggestion below)

> +			pte_t *ptep;
> +			unsigned int level;
> +			void *addr = __va(pfn << PAGE_SHIFT);
> +			ptep = lookup_address((unsigned long)addr, &level);
> +			set_pte(ptep, pfn_pte(pfn, PAGE_KERNEL));
> +		} else
> +			set_phys_to_machine(pfn, frame_list[i]);
>  
>  		/* Link back into the page tables if not highmem. */
> -		if (xen_pv_domain() && !PageHighMem(page)) {
> +		if (xen_pv_domain() && !PageHighMem(page) && 
> +		    !xen_feature(XENFEAT_auto_translated_physmap)) {

I think it would be more logical to put the PVH PT update here next to
the PV PT update and gate the set_phys_to_machine above on !a_t_pm, like
how you've done it in the decrease case.

(or perhaps per your comment below set_phys_to_machine isa NOP also on
PVH?)

> +
>  			int ret;
>  			ret = HYPERVISOR_update_va_mapping(
>  				(unsigned long)__va(pfn << PAGE_SHIFT),
> @@ -418,12 +429,21 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  		scrub_page(page);
>  
>  		if (xen_pv_domain() && !PageHighMem(page)) {
> -			ret = HYPERVISOR_update_va_mapping(
> -				(unsigned long)__va(pfn << PAGE_SHIFT),
> -				__pte_ma(0), 0);
> -			BUG_ON(ret);
> +			if (xen_feature(XENFEAT_auto_translated_physmap)) {
> +				unsigned int level;
> +				pte_t *ptep;
> +				void *addr = __va(pfn << PAGE_SHIFT);
> +				ptep = lookup_address((unsigned long)addr, 
> +							&level);
> +				set_pte(ptep, __pte(0));
> +
> +			} else {
> +				ret = HYPERVISOR_update_va_mapping(
> +					(unsigned long)__va(pfn << PAGE_SHIFT),
> +					__pte_ma(0), 0);
> +				BUG_ON(ret);
> +			}
>  		}
> -
>  	}
>  
>  	/* Ensure that ballooned highmem pages don't have kmaps. */
> @@ -433,6 +453,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  	/* No more mappings: invalidate P2M and add to balloon. */
>  	for (i = 0; i < nr_pages; i++) {
>  		pfn = mfn_to_pfn(frame_list[i]);
> +		/* PVH note: following will noop for auto translated */
>  		__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
>  		balloon_append(pfn_to_page(pfn));
>  	}
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 1ffd03b..35d1e3c 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -802,7 +802,8 @@ static int __init gntdev_init(void)
>  	if (!xen_domain())
>  		return -ENODEV;
>  
> -	use_ptemod = xen_pv_domain();
> +	use_ptemod = xen_pv_domain() && 
> +		     !xen_feature(XENFEAT_auto_translated_physmap);

I keep wondering if
#define XENFEAT(x) xen_feature(XENFEAT_#x)
might not be useful to keep some of these long lines under control.

Probably not, since the bulk is the "x" bit...

Ian.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1 6/8]: PVH ballooning and grant changes
  2012-09-24 13:55 ` Stefano Stabellini
@ 2012-09-26  1:22   ` Mukesh Rathor
  0 siblings, 0 replies; 5+ messages in thread
From: Mukesh Rathor @ 2012-09-26  1:22 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Xen-devel@lists.xensource.com, Ian Campbell,
	Konrad Rzeszutek Wilk

On Mon, 24 Sep 2012 14:55:06 +0100
Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:

> On Fri, 21 Sep 2012, Mukesh Rathor wrote:
> > ---
> >  drivers/xen/balloon.c     |   35
> > ++++++++++++++++++++++++++++------- drivers/xen/gntdev.c      |
> > 3 ++- drivers/xen/grant-table.c |   25 +++++++++++++++++++++----
> >  3 files changed, 51 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> > index 31ab82f..85a6917 100644
> > --- a/drivers/xen/balloon.c
> > +++ b/drivers/xen/balloon.c
> > @@ -358,10 +358,21 @@ static enum bp_state
> > increase_reservation(unsigned long nr_pages)
> > BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) &&
> > phys_to_machine_mapping_valid(pfn)); 
> > -		set_phys_to_machine(pfn, frame_list[i]);
> > +		if (xen_pv_domain() && 
> > +		    xen_feature(XENFEAT_auto_translated_physmap)) {
> > +			/* PVH: we just need to update native page
> > table */
> > +			pte_t *ptep;
> > +			unsigned int level;
> > +			void *addr = __va(pfn << PAGE_SHIFT);
> > +			ptep = lookup_address((unsigned long)addr,
> > &level);
> > +			set_pte(ptep, pfn_pte(pfn, PAGE_KERNEL));
> > +		} else
> > +			set_phys_to_machine(pfn, frame_list[i]);
> 
> 
> 
> >  		/* Link back into the page tables if not highmem.
> > */
> > -		if (xen_pv_domain() && !PageHighMem(page)) {
> > +		if (xen_pv_domain() && !PageHighMem(page) && 
> > +		    !xen_feature(XENFEAT_auto_translated_physmap))
> > { +
> >  			int ret;
> >  			ret = HYPERVISOR_update_va_mapping(
> >  				(unsigned long)__va(pfn <<
> > PAGE_SHIFT), @@ -418,12 +429,21 @@ static enum bp_state
> > decrease_reservation(unsigned long nr_pages, gfp_t gfp)
> > scrub_page(page); 
> >  		if (xen_pv_domain() && !PageHighMem(page)) {
> > -			ret = HYPERVISOR_update_va_mapping(
> > -				(unsigned long)__va(pfn <<
> > PAGE_SHIFT),
> > -				__pte_ma(0), 0);
> > -			BUG_ON(ret);
> > +			if
> > (xen_feature(XENFEAT_auto_translated_physmap)) {
> > +				unsigned int level;
> > +				pte_t *ptep;
> > +				void *addr = __va(pfn <<
> > PAGE_SHIFT);
> > +				ptep = lookup_address((unsigned
> > long)addr, 
> > +							&level);
> > +				set_pte(ptep, __pte(0));
> > +
> > +			} else {
> > +				ret = HYPERVISOR_update_va_mapping(
> > +					(unsigned long)__va(pfn <<
> > PAGE_SHIFT),
> > +					__pte_ma(0), 0);
> > +				BUG_ON(ret);
> > +			}
> >  		}
> > -
> >  	}
> >  
> >  	/* Ensure that ballooned highmem pages don't have kmaps. */
> > @@ -433,6 +453,7 @@ static enum bp_state
> > decrease_reservation(unsigned long nr_pages, gfp_t gfp) /* No more
> > mappings: invalidate P2M and add to balloon. */ for (i = 0; i <
> > nr_pages; i++) { pfn = mfn_to_pfn(frame_list[i]);
> > +		/* PVH note: following will noop for auto
> > translated */ __set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
> >  		balloon_append(pfn_to_page(pfn));
> >  	}
> 
> Maybe we need a function "update_mapping" internal to balloon.c that
> would be set to HYPERVISOR_update_va_mapping for pv guests, set_pte
> for pvh guests and nothing for pv on hvm guests.
> 
> Speaking of which, why do pvh guests need a set_pte while pv on hvm
> guests don't? I would think that their behaviour should be the same
> regarding ballooning.

you are right, I can remove it. Not sure why I've it there. Cant
recall.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1 6/8]: PVH ballooning and grant changes
  2012-09-21 19:19 [PATCH v1 6/8]: PVH ballooning and grant changes Mukesh Rathor
  2012-09-24 13:55 ` Stefano Stabellini
  2012-09-25 13:39 ` Ian Campbell
@ 2012-10-02 10:52 ` Stefano Stabellini
  2 siblings, 0 replies; 5+ messages in thread
From: Stefano Stabellini @ 2012-10-02 10:52 UTC (permalink / raw)
  To: Mukesh Rathor
  Cc: Xen-devel@lists.xensource.com, Ian Campbell, Stefano Stabellini,
	Konrad Rzeszutek Wilk

On Fri, 21 Sep 2012, Mukesh Rathor wrote:
> 
> ---
>  drivers/xen/balloon.c     |   35 ++++++++++++++++++++++++++++-------
>  drivers/xen/gntdev.c      |    3 ++-
>  drivers/xen/grant-table.c |   25 +++++++++++++++++++++----
>  3 files changed, 51 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 31ab82f..85a6917 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -358,10 +358,21 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
>  		BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) &&
>  		       phys_to_machine_mapping_valid(pfn));
>  
> -		set_phys_to_machine(pfn, frame_list[i]);
> +		if (xen_pv_domain() && 
> +		    xen_feature(XENFEAT_auto_translated_physmap)) {
> +			/* PVH: we just need to update native page table */
> +			pte_t *ptep;
> +			unsigned int level;
> +			void *addr = __va(pfn << PAGE_SHIFT);
> +			ptep = lookup_address((unsigned long)addr, &level);
> +			set_pte(ptep, pfn_pte(pfn, PAGE_KERNEL));
> +		} else
> +			set_phys_to_machine(pfn, frame_list[i]);
>  
>  		/* Link back into the page tables if not highmem. */
> -		if (xen_pv_domain() && !PageHighMem(page)) {
> +		if (xen_pv_domain() && !PageHighMem(page) && 
> +		    !xen_feature(XENFEAT_auto_translated_physmap)) {
> +
>  			int ret;
>  			ret = HYPERVISOR_update_va_mapping(
>  				(unsigned long)__va(pfn << PAGE_SHIFT),
> @@ -418,12 +429,21 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  		scrub_page(page);
>  
>  		if (xen_pv_domain() && !PageHighMem(page)) {
> -			ret = HYPERVISOR_update_va_mapping(
> -				(unsigned long)__va(pfn << PAGE_SHIFT),
> -				__pte_ma(0), 0);
> -			BUG_ON(ret);
> +			if (xen_feature(XENFEAT_auto_translated_physmap)) {
> +				unsigned int level;
> +				pte_t *ptep;
> +				void *addr = __va(pfn << PAGE_SHIFT);
> +				ptep = lookup_address((unsigned long)addr, 
> +							&level);
> +				set_pte(ptep, __pte(0));
> +
> +			} else {
> +				ret = HYPERVISOR_update_va_mapping(
> +					(unsigned long)__va(pfn << PAGE_SHIFT),
> +					__pte_ma(0), 0);
> +				BUG_ON(ret);
> +			}
>  		}
> -
>  	}
>  
>  	/* Ensure that ballooned highmem pages don't have kmaps. */
> @@ -433,6 +453,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  	/* No more mappings: invalidate P2M and add to balloon. */
>  	for (i = 0; i < nr_pages; i++) {
>  		pfn = mfn_to_pfn(frame_list[i]);
> +		/* PVH note: following will noop for auto translated */
>  		__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
>  		balloon_append(pfn_to_page(pfn));
>  	}
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 1ffd03b..35d1e3c 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -802,7 +802,8 @@ static int __init gntdev_init(void)
>  	if (!xen_domain())
>  		return -ENODEV;
>  
> -	use_ptemod = xen_pv_domain();
> +	use_ptemod = xen_pv_domain() && 
> +		     !xen_feature(XENFEAT_auto_translated_physmap);
>  
>  	err = misc_register(&gntdev_miscdev);
>  	if (err != 0) {
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 0bfc1ef..d831d52 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -974,14 +974,19 @@ static void gnttab_unmap_frames_v2(void)
>  static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
>  {
>  	struct gnttab_setup_table setup;
> -	unsigned long *frames;
> +	unsigned long *frames, start_gpfn;
>  	unsigned int nr_gframes = end_idx + 1;
>  	int rc;
>  
> -	if (xen_hvm_domain()) {
> +	if (xen_hvm_domain() || xen_feature(XENFEAT_auto_translated_physmap)) {
>  		struct xen_add_to_physmap xatp;
>  		unsigned int i = end_idx;
>  		rc = 0;
> +
> +		if (xen_hvm_domain())
> +			start_gpfn = xen_hvm_resume_frames >> PAGE_SHIFT;
> +		else
> +			start_gpfn = virt_to_pfn(gnttab_shared.addr);
>  		/*
>  		 * Loop backwards, so that the first hypercall has the largest
>  		 * index, ensuring that the table will grow only once.
> @@ -990,7 +995,7 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
>  			xatp.domid = DOMID_SELF;
>  			xatp.idx = i;
>  			xatp.space = XENMAPSPACE_grant_table;
> -			xatp.gpfn = (xen_hvm_resume_frames >> PAGE_SHIFT) + i;
> +			xatp.gpfn = start_gpfn + i;
>  			rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
>  			if (rc != 0) {
>  				printk(KERN_WARNING
> @@ -1053,7 +1058,7 @@ static void gnttab_request_version(void)
>  	int rc;
>  	struct gnttab_set_version gsv;
>  
> -	if (xen_hvm_domain())
> +	if (xen_hvm_domain() || xen_feature(XENFEAT_auto_translated_physmap))
>  		gsv.version = 1;
>  	else
>  		gsv.version = 2;
> @@ -1081,12 +1086,24 @@ static void gnttab_request_version(void)
>  int gnttab_resume(void)
>  {
>  	unsigned int max_nr_gframes;
> +	char *kmsg="Failed to kmalloc pages for pv in hvm grant frames\n";

msg = "Failed

>  
>  	gnttab_request_version();
>  	max_nr_gframes = gnttab_max_grant_frames();
>  	if (max_nr_gframes < nr_grant_frames)
>  		return -ENOSYS;
>  
> +	/* PVH note: xen will free existing kmalloc'd mfn in 
> +	 * XENMEM_add_to_physmap */
> +	if (xen_pv_domain() && xen_feature(XENFEAT_auto_translated_physmap) && 
> +	    !gnttab_shared.addr) {
> +		gnttab_shared.addr =
> +			kmalloc(max_nr_gframes * PAGE_SIZE, GFP_KERNEL);
> +		if ( !gnttab_shared.addr ) {

if (!gnttab_shared.addr) {


> +			printk(KERN_WARNING "%s", kmsg);
> +			return -ENOMEM;
> +		}
> +	}
>  	if (xen_pv_domain())
>  		return gnttab_map(0, nr_grant_frames - 1);
>  
> -- 
> 1.7.2.3
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-10-02 10:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-21 19:19 [PATCH v1 6/8]: PVH ballooning and grant changes Mukesh Rathor
2012-09-24 13:55 ` Stefano Stabellini
2012-09-26  1:22   ` Mukesh Rathor
2012-09-25 13:39 ` Ian Campbell
2012-10-02 10:52 ` Stefano Stabellini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).