Xen devel - Oct 2008 - [PATCH, RFC] i386: highmem access assistance hypercalls

While looking at the origin of very frequently executed hypercalls I
realized that the high page accessor functions in Linux would be good
candidates to handle in the hypervisor - clearing or copying to/from
a high page is a pretty frequent operation (provided there''s enough
memory in the domain). While prior to the first submission I only
measured kernel builds (where the results are not hinting at a
meaningful improvement), I now found time to do a more specific
analysis: page clearing is being improved by about 20%, page copying
doesn''t seem to significantly benefit (though that may be an effect of
the simplistic copy_page() implementation Xen currently uses) -
nevertheless I would think that if one function is supported by the
hypervisor, then the other should also be.

The hypervisor patch is below (using temporary numbers for the newly
added sub-hypercalls), the Linux patch didn''t change over the first
submission (if the patch here is acceptable, I''ll create a version
that applies to the 2.6.18 tree).

Opinions?

Signed-off-by: Jan Beulich <jbeulich@novell.com>

Index: 2008-09-19/xen/arch/x86/mm.c
==================================================================---
2008-09-19.orig/xen/arch/x86/mm.c	2008-09-17 09:26:41.000000000 +0200
+++ 2008-09-19/xen/arch/x86/mm.c	2008-09-19 14:00:01.000000000 +0200
@@ -2432,6 +2432,29 @@ static inline cpumask_t vcpumask_to_pcpu
     return pmask;
 }
 
+#ifdef __i386__
+static inline void *fixmap_domain_page(unsigned long mfn)
+{
+    unsigned int cpu = smp_processor_id();
+    void *ptr = (void *)fix_to_virt(FIX_PAE_HIGHMEM_0 + cpu);
+
+    l1e_write(fix_pae_highmem_pl1e - cpu,
+              l1e_from_pfn(mfn, __PAGE_HYPERVISOR));
+    flush_tlb_one_local(ptr);
+    return ptr;
+}
+static inline void fixunmap_domain_page(const void *ptr)
+{
+    unsigned int cpu = virt_to_fix((unsigned long)ptr) - FIX_PAE_HIGHMEM_0;
+
+    l1e_write(fix_pae_highmem_pl1e - cpu, l1e_empty());
+    this_cpu(make_cr3_timestamp) = this_cpu(tlbflush_time);
+}
+#else
+#define fixmap_domain_page(mfn) mfn_to_virt(mfn)
+#define fixunmap_domain_page(ptr) ((void)(ptr))
+#endif
+
 int do_mmuext_op(
     XEN_GUEST_HANDLE(mmuext_op_t) uops,
     unsigned int count,
@@ -2701,6 +2724,66 @@ int do_mmuext_op(
             break;
         }
 
+        case MMUEXT_CLEAR_PAGE:
+        {
+            unsigned char *ptr;
+
+            okay = !get_page_and_type_from_pagenr(mfn, PGT_writable_page,
+                                                  FOREIGNDOM, 0);
+            if ( unlikely(!okay) )
+            {
+                MEM_LOG("Error while clearing mfn %lx", mfn);
+                break;
+            }
+
+            /* A page is dirtied when it''s being cleared. */
+            paging_mark_dirty(d, mfn);
+
+            ptr = fixmap_domain_page(mfn);
+            clear_page(ptr);
+            fixunmap_domain_page(ptr);
+
+            put_page_and_type(page);
+            break;
+        }
+
+        case MMUEXT_COPY_PAGE:
+        {
+            const unsigned char *src;
+            unsigned char *dst;
+            unsigned long src_mfn;
+
+            src_mfn = gmfn_to_mfn(FOREIGNDOM, op.arg2.src_mfn);
+            okay = get_page_from_pagenr(src_mfn, FOREIGNDOM);
+            if ( unlikely(!okay) )
+            {
+                MEM_LOG("Error while copying from mfn %lx", src_mfn);
+                break;
+            }
+
+            okay = !get_page_and_type_from_pagenr(mfn, PGT_writable_page,
+                                                  FOREIGNDOM, 0);
+            if ( unlikely(!okay) )
+            {
+                put_page(mfn_to_page(src_mfn));
+                MEM_LOG("Error while copying to mfn %lx", mfn);
+                break;
+            }
+
+            /* A page is dirtied when it''s being copied to. */
+            paging_mark_dirty(d, mfn);
+
+            src = map_domain_page(src_mfn);
+            dst = fixmap_domain_page(mfn);
+            copy_page(dst, src);
+            fixunmap_domain_page(dst);
+            unmap_domain_page(src);
+
+            put_page_and_type(page);
+            put_page(mfn_to_page(src_mfn));
+            break;
+        }
+
         default:
             MEM_LOG("Invalid extended pt command 0x%x", op.cmd);
             rc = -ENOSYS;
Index: 2008-09-19/xen/arch/x86/x86_32/domain_page.c
==================================================================---
2008-09-19.orig/xen/arch/x86/x86_32/domain_page.c	2008-09-19 13:59:19.000000000
+0200
+++ 2008-09-19/xen/arch/x86/x86_32/domain_page.c	2008-09-19 14:00:01.000000000
+0200
@@ -114,7 +114,7 @@ void *map_domain_page(unsigned long mfn)
     return (void *)va;
 }
 
-void unmap_domain_page(void *va)
+void unmap_domain_page(const void *va)
 {
     unsigned int idx;
     struct vcpu *v;
@@ -241,7 +241,7 @@ void *map_domain_page_global(unsigned lo
     return (void *)va;
 }
 
-void unmap_domain_page_global(void *va)
+void unmap_domain_page_global(const void *va)
 {
     unsigned long __va = (unsigned long)va;
     l2_pgentry_t *pl2e;
Index: 2008-09-19/xen/arch/x86/x86_64/compat/mm.c
==================================================================---
2008-09-19.orig/xen/arch/x86/x86_64/compat/mm.c	2008-09-15 11:25:43.000000000
+0200
+++ 2008-09-19/xen/arch/x86/x86_64/compat/mm.c	2008-09-19 14:00:01.000000000
+0200
@@ -231,6 +231,8 @@ int compat_mmuext_op(XEN_GUEST_HANDLE(mm
             case MMUEXT_PIN_L4_TABLE:
             case MMUEXT_UNPIN_TABLE:
             case MMUEXT_NEW_BASEPTR:
+            case MMUEXT_CLEAR_PAGE:
+            case MMUEXT_COPY_PAGE:
                 arg1 = XLAT_mmuext_op_arg1_mfn;
                 break;
             default:
@@ -258,6 +260,9 @@ int compat_mmuext_op(XEN_GUEST_HANDLE(mm
             case MMUEXT_INVLPG_MULTI:
                 arg2 = XLAT_mmuext_op_arg2_vcpumask;
                 break;
+            case MMUEXT_COPY_PAGE:
+                arg2 = XLAT_mmuext_op_arg2_src_mfn;
+                break;
             default:
                 arg2 = -1;
                 break;
Index: 2008-09-19/xen/include/asm-x86/fixmap.h
==================================================================---
2008-09-19.orig/xen/include/asm-x86/fixmap.h	2008-09-15 11:25:43.000000000 +0200
+++ 2008-09-19/xen/include/asm-x86/fixmap.h	2008-09-19 14:00:01.000000000 +0200
@@ -29,6 +29,7 @@
  * from the end of virtual memory backwards.
  */
 enum fixed_addresses {
+    FIX_HOLE,
 #ifdef __i386__
     FIX_PAE_HIGHMEM_0,
     FIX_PAE_HIGHMEM_END = FIX_PAE_HIGHMEM_0 + NR_CPUS-1,
Index: 2008-09-19/xen/include/public/xen.h
==================================================================---
2008-09-19.orig/xen/include/public/xen.h	2008-09-15 11:25:43.000000000 +0200
+++ 2008-09-19/xen/include/public/xen.h	2008-09-19 14:00:01.000000000 +0200
@@ -231,6 +231,13 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t);
  * cmd: MMUEXT_SET_LDT
  * linear_addr: Linear address of LDT base (NB. must be page-aligned).
  * nr_ents: Number of entries in LDT.
+ *
+ * cmd: MMUEXT_CLEAR_PAGE
+ * mfn: Machine frame number to be cleared.
+ *
+ * cmd: MMUEXT_COPY_PAGE
+ * mfn: Machine frame number of the destination page.
+ * src_mfn: Machine frame number of the source page.
  */
 #define MMUEXT_PIN_L1_TABLE      0
 #define MMUEXT_PIN_L2_TABLE      1
@@ -247,12 +254,15 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t);
 #define MMUEXT_FLUSH_CACHE      12
 #define MMUEXT_SET_LDT          13
 #define MMUEXT_NEW_USER_BASEPTR 15
+#define MMUEXT_CLEAR_PAGE   0x1000
+#define MMUEXT_COPY_PAGE    0x1001
 
 #ifndef __ASSEMBLY__
 struct mmuext_op {
     unsigned int cmd;
     union {
-        /* [UN]PIN_TABLE, NEW_BASEPTR, NEW_USER_BASEPTR */
+        /* [UN]PIN_TABLE, NEW_BASEPTR, NEW_USER_BASEPTR
+         * CLEAR_PAGE, COPY_PAGE */
         xen_pfn_t     mfn;
         /* INVLPG_LOCAL, INVLPG_ALL, SET_LDT */
         unsigned long linear_addr;
@@ -266,6 +276,8 @@ struct mmuext_op {
 #else
         void *vcpumask;
 #endif
+        /* COPY_PAGE */
+        xen_pfn_t src_mfn;
     } arg2;
 };
 typedef struct mmuext_op mmuext_op_t;
Index: 2008-09-19/xen/include/xen/domain_page.h
==================================================================---
2008-09-19.orig/xen/include/xen/domain_page.h	2008-09-15 11:25:43.000000000
+0200
+++ 2008-09-19/xen/include/xen/domain_page.h	2008-09-19 14:00:01.000000000 +0200
@@ -24,7 +24,7 @@ void *map_domain_page(unsigned long mfn)
  * Pass a VA within a page previously mapped in the context of the
  * currently-executing VCPU via a call to map_domain_page().
  */
-void unmap_domain_page(void *va);
+void unmap_domain_page(const void *va);
 
 /*
  * Similar to the above calls, except the mapping is accessible in all
@@ -32,7 +32,7 @@ void unmap_domain_page(void *va);
  * mappings can also be unmapped from any context.
  */
 void *map_domain_page_global(unsigned long mfn);
-void unmap_domain_page_global(void *va);
+void unmap_domain_page_global(const void *va);
 
 #define DMCACHE_ENTRY_VALID 1U
 #define DMCACHE_ENTRY_HELD  2U
@@ -75,7 +75,7 @@ map_domain_page_with_cache(unsigned long
 }
 
 static inline void
-unmap_domain_page_with_cache(void *va, struct domain_mmap_cache *cache)
+unmap_domain_page_with_cache(const void *va, struct domain_mmap_cache *cache)
 {
     ASSERT(cache != NULL);
     cache->flags &= ~DMCACHE_ENTRY_HELD;



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> "Jan Beulich" <jbeulich@novell.com> 17.10.08 17:28
>>>
>While looking at the origin of very frequently executed hypercalls I
>realized that the high page accessor functions in Linux would be good
>candidates to handle in the hypervisor - clearing or copying to/from
>a high page is a pretty frequent operation (provided there''s enough
>memory in the domain). While prior to the first submission I only
>measured kernel builds (where the results are not hinting at a
>meaningful improvement), I now found time to do a more specific
>analysis: page clearing is being improved by about 20%, page copying
>doesn''t seem to significantly benefit (though that may be an effect
of
>the simplistic copy_page() implementation Xen currently uses) -
>nevertheless I would think that if one function is supported by the
>hypervisor, then the other should also be.
Actually, the workload I used for measuring the effect on page copying
was not meaningful. On an (artificial) workload that really exercises
page copying in a meaningful way, the improvement is about 15%.

Also, the improvements on a 64-bit hypervisor are, as expected, even a
little better (close to 25% for page clearing and close to 20% for page
copying - with the original numbers already having been slightly better
than on a 32-bit hypervisor).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 17/10/08 16:28, "Jan Beulich" <jbeulich@novell.com> wrote:
> The hypervisor patch is below (using temporary numbers for the newly
> added sub-hypercalls), the Linux patch didn''t change over the
first
> submission (if the patch here is acceptable, I''ll create a version
> that applies to the 2.6.18 tree).
> 
> Opinions?
I''m fine in principle. A cursory glance at the substance of the patch
indicates that there are some secondary portions which could be discarded or
broken out into separate patches: the constifying of unmap_domain_page*()
and the addition of FIX_HOLE to fixmap.h. Please divide up as appropriate
and re-send.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> Keir Fraser <keir.fraser@eu.citrix.com> 20.10.08 16:27
>>>
>On 17/10/08 16:28, "Jan Beulich" <jbeulich@novell.com>
wrote:
>
>> The hypervisor patch is below (using temporary numbers for the newly
>> added sub-hypercalls), the Linux patch didn''t change over the
first
>> submission (if the patch here is acceptable, I''ll create a
version
>> that applies to the 2.6.18 tree).
>> 
>> Opinions?
>
>I''m fine in principle. A cursory glance at the substance of the
patch
>indicates that there are some secondary portions which could be discarded or
>broken out into separate patches: the constifying of unmap_domain_page*()
>and the addition of FIX_HOLE to fixmap.h. Please divide up as appropriate
>and re-send.
Will do. One question I forgot to ask with the original mail: Should this
functionality be announced via a new feature bit?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 21/10/08 07:38, "Jan Beulich" <jbeulich@novell.com> wrote:
>> I''m fine in principle. A cursory glance at the substance of
the patch
>> indicates that there are some secondary portions which could be
discarded or
>> broken out into separate patches: the constifying of
unmap_domain_page*()
>> and the addition of FIX_HOLE to fixmap.h. Please divide up as
appropriate
>> and re-send.
> 
> Will do. One question I forgot to ask with the original mail: Should this
> functionality be announced via a new feature bit?
I guess that would be convenient.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jan Beulich wrote:
> While looking at the origin of very frequently executed hypercalls I
> realized that the high page accessor functions in Linux would be good
> candidates to handle in the hypervisor - clearing or copying to/from
> a high page is a pretty frequent operation (provided there''s
enough
> memory in the domain). While prior to the first submission I only
> measured kernel builds (where the results are not hinting at a
> meaningful improvement), I now found time to do a more specific
> analysis: page clearing is being improved by about 20%, page copying
> doesn''t seem to significantly benefit (though that may be an
effect of
> the simplistic copy_page() implementation Xen currently uses) -
> nevertheless I would think that if one function is supported by the
> hypervisor, then the other should also be.
>
> The hypervisor patch is below (using temporary numbers for the newly
> added sub-hypercalls), the Linux patch didn''t change over the
first
> submission (if the patch here is acceptable, I''ll create a version
> that applies to the 2.6.18 tree).
>
> Opinions?
>   
Do you have any thoughts about what it would take to make this be useful 
in a pvops kernel?  If the Novell kernel is the only user, it seems like 
a bit of a dead end.

    J


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> Jeremy Fitzhardinge <jeremy@goop.org> 25.10.08 02:01
>>>
>Jan Beulich wrote:
>> While looking at the origin of very frequently executed hypercalls I
>> realized that the high page accessor functions in Linux would be good
>> candidates to handle in the hypervisor - clearing or copying to/from
>> a high page is a pretty frequent operation (provided there''s
enough
>> memory in the domain). While prior to the first submission I only
>> measured kernel builds (where the results are not hinting at a
>> meaningful improvement), I now found time to do a more specific
>> analysis: page clearing is being improved by about 20%, page copying
>> doesn''t seem to significantly benefit (though that may be an
effect of
>> the simplistic copy_page() implementation Xen currently uses) -
>> nevertheless I would think that if one function is supported by the
>> hypervisor, then the other should also be.
>>
>> The hypervisor patch is below (using temporary numbers for the newly
>> added sub-hypercalls), the Linux patch didn''t change over the
first
>> submission (if the patch here is acceptable, I''ll create a
version
>> that applies to the 2.6.18 tree).
>>
>> Opinions?
>>   
>
>Do you have any thoughts about what it would take to make this be useful 
>in a pvops kernel?  If the Novell kernel is the only user, it seems like 
>a bit of a dead end.
I don''t see the difficulty here: Just add two new operations to pv ops.
If it won''t get done before we switch to the pv ops kernel (which
implies
full functionality in Dom0 and DomU), I''d certainly merge up this and
any
other changes we carry along.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel