[PATCH for-4.5 v9 04/19] xen: Relocate p2m_mem_access_resume to mem_access common

[PATCH for-4.5 v9 04/19] xen: Relocate p2m_mem_access_resume to mem_access common

[Tamas K Lengyel]

Relocate p2m_mem_access_resume to common and abstract the new
p2m_mem_event_emulate_check into the p2m layer to.

Signed-off-by: Tamas K Lengyel <tklengyel@sec.in.tum.de>
---
v9: Pass the vcpu instead of the domain to emulate_check.
v8: Abstract p2m_mem_event_emulate_check.
v6: Keep the comment describing the function.
v5: Style fix.
---
 xen/arch/x86/mm/p2m.c        | 128 ++++++++++++++++++-------------------------
 xen/common/mem_access.c      |  28 +++++++++-
 xen/common/mem_event.c       |   2 +-
 xen/include/asm-arm/p2m.h    |   7 +++
 xen/include/asm-x86/p2m.h    |   7 ++-
 xen/include/xen/mem_access.h |   5 ++
 6 files changed, 99 insertions(+), 78 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 31d0d9e..08adaac 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1382,6 +1382,60 @@ static void p2m_mem_event_fill_regs(mem_event_request_t *req)
     req->x86_regs.cs_arbytes = seg.attr.bytes;
 }
 
+void p2m_mem_event_emulate_check(struct vcpu *v, const mem_event_response_t *rsp)
+{
+    /* Mark vcpu for skipping one instruction upon rescheduling. */
+    if ( rsp->flags & MEM_EVENT_FLAG_EMULATE )
+    {
+        struct domain *d = v->domain;
+        xenmem_access_t access;
+        bool_t violation = 1;
+
+        if ( p2m_get_mem_access(d, rsp->gfn, &access) == 0 )
+        {
+            switch ( access )
+            {
+            case XENMEM_access_n:
+            case XENMEM_access_n2rwx:
+            default:
+                violation = rsp->access_r || rsp->access_w || rsp->access_x;
+                break;
+
+            case XENMEM_access_r:
+                violation = rsp->access_w || rsp->access_x;
+                break;
+
+            case XENMEM_access_w:
+                violation = rsp->access_r || rsp->access_x;
+                break;
+
+            case XENMEM_access_x:
+                violation = rsp->access_r || rsp->access_w;
+                break;
+
+            case XENMEM_access_rx:
+            case XENMEM_access_rx2rw:
+                violation = rsp->access_w;
+                break;
+
+            case XENMEM_access_wx:
+                violation = rsp->access_r;
+                break;
+
+            case XENMEM_access_rw:
+                violation = rsp->access_x;
+                break;
+
+            case XENMEM_access_rwx:
+                violation = 0;
+                break;
+            }
+        }
+
+        v->arch.mem_event.emulate_flags = violation ? rsp->flags : 0;
+    }
+}
+
 bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
                             struct npfec npfec,
                             mem_event_request_t **req_ptr)
@@ -1509,80 +1563,6 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
     return (p2ma == p2m_access_n2rwx);
 }
 
-void p2m_mem_access_resume(struct domain *d)
-{
-    mem_event_response_t rsp;
-
-    /* Pull all responses off the ring */
-    while( mem_event_get_response(d, &d->mem_event->access, &rsp) )
-    {
-        struct vcpu *v;
-
-        if ( rsp.flags & MEM_EVENT_FLAG_DUMMY )
-            continue;
-
-        /* Validate the vcpu_id in the response. */
-        if ( (rsp.vcpu_id >= d->max_vcpus) || !d->vcpu[rsp.vcpu_id] )
-            continue;
-
-        v = d->vcpu[rsp.vcpu_id];
-
-        /* Mark vcpu for skipping one instruction upon rescheduling. */
-        if ( rsp.flags & MEM_EVENT_FLAG_EMULATE )
-        {
-            xenmem_access_t access;
-            bool_t violation = 1;
-
-            if ( p2m_get_mem_access(d, rsp.gfn, &access) == 0 )
-            {
-                switch ( access )
-                {
-                case XENMEM_access_n:
-                case XENMEM_access_n2rwx:
-                default:
-                    violation = rsp.access_r || rsp.access_w || rsp.access_x;
-                    break;
-
-                case XENMEM_access_r:
-                    violation = rsp.access_w || rsp.access_x;
-                    break;
-
-                case XENMEM_access_w:
-                    violation = rsp.access_r || rsp.access_x;
-                    break;
-
-                case XENMEM_access_x:
-                    violation = rsp.access_r || rsp.access_w;
-                    break;
-
-                case XENMEM_access_rx:
-                case XENMEM_access_rx2rw:
-                    violation = rsp.access_w;
-                    break;
-
-                case XENMEM_access_wx:
-                    violation = rsp.access_r;
-                    break;
-
-                case XENMEM_access_rw:
-                    violation = rsp.access_x;
-                    break;
-
-                case XENMEM_access_rwx:
-                    violation = 0;
-                    break;
-                }
-            }
-
-            v->arch.mem_event.emulate_flags = violation ? rsp.flags : 0;
-        }
-
-        /* Unpause domain */
-        if ( rsp.flags & MEM_EVENT_FLAG_VCPU_PAUSED )
-            mem_event_vcpu_unpause(v);
-    }
-}
-
 /* Set access type for a region of pfns.
  * If start_pfn == -1ul, sets the default access type */
 long p2m_set_mem_access(struct domain *d, unsigned long pfn, uint32_t nr,
diff --git a/xen/common/mem_access.c b/xen/common/mem_access.c
index 9a8c1a9..64e5301 100644
--- a/xen/common/mem_access.c
+++ b/xen/common/mem_access.c
@@ -29,6 +29,32 @@
 #include <asm/p2m.h>
 #include <xsm/xsm.h>
 
+void mem_access_resume(struct domain *d)
+{
+    mem_event_response_t rsp;
+
+    /* Pull all responses off the ring. */
+    while ( mem_event_get_response(d, &d->mem_event->access, &rsp) )
+    {
+        struct vcpu *v;
+
+        if ( rsp.flags & MEM_EVENT_FLAG_DUMMY )
+            continue;
+
+        /* Validate the vcpu_id in the response. */
+        if ( (rsp.vcpu_id >= d->max_vcpus) || !d->vcpu[rsp.vcpu_id] )
+            continue;
+
+        v = d->vcpu[rsp.vcpu_id];
+
+        p2m_mem_event_emulate_check(v, &rsp);
+
+        /* Unpause domain. */
+        if ( rsp.flags & MEM_EVENT_FLAG_VCPU_PAUSED )
+            mem_event_vcpu_unpause(v);
+    }
+}
+
 int mem_access_memop(unsigned long cmd,
                      XEN_GUEST_HANDLE_PARAM(xen_mem_access_op_t) arg)
 {
@@ -58,7 +84,7 @@ int mem_access_memop(unsigned long cmd,
     switch ( mao.op )
     {
     case XENMEM_access_op_resume:
-        p2m_mem_access_resume(d);
+        mem_access_resume(d);
         rc = 0;
         break;
 
diff --git a/xen/common/mem_event.c b/xen/common/mem_event.c
index 7cc99b3..9f1a1b0 100644
--- a/xen/common/mem_event.c
+++ b/xen/common/mem_event.c
@@ -439,7 +439,7 @@ static void mem_paging_notification(struct vcpu *v, unsigned int port)
 static void mem_access_notification(struct vcpu *v, unsigned int port)
 {
     if ( likely(v->domain->mem_event->access.ring_page != NULL) )
-        p2m_mem_access_resume(v->domain);
+        mem_access_resume(v->domain);
 }
 
 #ifdef HAS_MEM_SHARING
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index faf14d3..38e32e6 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -66,6 +66,13 @@ typedef enum {
     p2m_max_real_type,  /* Types after this won't be store in the p2m */
 } p2m_type_t;
 
+static inline
+void p2m_mem_event_emulate_check(struct vcpu *v,
+                                 const mem_event_response_t *rsp)
+{
+    /* Not supported on ARM. */
+};
+
 #define p2m_is_foreign(_t)  ((_t) == p2m_map_foreign)
 #define p2m_is_ram(_t)      ((_t) == p2m_ram_rw || (_t) == p2m_ram_ro)
 
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index a2a6289..1de493e 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -573,8 +573,6 @@ void p2m_mem_paging_resume(struct domain *d);
 bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
                             struct npfec npfec,
                             mem_event_request_t **req_ptr);
-/* Resumes the running of the VCPU, restarting the last instruction */
-void p2m_mem_access_resume(struct domain *d);
 
 /* Set access type for a region of pfns.
  * If start_pfn == -1ul, sets the default access type */
@@ -586,6 +584,11 @@ long p2m_set_mem_access(struct domain *d, unsigned long start_pfn, uint32_t nr,
 int p2m_get_mem_access(struct domain *d, unsigned long pfn,
                        xenmem_access_t *access);
 
+/* Check for emulation and mark vcpu for skipping one instruction
+ * upon rescheduling if required. */
+void p2m_mem_event_emulate_check(struct vcpu *v,
+                                 const mem_event_response_t *rsp);
+
 /* 
  * Internal functions, only called by other p2m code
  */
diff --git a/xen/include/xen/mem_access.h b/xen/include/xen/mem_access.h
index 19d1a2d..6ceb2a4 100644
--- a/xen/include/xen/mem_access.h
+++ b/xen/include/xen/mem_access.h
@@ -31,6 +31,9 @@ int mem_access_memop(unsigned long cmd,
                      XEN_GUEST_HANDLE_PARAM(xen_mem_access_op_t) arg);
 int mem_access_send_req(struct domain *d, mem_event_request_t *req);
 
+/* Resumes the running of the VCPU, restarting the last instruction */
+void mem_access_resume(struct domain *d);
+
 #else
 
 static inline
@@ -46,6 +49,8 @@ int mem_access_send_req(struct domain *d, mem_event_request_t *req)
     return -ENOSYS;
 }
 
+static inline void mem_access_resume(struct domain *d) {}
+
 #endif /* HAS_MEM_ACCESS */
 
 #endif /* _XEN_ASM_MEM_ACCESS_H */
-- 
2.1.0

[PATCH for-4.5 v9 06/19] xen: Relocate mem_event_op domctl and access_op memop into common.

[Tamas K Lengyel]

Signed-off-by: Tamas K Lengyel <tklengyel@sec.in.tum.de>
---
v9: Rename abstracted function to p2m_setup_introspection.
v8: Move the new enable_msr_exit_interception test into the p2m layer.
v6: Grouping style fix of #includes in common/memory.c.
v5: Move memop compat into common as well.
    Position domctl in switch relative to the domctl #.
v4: Don't remove memop handling from x86_64/compat and style fixes.
---
 xen/arch/x86/domctl.c           |  8 --------
 xen/arch/x86/mm/p2m.c           |  9 +++++++++
 xen/arch/x86/x86_64/compat/mm.c |  4 ----
 xen/arch/x86/x86_64/mm.c        |  4 ----
 xen/common/compat/memory.c      |  5 +++++
 xen/common/domctl.c             |  7 +++++++
 xen/common/mem_event.c          | 10 ++++------
 xen/common/memory.c             |  9 +++++++--
 xen/include/asm-arm/p2m.h       |  6 ++++++
 xen/include/asm-x86/p2m.h       |  3 +++
 10 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 8731e7f..ec77555 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1131,14 +1131,6 @@ long arch_do_domctl(
     }
     break;
 
-    case XEN_DOMCTL_mem_event_op:
-    {
-        ret = mem_event_domctl(d, &domctl->u.mem_event_op,
-                              guest_handle_cast(u_domctl, void));
-        copyback = 1;
-    }
-    break;
-
     case XEN_DOMCTL_mem_sharing_op:
     {
         ret = mem_sharing_domctl(d, &domctl->u.mem_sharing_op);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 08adaac..728e4eb 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1436,6 +1436,15 @@ void p2m_mem_event_emulate_check(struct vcpu *v, const mem_event_response_t *rsp
     }
 }
 
+void p2m_setup_introspection(struct domain *d)
+{
+    if ( hvm_funcs.enable_msr_exit_interception )
+    {
+        d->arch.hvm_domain.introspection_enabled = 1;
+        hvm_funcs.enable_msr_exit_interception(d);
+    }
+}
+
 bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
                             struct npfec npfec,
                             mem_event_request_t **req_ptr)
diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c
index c079702..54f25b7 100644
--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -198,10 +198,6 @@ int compat_arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
     }
 
-    case XENMEM_access_op:
-        rc = mem_access_memop(cmd, guest_handle_cast(arg, xen_mem_access_op_t));
-        break;
-
     case XENMEM_sharing_op:
     {
         xen_mem_sharing_op_t mso;
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index cce1406..8e5a1a1 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -995,10 +995,6 @@ long subarch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
     }
 
-    case XENMEM_access_op:
-        rc = mem_access_memop(cmd, guest_handle_cast(arg, xen_mem_access_op_t));
-        break;
-
     case XENMEM_sharing_op:
     {
         xen_mem_sharing_op_t mso;
diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c
index 25dc016..43d02bc 100644
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -4,6 +4,7 @@
 #include <xen/guest_access.h>
 #include <xen/sched.h>
 #include <xen/event.h>
+#include <xen/mem_access.h>
 #include <asm/current.h>
 #include <compat/memory.h>
 
@@ -381,6 +382,10 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat)
             break;
         }
 
+        case XENMEM_access_op:
+            rc = mem_access_memop(cmd, guest_handle_cast(compat, xen_mem_access_op_t));
+            break;
+
         case XENMEM_add_to_physmap_batch:
             start_extent = end_extent;
             break;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 329e535..fd8dd44 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -24,6 +24,7 @@
 #include <xen/bitmap.h>
 #include <xen/paging.h>
 #include <xen/hypercall.h>
+#include <xen/mem_event.h>
 #include <asm/current.h>
 #include <asm/irq.h>
 #include <asm/page.h>
@@ -1111,6 +1112,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
     }
     break;
 
+    case XEN_DOMCTL_mem_event_op:
+        ret = mem_event_domctl(d, &op->u.mem_event_op,
+                               guest_handle_cast(u_domctl, void));
+        copyback = 1;
+        break;
+
     case XEN_DOMCTL_disable_migrate:
     {
         d->disable_migrate = op->u.disable_migrate.disable;
diff --git a/xen/common/mem_event.c b/xen/common/mem_event.c
index 9f1a1b0..4338826 100644
--- a/xen/common/mem_event.c
+++ b/xen/common/mem_event.c
@@ -623,12 +623,10 @@ int mem_event_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
                                     HVM_PARAM_ACCESS_RING_PFN,
                                     mem_access_notification);
 
-            if ( mec->op != XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE &&
-                 rc == 0 && hvm_funcs.enable_msr_exit_interception )
-            {
-                d->arch.hvm_domain.introspection_enabled = 1;
-                hvm_funcs.enable_msr_exit_interception(d);
-            }
+            if ( mec->op == XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE_INTROSPECTION
+                 && !rc )
+                p2m_setup_introspection(d);
+
         }
         break;
 
diff --git a/xen/common/memory.c b/xen/common/memory.c
index bad50cb..cc36e39 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -21,13 +21,14 @@
 #include <xen/errno.h>
 #include <xen/tmem.h>
 #include <xen/tmem_xen.h>
+#include <xen/numa.h>
+#include <xen/mem_access.h>
+#include <xen/trace.h>
 #include <asm/current.h>
 #include <asm/hardirq.h>
 #include <asm/p2m.h>
-#include <xen/numa.h>
 #include <public/memory.h>
 #include <xsm/xsm.h>
-#include <xen/trace.h>
 
 struct memop_args {
     /* INPUT */
@@ -939,6 +940,10 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
     }
 
+    case XENMEM_access_op:
+        rc = mem_access_memop(cmd, guest_handle_cast(arg, xen_mem_access_op_t));
+        break;
+
     case XENMEM_claim_pages:
         if ( copy_from_guest(&reservation, arg, 1) )
             return -EFAULT;
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 4d5570a..10bf111 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -77,6 +77,12 @@ void p2m_mem_event_emulate_check(struct vcpu *v,
     /* Not supported on ARM. */
 };
 
+static inline
+void p2m_setup_introspection(struct domain *d)
+{
+    /* No special setup on ARM. */
+}
+
 #define p2m_is_foreign(_t)  ((_t) == p2m_map_foreign)
 #define p2m_is_ram(_t)      ((_t) == p2m_ram_rw || (_t) == p2m_ram_ro)
 
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 1de493e..a0e1704 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -589,6 +589,9 @@ int p2m_get_mem_access(struct domain *d, unsigned long pfn,
 void p2m_mem_event_emulate_check(struct vcpu *v,
                                  const mem_event_response_t *rsp);
 
+/* Enable arch specific introspection options (such as MSR interception). */
+void p2m_setup_introspection(struct domain *d);
+
 /* 
  * Internal functions, only called by other p2m code
  */
-- 
2.1.0

Re: [PATCH for-4.5 v9 04/19] xen: Relocate p2m_mem_access_resume to mem_access common

[Jan Beulich]

>>> On 24.09.14 at 11:09, <tklengyel@sec.in.tum.de> wrote:
> Relocate p2m_mem_access_resume to common and abstract the new
> p2m_mem_event_emulate_check into the p2m layer to.
> 
> Signed-off-by: Tamas K Lengyel <tklengyel@sec.in.tum.de>
> ---
> v9: Pass the vcpu instead of the domain to emulate_check.

Is it correct that you resent just patches 4 and 6 as v9?

> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1382,6 +1382,60 @@ static void p2m_mem_event_fill_regs(mem_event_request_t *req)
>      req->x86_regs.cs_arbytes = seg.attr.bytes;
>  }
>  
> +void p2m_mem_event_emulate_check(struct vcpu *v, const mem_event_response_t *rsp)
> +{
> +    /* Mark vcpu for skipping one instruction upon rescheduling. */
> +    if ( rsp->flags & MEM_EVENT_FLAG_EMULATE )
> +    {
> +        struct domain *d = v->domain;
> +        xenmem_access_t access;
> +        bool_t violation = 1;
> +
> +        if ( p2m_get_mem_access(d, rsp->gfn, &access) == 0 )

While it's certainly not wrong, I personally dislike such single use
local variables - you could easily (and without hampering readability)
pass v->domain here.

Jan

Re: [PATCH for-4.5 v9 06/19] xen: Relocate mem_event_op domctl and access_op memop into common.

[Jan Beulich]

>>> On 24.09.14 at 11:09, <tklengyel@sec.in.tum.de> wrote:
> Signed-off-by: Tamas K Lengyel <tklengyel@sec.in.tum.de>

for the non-ARM, non-x86/mm parts
Acked-by: Jan Beulich <jbeulich@suse.com>

But you failed to Cc relevant people for those other parts (and you
Cc-ed too few for the common code changes).

Jan

> ---
> v9: Rename abstracted function to p2m_setup_introspection.
> v8: Move the new enable_msr_exit_interception test into the p2m layer.
> v6: Grouping style fix of #includes in common/memory.c.
> v5: Move memop compat into common as well.
>     Position domctl in switch relative to the domctl #.
> v4: Don't remove memop handling from x86_64/compat and style fixes.
> ---
>  xen/arch/x86/domctl.c           |  8 --------
>  xen/arch/x86/mm/p2m.c           |  9 +++++++++
>  xen/arch/x86/x86_64/compat/mm.c |  4 ----
>  xen/arch/x86/x86_64/mm.c        |  4 ----
>  xen/common/compat/memory.c      |  5 +++++
>  xen/common/domctl.c             |  7 +++++++
>  xen/common/mem_event.c          | 10 ++++------
>  xen/common/memory.c             |  9 +++++++--
>  xen/include/asm-arm/p2m.h       |  6 ++++++
>  xen/include/asm-x86/p2m.h       |  3 +++
>  10 files changed, 41 insertions(+), 24 deletions(-)
> 
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 8731e7f..ec77555 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1131,14 +1131,6 @@ long arch_do_domctl(
>      }
>      break;
>  
> -    case XEN_DOMCTL_mem_event_op:
> -    {
> -        ret = mem_event_domctl(d, &domctl->u.mem_event_op,
> -                              guest_handle_cast(u_domctl, void));
> -        copyback = 1;
> -    }
> -    break;
> -
>      case XEN_DOMCTL_mem_sharing_op:
>      {
>          ret = mem_sharing_domctl(d, &domctl->u.mem_sharing_op);
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 08adaac..728e4eb 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1436,6 +1436,15 @@ void p2m_mem_event_emulate_check(struct vcpu *v, const 
> mem_event_response_t *rsp
>      }
>  }
>  
> +void p2m_setup_introspection(struct domain *d)
> +{
> +    if ( hvm_funcs.enable_msr_exit_interception )
> +    {
> +        d->arch.hvm_domain.introspection_enabled = 1;
> +        hvm_funcs.enable_msr_exit_interception(d);
> +    }
> +}
> +
>  bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
>                              struct npfec npfec,
>                              mem_event_request_t **req_ptr)
> diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c
> index c079702..54f25b7 100644
> --- a/xen/arch/x86/x86_64/compat/mm.c
> +++ b/xen/arch/x86/x86_64/compat/mm.c
> @@ -198,10 +198,6 @@ int compat_arch_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>          break;
>      }
>  
> -    case XENMEM_access_op:
> -        rc = mem_access_memop(cmd, guest_handle_cast(arg, 
> xen_mem_access_op_t));
> -        break;
> -
>      case XENMEM_sharing_op:
>      {
>          xen_mem_sharing_op_t mso;
> diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
> index cce1406..8e5a1a1 100644
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -995,10 +995,6 @@ long subarch_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>          break;
>      }
>  
> -    case XENMEM_access_op:
> -        rc = mem_access_memop(cmd, guest_handle_cast(arg, 
> xen_mem_access_op_t));
> -        break;
> -
>      case XENMEM_sharing_op:
>      {
>          xen_mem_sharing_op_t mso;
> diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c
> index 25dc016..43d02bc 100644
> --- a/xen/common/compat/memory.c
> +++ b/xen/common/compat/memory.c
> @@ -4,6 +4,7 @@
>  #include <xen/guest_access.h>
>  #include <xen/sched.h>
>  #include <xen/event.h>
> +#include <xen/mem_access.h>
>  #include <asm/current.h>
>  #include <compat/memory.h>
>  
> @@ -381,6 +382,10 @@ int compat_memory_op(unsigned int cmd, 
> XEN_GUEST_HANDLE_PARAM(void) compat)
>              break;
>          }
>  
> +        case XENMEM_access_op:
> +            rc = mem_access_memop(cmd, guest_handle_cast(compat, 
> xen_mem_access_op_t));
> +            break;
> +
>          case XENMEM_add_to_physmap_batch:
>              start_extent = end_extent;
>              break;
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 329e535..fd8dd44 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -24,6 +24,7 @@
>  #include <xen/bitmap.h>
>  #include <xen/paging.h>
>  #include <xen/hypercall.h>
> +#include <xen/mem_event.h>
>  #include <asm/current.h>
>  #include <asm/irq.h>
>  #include <asm/page.h>
> @@ -1111,6 +1112,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>      }
>      break;
>  
> +    case XEN_DOMCTL_mem_event_op:
> +        ret = mem_event_domctl(d, &op->u.mem_event_op,
> +                               guest_handle_cast(u_domctl, void));
> +        copyback = 1;
> +        break;
> +
>      case XEN_DOMCTL_disable_migrate:
>      {
>          d->disable_migrate = op->u.disable_migrate.disable;
> diff --git a/xen/common/mem_event.c b/xen/common/mem_event.c
> index 9f1a1b0..4338826 100644
> --- a/xen/common/mem_event.c
> +++ b/xen/common/mem_event.c
> @@ -623,12 +623,10 @@ int mem_event_domctl(struct domain *d, 
> xen_domctl_mem_event_op_t *mec,
>                                      HVM_PARAM_ACCESS_RING_PFN,
>                                      mem_access_notification);
>  
> -            if ( mec->op != XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE &&
> -                 rc == 0 && hvm_funcs.enable_msr_exit_interception )
> -            {
> -                d->arch.hvm_domain.introspection_enabled = 1;
> -                hvm_funcs.enable_msr_exit_interception(d);
> -            }
> +            if ( mec->op == 
> XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE_INTROSPECTION
> +                 && !rc )
> +                p2m_setup_introspection(d);
> +
>          }
>          break;
>  
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index bad50cb..cc36e39 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -21,13 +21,14 @@
>  #include <xen/errno.h>
>  #include <xen/tmem.h>
>  #include <xen/tmem_xen.h>
> +#include <xen/numa.h>
> +#include <xen/mem_access.h>
> +#include <xen/trace.h>
>  #include <asm/current.h>
>  #include <asm/hardirq.h>
>  #include <asm/p2m.h>
> -#include <xen/numa.h>
>  #include <public/memory.h>
>  #include <xsm/xsm.h>
> -#include <xen/trace.h>
>  
>  struct memop_args {
>      /* INPUT */
> @@ -939,6 +940,10 @@ long do_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>          break;
>      }
>  
> +    case XENMEM_access_op:
> +        rc = mem_access_memop(cmd, guest_handle_cast(arg, 
> xen_mem_access_op_t));
> +        break;
> +
>      case XENMEM_claim_pages:
>          if ( copy_from_guest(&reservation, arg, 1) )
>              return -EFAULT;
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 4d5570a..10bf111 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -77,6 +77,12 @@ void p2m_mem_event_emulate_check(struct vcpu *v,
>      /* Not supported on ARM. */
>  };
>  
> +static inline
> +void p2m_setup_introspection(struct domain *d)
> +{
> +    /* No special setup on ARM. */
> +}
> +
>  #define p2m_is_foreign(_t)  ((_t) == p2m_map_foreign)
>  #define p2m_is_ram(_t)      ((_t) == p2m_ram_rw || (_t) == p2m_ram_ro)
>  
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index 1de493e..a0e1704 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -589,6 +589,9 @@ int p2m_get_mem_access(struct domain *d, unsigned long 
> pfn,
>  void p2m_mem_event_emulate_check(struct vcpu *v,
>                                   const mem_event_response_t *rsp);
>  
> +/* Enable arch specific introspection options (such as MSR interception). 
> */
> +void p2m_setup_introspection(struct domain *d);
> +
>  /* 
>   * Internal functions, only called by other p2m code
>   */
> -- 
> 2.1.0

Re: [PATCH for-4.5 v9 04/19] xen: Relocate p2m_mem_access_resume to mem_access common

[Tamas K Lengyel]

[-- Attachment #1.1: Type: text/plain, Size: 1543 bytes --]

On Wed, Sep 24, 2014 at 2:19 PM, Jan Beulich <JBeulich@suse.com> wrote:

> >>> On 24.09.14 at 11:09, <tklengyel@sec.in.tum.de> wrote:
> > Relocate p2m_mem_access_resume to common and abstract the new
> > p2m_mem_event_emulate_check into the p2m layer to.
> >
> > Signed-off-by: Tamas K Lengyel <tklengyel@sec.in.tum.de>
> > ---
> > v9: Pass the vcpu instead of the domain to emulate_check.
>
> Is it correct that you resent just patches 4 and 6 as v9?
>

I didn't want to resend the entire 19 set series for just these two updates.


>
> > --- a/xen/arch/x86/mm/p2m.c
> > +++ b/xen/arch/x86/mm/p2m.c
> > @@ -1382,6 +1382,60 @@ static void
> p2m_mem_event_fill_regs(mem_event_request_t *req)
> >      req->x86_regs.cs_arbytes = seg.attr.bytes;
> >  }
> >
> > +void p2m_mem_event_emulate_check(struct vcpu *v, const
> mem_event_response_t *rsp)
> > +{
> > +    /* Mark vcpu for skipping one instruction upon rescheduling. */
> > +    if ( rsp->flags & MEM_EVENT_FLAG_EMULATE )
> > +    {
> > +        struct domain *d = v->domain;
> > +        xenmem_access_t access;
> > +        bool_t violation = 1;
> > +
> > +        if ( p2m_get_mem_access(d, rsp->gfn, &access) == 0 )
>
> While it's certainly not wrong, I personally dislike such single use
> local variables - you could easily (and without hampering readability)
> pass v->domain here.
>

Certainly. Does it worth another resend?

Tamas


>
> Jan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

[-- Attachment #1.2: Type: text/html, Size: 2726 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH for-4.5 v9 06/19] xen: Relocate mem_event_op domctl and access_op memop into common.

[Tamas K Lengyel]

[-- Attachment #1.1: Type: text/plain, Size: 8926 bytes --]

On Wed, Sep 24, 2014 at 2:22 PM, Jan Beulich <JBeulich@suse.com> wrote:

> >>> On 24.09.14 at 11:09, <tklengyel@sec.in.tum.de> wrote:
> > Signed-off-by: Tamas K Lengyel <tklengyel@sec.in.tum.de>
>
> for the non-ARM, non-x86/mm parts
> Acked-by: Jan Beulich <jbeulich@suse.com>
>
> But you failed to Cc relevant people for those other parts (and you
> Cc-ed too few for the common code changes).
>

Thanks. I used the get_maintainer.pl script to grab only the relevant folks
for these two patches. I'm not sure why it did miss folks.

Tamas


>
> Jan
>
> > ---
> > v9: Rename abstracted function to p2m_setup_introspection.
> > v8: Move the new enable_msr_exit_interception test into the p2m layer.
> > v6: Grouping style fix of #includes in common/memory.c.
> > v5: Move memop compat into common as well.
> >     Position domctl in switch relative to the domctl #.
> > v4: Don't remove memop handling from x86_64/compat and style fixes.
> > ---
> >  xen/arch/x86/domctl.c           |  8 --------
> >  xen/arch/x86/mm/p2m.c           |  9 +++++++++
> >  xen/arch/x86/x86_64/compat/mm.c |  4 ----
> >  xen/arch/x86/x86_64/mm.c        |  4 ----
> >  xen/common/compat/memory.c      |  5 +++++
> >  xen/common/domctl.c             |  7 +++++++
> >  xen/common/mem_event.c          | 10 ++++------
> >  xen/common/memory.c             |  9 +++++++--
> >  xen/include/asm-arm/p2m.h       |  6 ++++++
> >  xen/include/asm-x86/p2m.h       |  3 +++
> >  10 files changed, 41 insertions(+), 24 deletions(-)
> >
> > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> > index 8731e7f..ec77555 100644
> > --- a/xen/arch/x86/domctl.c
> > +++ b/xen/arch/x86/domctl.c
> > @@ -1131,14 +1131,6 @@ long arch_do_domctl(
> >      }
> >      break;
> >
> > -    case XEN_DOMCTL_mem_event_op:
> > -    {
> > -        ret = mem_event_domctl(d, &domctl->u.mem_event_op,
> > -                              guest_handle_cast(u_domctl, void));
> > -        copyback = 1;
> > -    }
> > -    break;
> > -
> >      case XEN_DOMCTL_mem_sharing_op:
> >      {
> >          ret = mem_sharing_domctl(d, &domctl->u.mem_sharing_op);
> > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> > index 08adaac..728e4eb 100644
> > --- a/xen/arch/x86/mm/p2m.c
> > +++ b/xen/arch/x86/mm/p2m.c
> > @@ -1436,6 +1436,15 @@ void p2m_mem_event_emulate_check(struct vcpu *v,
> const
> > mem_event_response_t *rsp
> >      }
> >  }
> >
> > +void p2m_setup_introspection(struct domain *d)
> > +{
> > +    if ( hvm_funcs.enable_msr_exit_interception )
> > +    {
> > +        d->arch.hvm_domain.introspection_enabled = 1;
> > +        hvm_funcs.enable_msr_exit_interception(d);
> > +    }
> > +}
> > +
> >  bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
> >                              struct npfec npfec,
> >                              mem_event_request_t **req_ptr)
> > diff --git a/xen/arch/x86/x86_64/compat/mm.c
> b/xen/arch/x86/x86_64/compat/mm.c
> > index c079702..54f25b7 100644
> > --- a/xen/arch/x86/x86_64/compat/mm.c
> > +++ b/xen/arch/x86/x86_64/compat/mm.c
> > @@ -198,10 +198,6 @@ int compat_arch_memory_op(unsigned long cmd,
> > XEN_GUEST_HANDLE_PARAM(void) arg)
> >          break;
> >      }
> >
> > -    case XENMEM_access_op:
> > -        rc = mem_access_memop(cmd, guest_handle_cast(arg,
> > xen_mem_access_op_t));
> > -        break;
> > -
> >      case XENMEM_sharing_op:
> >      {
> >          xen_mem_sharing_op_t mso;
> > diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
> > index cce1406..8e5a1a1 100644
> > --- a/xen/arch/x86/x86_64/mm.c
> > +++ b/xen/arch/x86/x86_64/mm.c
> > @@ -995,10 +995,6 @@ long subarch_memory_op(unsigned long cmd,
> > XEN_GUEST_HANDLE_PARAM(void) arg)
> >          break;
> >      }
> >
> > -    case XENMEM_access_op:
> > -        rc = mem_access_memop(cmd, guest_handle_cast(arg,
> > xen_mem_access_op_t));
> > -        break;
> > -
> >      case XENMEM_sharing_op:
> >      {
> >          xen_mem_sharing_op_t mso;
> > diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c
> > index 25dc016..43d02bc 100644
> > --- a/xen/common/compat/memory.c
> > +++ b/xen/common/compat/memory.c
> > @@ -4,6 +4,7 @@
> >  #include <xen/guest_access.h>
> >  #include <xen/sched.h>
> >  #include <xen/event.h>
> > +#include <xen/mem_access.h>
> >  #include <asm/current.h>
> >  #include <compat/memory.h>
> >
> > @@ -381,6 +382,10 @@ int compat_memory_op(unsigned int cmd,
> > XEN_GUEST_HANDLE_PARAM(void) compat)
> >              break;
> >          }
> >
> > +        case XENMEM_access_op:
> > +            rc = mem_access_memop(cmd, guest_handle_cast(compat,
> > xen_mem_access_op_t));
> > +            break;
> > +
> >          case XENMEM_add_to_physmap_batch:
> >              start_extent = end_extent;
> >              break;
> > diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> > index 329e535..fd8dd44 100644
> > --- a/xen/common/domctl.c
> > +++ b/xen/common/domctl.c
> > @@ -24,6 +24,7 @@
> >  #include <xen/bitmap.h>
> >  #include <xen/paging.h>
> >  #include <xen/hypercall.h>
> > +#include <xen/mem_event.h>
> >  #include <asm/current.h>
> >  #include <asm/irq.h>
> >  #include <asm/page.h>
> > @@ -1111,6 +1112,12 @@ long
> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
> > u_domctl)
> >      }
> >      break;
> >
> > +    case XEN_DOMCTL_mem_event_op:
> > +        ret = mem_event_domctl(d, &op->u.mem_event_op,
> > +                               guest_handle_cast(u_domctl, void));
> > +        copyback = 1;
> > +        break;
> > +
> >      case XEN_DOMCTL_disable_migrate:
> >      {
> >          d->disable_migrate = op->u.disable_migrate.disable;
> > diff --git a/xen/common/mem_event.c b/xen/common/mem_event.c
> > index 9f1a1b0..4338826 100644
> > --- a/xen/common/mem_event.c
> > +++ b/xen/common/mem_event.c
> > @@ -623,12 +623,10 @@ int mem_event_domctl(struct domain *d,
> > xen_domctl_mem_event_op_t *mec,
> >                                      HVM_PARAM_ACCESS_RING_PFN,
> >                                      mem_access_notification);
> >
> > -            if ( mec->op != XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE &&
> > -                 rc == 0 && hvm_funcs.enable_msr_exit_interception )
> > -            {
> > -                d->arch.hvm_domain.introspection_enabled = 1;
> > -                hvm_funcs.enable_msr_exit_interception(d);
> > -            }
> > +            if ( mec->op ==
> > XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE_INTROSPECTION
> > +                 && !rc )
> > +                p2m_setup_introspection(d);
> > +
> >          }
> >          break;
> >
> > diff --git a/xen/common/memory.c b/xen/common/memory.c
> > index bad50cb..cc36e39 100644
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -21,13 +21,14 @@
> >  #include <xen/errno.h>
> >  #include <xen/tmem.h>
> >  #include <xen/tmem_xen.h>
> > +#include <xen/numa.h>
> > +#include <xen/mem_access.h>
> > +#include <xen/trace.h>
> >  #include <asm/current.h>
> >  #include <asm/hardirq.h>
> >  #include <asm/p2m.h>
> > -#include <xen/numa.h>
> >  #include <public/memory.h>
> >  #include <xsm/xsm.h>
> > -#include <xen/trace.h>
> >
> >  struct memop_args {
> >      /* INPUT */
> > @@ -939,6 +940,10 @@ long do_memory_op(unsigned long cmd,
> > XEN_GUEST_HANDLE_PARAM(void) arg)
> >          break;
> >      }
> >
> > +    case XENMEM_access_op:
> > +        rc = mem_access_memop(cmd, guest_handle_cast(arg,
> > xen_mem_access_op_t));
> > +        break;
> > +
> >      case XENMEM_claim_pages:
> >          if ( copy_from_guest(&reservation, arg, 1) )
> >              return -EFAULT;
> > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> > index 4d5570a..10bf111 100644
> > --- a/xen/include/asm-arm/p2m.h
> > +++ b/xen/include/asm-arm/p2m.h
> > @@ -77,6 +77,12 @@ void p2m_mem_event_emulate_check(struct vcpu *v,
> >      /* Not supported on ARM. */
> >  };
> >
> > +static inline
> > +void p2m_setup_introspection(struct domain *d)
> > +{
> > +    /* No special setup on ARM. */
> > +}
> > +
> >  #define p2m_is_foreign(_t)  ((_t) == p2m_map_foreign)
> >  #define p2m_is_ram(_t)      ((_t) == p2m_ram_rw || (_t) == p2m_ram_ro)
> >
> > diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> > index 1de493e..a0e1704 100644
> > --- a/xen/include/asm-x86/p2m.h
> > +++ b/xen/include/asm-x86/p2m.h
> > @@ -589,6 +589,9 @@ int p2m_get_mem_access(struct domain *d, unsigned
> long
> > pfn,
> >  void p2m_mem_event_emulate_check(struct vcpu *v,
> >                                   const mem_event_response_t *rsp);
> >
> > +/* Enable arch specific introspection options (such as MSR
> interception).
> > */
> > +void p2m_setup_introspection(struct domain *d);
> > +
> >  /*
> >   * Internal functions, only called by other p2m code
> >   */
> > --
> > 2.1.0
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

[-- Attachment #1.2: Type: text/html, Size: 11540 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH for-4.5 v9 04/19] xen: Relocate p2m_mem_access_resume to mem_access common

[Jan Beulich]

>>> On 24.09.14 at 15:05, <tamas.lengyel@zentific.com> wrote:
> On Wed, Sep 24, 2014 at 2:19 PM, Jan Beulich <JBeulich@suse.com> wrote:
> 
>> >>> On 24.09.14 at 11:09, <tklengyel@sec.in.tum.de> wrote:
>> > Relocate p2m_mem_access_resume to common and abstract the new
>> > p2m_mem_event_emulate_check into the p2m layer to.
>> >
>> > Signed-off-by: Tamas K Lengyel <tklengyel@sec.in.tum.de>
>> > ---
>> > v9: Pass the vcpu instead of the domain to emulate_check.
>>
>> Is it correct that you resent just patches 4 and 6 as v9?
>>
> 
> I didn't want to resend the entire 19 set series for just these two updates.

Which is fine I guess, I just wanted to double check.

>> > --- a/xen/arch/x86/mm/p2m.c
>> > +++ b/xen/arch/x86/mm/p2m.c
>> > @@ -1382,6 +1382,60 @@ static void
>> p2m_mem_event_fill_regs(mem_event_request_t *req)
>> >      req->x86_regs.cs_arbytes = seg.attr.bytes;
>> >  }
>> >
>> > +void p2m_mem_event_emulate_check(struct vcpu *v, const
>> mem_event_response_t *rsp)
>> > +{
>> > +    /* Mark vcpu for skipping one instruction upon rescheduling. */
>> > +    if ( rsp->flags & MEM_EVENT_FLAG_EMULATE )
>> > +    {
>> > +        struct domain *d = v->domain;
>> > +        xenmem_access_t access;
>> > +        bool_t violation = 1;
>> > +
>> > +        if ( p2m_get_mem_access(d, rsp->gfn, &access) == 0 )
>>
>> While it's certainly not wrong, I personally dislike such single use
>> local variables - you could easily (and without hampering readability)
>> pass v->domain here.
>>
> 
> Certainly. Does it worth another resend?

Probably not if no other need for resending arises.

Jan

Re: [PATCH for-4.5 v9 04/19] xen: Relocate p2m_mem_access_resume to mem_access common

[Jan Beulich]

>>> On 24.09.14 at 15:05, <tamas.lengyel@zentific.com> wrote:
> On Wed, Sep 24, 2014 at 2:19 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> >>> On 24.09.14 at 11:09, <tklengyel@sec.in.tum.de> wrote:
>> > --- a/xen/arch/x86/mm/p2m.c
>> > +++ b/xen/arch/x86/mm/p2m.c
>> > @@ -1382,6 +1382,60 @@ static void
>> p2m_mem_event_fill_regs(mem_event_request_t *req)
>> >      req->x86_regs.cs_arbytes = seg.attr.bytes;
>> >  }
>> >
>> > +void p2m_mem_event_emulate_check(struct vcpu *v, const
>> mem_event_response_t *rsp)
>> > +{
>> > +    /* Mark vcpu for skipping one instruction upon rescheduling. */
>> > +    if ( rsp->flags & MEM_EVENT_FLAG_EMULATE )
>> > +    {
>> > +        struct domain *d = v->domain;
>> > +        xenmem_access_t access;
>> > +        bool_t violation = 1;
>> > +
>> > +        if ( p2m_get_mem_access(d, rsp->gfn, &access) == 0 )
>>
>> While it's certainly not wrong, I personally dislike such single use
>> local variables - you could easily (and without hampering readability)
>> pass v->domain here.
>>
> 
> Certainly. Does it worth another resend?

Actually (correcting my earlier reply) I think together with the missed
Cc-s on both v9 patches, re-sending would be desirable, in which
case the cosmetic change above should be done at once.

Jan

Re: [PATCH for-4.5 v9 04/19] xen: Relocate p2m_mem_access_resume to mem_access common

[Tamas K Lengyel]

[-- Attachment #1.1: Type: text/plain, Size: 1608 bytes --]

On Wed, Sep 24, 2014 at 3:42 PM, Jan Beulich <JBeulich@suse.com> wrote:

> >>> On 24.09.14 at 15:05, <tamas.lengyel@zentific.com> wrote:
> > On Wed, Sep 24, 2014 at 2:19 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >> >>> On 24.09.14 at 11:09, <tklengyel@sec.in.tum.de> wrote:
> >> > --- a/xen/arch/x86/mm/p2m.c
> >> > +++ b/xen/arch/x86/mm/p2m.c
> >> > @@ -1382,6 +1382,60 @@ static void
> >> p2m_mem_event_fill_regs(mem_event_request_t *req)
> >> >      req->x86_regs.cs_arbytes = seg.attr.bytes;
> >> >  }
> >> >
> >> > +void p2m_mem_event_emulate_check(struct vcpu *v, const
> >> mem_event_response_t *rsp)
> >> > +{
> >> > +    /* Mark vcpu for skipping one instruction upon rescheduling. */
> >> > +    if ( rsp->flags & MEM_EVENT_FLAG_EMULATE )
> >> > +    {
> >> > +        struct domain *d = v->domain;
> >> > +        xenmem_access_t access;
> >> > +        bool_t violation = 1;
> >> > +
> >> > +        if ( p2m_get_mem_access(d, rsp->gfn, &access) == 0 )
> >>
> >> While it's certainly not wrong, I personally dislike such single use
> >> local variables - you could easily (and without hampering readability)
> >> pass v->domain here.
> >>
> >
> > Certainly. Does it worth another resend?
>
> Actually (correcting my earlier reply) I think together with the missed
> Cc-s on both v9 patches, re-sending would be desirable, in which
> case the cosmetic change above should be done at once.
>
> Jan
>

According to get_maintainer I only missed Tim from the original cc list. I
don't want to spam other maintainers with just these updates, so do you see
anyone else that should be on this?

Tamas

[-- Attachment #1.2: Type: text/html, Size: 2566 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH for-4.5 v9 04/19] xen: Relocate p2m_mem_access_resume to mem_access common

[Jan Beulich]

>>> On 24.09.14 at 16:47, <tamas.lengyel@zentific.com> wrote:
> On Wed, Sep 24, 2014 at 3:42 PM, Jan Beulich <JBeulich@suse.com> wrote:
> 
>> >>> On 24.09.14 at 15:05, <tamas.lengyel@zentific.com> wrote:
>> > On Wed, Sep 24, 2014 at 2:19 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> >> >>> On 24.09.14 at 11:09, <tklengyel@sec.in.tum.de> wrote:
>> >> > --- a/xen/arch/x86/mm/p2m.c
>> >> > +++ b/xen/arch/x86/mm/p2m.c
>> >> > @@ -1382,6 +1382,60 @@ static void
>> >> p2m_mem_event_fill_regs(mem_event_request_t *req)
>> >> >      req->x86_regs.cs_arbytes = seg.attr.bytes;
>> >> >  }
>> >> >
>> >> > +void p2m_mem_event_emulate_check(struct vcpu *v, const
>> >> mem_event_response_t *rsp)
>> >> > +{
>> >> > +    /* Mark vcpu for skipping one instruction upon rescheduling. */
>> >> > +    if ( rsp->flags & MEM_EVENT_FLAG_EMULATE )
>> >> > +    {
>> >> > +        struct domain *d = v->domain;
>> >> > +        xenmem_access_t access;
>> >> > +        bool_t violation = 1;
>> >> > +
>> >> > +        if ( p2m_get_mem_access(d, rsp->gfn, &access) == 0 )
>> >>
>> >> While it's certainly not wrong, I personally dislike such single use
>> >> local variables - you could easily (and without hampering readability)
>> >> pass v->domain here.
>> >>
>> >
>> > Certainly. Does it worth another resend?
>>
>> Actually (correcting my earlier reply) I think together with the missed
>> Cc-s on both v9 patches, re-sending would be desirable, in which
>> case the cosmetic change above should be done at once.
> 
> According to get_maintainer I only missed Tim from the original cc list. I
> don't want to spam other maintainers with just these updates, so do you see
> anyone else that should be on this?

IanC as another ARM maintainer? (And Tim is really kind of important
to Cc here.)

Jan

Re: [PATCH for-4.5 v9 04/19] xen: Relocate p2m_mem_access_resume to mem_access common

[Tamas K Lengyel]

[-- Attachment #1.1: Type: text/plain, Size: 2044 bytes --]

On Wed, Sep 24, 2014 at 4:52 PM, Jan Beulich <JBeulich@suse.com> wrote:

> >>> On 24.09.14 at 16:47, <tamas.lengyel@zentific.com> wrote:
> > On Wed, Sep 24, 2014 at 3:42 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >
> >> >>> On 24.09.14 at 15:05, <tamas.lengyel@zentific.com> wrote:
> >> > On Wed, Sep 24, 2014 at 2:19 PM, Jan Beulich <JBeulich@suse.com>
> wrote:
> >> >> >>> On 24.09.14 at 11:09, <tklengyel@sec.in.tum.de> wrote:
> >> >> > --- a/xen/arch/x86/mm/p2m.c
> >> >> > +++ b/xen/arch/x86/mm/p2m.c
> >> >> > @@ -1382,6 +1382,60 @@ static void
> >> >> p2m_mem_event_fill_regs(mem_event_request_t *req)
> >> >> >      req->x86_regs.cs_arbytes = seg.attr.bytes;
> >> >> >  }
> >> >> >
> >> >> > +void p2m_mem_event_emulate_check(struct vcpu *v, const
> >> >> mem_event_response_t *rsp)
> >> >> > +{
> >> >> > +    /* Mark vcpu for skipping one instruction upon rescheduling.
> */
> >> >> > +    if ( rsp->flags & MEM_EVENT_FLAG_EMULATE )
> >> >> > +    {
> >> >> > +        struct domain *d = v->domain;
> >> >> > +        xenmem_access_t access;
> >> >> > +        bool_t violation = 1;
> >> >> > +
> >> >> > +        if ( p2m_get_mem_access(d, rsp->gfn, &access) == 0 )
> >> >>
> >> >> While it's certainly not wrong, I personally dislike such single use
> >> >> local variables - you could easily (and without hampering
> readability)
> >> >> pass v->domain here.
> >> >>
> >> >
> >> > Certainly. Does it worth another resend?
> >>
> >> Actually (correcting my earlier reply) I think together with the missed
> >> Cc-s on both v9 patches, re-sending would be desirable, in which
> >> case the cosmetic change above should be done at once.
> >
> > According to get_maintainer I only missed Tim from the original cc list.
> I
> > don't want to spam other maintainers with just these updates, so do you
> see
> > anyone else that should be on this?
>
> IanC as another ARM maintainer? (And Tim is really kind of important
> to Cc here.)
>
> Jan
>

OK, IanC is not popping up for these patches via get_maintainer for some
reason..

Tamas

[-- Attachment #1.2: Type: text/html, Size: 3342 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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