[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 7/7] xen/mm: Clean up share_xen_page_with_guest() API

To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxx>
From: Julien Grall <julien.grall@xxxxxxx>
Date: Sun, 11 Mar 2018 20:29:50 +0000
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
Delivery-date: Sun, 11 Mar 2018 20:29:59 +0000
List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Andrew,

On 03/09/2018 01:18 PM, Andrew Cooper wrote:

The share_xen_page_with_guest() functions are used by common code, and are
implemented the same by each arch.  Move the declarations into the common mm.h
rather than duplicating them in each arch/mm.h

Turn an int readonly into a boolean enum, to retain ro/rw context at the
callsites, but use shorter labels which avoids a large number of split lines.

Implement share_xen_page_with_privileged_guests() as a static inline wrapper
around share_xen_page_with_guest() to avoid having a call into a separate
translation unit whose only purpose is to shuffle function arguments.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>


Acked-by: Julien Grall <julien.grall@xxxxxxx>

Cheers,

---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>
CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
  xen/arch/arm/domain.c             |  3 +--
  xen/arch/arm/mm.c                 | 13 ++++---------
  xen/arch/x86/domain.c             |  3 +--
  xen/arch/x86/hvm/dom0_build.c     |  2 +-
  xen/arch/x86/hvm/vmx/vmx.c        |  2 +-
  xen/arch/x86/mm.c                 | 20 +++++++-------------
  xen/arch/x86/pv/shim.c            |  6 ++----
  xen/arch/x86/x86_64/mm.c          | 16 ++++++----------
  xen/common/trace.c                |  9 +++------
  xen/common/xenoprof.c             |  3 +--
  xen/include/asm-arm/grant_table.h |  3 +--
  xen/include/asm-arm/mm.h          |  7 -------
  xen/include/asm-x86/grant_table.h |  6 ++----
  xen/include/asm-x86/mm.h          |  8 --------
  xen/include/xen/mm.h              | 14 ++++++++++++++
  15 files changed, 44 insertions(+), 71 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 4b45fad..23dac5d 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -602,8 +602,7 @@ int arch_domain_create(struct domain *d,
          goto fail;

clear_page(d->shared_info);

-    share_xen_page_with_guest(
-        virt_to_page(d->shared_info), d, XENSHARE_writable);
+    share_xen_page_with_guest(virt_to_page(d->shared_info), d, SHARE_rw);

switch ( config->config.gic_version )

      {
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index a09bea2..baa3b0d 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1187,8 +1187,8 @@ unsigned long domain_get_maximum_gpfn(struct domain *d)
      return gfn_x(d->arch.p2m.max_mapped_gfn);
  }

-void share_xen_page_with_guest(struct page_info *page,

-                          struct domain *d, int readonly)
+void share_xen_page_with_guest(struct page_info *page, struct domain *d,
+                               enum XENSHARE_flags flags)
  {
      if ( page_get_owner(page) == d )
          return;
@@ -1196,7 +1196,8 @@ void share_xen_page_with_guest(struct page_info *page,
      spin_lock(&d->page_alloc_lock);

/* The incremented type count pins as writable or read-only. */

-    page->u.inuse.type_info = (readonly ? PGT_none : PGT_writable_page) | 1;
+    page->u.inuse.type_info =
+        (flags == SHARE_ro ? PGT_none : PGT_writable_page) | 1;

page_set_owner(page, d);

      smp_wmb(); /* install valid domain ptr before updating refcnt. */
@@ -1214,12 +1215,6 @@ void share_xen_page_with_guest(struct page_info *page,
      spin_unlock(&d->page_alloc_lock);
  }

-void share_xen_page_with_privileged_guests(

-    struct page_info *page, int readonly)
-{
-    share_xen_page_with_guest(page, dom_xen, readonly);
-}
-
  int xenmem_add_to_physmap_one(
      struct domain *d,
      unsigned int space,
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 12d0766..8006bed 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -528,8 +528,7 @@ int arch_domain_create(struct domain *d,
          goto fail;

clear_page(d->shared_info);

-    share_xen_page_with_guest(
-        virt_to_page(d->shared_info), d, XENSHARE_writable);
+    share_xen_page_with_guest(virt_to_page(d->shared_info), d, SHARE_rw);

if ( (rc = init_domain_irq_mapping(d)) != 0 )

          goto fail;
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index afebaec..1c70416 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -297,7 +297,7 @@ static void __init pvh_steal_low_ram(struct domain *d, 
unsigned long start,
              continue;
          }

- share_xen_page_with_guest(pg, d, XENSHARE_writable);

+        share_xen_page_with_guest(pg, d, SHARE_rw);
          rc = guest_physmap_add_entry(d, _gfn(mfn), _mfn(mfn), 0, p2m_ram_rw);
          if ( rc )
              printk("Unable to add mfn %#lx to p2m: %d\n", mfn, rc);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 18d8ce2..ebc6934 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2941,7 +2941,7 @@ static int vmx_alloc_vlapic_mapping(struct domain *d)
          return -ENOMEM;
      mfn = page_to_mfn(pg);
      clear_domain_page(_mfn(mfn));
-    share_xen_page_with_guest(pg, d, XENSHARE_writable);
+    share_xen_page_with_guest(pg, d, SHARE_rw);
      d->arch.hvm_domain.vmx.apic_access_mfn = mfn;
      set_mmio_p2m_entry(d, paddr_to_pfn(APIC_DEFAULT_PHYS_BASE), _mfn(mfn),
                         PAGE_ORDER_4K, p2m_get_hostp2m(d)->default_access);
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 1d4e396..17558e0 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -301,8 +301,7 @@ void __init arch_init_memory(void)
            i < (pvh_boot ? (1 + PFN_UP(trampoline_end - trampoline_start))
                          : 0x100);
            i++ )
-        share_xen_page_with_guest(mfn_to_page(_mfn(i)),
-                                  dom_io, XENSHARE_writable);
+        share_xen_page_with_guest(mfn_to_page(_mfn(i)), dom_io, SHARE_rw);

/* Any areas not specified as RAM by the e820 map are considered I/O. */

      for ( i = 0, pfn = 0; pfn < max_page; i++ )
@@ -342,8 +341,8 @@ void __init arch_init_memory(void)
          {
              if ( !mfn_valid(_mfn(pfn)) )
                  continue;
-            share_xen_page_with_guest(
-                mfn_to_page(_mfn(pfn)), dom_io, XENSHARE_writable);
+
+            share_xen_page_with_guest(mfn_to_page(_mfn(pfn)), dom_io, 
SHARE_rw);
          }

/* Skip the RAM region. */

@@ -439,8 +438,8 @@ unsigned long domain_get_maximum_gpfn(struct domain *d)
      return (arch_get_max_pfn(d) ?: 1) - 1;
  }

-void share_xen_page_with_guest(

-    struct page_info *page, struct domain *d, int readonly)
+void share_xen_page_with_guest(struct page_info *page, struct domain *d,
+                               enum XENSHARE_flags flags)
  {
      if ( page_get_owner(page) == d )
          return;
@@ -450,7 +449,8 @@ void share_xen_page_with_guest(
      spin_lock(&d->page_alloc_lock);

/* The incremented type count pins as writable or read-only. */

-    page->u.inuse.type_info  = (readonly ? PGT_none : PGT_writable_page);
+    page->u.inuse.type_info =
+        (flags == SHARE_ro ? PGT_none : PGT_writable_page);
      page->u.inuse.type_info |= PGT_validated | 1;

page_set_owner(page, d);

@@ -485,12 +485,6 @@ int __init unshare_xen_page_with_guest(struct page_info 
*page,
      return 0;
  }

-void share_xen_page_with_privileged_guests(

-    struct page_info *page, int readonly)
-{
-    share_xen_page_with_guest(page, dom_xen, readonly);
-}
-
  void free_shared_domheap_page(struct page_info *page)
  {
      if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index 534965c..dd76264 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -171,8 +171,7 @@ void __init pv_shim_setup_dom(struct domain *d, 
l4_pgentry_t *l4start,
      (si) = param;                                                             
 \
      if ( va )                                                                 
 \
      {                                                                         
 \
-        share_xen_page_with_guest(mfn_to_page(_mfn(param)), d,                 
\
-                                  XENSHARE_writable);                          
\
+        share_xen_page_with_guest(mfn_to_page(_mfn(param)), d, SHARE_rw);      
\
          replace_va_mapping(d, l4start, va, _mfn(param));                      
 \
          dom0_update_physmap(d, PFN_DOWN((va) - va_start), param, vphysmap);   
 \
      }                                                                         
 \
@@ -199,8 +198,7 @@ void __init pv_shim_setup_dom(struct domain *d, 
l4_pgentry_t *l4start,
          clear_page(page);
          console_mfn = virt_to_mfn(page);
          si->console.domU.mfn = mfn_x(console_mfn);
-        share_xen_page_with_guest(mfn_to_page(console_mfn), d,
-                                  XENSHARE_writable);
+        share_xen_page_with_guest(mfn_to_page(console_mfn), d, SHARE_rw);
          replace_va_mapping(d, l4start, console_va, console_mfn);
          dom0_update_physmap(d, (console_va - va_start) >> PAGE_SHIFT,
                              mfn_x(console_mfn), vphysmap);
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 9b37da6..8820f5c 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -185,7 +185,7 @@ static int share_hotadd_m2p_table(struct mem_hotadd_info 
*info)
          {
              struct page_info *page = mfn_to_page(m2p_start_mfn + i);
              if (hotadd_mem_valid(m2p_start_mfn + i, info))
-                share_xen_page_with_privileged_guests(page, XENSHARE_readonly);
+                share_xen_page_with_privileged_guests(page, SHARE_ro);
          }
      }

@@ -206,7 +206,7 @@ static int share_hotadd_m2p_table(struct mem_hotadd_info *info)

          {
              struct page_info *page = mfn_to_page(m2p_start_mfn + i);
              if (hotadd_mem_valid(m2p_start_mfn + i, info))
-                share_xen_page_with_privileged_guests(page, XENSHARE_readonly);
+                share_xen_page_with_privileged_guests(page, SHARE_ro);
          }
      }
      return 0;
@@ -860,10 +860,8 @@ void __init subarch_init_memory(void)
          }

for ( i = 0; i < n; i++ )

-        {
-            struct page_info *page = mfn_to_page(m2p_start_mfn + i);
-            share_xen_page_with_privileged_guests(page, XENSHARE_readonly);
-        }
+            share_xen_page_with_privileged_guests(
+                mfn_to_page(m2p_start_mfn + i), SHARE_ro);
      }

for ( v = RDWR_COMPAT_MPT_VIRT_START;

@@ -880,10 +878,8 @@ void __init subarch_init_memory(void)
          m2p_start_mfn = l2e_get_pfn(l2e);

for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ )

-        {
-            struct page_info *page = mfn_to_page(m2p_start_mfn + i);
-            share_xen_page_with_privileged_guests(page, XENSHARE_readonly);
-        }
+            share_xen_page_with_privileged_guests(
+                mfn_to_page(m2p_start_mfn + i), SHARE_ro);
      }

/* Mark all of direct map NX if hardware supports it. */

diff --git a/xen/common/trace.c b/xen/common/trace.c
index 2e18702..680f6ae 100644
--- a/xen/common/trace.c
+++ b/xen/common/trace.c
@@ -227,7 +227,6 @@ static int alloc_trace_bufs(unsigned int pages)
      for_each_online_cpu(cpu)
      {
          struct t_buf *buf;
-        struct page_info *pg;

spin_lock_init(&per_cpu(t_lock, cpu));@@ -242,16 +241,14 @@ static int alloc_trace_bufs(unsigned int pages)/* Now share the trace pages */

          for ( i = 0; i < pages; i++ )
-        {
-            pg = mfn_to_page(t_info_mfn_list[offset + i]);
-            share_xen_page_with_privileged_guests(pg, XENSHARE_writable);
-        }
+            share_xen_page_with_privileged_guests(
+                mfn_to_page(t_info_mfn_list[offset + i]), SHARE_rw);
      }

/* Finally, share the t_info page */

      for(i = 0; i < t_info_pages; i++)
          share_xen_page_with_privileged_guests(
-            virt_to_page(t_info) + i, XENSHARE_readonly);
+            virt_to_page(t_info) + i, SHARE_ro);

data_size = (pages * PAGE_SIZE - sizeof(struct t_buf));

      t_buf_highwater = data_size >> 1; /* 50% high water */
diff --git a/xen/common/xenoprof.c b/xen/common/xenoprof.c
index 5acdde5..c1b4303 100644
--- a/xen/common/xenoprof.c
+++ b/xen/common/xenoprof.c
@@ -159,8 +159,7 @@ share_xenoprof_page_with_guest(struct domain *d, mfn_t mfn, 
int npages)
      }

for ( i = 0; i < npages; i++ )

-        share_xen_page_with_guest(mfn_to_page(mfn_add(mfn, i)),
-                                  d, XENSHARE_writable);
+        share_xen_page_with_guest(mfn_to_page(mfn_add(mfn, i)), d, SHARE_rw);

return 0;

  }
diff --git a/xen/include/asm-arm/grant_table.h 
b/xen/include/asm-arm/grant_table.h
index 5b8994c..be7ed75 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -81,8 +81,7 @@ static inline unsigned int gnttab_dom0_max(void)
  #define gnttab_create_shared_page(d, t, i)                               \
      do {                                                                 \
          share_xen_page_with_guest(                                       \
-            virt_to_page((char *)(t)->shared_raw[i]),                    \
-            (d), XENSHARE_writable);                                     \
+            virt_to_page((char *)(t)->shared_raw[i]), (d), SHARE_rw);    \
      } while ( 0 )

#define gnttab_shared_gmfn(d, t, i) \

diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 4d5563b..a0e922f 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -159,13 +159,6 @@ extern vaddr_t xenheap_virt_start;

#define maddr_get_owner(ma) (page_get_owner(maddr_to_page((ma))))-#define XENSHARE_writable 0

-#define XENSHARE_readonly 1
-extern void share_xen_page_with_guest(
-    struct page_info *page, struct domain *d, int readonly);
-extern void share_xen_page_with_privileged_guests(
-    struct page_info *page, int readonly);
-
  #define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)
  /* PDX of the first page in the frame table. */
  extern unsigned long frametable_base_pdx;
diff --git a/xen/include/asm-x86/grant_table.h 
b/xen/include/asm-x86/grant_table.h
index 66e9742..8720159 100644
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -57,15 +57,13 @@ static inline unsigned int gnttab_dom0_max(void)
  #define gnttab_create_shared_page(d, t, i)                               \
      do {                                                                 \
          share_xen_page_with_guest(                                       \
-            virt_to_page((char *)(t)->shared_raw[i]),                    \
-            (d), XENSHARE_writable);                                     \
+            virt_to_page((char *)(t)->shared_raw[i]), (d), SHARE_rw);    \
      } while ( 0 )

#define gnttab_create_status_page(d, t, i) \

      do {                                                                 \
          share_xen_page_with_guest(                                       \
-           virt_to_page((char *)(t)->status[i]),                         \
-            (d), XENSHARE_writable);                                     \
+            virt_to_page((char *)(t)->status[i]), (d), SHARE_rw);        \
      } while ( 0 )

diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h

index 96f3a34..c115661 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -292,14 +292,6 @@ struct page_info

#define maddr_get_owner(ma) (page_get_owner(maddr_to_page((ma))))-#define XENSHARE_writable 0

-#define XENSHARE_readonly 1
-extern void share_xen_page_with_guest(
-    struct page_info *page, struct domain *d, int readonly);
-extern int unshare_xen_page_with_guest(struct page_info *page,
-                                       struct domain *d);
-extern void share_xen_page_with_privileged_guests(
-    struct page_info *page, int readonly);
  extern void free_shared_domheap_page(struct page_info *page);

#define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)

diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 0e0e511..142aa73 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -634,4 +634,18 @@ static inline void filtered_flush_tlb_mask(uint32_t 
tlbflush_timestamp)
      }
  }

+enum XENSHARE_flags {

+    SHARE_rw,
+    SHARE_ro,
+};
+void share_xen_page_with_guest(struct page_info *page, struct domain *d,
+                               enum XENSHARE_flags flags);
+int unshare_xen_page_with_guest(struct page_info *page, struct domain *d);
+
+static inline void share_xen_page_with_privileged_guests(
+    struct page_info *page, enum XENSHARE_flags flags)
+{
+    share_xen_page_with_guest(page, dom_xen, flags);
+}
+
  #endif /* __XEN_MM_H__ */


--
Julien Grall

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

References:
- [Xen-devel] [PATCH 0/7] xen: More assorted improvements to domain creation
  - From: Andrew Cooper
- [Xen-devel] [PATCH 7/7] xen/mm: Clean up share_xen_page_with_guest() API
  - From: Andrew Cooper

Prev by Date: Re: [Xen-devel] [PATCH 3/3] docs: Remove redundant qemu-xen-security document
Next by Date: Re: [Xen-devel] preparations for 4.9.2 and 4.7.5
Previous by thread: Re: [Xen-devel] [PATCH 7/7] xen/mm: Clean up share_xen_page_with_guest() API
Next by thread: Re: [Xen-devel] [PATCH 7/7] xen/mm: Clean up share_xen_page_with_guest() API
Index(es):
- Date
- Thread

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.