diff mbox

[v3,2/9] monitor: Don't call vm_event_fill_regs from common

Message ID 1462373480-20206-2-git-send-email-tamas@tklengyel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tamas K Lengyel May 4, 2016, 2:51 p.m. UTC
The prototype of vm_event_fill_regs will differ on x86 and ARM so in this patch
we move components from common to arch-specific that use this function. As
part of this patch we rename and relocate vm_event_monitor_guest_request as
monitor_guest_request from vm_event to monitor.

Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
 xen/arch/arm/Makefile      |  1 +
 xen/arch/arm/hvm.c         |  4 ++--
 xen/arch/arm/monitor.c     | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/hvm/event.c   |  3 +++
 xen/arch/x86/hvm/hvm.c     |  3 ++-
 xen/arch/x86/monitor.c     | 18 +++++++++++++++++
 xen/common/vm_event.c      | 17 ----------------
 xen/include/xen/monitor.h  |  1 +
 xen/include/xen/vm_event.h |  2 --
 9 files changed, 75 insertions(+), 22 deletions(-)
 create mode 100644 xen/arch/arm/monitor.c

Comments

Razvan Cojocaru May 5, 2016, 9:34 a.m. UTC | #1
On 05/04/2016 05:51 PM, Tamas K Lengyel wrote:
> The prototype of vm_event_fill_regs will differ on x86 and ARM so in this patch
> we move components from common to arch-specific that use this function. As
> part of this patch we rename and relocate vm_event_monitor_guest_request as
> monitor_guest_request from vm_event to monitor.
> 
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> ---
>  xen/arch/arm/Makefile      |  1 +
>  xen/arch/arm/hvm.c         |  4 ++--
>  xen/arch/arm/monitor.c     | 48 ++++++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/hvm/event.c   |  3 +++
>  xen/arch/x86/hvm/hvm.c     |  3 ++-
>  xen/arch/x86/monitor.c     | 18 +++++++++++++++++
>  xen/common/vm_event.c      | 17 ----------------
>  xen/include/xen/monitor.h  |  1 +
>  xen/include/xen/vm_event.h |  2 --
>  9 files changed, 75 insertions(+), 22 deletions(-)
>  create mode 100644 xen/arch/arm/monitor.c

Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Julien Grall May 16, 2016, 9:48 a.m. UTC | #2
Hi Tamas,

On 04/05/16 15:51, Tamas K Lengyel wrote:
> The prototype of vm_event_fill_regs will differ on x86 and ARM so in this patch
> we move components from common to arch-specific that use this function. As
> part of this patch we rename and relocate vm_event_monitor_guest_request as
> monitor_guest_request from vm_event to monitor.

Would not it be possible to find a common prototype between ARM and x86?

 From patch #4, the ARM prototype is:

void vm_event_fill_regs(vm_event_request_t *req,
                         const struct cpu_user_regs *regs,
                         struct domain *d)

and the x86 one is

void vm_event_fill_regs(vm_event_request_t *req);

The parameter "regs" will always be equal to guest_cpu_user_regs(). And 
the domain will always be current->domain.

So, IHMO, there is no need to differ between ARM and x86. This would 
also keep the code simple.

Regards,
Tamas K Lengyel May 27, 2016, 6:58 p.m. UTC | #3
On Mon, May 16, 2016 at 3:48 AM, Julien Grall <julien.grall@arm.com> wrote:
> Hi Tamas,
>
> On 04/05/16 15:51, Tamas K Lengyel wrote:
>>
>> The prototype of vm_event_fill_regs will differ on x86 and ARM so in this
>> patch
>> we move components from common to arch-specific that use this function. As
>> part of this patch we rename and relocate vm_event_monitor_guest_request
>> as
>> monitor_guest_request from vm_event to monitor.
>
>
> Would not it be possible to find a common prototype between ARM and x86?
>
> From patch #4, the ARM prototype is:
>
> void vm_event_fill_regs(vm_event_request_t *req,
>                         const struct cpu_user_regs *regs,
>                         struct domain *d)
>
> and the x86 one is
>
> void vm_event_fill_regs(vm_event_request_t *req);
>
> The parameter "regs" will always be equal to guest_cpu_user_regs(). And the
> domain will always be current->domain.
>
> So, IHMO, there is no need to differ between ARM and x86. This would also
> keep the code simple.
>

Yeap, you are right, I'll get rid of this patch.

Thanks,
Tamas
diff mbox

Patch

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 0328b50..6e3dcff 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -40,6 +40,7 @@  obj-y += device.o
 obj-y += decode.o
 obj-y += processor.o
 obj-y += smc.o
+obj-y += monitor.o
 
 #obj-bin-y += ....o
 
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index c01123a..d999bde 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -22,7 +22,7 @@ 
 #include <xen/errno.h>
 #include <xen/guest_access.h>
 #include <xen/sched.h>
-#include <xen/vm_event.h>
+#include <xen/monitor.h>
 
 #include <xsm/xsm.h>
 
@@ -75,7 +75,7 @@  long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
 
     case HVMOP_guest_request_vm_event:
         if ( guest_handle_is_null(arg) )
-            vm_event_monitor_guest_request();
+            monitor_guest_request();
         else
             rc = -EINVAL;
         break;
diff --git a/xen/arch/arm/monitor.c b/xen/arch/arm/monitor.c
new file mode 100644
index 0000000..f957257
--- /dev/null
+++ b/xen/arch/arm/monitor.c
@@ -0,0 +1,48 @@ 
+/*
+ * arch/arm/monitor.c
+ *
+ * Arch-specific monitor_op domctl handler.
+ *
+ * Copyright (c) 2015-2016 Tamas K Lengyel (tamas@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <asm/vm_event.h>
+#include <public/vm_event.h>
+
+void monitor_guest_request(void)
+{
+    struct vcpu *curr = current;
+    struct domain *d = curr->domain;
+
+    if ( d->monitor.guest_request_enabled )
+    {
+        vm_event_request_t req = {
+            .reason = VM_EVENT_REASON_GUEST_REQUEST,
+            .vcpu_id = curr->vcpu_id,
+        };
+
+        vm_event_monitor_traps(curr, d->monitor.guest_request_sync, &req);
+    }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
index 56c5514..73d0a26 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -47,6 +47,7 @@  bool_t hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
             .u.write_ctrlreg.old_value = old
         };
 
+        vm_event_fill_regs(&req);
         vm_event_monitor_traps(curr, sync, &req);
         return 1;
     }
@@ -68,6 +69,7 @@  void hvm_event_msr(unsigned int msr, uint64_t value)
             .u.mov_to_msr.value = value,
         };
 
+        vm_event_fill_regs(&req);
         vm_event_monitor_traps(curr, 1, &req);
     }
 }
@@ -115,6 +117,7 @@  int hvm_event_breakpoint(unsigned long rip,
     }
 
     req.vcpu_id = curr->vcpu_id;
+    vm_event_fill_regs(&req);
 
     return vm_event_monitor_traps(curr, 1, &req);
 }
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 8cb6e9e..5d740f7 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -37,6 +37,7 @@ 
 #include <xen/mem_access.h>
 #include <xen/rangeset.h>
 #include <xen/vm_event.h>
+#include <xen/monitor.h>
 #include <asm/shadow.h>
 #include <asm/hap.h>
 #include <asm/current.h>
@@ -5695,7 +5696,7 @@  long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
 
     case HVMOP_guest_request_vm_event:
         if ( guest_handle_is_null(arg) )
-            vm_event_monitor_guest_request();
+            monitor_guest_request();
         else
             rc = -EINVAL;
         break;
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 621f91a..8598049 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -20,6 +20,7 @@ 
  */
 
 #include <asm/monitor.h>
+#include <asm/vm_event.h>
 #include <public/vm_event.h>
 
 int arch_monitor_domctl_event(struct domain *d,
@@ -136,6 +137,23 @@  int arch_monitor_domctl_event(struct domain *d,
     return 0;
 }
 
+void monitor_guest_request(void)
+{
+    struct vcpu *curr = current;
+    struct domain *d = curr->domain;
+
+    if ( d->monitor.guest_request_enabled )
+    {
+        vm_event_request_t req = {
+            .reason = VM_EVENT_REASON_GUEST_REQUEST,
+            .vcpu_id = curr->vcpu_id,
+        };
+
+        vm_event_fill_regs(&req);
+        vm_event_monitor_traps(curr, d->monitor.guest_request_sync, &req);
+    }
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 2906407..31a8830 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -818,28 +818,11 @@  int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
         req->altp2m_idx = altp2m_vcpu_idx(v);
     }
 
-    vm_event_fill_regs(req);
     vm_event_put_request(d, &d->vm_event->monitor, req);
 
     return 1;
 }
 
-void vm_event_monitor_guest_request(void)
-{
-    struct vcpu *curr = current;
-    struct domain *d = curr->domain;
-
-    if ( d->monitor.guest_request_enabled )
-    {
-        vm_event_request_t req = {
-            .reason = VM_EVENT_REASON_GUEST_REQUEST,
-            .vcpu_id = curr->vcpu_id,
-        };
-
-        vm_event_monitor_traps(curr, d->monitor.guest_request_sync, &req);
-    }
-}
-
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/monitor.h b/xen/include/xen/monitor.h
index 7015e6d..204d5cc 100644
--- a/xen/include/xen/monitor.h
+++ b/xen/include/xen/monitor.h
@@ -26,5 +26,6 @@  struct domain;
 struct xen_domctl_monitor_op;
 
 int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *op);
+void monitor_guest_request(void);
 
 #endif /* __XEN_MONITOR_H__ */
diff --git a/xen/include/xen/vm_event.h b/xen/include/xen/vm_event.h
index beda9fe..89e6243 100644
--- a/xen/include/xen/vm_event.h
+++ b/xen/include/xen/vm_event.h
@@ -81,8 +81,6 @@  void vm_event_vcpu_unpause(struct vcpu *v);
 int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
                            vm_event_request_t *req);
 
-void vm_event_monitor_guest_request(void);
-
 #endif /* __VM_EVENT_H__ */