xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 6/8]: Ballooning changes
@ 2012-08-16  1:05 Mukesh Rathor
  2012-08-16 14:13 ` Stefano Stabellini
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Mukesh Rathor @ 2012-08-16  1:05 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Xen-devel@lists.xensource.com


---
 drivers/xen/balloon.c |   26 +++++++++++++++++++++-----
 1 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 31ab82f..57960a1 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -358,10 +358,18 @@ 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_pvh_domain()) {
+			set_phys_to_machine(pfn, frame_list[i]);
+		} else {
+			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));
+		}
 		/* Link back into the page tables if not highmem. */
-		if (xen_pv_domain() && !PageHighMem(page)) {
+		if (xen_pv_domain() && !PageHighMem(page) &&
+		    !xen_pvh_domain()) {
 			int ret;
 			ret = HYPERVISOR_update_va_mapping(
 				(unsigned long)__va(pfn << PAGE_SHIFT),
@@ -417,7 +425,14 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 
 		scrub_page(page);
 
-		if (xen_pv_domain() && !PageHighMem(page)) {
+		if (xen_pvh_domain() && !PageHighMem(page)) {
+			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 if (xen_pv_domain() && !PageHighMem(page)) {
 			ret = HYPERVISOR_update_va_mapping(
 				(unsigned long)__va(pfn << PAGE_SHIFT),
 				__pte_ma(0), 0);
@@ -433,7 +448,8 @@ 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]);
-		__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
+		if (!xen_pvh_domain())
+			__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
 		balloon_append(pfn_to_page(pfn));
 	}
 
-- 
1.7.2.3

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

* Re: [RFC PATCH 6/8]: Ballooning changes
  2012-08-16  1:05 [RFC PATCH 6/8]: Ballooning changes Mukesh Rathor
@ 2012-08-16 14:13 ` Stefano Stabellini
  2012-08-17  9:46 ` Ian Campbell
  2012-09-13 16:33 ` Ian Campbell
  2 siblings, 0 replies; 4+ messages in thread
From: Stefano Stabellini @ 2012-08-16 14:13 UTC (permalink / raw)
  To: Mukesh Rathor; +Cc: Xen-devel@lists.xensource.com, Konrad Rzeszutek Wilk

And patches like this one are the reason why I don't want xen_pvh_domain
in non-x86 xen files: I am pretty sure that if you didn't use
xen_pvh_domain here but a combination of XENFEAT_auto_translated_physmap
and xen_hvm_domain I would be able to reuse this work on ARM (that
doesn't have a working balloon driver yet) as is.

On Thu, 16 Aug 2012, Mukesh Rathor wrote:
> ---
>  drivers/xen/balloon.c |   26 +++++++++++++++++++++-----
>  1 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 31ab82f..57960a1 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -358,10 +358,18 @@ 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_pvh_domain()) {
> +			set_phys_to_machine(pfn, frame_list[i]);
> +		} else {
> +			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));
> +		}
>  		/* Link back into the page tables if not highmem. */
> -		if (xen_pv_domain() && !PageHighMem(page)) {
> +		if (xen_pv_domain() && !PageHighMem(page) &&
> +		    !xen_pvh_domain()) {
>  			int ret;
>  			ret = HYPERVISOR_update_va_mapping(
>  				(unsigned long)__va(pfn << PAGE_SHIFT),
> @@ -417,7 +425,14 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  
>  		scrub_page(page);
>  
> -		if (xen_pv_domain() && !PageHighMem(page)) {
> +		if (xen_pvh_domain() && !PageHighMem(page)) {
> +			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 if (xen_pv_domain() && !PageHighMem(page)) {
>  			ret = HYPERVISOR_update_va_mapping(
>  				(unsigned long)__va(pfn << PAGE_SHIFT),
>  				__pte_ma(0), 0);
> @@ -433,7 +448,8 @@ 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]);
> -		__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
> +		if (!xen_pvh_domain())
> +			__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
>  		balloon_append(pfn_to_page(pfn));
>  	}
>  
> -- 
> 1.7.2.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

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

* Re: [RFC PATCH 6/8]: Ballooning changes
  2012-08-16  1:05 [RFC PATCH 6/8]: Ballooning changes Mukesh Rathor
  2012-08-16 14:13 ` Stefano Stabellini
@ 2012-08-17  9:46 ` Ian Campbell
  2012-09-13 16:33 ` Ian Campbell
  2 siblings, 0 replies; 4+ messages in thread
From: Ian Campbell @ 2012-08-17  9:46 UTC (permalink / raw)
  To: Mukesh Rathor; +Cc: Xen-devel@lists.xensource.com, Konrad Rzeszutek Wilk

On Thu, 2012-08-16 at 02:05 +0100, Mukesh Rathor wrote:
> ---
>  drivers/xen/balloon.c |   26 +++++++++++++++++++++-----
>  1 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 31ab82f..57960a1 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -358,10 +358,18 @@ 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_pvh_domain()) {

XENFEAT_auto_translated...?

> +			set_phys_to_machine(pfn, frame_list[i]);
> +		} else {
> +			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));
> +		}
>  		/* Link back into the page tables if not highmem. */
> -		if (xen_pv_domain() && !PageHighMem(page)) {
> +		if (xen_pv_domain() && !PageHighMem(page) &&
> +		    !xen_pvh_domain()) {

It feels like this ought to be inside the !xen_pvh_domain above as well
as just hte set_phys_to_machine.

And is the else above not missing a !PageHighMem check?

>  			int ret;
>  			ret = HYPERVISOR_update_va_mapping(
>  				(unsigned long)__va(pfn << PAGE_SHIFT),
> @@ -417,7 +425,14 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  
>  		scrub_page(page);
>  
> -		if (xen_pv_domain() && !PageHighMem(page)) {
> +		if (xen_pvh_domain() && !PageHighMem(page)) {
> +			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 if (xen_pv_domain() && !PageHighMem(page)) {
>  			ret = HYPERVISOR_update_va_mapping(
>  				(unsigned long)__va(pfn << PAGE_SHIFT),
>  				__pte_ma(0), 0);

This pattern:

	if ( xen_pvh_... )
		... lookup_address / set_pte ...
	else
		... HYOERVISOR_update_va_mapping

Was present above too -- candidate for a helper? (I was a bit surprised
there wasn't already one...)

> @@ -433,7 +448,8 @@ 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]);
> -		__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
> +		if (!xen_pvh_domain())

XENFEAT_something?

> +			__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
>  		balloon_append(pfn_to_page(pfn));
>  	}
>  

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

* Re: [RFC PATCH 6/8]: Ballooning changes
  2012-08-16  1:05 [RFC PATCH 6/8]: Ballooning changes Mukesh Rathor
  2012-08-16 14:13 ` Stefano Stabellini
  2012-08-17  9:46 ` Ian Campbell
@ 2012-09-13 16:33 ` Ian Campbell
  2 siblings, 0 replies; 4+ messages in thread
From: Ian Campbell @ 2012-09-13 16:33 UTC (permalink / raw)
  To: Mukesh Rathor; +Cc: Xen-devel@lists.xensource.com, Konrad Rzeszutek Wilk

On Thu, 2012-08-16 at 02:05 +0100, Mukesh Rathor wrote:
> ---
>  drivers/xen/balloon.c |   26 +++++++++++++++++++++-----
>  1 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 31ab82f..57960a1 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -358,10 +358,18 @@ 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_pvh_domain()) {
> +			set_phys_to_machine(pfn, frame_list[i]);
> +		} else {
> +			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));

Another thing I just tripped over on ARM which might be pertinent on x86
PVH too is that lowmem mappings on ARM are super 2M mappings, so trying
to clear the PTE here fails.

I was a bit wary of leaving stage 1 mappings in place for pfn's with no
backing mfn but this 2M mapping issue has caused me to revise that
opinion -- there's no need to worry about the stage 1 mapping still
being present as long as you guarantee you never touch the associated
virtual address, which the balloon driver should be able to do. The
other option is to shatter such mappings which is just too much pain to
contemplate.

Long story short I reckon you can drop this hunk (and associated similar
changes) and rely on the XENFEAT_auto_translated_physmap check inside
set_phys_to_machine to do the right thing.

I guess PVH currently suppresses superpage mappings on x86 (probably
inherited from PV) but undoing that might be something to investigate
for the future? It'll help perf I expect.

Ian.

> +		}
>  		/* Link back into the page tables if not highmem. */
> -		if (xen_pv_domain() && !PageHighMem(page)) {
> +		if (xen_pv_domain() && !PageHighMem(page) &&
> +		    !xen_pvh_domain()) {
>  			int ret;
>  			ret = HYPERVISOR_update_va_mapping(
>  				(unsigned long)__va(pfn << PAGE_SHIFT),
> @@ -417,7 +425,14 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  
>  		scrub_page(page);
>  
> -		if (xen_pv_domain() && !PageHighMem(page)) {
> +		if (xen_pvh_domain() && !PageHighMem(page)) {
> +			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 if (xen_pv_domain() && !PageHighMem(page)) {
>  			ret = HYPERVISOR_update_va_mapping(
>  				(unsigned long)__va(pfn << PAGE_SHIFT),
>  				__pte_ma(0), 0);
> @@ -433,7 +448,8 @@ 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]);
> -		__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
> +		if (!xen_pvh_domain())
> +			__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
>  		balloon_append(pfn_to_page(pfn));
>  	}
>  

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

end of thread, other threads:[~2012-09-13 16:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-16  1:05 [RFC PATCH 6/8]: Ballooning changes Mukesh Rathor
2012-08-16 14:13 ` Stefano Stabellini
2012-08-17  9:46 ` Ian Campbell
2012-09-13 16:33 ` Ian Campbell

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).