Re: [Xen-devel] [PATCH 08/14] xen/memory: Drop ARM put_gfn() stub

2018-11-22 Thread Jan Beulich 
>>> On 21.11.18 at 14:21,  wrote:
> On x86, get_gfn_*() and put_gfn() are reference counting pairs.  All the
> get_gfn_*() functions are called from within CONFIG_X86 sections, but
> put_gfn() is stubbed out on ARM.
> 
> As a result, the common code reads as if ARM is dropping references it never
> acquired.
> 
> Put all put_gfn() calls in common code inside CONFIG_X86 to make the code
> properly balanced, and drop the ARM stub.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 08/14] xen/memory: Drop ARM put_gfn() stub

2018-11-22 Thread Julien Grall 

Hi Andrew,

On 11/21/18 1:21 PM, Andrew Cooper wrote:

On x86, get_gfn_*() and put_gfn() are reference counting pairs.  All the
get_gfn_*() functions are called from within CONFIG_X86 sections, but
put_gfn() is stubbed out on ARM.

As a result, the common code reads as if ARM is dropping references it never
acquired.

Put all put_gfn() calls in common code inside CONFIG_X86 to make the code
properly balanced, and drop the ARM stub.

Signed-off-by: Andrew Cooper 


Acked-by: Julien Grall 

Cheers,


---
CC: Stefano Stabellini 
CC: Julien Grall 
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 
---
  xen/common/grant_table.c |  8 
  xen/common/memory.c  | 15 ++-
  xen/include/asm-arm/mm.h |  2 --
  3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index fc41b65..f7860f6 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2110,7 +2110,9 @@ gnttab_transfer(
  /* Check the passed page frame for basic validity. */
  if ( unlikely(!mfn_valid(mfn)) )
  {
+#ifdef CONFIG_X86
  put_gfn(d, gop.mfn);
+#endif
  gdprintk(XENLOG_INFO, "out-of-range %lx\n", (unsigned 
long)gop.mfn);
  gop.status = GNTST_bad_page;
  goto copyback;
@@ -2119,7 +2121,9 @@ gnttab_transfer(
  page = mfn_to_page(mfn);
  if ( (rc = steal_page(d, page, 0)) < 0 )
  {
+#ifdef CONFIG_X86
  put_gfn(d, gop.mfn);
+#endif
  gop.status = rc == -EINVAL ? GNTST_bad_page : GNTST_general_error;
  goto copyback;
  }
@@ -2149,7 +2153,9 @@ gnttab_transfer(
  unlock_and_copyback:
  rcu_unlock_domain(e);
  put_gfn_and_copyback:
+#ifdef CONFIG_X86
  put_gfn(d, gop.mfn);
+#endif
  page->count_info &= ~(PGC_count_mask|PGC_allocated);
  free_domheap_page(page);
  goto copyback;
@@ -2236,7 +2242,9 @@ gnttab_transfer(
  page_set_owner(page, e);
  
  spin_unlock(&e->page_alloc_lock);

+#ifdef CONFIG_X86
  put_gfn(d, gop.mfn);
+#endif
  
  TRACE_1D(TRC_MEM_PAGE_GRANT_TRANSFER, e->domain_id);
  
diff --git a/xen/common/memory.c b/xen/common/memory.c

index 58194b9..175bd62 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -358,7 +358,9 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
  #endif
  if ( unlikely(!mfn_valid(mfn)) )
  {
+#ifdef CONFIG_X86
  put_gfn(d, gmfn);
+#endif
  gdprintk(XENLOG_INFO, "Domain %u page number %lx invalid\n",
  d->domain_id, gmfn);
  
@@ -388,7 +390,9 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)

  page = mfn_to_page(mfn);
  if ( unlikely(!get_page(page, d)) )
  {
+#ifdef CONFIG_X86
  put_gfn(d, gmfn);
+#endif
  gdprintk(XENLOG_INFO, "Bad page free for domain %u\n", d->domain_id);
  
  return -ENXIO;

@@ -409,8 +413,11 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
  put_page(page);
  
  put_page(page);

- out_put_gfn: __maybe_unused;
+
+#ifdef CONFIG_X86
+ out_put_gfn:
  put_gfn(d, gmfn);
+#endif
  
  /*

   * Filter out -ENOENT return values that aren't a result of an empty p2m
@@ -656,7 +663,9 @@ static long 
memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
  #endif
  if ( unlikely(!mfn_valid(mfn)) )
  {
+#ifdef CONFIG_X86
  put_gfn(d, gmfn + k);
+#endif
  rc = -EINVAL;
  goto fail;
  }
@@ -666,12 +675,16 @@ static long 
memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
  rc = steal_page(d, page, MEMF_no_refcount);
  if ( unlikely(rc) )
  {
+#ifdef CONFIG_X86
  put_gfn(d, gmfn + k);
+#endif
  goto fail;
  }
  
  page_list_add(page, &in_chunk_list);

+#ifdef CONFIG_X86
  put_gfn(d, gmfn + k);
+#endif
  }
  }
  
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h

index 940b74b..b2f6104 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -313,8 +313,6 @@ static inline void *page_to_virt(const struct page_info *pg)
  struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
  unsigned long flags);
  
-static inline void put_gfn(struct domain *d, unsigned long gfn) {}

-
  /*
   * Arm does not have an M2P, but common code expects a handful of
   * M2P-related defines and functions. Provide dummy versions of these.



--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 08/14] xen/memory: Drop ARM put_gfn() stub

2018-11-21 Thread Andrew Cooper 
On x86, get_gfn_*() and put_gfn() are reference counting pairs.  All the
get_gfn_*() functions are called from within CONFIG_X86 sections, but
put_gfn() is stubbed out on ARM.

As a result, the common code reads as if ARM is dropping references it never
acquired.

Put all put_gfn() calls in common code inside CONFIG_X86 to make the code
properly balanced, and drop the ARM stub.

Signed-off-by: Andrew Cooper 
---
CC: Stefano Stabellini 
CC: Julien Grall 
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 
---
 xen/common/grant_table.c |  8 
 xen/common/memory.c  | 15 ++-
 xen/include/asm-arm/mm.h |  2 --
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index fc41b65..f7860f6 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2110,7 +2110,9 @@ gnttab_transfer(
 /* Check the passed page frame for basic validity. */
 if ( unlikely(!mfn_valid(mfn)) )
 {
+#ifdef CONFIG_X86
 put_gfn(d, gop.mfn);
+#endif
 gdprintk(XENLOG_INFO, "out-of-range %lx\n", (unsigned 
long)gop.mfn);
 gop.status = GNTST_bad_page;
 goto copyback;
@@ -2119,7 +2121,9 @@ gnttab_transfer(
 page = mfn_to_page(mfn);
 if ( (rc = steal_page(d, page, 0)) < 0 )
 {
+#ifdef CONFIG_X86
 put_gfn(d, gop.mfn);
+#endif
 gop.status = rc == -EINVAL ? GNTST_bad_page : GNTST_general_error;
 goto copyback;
 }
@@ -2149,7 +2153,9 @@ gnttab_transfer(
 unlock_and_copyback:
 rcu_unlock_domain(e);
 put_gfn_and_copyback:
+#ifdef CONFIG_X86
 put_gfn(d, gop.mfn);
+#endif
 page->count_info &= ~(PGC_count_mask|PGC_allocated);
 free_domheap_page(page);
 goto copyback;
@@ -2236,7 +2242,9 @@ gnttab_transfer(
 page_set_owner(page, e);
 
 spin_unlock(&e->page_alloc_lock);
+#ifdef CONFIG_X86
 put_gfn(d, gop.mfn);
+#endif
 
 TRACE_1D(TRC_MEM_PAGE_GRANT_TRANSFER, e->domain_id);
 
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 58194b9..175bd62 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -358,7 +358,9 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
 #endif
 if ( unlikely(!mfn_valid(mfn)) )
 {
+#ifdef CONFIG_X86
 put_gfn(d, gmfn);
+#endif
 gdprintk(XENLOG_INFO, "Domain %u page number %lx invalid\n",
 d->domain_id, gmfn);
 
@@ -388,7 +390,9 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
 page = mfn_to_page(mfn);
 if ( unlikely(!get_page(page, d)) )
 {
+#ifdef CONFIG_X86
 put_gfn(d, gmfn);
+#endif
 gdprintk(XENLOG_INFO, "Bad page free for domain %u\n", d->domain_id);
 
 return -ENXIO;
@@ -409,8 +413,11 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
 put_page(page);
 
 put_page(page);
- out_put_gfn: __maybe_unused;
+
+#ifdef CONFIG_X86
+ out_put_gfn:
 put_gfn(d, gmfn);
+#endif
 
 /*
  * Filter out -ENOENT return values that aren't a result of an empty p2m
@@ -656,7 +663,9 @@ static long 
memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
 #endif
 if ( unlikely(!mfn_valid(mfn)) )
 {
+#ifdef CONFIG_X86
 put_gfn(d, gmfn + k);
+#endif
 rc = -EINVAL;
 goto fail;
 }
@@ -666,12 +675,16 @@ static long 
memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
 rc = steal_page(d, page, MEMF_no_refcount);
 if ( unlikely(rc) )
 {
+#ifdef CONFIG_X86
 put_gfn(d, gmfn + k);
+#endif
 goto fail;
 }
 
 page_list_add(page, &in_chunk_list);
+#ifdef CONFIG_X86
 put_gfn(d, gmfn + k);
+#endif
 }
 }
 
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 940b74b..b2f6104 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -313,8 +313,6 @@ static inline void *page_to_virt(const struct page_info *pg)
 struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
 unsigned long flags);
 
-static inline void put_gfn(struct domain *d, unsigned long gfn) {}
-
 /*
  * Arm does not have an M2P, but common code expects a handful of
  * M2P-related defines and functions. Provide dummy versions of these.
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel