* [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps
@ 2016-06-02 22:52 Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 2/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
` (10 more replies)
0 siblings, 11 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-02 22:52 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Tamas K Lengyel, Jun Nakajima, Razvan Cojocaru,
Andrew Cooper, Jan Beulich
The return value has not been clearly defined, with the function
never returning 0 which seemingly indicated a condition where the
guest should crash. In this patch we define -rc as error condition
where the notification was not sent, 0 where the notification was sent
and the vCPU is not paused and 1 that the notification was sent and
that the vCPU is paused.
Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
xen/arch/x86/hvm/event.c | 4 ++--
xen/arch/x86/hvm/vmx/vmx.c | 6 +++---
xen/common/vm_event.c | 5 +++--
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
index 56c5514..5772c6b 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -47,8 +47,8 @@ bool_t hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
.u.write_ctrlreg.old_value = old
};
- vm_event_monitor_traps(curr, sync, &req);
- return 1;
+ if ( vm_event_monitor_traps(curr, sync, &req) >= 0 )
+ return 1;
}
return 0;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 3acf1ab..097d97c 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3392,11 +3392,11 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
break;
}
else {
- int handled =
+ int rc =
hvm_event_breakpoint(regs->eip,
HVM_EVENT_SOFTWARE_BREAKPOINT);
- if ( handled < 0 )
+ if ( !rc )
{
struct hvm_trap trap = {
.vector = TRAP_int3,
@@ -3410,7 +3410,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
hvm_inject_trap(&trap);
break;
}
- else if ( handled )
+ else if ( rc > 0 )
break;
}
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 2906407..fe86fb9 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -801,7 +801,7 @@ int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
* If there was no ring to handle the event, then
* simply continue executing normally.
*/
- return 1;
+ return 0;
default:
return rc;
};
@@ -810,6 +810,7 @@ int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
{
req->flags |= VM_EVENT_FLAG_VCPU_PAUSED;
vm_event_vcpu_pause(v);
+ rc = 1;
}
if ( altp2m_active(d) )
@@ -821,7 +822,7 @@ int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
vm_event_fill_regs(req);
vm_event_put_request(d, &d->vm_event->monitor, req);
- return 1;
+ return rc;
}
void vm_event_monitor_guest_request(void)
--
2.8.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v5 2/9] monitor: Rename vm_event_monitor_get_capabilities
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
@ 2016-06-02 22:52 ` Tamas K Lengyel
2016-06-17 19:07 ` Tamas K Lengyel
2016-06-21 9:20 ` Julien Grall
2016-06-02 22:52 ` [PATCH v5 3/9] monitor: Rename vm_event_monitor_guest_request Tamas K Lengyel
` (9 subsequent siblings)
10 siblings, 2 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-02 22:52 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, Tamas K Lengyel, Stefano Stabellini, Jan Beulich
The monitor_get_capabilities check actually belongs to the monitor subsystem so
relocating and renaming it to sanitize the code's name and location. Mechanical
patch, no code changes introduced.
Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
xen/arch/x86/monitor.c | 2 +-
xen/common/monitor.c | 5 ++---
xen/include/asm-arm/monitor.h | 11 ++++++++++-
xen/include/asm-arm/vm_event.h | 9 ---------
xen/include/asm-x86/monitor.h | 23 +++++++++++++++++++++++
xen/include/asm-x86/vm_event.h | 23 -----------------------
6 files changed, 36 insertions(+), 37 deletions(-)
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 1fec412..621f91a 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -126,7 +126,7 @@ int arch_monitor_domctl_event(struct domain *d,
default:
/*
- * Should not be reached unless vm_event_monitor_get_capabilities() is
+ * Should not be reached unless arch_monitor_get_capabilities() is
* not properly implemented.
*/
ASSERT_UNREACHABLE();
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index d950a7c..7c3d1c8 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -24,7 +24,6 @@
#include <xsm/xsm.h>
#include <public/domctl.h>
#include <asm/monitor.h>
-#include <asm/vm_event.h>
int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
{
@@ -48,12 +47,12 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
if ( unlikely(mop->event > 31) )
return -EINVAL;
/* Check if event type is available. */
- if ( unlikely(!(vm_event_monitor_get_capabilities(d) & (1U << mop->event))) )
+ if ( unlikely(!(arch_monitor_get_capabilities(d) & (1U << mop->event))) )
return -EOPNOTSUPP;
break;
case XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES:
- mop->event = vm_event_monitor_get_capabilities(d);
+ mop->event = arch_monitor_get_capabilities(d);
return 0;
default:
diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h
index 6e36e99..3fd3c9d 100644
--- a/xen/include/asm-arm/monitor.h
+++ b/xen/include/asm-arm/monitor.h
@@ -39,11 +39,20 @@ int arch_monitor_domctl_event(struct domain *d,
/*
* No arch-specific monitor vm-events on ARM.
*
- * Should not be reached unless vm_event_monitor_get_capabilities() is not
+ * Should not be reached unless arch_monitor_get_capabilities() is not
* properly implemented.
*/
ASSERT_UNREACHABLE();
return -EOPNOTSUPP;
}
+static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
+{
+ uint32_t capabilities = 0;
+
+ capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+
+ return capabilities;
+}
+
#endif /* __ASM_ARM_MONITOR_H__ */
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index 014d9ba..a3fc4ce 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -59,13 +59,4 @@ static inline void vm_event_fill_regs(vm_event_request_t *req)
/* Not supported on ARM. */
}
-static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d)
-{
- uint32_t capabilities = 0;
-
- capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
-
- return capabilities;
-}
-
#endif /* __ASM_ARM_VM_EVENT_H__ */
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index d367099..0fee750 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -60,4 +60,27 @@ int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
int arch_monitor_domctl_event(struct domain *d,
struct xen_domctl_monitor_op *mop);
+static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
+{
+ uint32_t capabilities = 0;
+
+ /*
+ * At the moment only Intel HVM domains are supported. However, event
+ * delivery could be extended to AMD and PV domains.
+ */
+ if ( !is_hvm_domain(d) || !cpu_has_vmx )
+ return capabilities;
+
+ capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
+ (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
+ (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
+ (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+
+ /* Since we know this is on VMX, we can just call the hvm func */
+ if ( hvm_is_singlestep_supported() )
+ capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
+
+ return capabilities;
+}
+
#endif /* __ASM_X86_MONITOR_H__ */
diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
index 0470240..026f42e 100644
--- a/xen/include/asm-x86/vm_event.h
+++ b/xen/include/asm-x86/vm_event.h
@@ -44,27 +44,4 @@ void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp);
void vm_event_fill_regs(vm_event_request_t *req);
-static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d)
-{
- uint32_t capabilities = 0;
-
- /*
- * At the moment only Intel HVM domains are supported. However, event
- * delivery could be extended to AMD and PV domains.
- */
- if ( !is_hvm_domain(d) || !cpu_has_vmx )
- return capabilities;
-
- capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
- (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
- (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
- (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
-
- /* Since we know this is on VMX, we can just call the hvm func */
- if ( hvm_is_singlestep_supported() )
- capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
-
- return capabilities;
-}
-
#endif /* __ASM_X86_VM_EVENT_H__ */
--
2.8.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v5 3/9] monitor: Rename vm_event_monitor_guest_request
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 2/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
@ 2016-06-02 22:52 ` Tamas K Lengyel
2016-06-17 19:10 ` Tamas K Lengyel
2016-06-21 9:18 ` Julien Grall
2016-06-02 22:52 ` [PATCH v5 4/9] monitor: Rename hvm/event to hvm/monitor Tamas K Lengyel
` (8 subsequent siblings)
10 siblings, 2 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-02 22:52 UTC (permalink / raw)
To: xen-devel
Cc: Andrew Cooper, Julien Grall, Tamas K Lengyel, Stefano Stabellini
Mechanical renaming and relocation to the monitor subsystem.
Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
xen/arch/arm/hvm.c | 4 ++--
xen/arch/x86/hvm/hvm.c | 3 ++-
xen/common/monitor.c | 17 +++++++++++++++++
xen/common/vm_event.c | 16 ----------------
xen/include/xen/monitor.h | 1 +
xen/include/xen/vm_event.h | 2 --
6 files changed, 22 insertions(+), 21 deletions(-)
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/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 5040a5c..7bf6a36 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>
@@ -5704,7 +5705,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/common/monitor.c b/xen/common/monitor.c
index 7c3d1c8..436214a 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -21,6 +21,7 @@
#include <xen/monitor.h>
#include <xen/sched.h>
+#include <xen/vm_event.h>
#include <xsm/xsm.h>
#include <public/domctl.h>
#include <asm/monitor.h>
@@ -84,6 +85,22 @@ int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
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_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 fe86fb9..068d587 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -825,22 +825,6 @@ int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
return rc;
}
-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__ */
--
2.8.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v5 4/9] monitor: Rename hvm/event to hvm/monitor
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 2/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 3/9] monitor: Rename vm_event_monitor_guest_request Tamas K Lengyel
@ 2016-06-02 22:52 ` Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 5/9] monitor: ARM SMC events Tamas K Lengyel
` (7 subsequent siblings)
10 siblings, 0 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-02 22:52 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Tamas K Lengyel, Jun Nakajima
Mechanical renaming to better describe that the code in hvm/event is part of
the monitor subsystem.
Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
---
xen/arch/x86/hvm/Makefile | 2 +-
xen/arch/x86/hvm/hvm.c | 12 +++++------
xen/arch/x86/hvm/{event.c => monitor.c} | 17 ++++++++-------
xen/arch/x86/hvm/vmx/vmx.c | 13 +++++------
xen/include/asm-x86/hvm/{event.h => monitor.h} | 30 +++++++++++++-------------
5 files changed, 38 insertions(+), 36 deletions(-)
rename xen/arch/x86/hvm/{event.c => monitor.c} (88%)
rename xen/include/asm-x86/hvm/{event.h => monitor.h} (59%)
diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
index 8bc55a9..f750d13 100644
--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -3,7 +3,6 @@ subdir-y += vmx
obj-y += asid.o
obj-y += emulate.o
-obj-y += event.o
obj-y += hpet.o
obj-y += hvm.o
obj-y += i8254.o
@@ -11,6 +10,7 @@ obj-y += intercept.o
obj-y += io.o
obj-y += ioreq.o
obj-y += irq.o
+obj-y += monitor.o
obj-y += mtrr.o
obj-y += nestedhvm.o
obj-y += pmtimer.o
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 7bf6a36..c4089be 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -60,7 +60,7 @@
#include <asm/hvm/cacheattr.h>
#include <asm/hvm/trace.h>
#include <asm/hvm/nestedhvm.h>
-#include <asm/hvm/event.h>
+#include <asm/hvm/monitor.h>
#include <asm/hvm/ioreq.h>
#include <asm/hvm/vmx/vmx.h>
#include <asm/altp2m.h>
@@ -1961,7 +1961,7 @@ int hvm_handle_xsetbv(u32 index, u64 new_bv)
{
int rc;
- hvm_event_crX(XCR0, new_bv, current->arch.xcr0);
+ hvm_monitor_crX(XCR0, new_bv, current->arch.xcr0);
rc = handle_xsetbv(index, new_bv);
if ( rc )
@@ -2197,7 +2197,7 @@ int hvm_set_cr0(unsigned long value, bool_t may_defer)
{
ASSERT(v->arch.vm_event);
- if ( hvm_event_crX(CR0, value, old_value) )
+ if ( hvm_monitor_crX(CR0, value, old_value) )
{
/* The actual write will occur in hvm_do_resume(), if permitted. */
v->arch.vm_event->write_data.do_write.cr0 = 1;
@@ -2299,7 +2299,7 @@ int hvm_set_cr3(unsigned long value, bool_t may_defer)
{
ASSERT(v->arch.vm_event);
- if ( hvm_event_crX(CR3, value, old) )
+ if ( hvm_monitor_crX(CR3, value, old) )
{
/* The actual write will occur in hvm_do_resume(), if permitted. */
v->arch.vm_event->write_data.do_write.cr3 = 1;
@@ -2379,7 +2379,7 @@ int hvm_set_cr4(unsigned long value, bool_t may_defer)
{
ASSERT(v->arch.vm_event);
- if ( hvm_event_crX(CR4, value, old_cr) )
+ if ( hvm_monitor_crX(CR4, value, old_cr) )
{
/* The actual write will occur in hvm_do_resume(), if permitted. */
v->arch.vm_event->write_data.do_write.cr4 = 1;
@@ -3722,7 +3722,7 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
v->arch.vm_event->write_data.msr = msr;
v->arch.vm_event->write_data.value = msr_content;
- hvm_event_msr(msr, msr_content);
+ hvm_monitor_msr(msr, msr_content);
return X86EMUL_OKAY;
}
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/monitor.c
similarity index 88%
rename from xen/arch/x86/hvm/event.c
rename to xen/arch/x86/hvm/monitor.c
index 5772c6b..764c3e8 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -1,5 +1,5 @@
/*
- * arch/x86/hvm/event.c
+ * arch/x86/hvm/monitor.c
*
* Arch-specific hardware virtual machine event abstractions.
*
@@ -7,6 +7,7 @@
* Copyright (c) 2005, International Business Machines Corporation.
* Copyright (c) 2008, Citrix Systems, Inc.
* Copyright (c) 2016, Bitdefender S.R.L.
+ * Copyright (c) 2016, Tamas K Lengyel (tamas@tklengyel.com)
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -22,12 +23,12 @@
*/
#include <xen/vm_event.h>
-#include <asm/hvm/event.h>
+#include <asm/hvm/monitor.h>
#include <asm/monitor.h>
#include <asm/vm_event.h>
#include <public/vm_event.h>
-bool_t hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
+bool_t hvm_monitor_cr(unsigned int index, unsigned long value, unsigned long old)
{
struct vcpu *curr = current;
struct arch_domain *ad = &curr->domain->arch;
@@ -54,7 +55,7 @@ bool_t hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
return 0;
}
-void hvm_event_msr(unsigned int msr, uint64_t value)
+void hvm_monitor_msr(unsigned int msr, uint64_t value)
{
struct vcpu *curr = current;
struct arch_domain *ad = &curr->domain->arch;
@@ -87,8 +88,8 @@ static inline unsigned long gfn_of_rip(unsigned long rip)
return paging_gva_to_gfn(curr, sreg.base + rip, &pfec);
}
-int hvm_event_breakpoint(unsigned long rip,
- enum hvm_event_breakpoint_type type)
+int hvm_monitor_breakpoint(unsigned long rip,
+ enum hvm_monitor_breakpoint_type type)
{
struct vcpu *curr = current;
struct arch_domain *ad = &curr->domain->arch;
@@ -96,14 +97,14 @@ int hvm_event_breakpoint(unsigned long rip,
switch ( type )
{
- case HVM_EVENT_SOFTWARE_BREAKPOINT:
+ case HVM_MONITOR_SOFTWARE_BREAKPOINT:
if ( !ad->monitor.software_breakpoint_enabled )
return 0;
req.reason = VM_EVENT_REASON_SOFTWARE_BREAKPOINT;
req.u.software_breakpoint.gfn = gfn_of_rip(rip);
break;
- case HVM_EVENT_SINGLESTEP_BREAKPOINT:
+ case HVM_MONITOR_SINGLESTEP_BREAKPOINT:
if ( !ad->monitor.singlestep_enabled )
return 0;
req.reason = VM_EVENT_REASON_SINGLESTEP;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 097d97c..4981574 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -50,7 +50,7 @@
#include <asm/hvm/vpt.h>
#include <public/hvm/save.h>
#include <asm/hvm/trace.h>
-#include <asm/hvm/event.h>
+#include <asm/hvm/monitor.h>
#include <asm/xenoprof.h>
#include <asm/debugger.h>
#include <asm/apic.h>
@@ -2477,10 +2477,10 @@ static int vmx_cr_access(unsigned long exit_qualification)
/*
* Special case unlikely to be interesting to a
- * VM_EVENT_FLAG_DENY-capable application, so the hvm_event_crX()
+ * VM_EVENT_FLAG_DENY-capable application, so the hvm_monitor_crX()
* return value is ignored for now.
*/
- hvm_event_crX(CR0, value, old);
+ hvm_monitor_crX(CR0, value, old);
curr->arch.hvm_vcpu.guest_cr[0] = value;
vmx_update_guest_cr(curr, 0);
HVMTRACE_0D(CLTS);
@@ -3393,8 +3393,8 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
}
else {
int rc =
- hvm_event_breakpoint(regs->eip,
- HVM_EVENT_SOFTWARE_BREAKPOINT);
+ hvm_monitor_breakpoint(regs->eip,
+ HVM_MONITOR_SOFTWARE_BREAKPOINT);
if ( !rc )
{
@@ -3721,7 +3721,8 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
vmx_update_cpu_exec_control(v);
if ( v->arch.hvm_vcpu.single_step )
{
- hvm_event_breakpoint(regs->eip, HVM_EVENT_SINGLESTEP_BREAKPOINT);
+ hvm_monitor_breakpoint(regs->eip,
+ HVM_MONITOR_SINGLESTEP_BREAKPOINT);
if ( v->domain->debugger_attached )
domain_pause_for_debugger();
}
diff --git a/xen/include/asm-x86/hvm/event.h b/xen/include/asm-x86/hvm/monitor.h
similarity index 59%
rename from xen/include/asm-x86/hvm/event.h
rename to xen/include/asm-x86/hvm/monitor.h
index 03f7fee..55d435e 100644
--- a/xen/include/asm-x86/hvm/event.h
+++ b/xen/include/asm-x86/hvm/monitor.h
@@ -1,7 +1,7 @@
/*
- * include/asm-x86/hvm/event.h
+ * include/asm-x86/hvm/monitor.h
*
- * Arch-specific hardware virtual machine event abstractions.
+ * Arch-specific hardware virtual machine monitor abstractions.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -16,17 +16,17 @@
* this program; If not, see <http://www.gnu.org/licenses/>.
*/
-#ifndef __ASM_X86_HVM_EVENT_H__
-#define __ASM_X86_HVM_EVENT_H__
+#ifndef __ASM_X86_HVM_MONITOR_H__
+#define __ASM_X86_HVM_MONITOR_H__
#include <xen/sched.h>
#include <xen/paging.h>
#include <public/vm_event.h>
-enum hvm_event_breakpoint_type
+enum hvm_monitor_breakpoint_type
{
- HVM_EVENT_SOFTWARE_BREAKPOINT,
- HVM_EVENT_SINGLESTEP_BREAKPOINT,
+ HVM_MONITOR_SOFTWARE_BREAKPOINT,
+ HVM_MONITOR_SINGLESTEP_BREAKPOINT,
};
/*
@@ -34,15 +34,15 @@ enum hvm_event_breakpoint_type
* The event might not fire if the client has subscribed to it in onchangeonly
* mode, hence the bool_t return type for control register write events.
*/
-bool_t hvm_event_cr(unsigned int index, unsigned long value,
- unsigned long old);
-#define hvm_event_crX(cr, new, old) \
- hvm_event_cr(VM_EVENT_X86_##cr, new, old)
-void hvm_event_msr(unsigned int msr, uint64_t value);
-int hvm_event_breakpoint(unsigned long rip,
- enum hvm_event_breakpoint_type type);
+bool_t hvm_monitor_cr(unsigned int index, unsigned long value,
+ unsigned long old);
+#define hvm_monitor_crX(cr, new, old) \
+ hvm_monitor_cr(VM_EVENT_X86_##cr, new, old)
+void hvm_monitor_msr(unsigned int msr, uint64_t value);
+int hvm_monitor_breakpoint(unsigned long rip,
+ enum hvm_monitor_breakpoint_type type);
-#endif /* __ASM_X86_HVM_EVENT_H__ */
+#endif /* __ASM_X86_HVM_MONITOR_H__ */
/*
* Local variables:
--
2.8.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v5 5/9] monitor: ARM SMC events
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
` (2 preceding siblings ...)
2016-06-02 22:52 ` [PATCH v5 4/9] monitor: Rename hvm/event to hvm/monitor Tamas K Lengyel
@ 2016-06-02 22:52 ` Tamas K Lengyel
2016-06-03 9:49 ` Julien Grall
2016-06-02 22:52 ` [PATCH v5 6/9] arm/vm_event: get/set registers Tamas K Lengyel
` (6 subsequent siblings)
10 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-02 22:52 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, Tamas K Lengyel, Stefano Stabellini
Add support for monitoring ARM SMC events. This patch only adds the required
bits to enable/disable monitoring and forwarding the event through vm_event.
Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
v5: Add struct vm_event_privcall to hold the SMM call# (ESR_EL2.iss)
v4: Style fixes
v3: Split parts off as separate patches
Union for arm32/64 register structs in vm_event
Cosmetic fixes
---
xen/arch/arm/Makefile | 1 +
xen/arch/arm/monitor.c | 84 +++++++++++++++++++++++++++++++++++++++++++
xen/arch/arm/traps.c | 13 +++++--
xen/include/asm-arm/domain.h | 5 +++
xen/include/asm-arm/monitor.h | 24 ++++---------
xen/include/public/domctl.h | 1 +
xen/include/public/vm_event.h | 10 ++++++
7 files changed, 118 insertions(+), 20 deletions(-)
create mode 100644 xen/arch/arm/monitor.c
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index ead0cc0..344d3ad 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -41,6 +41,7 @@ obj-y += decode.o
obj-y += processor.o
obj-y += smc.o
obj-$(CONFIG_XSPLICE) += xsplice.o
+obj-y += monitor.o
#obj-bin-y += ....o
diff --git a/xen/arch/arm/monitor.c b/xen/arch/arm/monitor.c
new file mode 100644
index 0000000..90a13dd
--- /dev/null
+++ b/xen/arch/arm/monitor.c
@@ -0,0 +1,84 @@
+/*
+ * arch/arm/monitor.c
+ *
+ * Arch-specific monitor_op domctl handler.
+ *
+ * Copyright (c) 2016 Tamas K Lengyel (tamas@tklengyel.com)
+ *
+ * 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>
+
+int arch_monitor_domctl_event(struct domain *d,
+ struct xen_domctl_monitor_op *mop)
+{
+ struct arch_domain *ad = &d->arch;
+ bool_t requested_status = (XEN_DOMCTL_MONITOR_OP_ENABLE == mop->op);
+
+ switch ( mop->event )
+ {
+ case XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL:
+ {
+ bool_t old_status = ad->monitor.privileged_call_enabled;
+
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
+
+ domain_pause(d);
+ ad->monitor.privileged_call_enabled = requested_status;
+ domain_unpause(d);
+ break;
+ }
+
+ default:
+ /*
+ * Should not be reached unless arch_monitor_get_capabilities() is
+ * not properly implemented.
+ */
+ ASSERT_UNREACHABLE();
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+bool_t monitor_smc(unsigned long iss, const struct cpu_user_regs *regs)
+{
+ struct vcpu *curr = current;
+
+ if ( curr->domain->arch.monitor.privileged_call_enabled )
+ {
+ vm_event_request_t req = { 0 };
+
+ req.reason = VM_EVENT_REASON_PRIVILEGED_CALL;
+ req.vcpu_id = curr->vcpu_id;
+ req.u.privcall.type = VM_EVENT_PRIVCALL_SMC;
+ req.u.privcall.vector = iss;
+
+ if ( vm_event_monitor_traps(curr, 1, &req) > 0 )
+ return 1;
+ }
+
+ return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index aa3e3c2..965c151 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -42,6 +42,7 @@
#include <asm/mmio.h>
#include <asm/cpufeature.h>
#include <asm/flushtlb.h>
+#include <asm/monitor.h>
#include "decode.h"
#include "vtimer.h"
@@ -2506,6 +2507,14 @@ bad_data_abort:
inject_dabt_exception(regs, info.gva, hsr.len);
}
+static void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
+{
+ bool_t handled = monitor_smc(hsr.iss, regs);
+
+ if ( !handled )
+ inject_undef_exception(regs, hsr);
+}
+
static void enter_hypervisor_head(struct cpu_user_regs *regs)
{
if ( guest_mode(regs) )
@@ -2581,7 +2590,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
*/
GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
perfc_incr(trap_smc32);
- inject_undef32_exception(regs);
+ do_trap_smc(regs, hsr);
break;
case HSR_EC_HVC32:
GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
@@ -2614,7 +2623,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
*/
GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
perfc_incr(trap_smc64);
- inject_undef64_exception(regs, hsr.len);
+ do_trap_smc(regs, hsr);
break;
case HSR_EC_SYSREG:
GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 370cdeb..ed56fc9 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -127,6 +127,11 @@ struct arch_domain
paddr_t efi_acpi_gpa;
paddr_t efi_acpi_len;
#endif
+
+ /* Monitor options */
+ struct {
+ uint8_t privileged_call_enabled : 1;
+ } monitor;
} __cacheline_aligned;
struct arch_vcpu
diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h
index 3fd3c9d..0097be2 100644
--- a/xen/include/asm-arm/monitor.h
+++ b/xen/include/asm-arm/monitor.h
@@ -3,7 +3,7 @@
*
* Arch-specific monitor_op domctl handler.
*
- * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
+ * 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
@@ -32,27 +32,15 @@ int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
return -EOPNOTSUPP;
}
-static inline
int arch_monitor_domctl_event(struct domain *d,
- struct xen_domctl_monitor_op *mop)
-{
- /*
- * No arch-specific monitor vm-events on ARM.
- *
- * Should not be reached unless arch_monitor_get_capabilities() is not
- * properly implemented.
- */
- ASSERT_UNREACHABLE();
- return -EOPNOTSUPP;
-}
+ struct xen_domctl_monitor_op *mop);
static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
{
- uint32_t capabilities = 0;
-
- capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
-
- return capabilities;
+ return (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
+ (1U << XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL);
}
+bool_t monitor_smc(unsigned long iss, const struct cpu_user_regs *regs);
+
#endif /* __ASM_ARM_MONITOR_H__ */
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 2457698..35adce2 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1080,6 +1080,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
#define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2
#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3
#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
+#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 5
struct xen_domctl_monitor_op {
uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 9270d52..7976080 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -119,6 +119,8 @@
#define VM_EVENT_REASON_SINGLESTEP 7
/* An event has been requested via HVMOP_guest_request_vm_event. */
#define VM_EVENT_REASON_GUEST_REQUEST 8
+/* Privileged call executed (e.g. SMC) */
+#define VM_EVENT_REASON_PRIVILEGED_CALL 9
/* Supported values for the vm_event_write_ctrlreg index. */
#define VM_EVENT_X86_CR0 0
@@ -212,6 +214,13 @@ struct vm_event_mov_to_msr {
uint64_t value;
};
+#define VM_EVENT_PRIVCALL_SMC 0
+
+struct vm_event_privcall {
+ uint32_t type;
+ uint32_t vector; /* ESR_EL2.ISS for SMC calls */
+};
+
#define MEM_PAGING_DROP_PAGE (1 << 0)
#define MEM_PAGING_EVICT_FAIL (1 << 1)
@@ -249,6 +258,7 @@ typedef struct vm_event_st {
struct vm_event_mov_to_msr mov_to_msr;
struct vm_event_debug software_breakpoint;
struct vm_event_debug singlestep;
+ struct vm_event_privcall privcall;
} u;
union {
--
2.8.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v5 6/9] arm/vm_event: get/set registers
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
` (3 preceding siblings ...)
2016-06-02 22:52 ` [PATCH v5 5/9] monitor: ARM SMC events Tamas K Lengyel
@ 2016-06-02 22:52 ` Tamas K Lengyel
2016-06-03 10:34 ` Jan Beulich
2016-06-02 22:52 ` [PATCH v5 7/9] tools/libxc: add xc_monitor_privileged_call Tamas K Lengyel
` (5 subsequent siblings)
10 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-02 22:52 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, Tamas K Lengyel, Stefano Stabellini
Add support for getting/setting registers through vm_event on ARM.
The set of registers can be expanded in the future to include other registers
as well if required. The set is limited to the GPRs, PC, CPSR and TTBR0/1 in
this patch.
Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
v5: Use x29/x30 as names instead of the Xen internal names on 64-bit
Transmit all GPRs on 64-bit
v4: Use psr mode to determine whether to full 32-bit or 64-bit structs
---
xen/arch/arm/Makefile | 1 +
xen/arch/arm/vm_event.c | 159 +++++++++++++++++++++++++++++++++++++++++
xen/include/asm-arm/vm_event.h | 11 ---
xen/include/asm-x86/vm_event.h | 4 --
xen/include/public/vm_event.h | 74 ++++++++++++++++++-
xen/include/xen/vm_event.h | 3 +
6 files changed, 234 insertions(+), 18 deletions(-)
create mode 100644 xen/arch/arm/vm_event.c
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 344d3ad..7d2641c 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -42,6 +42,7 @@ obj-y += processor.o
obj-y += smc.o
obj-$(CONFIG_XSPLICE) += xsplice.o
obj-y += monitor.o
+obj-y += vm_event.o
#obj-bin-y += ....o
diff --git a/xen/arch/arm/vm_event.c b/xen/arch/arm/vm_event.c
new file mode 100644
index 0000000..6e92f8b
--- /dev/null
+++ b/xen/arch/arm/vm_event.c
@@ -0,0 +1,159 @@
+/*
+ * arch/arm/vm_event.c
+ *
+ * Architecture-specific vm_event handling routines
+ *
+ * Copyright (c) 2016 Tamas K Lengyel (tamas@tklengyel.com)
+ *
+ * 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 <xen/sched.h>
+#include <asm/vm_event.h>
+
+void vm_event_fill_regs(vm_event_request_t *req)
+{
+ const struct cpu_user_regs *regs = guest_cpu_user_regs();
+
+ req->data.regs.arm.cpsr = regs->cpsr;
+ req->data.regs.arm.ttbr0 = READ_SYSREG64(TTBR0_EL1);
+ req->data.regs.arm.ttbr1 = READ_SYSREG64(TTBR1_EL1);
+
+ if ( psr_mode_is_32bit(regs->cpsr) )
+ {
+ req->data.regs.arm.arch.arm32.r0 = regs->r0;
+ req->data.regs.arm.arch.arm32.r1 = regs->r1;
+ req->data.regs.arm.arch.arm32.r2 = regs->r2;
+ req->data.regs.arm.arch.arm32.r3 = regs->r3;
+ req->data.regs.arm.arch.arm32.r4 = regs->r4;
+ req->data.regs.arm.arch.arm32.r5 = regs->r5;
+ req->data.regs.arm.arch.arm32.r6 = regs->r6;
+ req->data.regs.arm.arch.arm32.r7 = regs->r7;
+ req->data.regs.arm.arch.arm32.r8 = regs->r8;
+ req->data.regs.arm.arch.arm32.r9 = regs->r9;
+ req->data.regs.arm.arch.arm32.r10 = regs->r10;
+ req->data.regs.arm.arch.arm32.r11 = regs->fp;
+ req->data.regs.arm.arch.arm32.r12 = regs->r12;
+ req->data.regs.arm.arch.arm32.pc = regs->pc32;
+ }
+#ifdef CONFIG_ARM_64
+ else
+ {
+ req->data.regs.arm.arch.arm64.x0 = regs->x0;
+ req->data.regs.arm.arch.arm64.x1 = regs->x1;
+ req->data.regs.arm.arch.arm64.x2 = regs->x2;
+ req->data.regs.arm.arch.arm64.x3 = regs->x3;
+ req->data.regs.arm.arch.arm64.x4 = regs->x4;
+ req->data.regs.arm.arch.arm64.x5 = regs->x5;
+ req->data.regs.arm.arch.arm64.x6 = regs->x6;
+ req->data.regs.arm.arch.arm64.x7 = regs->x7;
+ req->data.regs.arm.arch.arm64.x8 = regs->x8;
+ req->data.regs.arm.arch.arm64.x9 = regs->x9;
+ req->data.regs.arm.arch.arm64.x10 = regs->x10;
+ req->data.regs.arm.arch.arm64.x11 = regs->x11;
+ req->data.regs.arm.arch.arm64.x12 = regs->x12;
+ req->data.regs.arm.arch.arm64.x13 = regs->x13;
+ req->data.regs.arm.arch.arm64.x14 = regs->x14;
+ req->data.regs.arm.arch.arm64.x15 = regs->x15;
+ req->data.regs.arm.arch.arm64.x16 = regs->x16;
+ req->data.regs.arm.arch.arm64.x17 = regs->x17;
+ req->data.regs.arm.arch.arm64.x18 = regs->x18;
+ req->data.regs.arm.arch.arm64.x19 = regs->x19;
+ req->data.regs.arm.arch.arm64.x20 = regs->x20;
+ req->data.regs.arm.arch.arm64.x21 = regs->x21;
+ req->data.regs.arm.arch.arm64.x22 = regs->x22;
+ req->data.regs.arm.arch.arm64.x23 = regs->x23;
+ req->data.regs.arm.arch.arm64.x24 = regs->x24;
+ req->data.regs.arm.arch.arm64.x25 = regs->x25;
+ req->data.regs.arm.arch.arm64.x26 = regs->x26;
+ req->data.regs.arm.arch.arm64.x27 = regs->x27;
+ req->data.regs.arm.arch.arm64.x28 = regs->x28;
+ req->data.regs.arm.arch.arm64.x29 = regs->fp;
+ req->data.regs.arm.arch.arm64.x30 = regs->lr;
+ req->data.regs.arm.arch.arm64.pc = regs->pc;
+ }
+#endif
+}
+
+void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp)
+{
+ struct cpu_user_regs *regs = &v->arch.cpu_info->guest_cpu_user_regs;
+
+ regs->cpsr = rsp->data.regs.arm.cpsr;
+ v->arch.ttbr0 = rsp->data.regs.arm.ttbr0;
+ v->arch.ttbr1 = rsp->data.regs.arm.ttbr1;
+
+ if ( psr_mode_is_32bit(regs->cpsr) )
+ {
+ regs->r0 = rsp->data.regs.arm.arch.arm32.r0;
+ regs->r1 = rsp->data.regs.arm.arch.arm32.r1;
+ regs->r2 = rsp->data.regs.arm.arch.arm32.r2;
+ regs->r3 = rsp->data.regs.arm.arch.arm32.r3;
+ regs->r4 = rsp->data.regs.arm.arch.arm32.r4;
+ regs->r5 = rsp->data.regs.arm.arch.arm32.r5;
+ regs->r6 = rsp->data.regs.arm.arch.arm32.r6;
+ regs->r7 = rsp->data.regs.arm.arch.arm32.r7;
+ regs->r8 = rsp->data.regs.arm.arch.arm32.r8;
+ regs->r9 = rsp->data.regs.arm.arch.arm32.r9;
+ regs->r10 = rsp->data.regs.arm.arch.arm32.r10;
+ regs->fp = rsp->data.regs.arm.arch.arm32.r11;
+ regs->r12 = rsp->data.regs.arm.arch.arm32.r12;
+ regs->pc32 = rsp->data.regs.arm.arch.arm32.pc;
+ }
+#ifdef CONFIG_ARM_64
+ else
+ {
+ regs->x0 = rsp->data.regs.arm.arch.arm64.x0;
+ regs->x1 = rsp->data.regs.arm.arch.arm64.x1;
+ regs->x2 = rsp->data.regs.arm.arch.arm64.x2;
+ regs->x3 = rsp->data.regs.arm.arch.arm64.x3;
+ regs->x4 = rsp->data.regs.arm.arch.arm64.x4;
+ regs->x5 = rsp->data.regs.arm.arch.arm64.x5;
+ regs->x6 = rsp->data.regs.arm.arch.arm64.x6;
+ regs->x7 = rsp->data.regs.arm.arch.arm64.x7;
+ regs->x8 = rsp->data.regs.arm.arch.arm64.x8;
+ regs->x9 = rsp->data.regs.arm.arch.arm64.x9;
+ regs->x10 = rsp->data.regs.arm.arch.arm64.x10;
+ regs->x11 = rsp->data.regs.arm.arch.arm64.x11;
+ regs->x12 = rsp->data.regs.arm.arch.arm64.x12;
+ regs->x13 = rsp->data.regs.arm.arch.arm64.x13;
+ regs->x14 = rsp->data.regs.arm.arch.arm64.x14;
+ regs->x15 = rsp->data.regs.arm.arch.arm64.x15;
+ regs->x16 = rsp->data.regs.arm.arch.arm64.x16;
+ regs->x17 = rsp->data.regs.arm.arch.arm64.x17;
+ regs->x18 = rsp->data.regs.arm.arch.arm64.x18;
+ regs->x19 = rsp->data.regs.arm.arch.arm64.x19;
+ regs->x20 = rsp->data.regs.arm.arch.arm64.x20;
+ regs->x21 = rsp->data.regs.arm.arch.arm64.x21;
+ regs->x22 = rsp->data.regs.arm.arch.arm64.x22;
+ regs->x23 = rsp->data.regs.arm.arch.arm64.x23;
+ regs->x24 = rsp->data.regs.arm.arch.arm64.x24;
+ regs->x25 = rsp->data.regs.arm.arch.arm64.x25;
+ regs->x26 = rsp->data.regs.arm.arch.arm64.x26;
+ regs->x27 = rsp->data.regs.arm.arch.arm64.x27;
+ regs->x28 = rsp->data.regs.arm.arch.arm64.x28;
+ regs->fp = rsp->data.regs.arm.arch.arm64.x29;
+ regs->lr = rsp->data.regs.arm.arch.arm64.x30;
+ regs->pc = rsp->data.regs.arm.arch.arm64.pc;
+ }
+#endif
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index a3fc4ce..a4922b3 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -48,15 +48,4 @@ void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
/* Not supported on ARM. */
}
-static inline
-void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp)
-{
- /* Not supported on ARM. */
-}
-
-static inline void vm_event_fill_regs(vm_event_request_t *req)
-{
- /* Not supported on ARM. */
-}
-
#endif /* __ASM_ARM_VM_EVENT_H__ */
diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
index 026f42e..cf2077c 100644
--- a/xen/include/asm-x86/vm_event.h
+++ b/xen/include/asm-x86/vm_event.h
@@ -40,8 +40,4 @@ void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v);
void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp);
-void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp);
-
-void vm_event_fill_regs(vm_event_request_t *req);
-
#endif /* __ASM_X86_VM_EVENT_H__ */
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 7976080..31801b2 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -129,8 +129,8 @@
#define VM_EVENT_X86_XCR0 3
/*
- * Using a custom struct (not hvm_hw_cpu) so as to not fill
- * the vm_event ring buffer too quickly.
+ * Using custom vCPU structs (i.e. not hvm_hw_cpu) for both x86 and ARM
+ * so as to not fill the vm_event ring buffer too quickly.
*/
struct vm_event_regs_x86 {
uint64_t rax;
@@ -168,6 +168,69 @@ struct vm_event_regs_x86 {
uint32_t _pad;
};
+struct vm_event_regs_arm32 {
+ uint32_t r0;
+ uint32_t r1;
+ uint32_t r2;
+ uint32_t r3;
+ uint32_t r4;
+ uint32_t r5;
+ uint32_t r6;
+ uint32_t r7;
+ uint32_t r8;
+ uint32_t r9;
+ uint32_t r10;
+ uint32_t r11;
+ uint32_t r12;
+ uint32_t pc;
+};
+
+struct vm_event_regs_arm64 {
+ uint64_t x0;
+ uint64_t x1;
+ uint64_t x2;
+ uint64_t x3;
+ uint64_t x4;
+ uint64_t x5;
+ uint64_t x6;
+ uint64_t x7;
+ uint64_t x8;
+ uint64_t x9;
+ uint64_t x10;
+ uint64_t x11;
+ uint64_t x12;
+ uint64_t x13;
+ uint64_t x14;
+ uint64_t x15;
+ uint64_t x16;
+ uint64_t x17;
+ uint64_t x18;
+ uint64_t x19;
+ uint64_t x20;
+ uint64_t x21;
+ uint64_t x22;
+ uint64_t x23;
+ uint64_t x24;
+ uint64_t x25;
+ uint64_t x26;
+ uint64_t x27;
+ uint64_t x28;
+ uint64_t x29;
+ uint64_t x30;
+ uint64_t pc;
+};
+
+struct vm_event_regs_arm {
+ uint32_t cpsr; /* PSR_MODE_BIT is set iff arm32 is used below */
+ uint32_t _pad;
+ uint64_t ttbr0;
+ uint64_t ttbr1;
+ union {
+ struct vm_event_regs_arm32 arm32;
+ struct vm_event_regs_arm64 arm64;
+ } arch;
+};
+
/*
* mem_access flag definitions
*
@@ -236,10 +299,14 @@ struct vm_event_sharing {
uint32_t _pad;
};
+#define VM_EVENT_MAX_DATA_SIZE \
+ (sizeof(struct vm_event_regs_x86) > sizeof(struct vm_event_regs_arm) ? \
+ sizeof(struct vm_event_regs_x86) : sizeof(struct vm_event_regs_arm))
+
struct vm_event_emul_read_data {
uint32_t size;
/* The struct is used in a union with vm_event_regs_x86. */
- uint8_t data[sizeof(struct vm_event_regs_x86) - sizeof(uint32_t)];
+ uint8_t data[VM_EVENT_MAX_DATA_SIZE - sizeof(uint32_t)];
};
typedef struct vm_event_st {
@@ -264,6 +331,7 @@ typedef struct vm_event_st {
union {
union {
struct vm_event_regs_x86 x86;
+ struct vm_event_regs_arm arm;
} regs;
struct vm_event_emul_read_data emul_read_data;
diff --git a/xen/include/xen/vm_event.h b/xen/include/xen/vm_event.h
index 89e6243..a5767ab 100644
--- a/xen/include/xen/vm_event.h
+++ b/xen/include/xen/vm_event.h
@@ -75,6 +75,9 @@ int vm_event_domctl(struct domain *d, xen_domctl_vm_event_op_t *vec,
void vm_event_vcpu_pause(struct vcpu *v);
void vm_event_vcpu_unpause(struct vcpu *v);
+void vm_event_fill_regs(vm_event_request_t *req);
+void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp);
+
/*
* Monitor vm-events
*/
--
2.8.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v5 7/9] tools/libxc: add xc_monitor_privileged_call
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
` (4 preceding siblings ...)
2016-06-02 22:52 ` [PATCH v5 6/9] arm/vm_event: get/set registers Tamas K Lengyel
@ 2016-06-02 22:52 ` Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 8/9] x86/vm_event: Add HVM debug exception vm_events Tamas K Lengyel
` (4 subsequent siblings)
10 siblings, 0 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-02 22:52 UTC (permalink / raw)
To: xen-devel; +Cc: Tamas K Lengyel, Ian Jackson
These are the user-space components for the new ARM SMC events.
Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxc/include/xenctrl.h | 2 ++
tools/libxc/xc_monitor.c | 24 ++++++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index dc54612..3085130 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2160,6 +2160,8 @@ int xc_monitor_software_breakpoint(xc_interface *xch, domid_t domain_id,
bool enable);
int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
bool enable, bool sync);
+int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
+ bool enable);
/**
* This function enables / disables emulation for each REP for a
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index b1705dd..1e4a9d2 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -156,3 +156,27 @@ int xc_monitor_emulate_each_rep(xc_interface *xch, domid_t domain_id,
return do_domctl(xch, &domctl);
}
+
+int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
+ bool enable)
+{
+ DECLARE_DOMCTL;
+
+ domctl.cmd = XEN_DOMCTL_monitor_op;
+ domctl.domain = domain_id;
+ domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+ : XEN_DOMCTL_MONITOR_OP_DISABLE;
+ domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL;
+
+ return do_domctl(xch, &domctl);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.8.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v5 8/9] x86/vm_event: Add HVM debug exception vm_events
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
` (5 preceding siblings ...)
2016-06-02 22:52 ` [PATCH v5 7/9] tools/libxc: add xc_monitor_privileged_call Tamas K Lengyel
@ 2016-06-02 22:52 ` Tamas K Lengyel
2016-06-03 10:49 ` Jan Beulich
2016-06-02 22:52 ` [PATCH v5 9/9] tools/xen-access: add test-case for ARM SMC Tamas K Lengyel
` (3 subsequent siblings)
10 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-02 22:52 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Tamas K Lengyel, Wei Liu, Jun Nakajima,
Razvan Cojocaru, Andrew Cooper, Ian Jackson, Jan Beulich
Since in-guest debug exceptions are now unconditionally trapped to Xen, adding
a hook for vm_event subscribers to tap into this new always-on guest event. We
rename along the way hvm_event_breakpoint_type to hvm_event_type to better
match the various events that can be passed with it. We also introduce the
necessary monitor_op domctl's to enable subscribing to the events.
Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
v5: Transmit the proper insn_length for int3 events as well to fix
the current bug with prefixed int3 instructions.
---
tools/libxc/include/xenctrl.h | 3 +-
tools/libxc/xc_monitor.c | 15 +++++++++
tools/tests/xen-access/xen-access.c | 63 ++++++++++++++++++++++++++++++++-----
xen/arch/x86/hvm/monitor.c | 21 +++++++++++--
xen/arch/x86/hvm/vmx/vmx.c | 47 +++++++++++++++++++++------
xen/arch/x86/monitor.c | 16 ++++++++++
xen/include/asm-x86/domain.h | 2 ++
xen/include/asm-x86/hvm/monitor.h | 7 +++--
xen/include/asm-x86/monitor.h | 3 +-
xen/include/public/domctl.h | 6 ++++
xen/include/public/vm_event.h | 14 +++++++--
11 files changed, 169 insertions(+), 28 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 3085130..29d983e 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2162,7 +2162,8 @@ int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
bool enable, bool sync);
int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
bool enable);
-
+int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
+ bool enable, bool sync);
/**
* This function enables / disables emulation for each REP for a
* REP-compatible instruction.
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index 1e4a9d2..b0bf708 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -171,6 +171,21 @@ int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
return do_domctl(xch, &domctl);
}
+int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
+ bool enable, bool sync)
+{
+ DECLARE_DOMCTL;
+
+ domctl.cmd = XEN_DOMCTL_monitor_op;
+ domctl.domain = domain_id;
+ domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+ : XEN_DOMCTL_MONITOR_OP_DISABLE;
+ domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION;
+ domctl.u.monitor_op.u.debug_exception.sync = sync;
+
+ return do_domctl(xch, &domctl);
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index f26e723..e615476 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -53,6 +53,10 @@
#define ERROR(a, b...) fprintf(stderr, a "\n", ## b)
#define PERROR(a, b...) fprintf(stderr, a ": %s\n", ## b, strerror(errno))
+/* From xen/include/asm-x86/processor.h */
+#define X86_TRAP_DEBUG 1
+#define X86_TRAP_INT3 3
+
typedef struct vm_event {
domid_t domain_id;
xenevtchn_handle *xce_handle;
@@ -333,7 +337,7 @@ void usage(char* progname)
{
fprintf(stderr, "Usage: %s [-m] <domain_id> write|exec", progname);
#if defined(__i386__) || defined(__x86_64__)
- fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec");
+ fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug");
#endif
fprintf(stderr,
"\n"
@@ -354,10 +358,12 @@ int main(int argc, char *argv[])
xc_interface *xch;
xenmem_access_t default_access = XENMEM_access_rwx;
xenmem_access_t after_first_access = XENMEM_access_rwx;
+ int memaccess = 0;
int required = 0;
int breakpoint = 0;
int shutting_down = 0;
int altp2m = 0;
+ int debug = 0;
uint16_t altp2m_view_id = 0;
char* progname = argv[0];
@@ -391,11 +397,13 @@ int main(int argc, char *argv[])
{
default_access = XENMEM_access_rx;
after_first_access = XENMEM_access_rwx;
+ memaccess = 1;
}
else if ( !strcmp(argv[0], "exec") )
{
default_access = XENMEM_access_rw;
after_first_access = XENMEM_access_rwx;
+ memaccess = 1;
}
#if defined(__i386__) || defined(__x86_64__)
else if ( !strcmp(argv[0], "breakpoint") )
@@ -406,11 +414,17 @@ int main(int argc, char *argv[])
{
default_access = XENMEM_access_rx;
altp2m = 1;
+ memaccess = 1;
}
else if ( !strcmp(argv[0], "altp2m_exec") )
{
default_access = XENMEM_access_rw;
altp2m = 1;
+ memaccess = 1;
+ }
+ else if ( !strcmp(argv[0], "debug") )
+ {
+ debug = 1;
}
#endif
else
@@ -446,7 +460,7 @@ int main(int argc, char *argv[])
}
/* With altp2m we just create a new, restricted view of the memory */
- if ( altp2m )
+ if ( memaccess && altp2m )
{
xen_pfn_t gfn = 0;
unsigned long perm_set = 0;
@@ -493,7 +507,7 @@ int main(int argc, char *argv[])
}
}
- if ( !altp2m )
+ if ( memaccess && !altp2m )
{
/* Set the default access type and convert all pages to it */
rc = xc_set_mem_access(xch, domain_id, default_access, ~0ull, 0);
@@ -524,6 +538,16 @@ int main(int argc, char *argv[])
}
}
+ if ( debug )
+ {
+ rc = xc_monitor_debug_exceptions(xch, domain_id, 1, 1);
+ if ( rc < 0 )
+ {
+ ERROR("Error %d setting debug exception listener with vm_event\n", rc);
+ goto exit;
+ }
+ }
+
/* Wait for access */
for (;;)
{
@@ -534,6 +558,8 @@ int main(int argc, char *argv[])
if ( breakpoint )
rc = xc_monitor_software_breakpoint(xch, domain_id, 0);
+ if ( debug )
+ rc = xc_monitor_debug_exceptions(xch, domain_id, 0, 0);
if ( altp2m )
{
@@ -635,22 +661,22 @@ int main(int argc, char *argv[])
rsp.u.mem_access = req.u.mem_access;
break;
case VM_EVENT_REASON_SOFTWARE_BREAKPOINT:
- printf("Breakpoint: rip=%016"PRIx64", gfn=%"PRIx64" (vcpu %d)\n",
+ printf("Breakpoint: rip=%016"PRIx64" gfn=%"PRIx64" (vcpu %d)\n",
req.data.regs.x86.rip,
req.u.software_breakpoint.gfn,
req.vcpu_id);
/* Reinject */
- rc = xc_hvm_inject_trap(
- xch, domain_id, req.vcpu_id, 3,
- HVMOP_TRAP_sw_exc, -1, 0, 0);
+ rc = xc_hvm_inject_trap(xch, domain_id, req.vcpu_id,
+ X86_TRAP_INT3,
+ req.u.software_breakpoint.type, -1,
+ req.u.software_breakpoint.insn_length, 0);
if (rc < 0)
{
ERROR("Error %d injecting breakpoint\n", rc);
interrupted = -1;
continue;
}
-
break;
case VM_EVENT_REASON_SINGLESTEP:
printf("Singlestep: rip=%016"PRIx64", vcpu %d, altp2m %u\n",
@@ -669,6 +695,27 @@ int main(int argc, char *argv[])
rsp.flags |= VM_EVENT_FLAG_TOGGLE_SINGLESTEP;
break;
+ case VM_EVENT_REASON_DEBUG_EXCEPTION:
+ printf("Debug exception: rip=%016"PRIx64", vcpu %d. Type: %u. Length: %u\n",
+ req.data.regs.x86.rip,
+ req.vcpu_id,
+ req.u.debug_exception.type,
+ req.u.debug_exception.insn_length);
+
+ /* Reinject */
+ rc = xc_hvm_inject_trap(xch, domain_id, req.vcpu_id,
+ X86_TRAP_DEBUG,
+ req.u.debug_exception.type, -1,
+ req.u.debug_exception.insn_length,
+ req.data.regs.x86.cr2);
+ if (rc < 0)
+ {
+ ERROR("Error %d injecting breakpoint\n", rc);
+ interrupted = -1;
+ continue;
+ }
+
+ break;
default:
fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
}
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index 764c3e8..8458627 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -88,12 +88,13 @@ static inline unsigned long gfn_of_rip(unsigned long rip)
return paging_gva_to_gfn(curr, sreg.base + rip, &pfec);
}
-int hvm_monitor_breakpoint(unsigned long rip,
- enum hvm_monitor_breakpoint_type type)
+int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type,
+ unsigned long trap_type, unsigned long insn_length)
{
struct vcpu *curr = current;
struct arch_domain *ad = &curr->domain->arch;
vm_event_request_t req = {};
+ bool_t sync;
switch ( type )
{
@@ -102,6 +103,9 @@ int hvm_monitor_breakpoint(unsigned long rip,
return 0;
req.reason = VM_EVENT_REASON_SOFTWARE_BREAKPOINT;
req.u.software_breakpoint.gfn = gfn_of_rip(rip);
+ req.u.software_breakpoint.type = trap_type;
+ req.u.software_breakpoint.insn_length = insn_length;
+ sync = 1;
break;
case HVM_MONITOR_SINGLESTEP_BREAKPOINT:
@@ -109,6 +113,17 @@ int hvm_monitor_breakpoint(unsigned long rip,
return 0;
req.reason = VM_EVENT_REASON_SINGLESTEP;
req.u.singlestep.gfn = gfn_of_rip(rip);
+ sync = 1;
+ break;
+
+ case HVM_MONITOR_DEBUG_EXCEPTION:
+ if ( !ad->monitor.debug_exception_enabled )
+ return 0;
+ req.reason = VM_EVENT_REASON_DEBUG_EXCEPTION;
+ req.u.debug_exception.gfn = gfn_of_rip(rip);
+ req.u.debug_exception.type = trap_type;
+ req.u.debug_exception.insn_length = insn_length;
+ sync = !!ad->monitor.debug_exception_sync;
break;
default:
@@ -117,7 +132,7 @@ int hvm_monitor_breakpoint(unsigned long rip,
req.vcpu_id = curr->vcpu_id;
- return vm_event_monitor_traps(curr, 1, &req);
+ return vm_event_monitor_traps(curr, sync, &req);
}
/*
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 4981574..17cf1f7 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3377,10 +3377,33 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
if ( !v->domain->debugger_attached )
- vmx_propagate_intr(intr_info);
+ {
+ unsigned long insn_length = 0;
+ int rc;
+ unsigned long trap_type = MASK_EXTR(intr_info,
+ INTR_INFO_INTR_TYPE_MASK);
+
+ if( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
+ __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_length);
+
+ rc = hvm_monitor_debug(regs->eip,
+ HVM_MONITOR_DEBUG_EXCEPTION,
+ trap_type, insn_length);
+ if ( !rc )
+ {
+ vmx_propagate_intr(intr_info);
+ break;
+ }
+ else if ( rc > 0 )
+ break;
+ }
else
+ {
domain_pause_for_debugger();
- break;
+ break;
+ }
+
+ goto exit_and_crash;
case TRAP_int3:
{
HVMTRACE_1D(TRAP, vector);
@@ -3392,9 +3415,14 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
break;
}
else {
- int rc =
- hvm_monitor_breakpoint(regs->eip,
- HVM_MONITOR_SOFTWARE_BREAKPOINT);
+ unsigned long insn_len;
+ int rc;
+
+ __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len);
+ rc = hvm_monitor_debug(regs->eip,
+ HVM_MONITOR_SOFTWARE_BREAKPOINT,
+ X86_EVENTTYPE_SW_EXCEPTION,
+ insn_len);
if ( !rc )
{
@@ -3403,9 +3431,6 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
.type = X86_EVENTTYPE_SW_EXCEPTION,
.error_code = HVM_DELIVER_NO_ERROR_CODE,
};
- unsigned long insn_len;
-
- __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len);
trap.insn_len = insn_len;
hvm_inject_trap(&trap);
break;
@@ -3721,8 +3746,10 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
vmx_update_cpu_exec_control(v);
if ( v->arch.hvm_vcpu.single_step )
{
- hvm_monitor_breakpoint(regs->eip,
- HVM_MONITOR_SINGLESTEP_BREAKPOINT);
+ hvm_monitor_debug(regs->eip,
+ HVM_MONITOR_SINGLESTEP_BREAKPOINT,
+ 0, 0);
+
if ( v->domain->debugger_attached )
domain_pause_for_debugger();
}
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 621f91a..e8b79f6 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -124,6 +124,22 @@ int arch_monitor_domctl_event(struct domain *d,
break;
}
+ case XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION:
+ {
+ bool_t old_status = ad->monitor.debug_exception_enabled;
+
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
+
+ domain_pause(d);
+ ad->monitor.debug_exception_enabled = requested_status;
+ ad->monitor.debug_exception_sync = requested_status ?
+ mop->u.debug_exception.sync :
+ 0;
+ domain_unpause(d);
+ break;
+ }
+
default:
/*
* Should not be reached unless arch_monitor_get_capabilities() is
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 165e533..1cf97c3 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -405,6 +405,8 @@ struct arch_domain
unsigned int mov_to_msr_extended : 1;
unsigned int singlestep_enabled : 1;
unsigned int software_breakpoint_enabled : 1;
+ unsigned int debug_exception_enabled : 1;
+ unsigned int debug_exception_sync : 1;
} monitor;
/* Mem_access emulation control */
diff --git a/xen/include/asm-x86/hvm/monitor.h b/xen/include/asm-x86/hvm/monitor.h
index 55d435e..8b0f119 100644
--- a/xen/include/asm-x86/hvm/monitor.h
+++ b/xen/include/asm-x86/hvm/monitor.h
@@ -23,10 +23,11 @@
#include <xen/paging.h>
#include <public/vm_event.h>
-enum hvm_monitor_breakpoint_type
+enum hvm_monitor_debug_type
{
HVM_MONITOR_SOFTWARE_BREAKPOINT,
HVM_MONITOR_SINGLESTEP_BREAKPOINT,
+ HVM_MONITOR_DEBUG_EXCEPTION,
};
/*
@@ -39,8 +40,8 @@ bool_t hvm_monitor_cr(unsigned int index, unsigned long value,
#define hvm_monitor_crX(cr, new, old) \
hvm_monitor_cr(VM_EVENT_X86_##cr, new, old)
void hvm_monitor_msr(unsigned int msr, uint64_t value);
-int hvm_monitor_breakpoint(unsigned long rip,
- enum hvm_monitor_breakpoint_type type);
+int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type,
+ unsigned long trap_type, unsigned long insn_length);
#endif /* __ASM_X86_HVM_MONITOR_H__ */
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 0fee750..74a08f0f 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -74,7 +74,8 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
(1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
(1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
- (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+ (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
+ (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION);
/* Since we know this is on VMX, we can just call the hvm func */
if ( hvm_is_singlestep_supported() )
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 35adce2..b69b1bb 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1081,6 +1081,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
#define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3
#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 5
+#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION 6
struct xen_domctl_monitor_op {
uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
@@ -1116,6 +1117,11 @@ struct xen_domctl_monitor_op {
/* Pause vCPU until response */
uint8_t sync;
} guest_request;
+
+ struct {
+ /* Pause vCPU until response */
+ uint8_t sync;
+ } debug_exception;
} u;
};
typedef struct xen_domctl_monitor_op xen_domctl_monitor_op_t;
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 31801b2..729b73d 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -29,7 +29,7 @@
#include "xen.h"
-#define VM_EVENT_INTERFACE_VERSION 0x00000001
+#define VM_EVENT_INTERFACE_VERSION 0x00000002
#if defined(__XEN__) || defined(__XEN_TOOLS__)
@@ -121,6 +121,8 @@
#define VM_EVENT_REASON_GUEST_REQUEST 8
/* Privileged call executed (e.g. SMC) */
#define VM_EVENT_REASON_PRIVILEGED_CALL 9
+/* A debug exception was caught */
+#define VM_EVENT_REASON_DEBUG_EXCEPTION 10
/* Supported values for the vm_event_write_ctrlreg index. */
#define VM_EVENT_X86_CR0 0
@@ -268,8 +270,15 @@ struct vm_event_write_ctrlreg {
uint64_t old_value;
};
+struct vm_event_singlestep {
+ uint64_t gfn;
+};
+
struct vm_event_debug {
uint64_t gfn;
+ uint8_t type; /* HVMOP_TRAP_* */
+ uint8_t insn_length;
+ uint8_t _pad[6];
};
struct vm_event_mov_to_msr {
@@ -324,7 +333,8 @@ typedef struct vm_event_st {
struct vm_event_write_ctrlreg write_ctrlreg;
struct vm_event_mov_to_msr mov_to_msr;
struct vm_event_debug software_breakpoint;
- struct vm_event_debug singlestep;
+ struct vm_event_singlestep singlestep;
+ struct vm_event_debug debug_exception;
struct vm_event_privcall privcall;
} u;
--
2.8.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v5 9/9] tools/xen-access: add test-case for ARM SMC
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
` (6 preceding siblings ...)
2016-06-02 22:52 ` [PATCH v5 8/9] x86/vm_event: Add HVM debug exception vm_events Tamas K Lengyel
@ 2016-06-02 22:52 ` Tamas K Lengyel
2016-06-03 7:08 ` [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Razvan Cojocaru
` (2 subsequent siblings)
10 siblings, 0 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-02 22:52 UTC (permalink / raw)
To: xen-devel; +Cc: Tamas K Lengyel, Ian Jackson
Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/tests/xen-access/xen-access.c | 45 ++++++++++++++++++++++++++++++++++++-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index e615476..e467c48 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -338,6 +338,8 @@ void usage(char* progname)
fprintf(stderr, "Usage: %s [-m] <domain_id> write|exec", progname);
#if defined(__i386__) || defined(__x86_64__)
fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug");
+#elif defined(__arm__) || defined(__aarch64__)
+ fprintf(stderr, "|privcall");
#endif
fprintf(stderr,
"\n"
@@ -426,6 +428,11 @@ int main(int argc, char *argv[])
{
debug = 1;
}
+#elif defined(__arm__) || defined(__aarch64__)
+ else if ( !strcmp(argv[0], "privcall") )
+ {
+ privcall = 1;
+ }
#endif
else
{
@@ -548,6 +555,16 @@ int main(int argc, char *argv[])
}
}
+ if ( privcall )
+ {
+ rc = xc_monitor_privileged_call(xch, domain_id, 1);
+ if ( rc < 0 )
+ {
+ ERROR("Error %d setting privileged call trapping with vm_event\n", rc);
+ goto exit;
+ }
+ }
+
/* Wait for access */
for (;;)
{
@@ -560,7 +577,8 @@ int main(int argc, char *argv[])
rc = xc_monitor_software_breakpoint(xch, domain_id, 0);
if ( debug )
rc = xc_monitor_debug_exceptions(xch, domain_id, 0, 0);
-
+ if ( privcall )
+ rc = xc_monitor_privileged_call(xch, domain_id, 0);
if ( altp2m )
{
rc = xc_altp2m_switch_to_view( xch, domain_id, 0 );
@@ -716,6 +734,31 @@ int main(int argc, char *argv[])
}
break;
+#if defined(__arm__) || defined(__aarch64__)
+ case VM_EVENT_REASON_PRIVILEGED_CALL:
+ {
+ const struct vm_event_regs_arm *in_regs = &req.data.regs.arm;
+ struct vm_event_regs_arm *out_regs = &rsp.data.regs.arm;
+ bool is32bit = !!(in_regs->cpsr & PSR_MODE_BIT);
+ uint64_t pc;
+
+ *out_regs = *in_regs;
+
+ if ( is32bit ) {
+ pc = in_regs->arch.arm32.pc;
+ out_regs->arch.arm32.pc += 4;
+ } else {
+ pc = in_regs->arch.arm64.pc;
+ out_regs->arch.arm64.pc += 8;
+ }
+
+ printf("Privileged call: pc=%016"PRIx64" (vcpu %d)\n",
+ pc, req.vcpu_id);
+
+ rsp.flags |= VM_EVENT_FLAG_SET_REGISTERS;
+ }
+ break;
+#endif
default:
fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
}
--
2.8.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
` (7 preceding siblings ...)
2016-06-02 22:52 ` [PATCH v5 9/9] tools/xen-access: add test-case for ARM SMC Tamas K Lengyel
@ 2016-06-03 7:08 ` Razvan Cojocaru
2016-06-03 15:54 ` Jan Beulich
2016-06-17 19:09 ` Tamas K Lengyel
10 siblings, 0 replies; 46+ messages in thread
From: Razvan Cojocaru @ 2016-06-03 7:08 UTC (permalink / raw)
To: Tamas K Lengyel, xen-devel
Cc: Andrew Cooper, Kevin Tian, Jan Beulich, Jun Nakajima
On 06/03/16 01:52, Tamas K Lengyel wrote:
> The return value has not been clearly defined, with the function
> never returning 0 which seemingly indicated a condition where the
> guest should crash. In this patch we define -rc as error condition
> where the notification was not sent, 0 where the notification was sent
> and the vCPU is not paused and 1 that the notification was sent and
> that the vCPU is paused.
>
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> ---
> xen/arch/x86/hvm/event.c | 4 ++--
> xen/arch/x86/hvm/vmx/vmx.c | 6 +++---
> xen/common/vm_event.c | 5 +++--
> 3 files changed, 8 insertions(+), 7 deletions(-)
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-02 22:52 ` [PATCH v5 5/9] monitor: ARM SMC events Tamas K Lengyel
@ 2016-06-03 9:49 ` Julien Grall
2016-06-03 13:40 ` Tamas K Lengyel
0 siblings, 1 reply; 46+ messages in thread
From: Julien Grall @ 2016-06-03 9:49 UTC (permalink / raw)
To: Tamas K Lengyel, xen-devel; +Cc: Stefano Stabellini
Hello Tamas,
On 02/06/16 23:52, Tamas K Lengyel wrote:
> diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
> index 9270d52..7976080 100644
> --- a/xen/include/public/vm_event.h
> +++ b/xen/include/public/vm_event.h
> @@ -119,6 +119,8 @@
> #define VM_EVENT_REASON_SINGLESTEP 7
> /* An event has been requested via HVMOP_guest_request_vm_event. */
> #define VM_EVENT_REASON_GUEST_REQUEST 8
> +/* Privileged call executed (e.g. SMC) */
> +#define VM_EVENT_REASON_PRIVILEGED_CALL 9
>
> /* Supported values for the vm_event_write_ctrlreg index. */
> #define VM_EVENT_X86_CR0 0
> @@ -212,6 +214,13 @@ struct vm_event_mov_to_msr {
> uint64_t value;
> };
>
> +#define VM_EVENT_PRIVCALL_SMC 0
> +
> +struct vm_event_privcall {
> + uint32_t type;
> + uint32_t vector; /* ESR_EL2.ISS for SMC calls */
How do you expect the introspection app to deal with it? As explained in
a previous mail [1], the ISS encoding is different between ARMv7 32-bit
and ARMv8 32-bit. The former is unknown (see B3-1431 in ARM DDI 0406C.c)
whilst the latter contains fields related to the condition (see D7-1897
in ARM DDI 0406C.c).
This is because on ARMv8, the conditional SMC issued in AArch32 state
may trap even if the condition has failed.
So the app would have to know whether the hypervisor is running on an
ARMv7 or ARMv8 platform. But I am not aware of an easy way to
differentiate it from the registers.
Furthermore, I think the vm_event app should only received SMCs whose
condition has succeeded, because they will be actual SMC. The others
should just be ignored.
IHMO, the vm_event should only contain the immediate. The rest only
matters for the hypervisor.
Regards,
[1]
http://lists.xenproject.org/archives/html/xen-devel/2016-06/msg00285.html
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 6/9] arm/vm_event: get/set registers
2016-06-02 22:52 ` [PATCH v5 6/9] arm/vm_event: get/set registers Tamas K Lengyel
@ 2016-06-03 10:34 ` Jan Beulich
2016-06-03 19:27 ` Tamas K Lengyel
0 siblings, 1 reply; 46+ messages in thread
From: Jan Beulich @ 2016-06-03 10:34 UTC (permalink / raw)
To: Tamas K Lengyel; +Cc: xen-devel, Julien Grall, Stefano Stabellini
>>> On 03.06.16 at 00:52, <tamas@tklengyel.com> wrote:
> @@ -168,6 +168,69 @@ struct vm_event_regs_x86 {
> uint32_t _pad;
> };
>
> +struct vm_event_regs_arm32 {
> + uint32_t r0;
> + uint32_t r1;
> + uint32_t r2;
> + uint32_t r3;
> + uint32_t r4;
> + uint32_t r5;
> + uint32_t r6;
> + uint32_t r7;
> + uint32_t r8;
> + uint32_t r9;
> + uint32_t r10;
> + uint32_t r11;
> + uint32_t r12;
> + uint32_t pc;
> +};
While I had given my v4 comment on the ARM64 variant, I certainly
meant it to apply here too: I'm missing r13/sp and r14/lr.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 8/9] x86/vm_event: Add HVM debug exception vm_events
2016-06-02 22:52 ` [PATCH v5 8/9] x86/vm_event: Add HVM debug exception vm_events Tamas K Lengyel
@ 2016-06-03 10:49 ` Jan Beulich
2016-06-03 13:29 ` Tamas K Lengyel
0 siblings, 1 reply; 46+ messages in thread
From: Jan Beulich @ 2016-06-03 10:49 UTC (permalink / raw)
To: Tamas K Lengyel
Cc: Kevin Tian, Wei Liu, Razvan Cojocaru, Andrew Cooper, Ian Jackson,
Jun Nakajima, xen-devel
>>> On 03.06.16 at 00:52, <tamas@tklengyel.com> wrote:
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3377,10 +3377,33 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
> HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
> write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
> if ( !v->domain->debugger_attached )
> - vmx_propagate_intr(intr_info);
> + {
> + unsigned long insn_length = 0;
It's insn_len further down - please try to be consistent.
> + int rc;
> + unsigned long trap_type = MASK_EXTR(intr_info,
> + INTR_INFO_INTR_TYPE_MASK);
> +
> + if( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
> + __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_length);
> +
> + rc = hvm_monitor_debug(regs->eip,
> + HVM_MONITOR_DEBUG_EXCEPTION,
> + trap_type, insn_length);
> + if ( !rc )
> + {
> + vmx_propagate_intr(intr_info);
> + break;
> + }
> + else if ( rc > 0 )
> + break;
So you've removed the odd / hard to understand return value
adjustment from hvm_monitor_debug(), but this isn't any better:
What does the return value being positive really mean? And btw.,
no point using "else" after an unconditional "break" in the previous
if().
> + }
> else
> + {
> domain_pause_for_debugger();
> - break;
> + break;
> + }
> +
> + goto exit_and_crash;
There was no such goto before, i.e. you introduce this. I'm rather
hesitant to see such getting added without a good reason, and
that good reason should be stated in a comment. Also it looks like
the change would be easier to grok if you didn't alter the code
down here, but instead inverted the earlier if:
if ( unlikely(rc < 0) )
/* ... */
goto exit_and_crash;
if ( !rc )
vmx_propagate_intr(intr_info);
Which imo would get us closer to code being at least half way
self-explanatory.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 8/9] x86/vm_event: Add HVM debug exception vm_events
2016-06-03 10:49 ` Jan Beulich
@ 2016-06-03 13:29 ` Tamas K Lengyel
2016-06-03 14:23 ` Jan Beulich
0 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-03 13:29 UTC (permalink / raw)
To: Jan Beulich
Cc: Kevin Tian, Wei Liu, Razvan Cojocaru, Andrew Cooper, Ian Jackson,
Jun Nakajima, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 2906 bytes --]
On Jun 3, 2016 04:49, "Jan Beulich" <JBeulich@suse.com> wrote:
>
> >>> On 03.06.16 at 00:52, <tamas@tklengyel.com> wrote:
> > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > @@ -3377,10 +3377,33 @@ void vmx_vmexit_handler(struct cpu_user_regs
*regs)
> > HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
> > write_debugreg(6, exit_qualification |
DR_STATUS_RESERVED_ONE);
> > if ( !v->domain->debugger_attached )
> > - vmx_propagate_intr(intr_info);
> > + {
> > + unsigned long insn_length = 0;
>
> It's insn_len further down - please try to be consistent.
>
> > + int rc;
> > + unsigned long trap_type = MASK_EXTR(intr_info,
> > +
INTR_INFO_INTR_TYPE_MASK);
> > +
> > + if( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
> > + __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_length);
> > +
> > + rc = hvm_monitor_debug(regs->eip,
> > + HVM_MONITOR_DEBUG_EXCEPTION,
> > + trap_type, insn_length);
> > + if ( !rc )
> > + {
> > + vmx_propagate_intr(intr_info);
> > + break;
> > + }
> > + else if ( rc > 0 )
> > + break;
>
> So you've removed the odd / hard to understand return value
> adjustment from hvm_monitor_debug(), but this isn't any better:
> What does the return value being positive really mean? And btw.,
> no point using "else" after an unconditional "break" in the previous
> if().
>
As the commit message explains in the other patch rc is 1 when the vCPU is
paused. This means a synchronous event where we are waiting for the
vm_event response thus work here is done.
> > + }
> > else
> > + {
> > domain_pause_for_debugger();
> > - break;
> > + break;
> > + }
> > +
> > + goto exit_and_crash;
>
> There was no such goto before, i.e. you introduce this. I'm rather
> hesitant to see such getting added without a good reason, and
> that good reason should be stated in a comment. Also it looks like
> the change would be easier to grok if you didn't alter the code
> down here, but instead inverted the earlier if:
>
> if ( unlikely(rc < 0) )
> /* ... */
> goto exit_and_crash;
> if ( !rc )
> vmx_propagate_intr(intr_info);
>
> Which imo would get us closer to code being at least half way
> self-explanatory.
>
I agree it may be more intuitive that way but adding the goto the way I did
is whats consistent with the already established handling of int3 events. I
either go for consistency or reworking more code at other spots too.
Tamas
[-- Attachment #1.2: Type: text/html, Size: 4139 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-03 9:49 ` Julien Grall
@ 2016-06-03 13:40 ` Tamas K Lengyel
2016-06-03 14:43 ` Julien Grall
0 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-03 13:40 UTC (permalink / raw)
To: Julien Grall; +Cc: Xen-devel, Stefano Stabellini
[-- Attachment #1.1: Type: text/plain, Size: 2309 bytes --]
On Jun 3, 2016 03:49, "Julien Grall" <julien.grall@arm.com> wrote:
>
> Hello Tamas,
>
>
> On 02/06/16 23:52, Tamas K Lengyel wrote:
>>
>> diff --git a/xen/include/public/vm_event.h
b/xen/include/public/vm_event.h
>> index 9270d52..7976080 100644
>> --- a/xen/include/public/vm_event.h
>> +++ b/xen/include/public/vm_event.h
>> @@ -119,6 +119,8 @@
>> #define VM_EVENT_REASON_SINGLESTEP 7
>> /* An event has been requested via HVMOP_guest_request_vm_event. */
>> #define VM_EVENT_REASON_GUEST_REQUEST 8
>> +/* Privileged call executed (e.g. SMC) */
>> +#define VM_EVENT_REASON_PRIVILEGED_CALL 9
>>
>> /* Supported values for the vm_event_write_ctrlreg index. */
>> #define VM_EVENT_X86_CR0 0
>> @@ -212,6 +214,13 @@ struct vm_event_mov_to_msr {
>> uint64_t value;
>> };
>>
>> +#define VM_EVENT_PRIVCALL_SMC 0
>> +
>> +struct vm_event_privcall {
>> + uint32_t type;
>> + uint32_t vector; /* ESR_EL2.ISS for SMC calls */
>
>
> How do you expect the introspection app to deal with it? As explained in
a previous mail [1], the ISS encoding is different between ARMv7 32-bit and
ARMv8 32-bit. The former is unknown (see B3-1431 in ARM DDI 0406C.c) whilst
the latter contains fields related to the condition (see D7-1897 in ARM DDI
0406C.c).
>
> This is because on ARMv8, the conditional SMC issued in AArch32 state may
trap even if the condition has failed.
>
> So the app would have to know whether the hypervisor is running on an
ARMv7 or ARMv8 platform. But I am not aware of an easy way to differentiate
it from the registers.
The app can certainly run other checks to determine what the CPU version
is, not being exclusively reliant on vm_event and running in a privileged
domain.
>
> Furthermore, I think the vm_event app should only received SMCs whose
condition has succeeded, because they will be actual SMC. The others should
just be ignored.
>
> IHMO, the vm_event should only contain the immediate. The rest only
matters for the hypervisor.
Absolutely not! The primary usecase I have for SMC trapping is kernel
execution monitoring by manually writing it into arbitrary kernel code
locations and hiding them from the guest with mem_access. If some SMCs may
silently get swallowed by the hypervisor the whole thing becomes unreliable.
Tamas
[-- Attachment #1.2: Type: text/html, Size: 2870 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 8/9] x86/vm_event: Add HVM debug exception vm_events
2016-06-03 13:29 ` Tamas K Lengyel
@ 2016-06-03 14:23 ` Jan Beulich
2016-06-03 14:34 ` Tamas K Lengyel
0 siblings, 1 reply; 46+ messages in thread
From: Jan Beulich @ 2016-06-03 14:23 UTC (permalink / raw)
To: Tamas K Lengyel
Cc: Kevin Tian, Wei Liu, Razvan Cojocaru, Andrew Cooper, Ian Jackson,
Jun Nakajima, xen-devel
>>> On 03.06.16 at 15:29, <tamas@tklengyel.com> wrote:
> On Jun 3, 2016 04:49, "Jan Beulich" <JBeulich@suse.com> wrote:
>>
>> >>> On 03.06.16 at 00:52, <tamas@tklengyel.com> wrote:
>> > --- a/xen/arch/x86/hvm/vmx/vmx.c
>> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> > @@ -3377,10 +3377,33 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>> > HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
>> > write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
>> > if ( !v->domain->debugger_attached )
>> > - vmx_propagate_intr(intr_info);
>> > + {
>> > + unsigned long insn_length = 0;
>>
>> It's insn_len further down - please try to be consistent.
>>
>> > + int rc;
>> > + unsigned long trap_type = MASK_EXTR(intr_info,
>> > +
> INTR_INFO_INTR_TYPE_MASK);
>> > +
>> > + if( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
>> > + __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_length);
>> > +
>> > + rc = hvm_monitor_debug(regs->eip,
>> > + HVM_MONITOR_DEBUG_EXCEPTION,
>> > + trap_type, insn_length);
>> > + if ( !rc )
>> > + {
>> > + vmx_propagate_intr(intr_info);
>> > + break;
>> > + }
>> > + else if ( rc > 0 )
>> > + break;
>>
>> So you've removed the odd / hard to understand return value
>> adjustment from hvm_monitor_debug(), but this isn't any better:
>> What does the return value being positive really mean? And btw.,
>> no point using "else" after an unconditional "break" in the previous
>> if().
>
> As the commit message explains in the other patch rc is 1 when the vCPU is
> paused. This means a synchronous event where we are waiting for the
> vm_event response thus work here is done.
The commit message of _another_ patch doesn't help at all a future
reader to understand what's going on here.
>> > + }
>> > else
>> > + {
>> > domain_pause_for_debugger();
>> > - break;
>> > + break;
>> > + }
>> > +
>> > + goto exit_and_crash;
>>
>> There was no such goto before, i.e. you introduce this. I'm rather
>> hesitant to see such getting added without a good reason, and
>> that good reason should be stated in a comment. Also it looks like
>> the change would be easier to grok if you didn't alter the code
>> down here, but instead inverted the earlier if:
>>
>> if ( unlikely(rc < 0) )
>> /* ... */
>> goto exit_and_crash;
>> if ( !rc )
>> vmx_propagate_intr(intr_info);
>>
>> Which imo would get us closer to code being at least half way
>> self-explanatory.
>
> I agree it may be more intuitive that way but adding the goto the way I did
> is whats consistent with the already established handling of int3 events. I
> either go for consistency or reworking more code at other spots too.
Well, as always I'll leave it to the maintainers to decide, but I think
my suggestion would make this code better readable, and doesn't
require immediate re-work elsewhere.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 8/9] x86/vm_event: Add HVM debug exception vm_events
2016-06-03 14:23 ` Jan Beulich
@ 2016-06-03 14:34 ` Tamas K Lengyel
2016-06-03 14:45 ` Jan Beulich
0 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-03 14:34 UTC (permalink / raw)
To: Jan Beulich
Cc: Kevin Tian, Wei Liu, Razvan Cojocaru, Andrew Cooper, Ian Jackson,
Jun Nakajima, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 4040 bytes --]
On Jun 3, 2016 08:23, "Jan Beulich" <JBeulich@suse.com> wrote:
>
> >>> On 03.06.16 at 15:29, <tamas@tklengyel.com> wrote:
> > On Jun 3, 2016 04:49, "Jan Beulich" <JBeulich@suse.com> wrote:
> >>
> >> >>> On 03.06.16 at 00:52, <tamas@tklengyel.com> wrote:
> >> > --- a/xen/arch/x86/hvm/vmx/vmx.c
> >> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> >> > @@ -3377,10 +3377,33 @@ void vmx_vmexit_handler(struct cpu_user_regs
*regs)
> >> > HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
> >> > write_debugreg(6, exit_qualification |
DR_STATUS_RESERVED_ONE);
> >> > if ( !v->domain->debugger_attached )
> >> > - vmx_propagate_intr(intr_info);
> >> > + {
> >> > + unsigned long insn_length = 0;
> >>
> >> It's insn_len further down - please try to be consistent.
> >>
> >> > + int rc;
> >> > + unsigned long trap_type = MASK_EXTR(intr_info,
> >> > +
> > INTR_INFO_INTR_TYPE_MASK);
> >> > +
> >> > + if( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
> >> > + __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_length);
> >> > +
> >> > + rc = hvm_monitor_debug(regs->eip,
> >> > + HVM_MONITOR_DEBUG_EXCEPTION,
> >> > + trap_type, insn_length);
> >> > + if ( !rc )
> >> > + {
> >> > + vmx_propagate_intr(intr_info);
> >> > + break;
> >> > + }
> >> > + else if ( rc > 0 )
> >> > + break;
> >>
> >> So you've removed the odd / hard to understand return value
> >> adjustment from hvm_monitor_debug(), but this isn't any better:
> >> What does the return value being positive really mean? And btw.,
> >> no point using "else" after an unconditional "break" in the previous
> >> if().
> >
> > As the commit message explains in the other patch rc is 1 when the vCPU
is
> > paused. This means a synchronous event where we are waiting for the
> > vm_event response thus work here is done.
>
> The commit message of _another_ patch doesn't help at all a future
> reader to understand what's going on here.
This is already used elsewhere in similar fashion so I don't see why we
would need to treat this case any differently. Its not like I'm introducing
a totally new way of doing this. So IMHO adding a comment would be an OK
middle ground but my goal is really not to rework everything.
> >> > + }
> >> > else
> >> > + {
> >> > domain_pause_for_debugger();
> >> > - break;
> >> > + break;
> >> > + }
> >> > +
> >> > + goto exit_and_crash;
> >>
> >> There was no such goto before, i.e. you introduce this. I'm rather
> >> hesitant to see such getting added without a good reason, and
> >> that good reason should be stated in a comment. Also it looks like
> >> the change would be easier to grok if you didn't alter the code
> >> down here, but instead inverted the earlier if:
> >>
> >> if ( unlikely(rc < 0) )
> >> /* ... */
> >> goto exit_and_crash;
> >> if ( !rc )
> >> vmx_propagate_intr(intr_info);
> >>
> >> Which imo would get us closer to code being at least half way
> >> self-explanatory.
> >
> > I agree it may be more intuitive that way but adding the goto the way I
did
> > is whats consistent with the already established handling of int3
events. I
> > either go for consistency or reworking more code at other spots too.
>
> Well, as always I'll leave it to the maintainers to decide, but I think
> my suggestion would make this code better readable, and doesn't
> require immediate re-work elsewhere.
>
If we are aiming for consistency then I think it does. Lets hear from the
maintainers and will go from there. I rather not start reworking
preexisting stuff because it tends to snowball into a lot more patches.
Tamas
[-- Attachment #1.2: Type: text/html, Size: 5882 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-03 13:40 ` Tamas K Lengyel
@ 2016-06-03 14:43 ` Julien Grall
2016-06-03 15:03 ` Tamas K Lengyel
2016-06-03 15:27 ` Tamas K Lengyel
0 siblings, 2 replies; 46+ messages in thread
From: Julien Grall @ 2016-06-03 14:43 UTC (permalink / raw)
To: Tamas K Lengyel; +Cc: Xen-devel, Stefano Stabellini, Steve Capper
Hello Tamas,
On 03/06/16 14:40, Tamas K Lengyel wrote:
>
> On Jun 3, 2016 03:49, "Julien Grall" <julien.grall@arm.com
> <mailto:julien.grall@arm.com>> wrote:
> >
> > Hello Tamas,
> >
> >
> > On 02/06/16 23:52, Tamas K Lengyel wrote:
> >>
> >> diff --git a/xen/include/public/vm_event.h
> b/xen/include/public/vm_event.h
> >> index 9270d52..797608Burrington0 100644
> >> --- a/xen/include/public/vm_event.h
> >> +++ b/xen/include/public/vm_event.h
> >> @@ -119,6 +119,8 @@
> >> #define VM_EVENT_REASON_SINGLESTEP 7
> >> /* An event has been requested via HVMOP_guest_request_vm_event. */
> >> #define VM_EVENT_REASON_GUEST_REQUEST 8
> >> +/* Privileged call executed (e.g. SMC) */
> >> +#define VM_EVENT_REASON_PRIVILEGED_CALL 9
> >>
> >> /* Supported values for the vm_event_write_ctrlreg index. */
> >> #define VM_EVENT_X86_CR0 0
> >> @@ -212,6 +214,13 @@ struct vm_event_mov_to_msr {
> >> uint64_t value;
> >> };
> >>
> >> +#define VM_EVENT_PRIVCALL_SMC 0
> >> +
> >> +struct vm_event_privcall {
> >> + uint32_t type;
> >> + uint32_t vector; /* ESR_EL2.ISS for SMC calls */
> >
> >
> > How do you expect the introspection app to deal with it? As explained `
> in a previous mail [1], the ISS encoding is different between ARMv7
> 32-bit and ARMv8 32-bit. The former is unknown (see B3-1431 in ARM DDI
> 0406C.c) whilst the latter contains fields related to the condition (see
> D7-1897 in ARM DDI 0406C.c).
> >
> > This is because on ARMv8, the conditional SMC issued in AArch32 state
> may trap even if the condition has failed.
> >
> > So the app would have to know whether the hypervisor is running on an
> ARMv7 or ARMv8 platform. But I am not aware of an easy way to
> differentiate it from the registers.
>
> The app can certainly run other checks to determine what the CPU version
> is, not being exclusively reliant on vm_event and running in a
> privileged domain.
Manufacturers are allowed to build their custom ARM processor based on
the ARM ARM. The number of CPU version to check will likely be huge and
you will not be future proof.
`
>
> >
> > Furthermore, I think the vm_event app should only received SMCs whose
> condition has succeeded, because they will be actual SMC. The others
> should just be ignored.
> >
> > IHMO, the vm_event should only contain the immediate. The rest only
> matters for the hypervisor.
>
> Absolutely not! The primary usecase I have for SMC trapping is kernel
> execution monitoring by manually writing it into arbitrary kernel code
> locations and hiding them from the guest with mem_access. If some SMCs
> may silently get swallowed by the hypervisor the whole thing becomes
> unreliable.
Please read what I wrote, on ARMv8, a conditional SMC issued in AArch32
state *may* trap even if the condition has failed. I.e an implementer
can design its CPU to not trap them (see D1-1506 on ARM DDI 0487A.i).
On ARMv7, only unconditional SMC and conditional SMC *which pass the
condition test* will be trapped. The others will be ignored.
So even if the hypervisor send an event for each SMC trapped, you may
not receive all the SMCs. This is already unreliable by the architecture.
If you want something reliable, you will have to inject unconditional
SMC or HVC which are always unconditional.
If you want to also trap all the SMCs written by a guest, then you will
have to live with the fact that some may be ignored. Although, I don't
think that an introspection app should care about instructions that
would be treated as a nop (for instance because the condition check has
failed).
Hence my suggestion to check in the hypervisor whether the condition has
failed and provide processor-agnostic information (the ISS is different
between ARMv7, ARMv8 and AArch32 and AArch64).
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 8/9] x86/vm_event: Add HVM debug exception vm_events
2016-06-03 14:34 ` Tamas K Lengyel
@ 2016-06-03 14:45 ` Jan Beulich
2016-06-03 14:51 ` Tamas K Lengyel
0 siblings, 1 reply; 46+ messages in thread
From: Jan Beulich @ 2016-06-03 14:45 UTC (permalink / raw)
To: Tamas K Lengyel
Cc: Kevin Tian, Wei Liu, Razvan Cojocaru, Andrew Cooper, Ian Jackson,
Jun Nakajima, xen-devel
>>> On 03.06.16 at 16:34, <tamas@tklengyel.com> wrote:
> On Jun 3, 2016 08:23, "Jan Beulich" <JBeulich@suse.com> wrote:
>>
>> >>> On 03.06.16 at 15:29, <tamas@tklengyel.com> wrote:
>> > On Jun 3, 2016 04:49, "Jan Beulich" <JBeulich@suse.com> wrote:
>> >>
>> >> >>> On 03.06.16 at 00:52, <tamas@tklengyel.com> wrote:
>> >> > --- a/xen/arch/x86/hvm/vmx/vmx.c
>> >> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> >> > @@ -3377,10 +3377,33 @@ void vmx_vmexit_handler(struct cpu_user_regs
> *regs)
>> >> > HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
>> >> > write_debugreg(6, exit_qualification |
> DR_STATUS_RESERVED_ONE);
>> >> > if ( !v->domain->debugger_attached )
>> >> > - vmx_propagate_intr(intr_info);
>> >> > + {
>> >> > + unsigned long insn_length = 0;
>> >>
>> >> It's insn_len further down - please try to be consistent.
>> >>
>> >> > + int rc;
>> >> > + unsigned long trap_type = MASK_EXTR(intr_info,
>> >> > +
>> > INTR_INFO_INTR_TYPE_MASK);
>> >> > +
>> >> > + if( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
>> >> > + __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_length);
>> >> > +
>> >> > + rc = hvm_monitor_debug(regs->eip,
>> >> > + HVM_MONITOR_DEBUG_EXCEPTION,
>> >> > + trap_type, insn_length);
>> >> > + if ( !rc )
>> >> > + {
>> >> > + vmx_propagate_intr(intr_info);
>> >> > + break;
>> >> > + }
>> >> > + else if ( rc > 0 )
>> >> > + break;
>> >>
>> >> So you've removed the odd / hard to understand return value
>> >> adjustment from hvm_monitor_debug(), but this isn't any better:
>> >> What does the return value being positive really mean? And btw.,
>> >> no point using "else" after an unconditional "break" in the previous
>> >> if().
>> >
>> > As the commit message explains in the other patch rc is 1 when the vCPU is
>> > paused. This means a synchronous event where we are waiting for the
>> > vm_event response thus work here is done.
>>
>> The commit message of _another_ patch doesn't help at all a future
>> reader to understand what's going on here.
>
> This is already used elsewhere in similar fashion so I don't see why we
> would need to treat this case any differently. Its not like I'm introducing
> a totally new way of doing this. So IMHO adding a comment would be an OK
> middle ground but my goal is really not to rework everything.
Nothing but a comment was what I was hoping for. And then later,
in the remark regarding the odd code structure further down, I did
say "Which imo would get us closer to code being at least half way
self-explanatory," to indicate that if that adjustment was done,
perhaps a comment may not even be needed.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 8/9] x86/vm_event: Add HVM debug exception vm_events
2016-06-03 14:45 ` Jan Beulich
@ 2016-06-03 14:51 ` Tamas K Lengyel
0 siblings, 0 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-03 14:51 UTC (permalink / raw)
To: Jan Beulich
Cc: Kevin Tian, Wei Liu, Razvan Cojocaru, Andrew Cooper, Ian Jackson,
Jun Nakajima, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 3136 bytes --]
On Jun 3, 2016 08:45, "Jan Beulich" <JBeulich@suse.com> wrote:
>
> >>> On 03.06.16 at 16:34, <tamas@tklengyel.com> wrote:
> > On Jun 3, 2016 08:23, "Jan Beulich" <JBeulich@suse.com> wrote:
> >>
> >> >>> On 03.06.16 at 15:29, <tamas@tklengyel.com> wrote:
> >> > On Jun 3, 2016 04:49, "Jan Beulich" <JBeulich@suse.com> wrote:
> >> >>
> >> >> >>> On 03.06.16 at 00:52, <tamas@tklengyel.com> wrote:
> >> >> > --- a/xen/arch/x86/hvm/vmx/vmx.c
> >> >> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> >> >> > @@ -3377,10 +3377,33 @@ void vmx_vmexit_handler(struct
cpu_user_regs
> > *regs)
> >> >> > HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
> >> >> > write_debugreg(6, exit_qualification |
> > DR_STATUS_RESERVED_ONE);
> >> >> > if ( !v->domain->debugger_attached )
> >> >> > - vmx_propagate_intr(intr_info);
> >> >> > + {
> >> >> > + unsigned long insn_length = 0;
> >> >>
> >> >> It's insn_len further down - please try to be consistent.
> >> >>
> >> >> > + int rc;
> >> >> > + unsigned long trap_type = MASK_EXTR(intr_info,
> >> >> > +
> >> > INTR_INFO_INTR_TYPE_MASK);
> >> >> > +
> >> >> > + if( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
> >> >> > + __vmread(VM_EXIT_INSTRUCTION_LEN,
&insn_length);
> >> >> > +
> >> >> > + rc = hvm_monitor_debug(regs->eip,
> >> >> > +
HVM_MONITOR_DEBUG_EXCEPTION,
> >> >> > + trap_type, insn_length);
> >> >> > + if ( !rc )
> >> >> > + {
> >> >> > + vmx_propagate_intr(intr_info);
> >> >> > + break;
> >> >> > + }
> >> >> > + else if ( rc > 0 )
> >> >> > + break;
> >> >>
> >> >> So you've removed the odd / hard to understand return value
> >> >> adjustment from hvm_monitor_debug(), but this isn't any better:
> >> >> What does the return value being positive really mean? And btw.,
> >> >> no point using "else" after an unconditional "break" in the previous
> >> >> if().
> >> >
> >> > As the commit message explains in the other patch rc is 1 when the
vCPU is
> >> > paused. This means a synchronous event where we are waiting for the
> >> > vm_event response thus work here is done.
> >>
> >> The commit message of _another_ patch doesn't help at all a future
> >> reader to understand what's going on here.
> >
> > This is already used elsewhere in similar fashion so I don't see why we
> > would need to treat this case any differently. Its not like I'm
introducing
> > a totally new way of doing this. So IMHO adding a comment would be an OK
> > middle ground but my goal is really not to rework everything.
>
> Nothing but a comment was what I was hoping for. And then later,
> in the remark regarding the odd code structure further down, I did
> say "Which imo would get us closer to code being at least half way
> self-explanatory," to indicate that if that adjustment was done,
> perhaps a comment may not even be needed.
>
Ack. I have nothing against adding a comment here.
Tamas
[-- Attachment #1.2: Type: text/html, Size: 4979 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-03 14:43 ` Julien Grall
@ 2016-06-03 15:03 ` Tamas K Lengyel
2016-06-03 15:06 ` Julien Grall
2016-06-03 15:27 ` Tamas K Lengyel
1 sibling, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-03 15:03 UTC (permalink / raw)
To: Julien Grall; +Cc: Xen-devel, Stefano Stabellini, Steve Capper
On Fri, Jun 3, 2016 at 8:43 AM, Julien Grall <julien.grall@arm.com> wrote:
> Hello Tamas,
>
> On 03/06/16 14:40, Tamas K Lengyel wrote:
>>
>>
>> On Jun 3, 2016 03:49, "Julien Grall" <julien.grall@arm.com
>> <mailto:julien.grall@arm.com>> wrote:
>> >
>> > Hello Tamas,
>> >
>> >
>> > On 02/06/16 23:52, Tamas K Lengyel wrote:
>> >>
>> >> diff --git a/xen/include/public/vm_event.h
>> b/xen/include/public/vm_event.h
>> >> index 9270d52..797608Burrington0 100644
>>
>> >> --- a/xen/include/public/vm_event.h
>> >> +++ b/xen/include/public/vm_event.h
>> >> @@ -119,6 +119,8 @@
>> >> #define VM_EVENT_REASON_SINGLESTEP 7
>> >> /* An event has been requested via HVMOP_guest_request_vm_event. */
>> >> #define VM_EVENT_REASON_GUEST_REQUEST 8
>> >> +/* Privileged call executed (e.g. SMC) */
>> >> +#define VM_EVENT_REASON_PRIVILEGED_CALL 9
>> >>
>> >> /* Supported values for the vm_event_write_ctrlreg index. */
>> >> #define VM_EVENT_X86_CR0 0
>> >> @@ -212,6 +214,13 @@ struct vm_event_mov_to_msr {
>> >> uint64_t value;
>> >> };
>> >>
>> >> +#define VM_EVENT_PRIVCALL_SMC 0
>> >> +
>> >> +struct vm_event_privcall {
>> >> + uint32_t type;
>> >> + uint32_t vector; /* ESR_EL2.ISS for SMC calls */
>> >
>> >
>> > How do you expect the introspection app to deal with it? As explained `
>> in a previous mail [1], the ISS encoding is different between ARMv7
>> 32-bit and ARMv8 32-bit. The former is unknown (see B3-1431 in ARM DDI
>> 0406C.c) whilst the latter contains fields related to the condition (see
>> D7-1897 in ARM DDI 0406C.c).
>> >
>> > This is because on ARMv8, the conditional SMC issued in AArch32 state
>> may trap even if the condition has failed.
>> >
>> > So the app would have to know whether the hypervisor is running on an
>> ARMv7 or ARMv8 platform. But I am not aware of an easy way to
>> differentiate it from the registers.
>>
>> The app can certainly run other checks to determine what the CPU version
>> is, not being exclusively reliant on vm_event and running in a
>> privileged domain.
>
>
> Manufacturers are allowed to build their custom ARM processor based on the
> ARM ARM. The number of CPU version to check will likely be huge and you will
> not be future proof.
> `
If transmitting the ISS to the user this way is not enough in your
opinion I may just leave this item up for a future user to implement
as my use-case doesn't need it.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-03 15:03 ` Tamas K Lengyel
@ 2016-06-03 15:06 ` Julien Grall
2016-06-03 15:42 ` Tamas K Lengyel
0 siblings, 1 reply; 46+ messages in thread
From: Julien Grall @ 2016-06-03 15:06 UTC (permalink / raw)
To: Tamas K Lengyel; +Cc: Xen-devel, Stefano Stabellini, Steve Capper
On 03/06/16 16:03, Tamas K Lengyel wrote:
> If transmitting the ISS to the user this way is not enough in your
> opinion I may just leave this item up for a future user to implement
> as my use-case doesn't need it.
It is up to you. However, it will likely mean to bump the interface
version of the VM Event.
BTW, I have not seen any change to the interface version within this
series. But the size of the structure will change in patch #9 which will
impact all the introspection app. Is it normal?
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-03 14:43 ` Julien Grall
2016-06-03 15:03 ` Tamas K Lengyel
@ 2016-06-03 15:27 ` Tamas K Lengyel
2016-06-03 15:34 ` Tamas K Lengyel
1 sibling, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-03 15:27 UTC (permalink / raw)
To: Julien Grall; +Cc: Xen-devel, Stefano Stabellini, Steve Capper
>> > Furthermore, I think the vm_event app should only received SMCs whose
>> condition has succeeded, because they will be actual SMC. The others
>> should just be ignored.
>> >
>> > IHMO, the vm_event should only contain the immediate. The rest only
>> matters for the hypervisor.
>>
>> Absolutely not! The primary usecase I have for SMC trapping is kernel
>> execution monitoring by manually writing it into arbitrary kernel code
>> locations and hiding them from the guest with mem_access. If some SMCs
>> may silently get swallowed by the hypervisor the whole thing becomes
>> unreliable.
>
>
> Please read what I wrote, on ARMv8, a conditional SMC issued in AArch32
> state *may* trap even if the condition has failed. I.e an implementer can
> design its CPU to not trap them (see D1-1506 on ARM DDI 0487A.i).
>
> On ARMv7, only unconditional SMC and conditional SMC *which pass the
> condition test* will be trapped. The others will be ignored.
>
> So even if the hypervisor send an event for each SMC trapped, you may not
> receive all the SMCs. This is already unreliable by the architecture.
>
> If you want something reliable, you will have to inject unconditional SMC or
> HVC which are always unconditional.
Can you tell me how a conditional SMC would look like in memory? Would
it be a variant of the instruction with the condition code mnemonic
embedded in it, or the condition code is like another instruction
following the SMC? From the ARM ARM it's not entirely clear to me
(SMC{cond} #imm4). If it's the latter then we indeed need more work
done during trapping since we would need to be aware of the context of
where we are writing SMC and make sure the following condition check
is also disabled. Otherwise we can just inject unconditional SMCs and
case closed. Either way, we can swallow the SMCs with failed condition
checks, but if it already trapped to the hypervisor, we might as well
forward it to the vm_event subscriber if there is one and let it
decide what it wants to do next (jump over the instruction or crash
the domain being the only paths available).
Thanks,
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-03 15:27 ` Tamas K Lengyel
@ 2016-06-03 15:34 ` Tamas K Lengyel
2016-06-04 9:03 ` Edgar E. Iglesias
0 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-03 15:34 UTC (permalink / raw)
To: Julien Grall; +Cc: Xen-devel, Stefano Stabellini, Steve Capper
On Fri, Jun 3, 2016 at 9:27 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
>>> > Furthermore, I think the vm_event app should only received SMCs whose
>>> condition has succeeded, because they will be actual SMC. The others
>>> should just be ignored.
>>> >
>>> > IHMO, the vm_event should only contain the immediate. The rest only
>>> matters for the hypervisor.
>>>
>>> Absolutely not! The primary usecase I have for SMC trapping is kernel
>>> execution monitoring by manually writing it into arbitrary kernel code
>>> locations and hiding them from the guest with mem_access. If some SMCs
>>> may silently get swallowed by the hypervisor the whole thing becomes
>>> unreliable.
>>
>>
>> Please read what I wrote, on ARMv8, a conditional SMC issued in AArch32
>> state *may* trap even if the condition has failed. I.e an implementer can
>> design its CPU to not trap them (see D1-1506 on ARM DDI 0487A.i).
>>
>> On ARMv7, only unconditional SMC and conditional SMC *which pass the
>> condition test* will be trapped. The others will be ignored.
>>
>> So even if the hypervisor send an event for each SMC trapped, you may not
>> receive all the SMCs. This is already unreliable by the architecture.
>>
>> If you want something reliable, you will have to inject unconditional SMC or
>> HVC which are always unconditional.
>
> Can you tell me how a conditional SMC would look like in memory? Would
> it be a variant of the instruction with the condition code mnemonic
> embedded in it, or the condition code is like another instruction
> following the SMC? From the ARM ARM it's not entirely clear to me
> (SMC{cond} #imm4). If it's the latter then we indeed need more work
> done during trapping since we would need to be aware of the context of
> where we are writing SMC and make sure the following condition check
> is also disabled. Otherwise we can just inject unconditional SMCs and
> case closed. Either way, we can swallow the SMCs with failed condition
> checks, but if it already trapped to the hypervisor, we might as well
> forward it to the vm_event subscriber if there is one and let it
> decide what it wants to do next (jump over the instruction or crash
> the domain being the only paths available).
>
Never mind, found the info "This condition is encoded in ARM
instructions". So yes, we are always injecting unconditional SMCs for
monitoring so SMCs with failed condition checks are of no interest. My
comment above still stands though, we might as well forward these too
if they trapped to the VMM.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-03 15:06 ` Julien Grall
@ 2016-06-03 15:42 ` Tamas K Lengyel
0 siblings, 0 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-03 15:42 UTC (permalink / raw)
To: Julien Grall; +Cc: Xen-devel, Stefano Stabellini, Steve Capper
On Fri, Jun 3, 2016 at 9:06 AM, Julien Grall <julien.grall@arm.com> wrote:
>
>
> On 03/06/16 16:03, Tamas K Lengyel wrote:
>>
>> If transmitting the ISS to the user this way is not enough in your
>> opinion I may just leave this item up for a future user to implement
>> as my use-case doesn't need it.
>
>
> It is up to you. However, it will likely mean to bump the interface version
> of the VM Event.
That's what it is for.
>
> BTW, I have not seen any change to the interface version within this series.
> But the size of the structure will change in patch #9 which will impact all
> the introspection app. Is it normal?
>
We are increasing the interface number later in the series. We only
need to increase it once per every merge-window, i.e. we can keep
morphing it under the umbrella of a single version bump until it's
only in unstable.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
` (8 preceding siblings ...)
2016-06-03 7:08 ` [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Razvan Cojocaru
@ 2016-06-03 15:54 ` Jan Beulich
2016-06-03 16:03 ` Tamas K Lengyel
2016-06-17 19:09 ` Tamas K Lengyel
10 siblings, 1 reply; 46+ messages in thread
From: Jan Beulich @ 2016-06-03 15:54 UTC (permalink / raw)
To: Tamas K Lengyel
Cc: Andrew Cooper, Kevin Tian, Jun Nakajima, Razvan Cojocaru, xen-devel
>>> On 03.06.16 at 00:52, <tamas@tklengyel.com> wrote:
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3392,11 +3392,11 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
> break;
> }
> else {
> - int handled =
> + int rc =
> hvm_event_breakpoint(regs->eip,
> HVM_EVENT_SOFTWARE_BREAKPOINT);
>
> - if ( handled < 0 )
> + if ( !rc )
> {
> struct hvm_trap trap = {
> .vector = TRAP_int3,
> @@ -3410,7 +3410,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
> hvm_inject_trap(&trap);
> break;
> }
> - else if ( handled )
> + else if ( rc > 0 )
> break;
> }
>
Ah, I guess that's what you were referring to on the other thread.
There's indeed quite a bit of cleanup potential here. The minimal
thing I'd like to ask for is to drop the pointless "else", as you're
touching that line anyway.
With that (also doable upon commit of course)
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps
2016-06-03 15:54 ` Jan Beulich
@ 2016-06-03 16:03 ` Tamas K Lengyel
0 siblings, 0 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-03 16:03 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Kevin Tian, Jun Nakajima, Razvan Cojocaru, Xen-devel
On Fri, Jun 3, 2016 at 9:54 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 03.06.16 at 00:52, <tamas@tklengyel.com> wrote:
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -3392,11 +3392,11 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>> break;
>> }
>> else {
>> - int handled =
>> + int rc =
>> hvm_event_breakpoint(regs->eip,
>> HVM_EVENT_SOFTWARE_BREAKPOINT);
>>
>> - if ( handled < 0 )
>> + if ( !rc )
>> {
>> struct hvm_trap trap = {
>> .vector = TRAP_int3,
>> @@ -3410,7 +3410,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>> hvm_inject_trap(&trap);
>> break;
>> }
>> - else if ( handled )
>> + else if ( rc > 0 )
>> break;
>> }
>>
>
> Ah, I guess that's what you were referring to on the other thread.
> There's indeed quite a bit of cleanup potential here. The minimal
> thing I'd like to ask for is to drop the pointless "else", as you're
> touching that line anyway.
Sounds good.
>
> With that (also doable upon commit of course)
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> Jan
Thanks,
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 6/9] arm/vm_event: get/set registers
2016-06-03 10:34 ` Jan Beulich
@ 2016-06-03 19:27 ` Tamas K Lengyel
0 siblings, 0 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-03 19:27 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel, Julien Grall, Stefano Stabellini
On Fri, Jun 3, 2016 at 4:34 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 03.06.16 at 00:52, <tamas@tklengyel.com> wrote:
>> @@ -168,6 +168,69 @@ struct vm_event_regs_x86 {
>> uint32_t _pad;
>> };
>>
>> +struct vm_event_regs_arm32 {
>> + uint32_t r0;
>> + uint32_t r1;
>> + uint32_t r2;
>> + uint32_t r3;
>> + uint32_t r4;
>> + uint32_t r5;
>> + uint32_t r6;
>> + uint32_t r7;
>> + uint32_t r8;
>> + uint32_t r9;
>> + uint32_t r10;
>> + uint32_t r11;
>> + uint32_t r12;
>> + uint32_t pc;
>> +};
>
> While I had given my v4 comment on the ARM64 variant, I certainly
> meant it to apply here too: I'm missing r13/sp and r14/lr.
Yeap, l've overlooked those. Also, we will need to send TTBCR/TCR_EL1
as well so the guest can properly understand the paging format used by
TTBR0/1..
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-03 15:34 ` Tamas K Lengyel
@ 2016-06-04 9:03 ` Edgar E. Iglesias
2016-06-04 17:40 ` Tamas K Lengyel
0 siblings, 1 reply; 46+ messages in thread
From: Edgar E. Iglesias @ 2016-06-04 9:03 UTC (permalink / raw)
To: Tamas K Lengyel; +Cc: Xen-devel, Julien Grall, Stefano Stabellini, Steve Capper
On Fri, Jun 03, 2016 at 09:34:20AM -0600, Tamas K Lengyel wrote:
> On Fri, Jun 3, 2016 at 9:27 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
> >>> > Furthermore, I think the vm_event app should only received SMCs whose
> >>> condition has succeeded, because they will be actual SMC. The others
> >>> should just be ignored.
> >>> >
> >>> > IHMO, the vm_event should only contain the immediate. The rest only
> >>> matters for the hypervisor.
> >>>
> >>> Absolutely not! The primary usecase I have for SMC trapping is kernel
> >>> execution monitoring by manually writing it into arbitrary kernel code
> >>> locations and hiding them from the guest with mem_access. If some SMCs
> >>> may silently get swallowed by the hypervisor the whole thing becomes
> >>> unreliable.
> >>
> >>
> >> Please read what I wrote, on ARMv8, a conditional SMC issued in AArch32
> >> state *may* trap even if the condition has failed. I.e an implementer can
> >> design its CPU to not trap them (see D1-1506 on ARM DDI 0487A.i).
> >>
> >> On ARMv7, only unconditional SMC and conditional SMC *which pass the
> >> condition test* will be trapped. The others will be ignored.
> >>
> >> So even if the hypervisor send an event for each SMC trapped, you may not
> >> receive all the SMCs. This is already unreliable by the architecture.
> >>
> >> If you want something reliable, you will have to inject unconditional SMC or
> >> HVC which are always unconditional.
> >
> > Can you tell me how a conditional SMC would look like in memory? Would
> > it be a variant of the instruction with the condition code mnemonic
> > embedded in it, or the condition code is like another instruction
> > following the SMC? From the ARM ARM it's not entirely clear to me
> > (SMC{cond} #imm4). If it's the latter then we indeed need more work
> > done during trapping since we would need to be aware of the context of
> > where we are writing SMC and make sure the following condition check
> > is also disabled. Otherwise we can just inject unconditional SMCs and
> > case closed. Either way, we can swallow the SMCs with failed condition
> > checks, but if it already trapped to the hypervisor, we might as well
> > forward it to the vm_event subscriber if there is one and let it
> > decide what it wants to do next (jump over the instruction or crash
> > the domain being the only paths available).
> >
>
> Never mind, found the info "This condition is encoded in ARM
> instructions". So yes, we are always injecting unconditional SMCs for
> monitoring so SMCs with failed condition checks are of no interest. My
> comment above still stands though, we might as well forward these too
> if they trapped to the VMM.
>
Hi,
Forwarding SMC events for SMC insns that didn't pass the condition tests
doesn't make any sense to me. It'll just make the receivers job harder.
Why would a receiver want to do anything else than drop these?
If it actually does look at them it'll be looking at implementation
defined HW behaviour that may vary between CPU implementations.
Cheers,
Edgar
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-04 9:03 ` Edgar E. Iglesias
@ 2016-06-04 17:40 ` Tamas K Lengyel
2016-06-06 10:07 ` Julien Grall
0 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-04 17:40 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: Xen-devel, Julien Grall, Stefano Stabellini, Steve Capper
On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> On Fri, Jun 03, 2016 at 09:34:20AM -0600, Tamas K Lengyel wrote:
>> On Fri, Jun 3, 2016 at 9:27 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
>> >>> > Furthermore, I think the vm_event app should only received SMCs whose
>> >>> condition has succeeded, because they will be actual SMC. The others
>> >>> should just be ignored.
>> >>> >
>> >>> > IHMO, the vm_event should only contain the immediate. The rest only
>> >>> matters for the hypervisor.
>> >>>
>> >>> Absolutely not! The primary usecase I have for SMC trapping is kernel
>> >>> execution monitoring by manually writing it into arbitrary kernel code
>> >>> locations and hiding them from the guest with mem_access. If some SMCs
>> >>> may silently get swallowed by the hypervisor the whole thing becomes
>> >>> unreliable.
>> >>
>> >>
>> >> Please read what I wrote, on ARMv8, a conditional SMC issued in AArch32
>> >> state *may* trap even if the condition has failed. I.e an implementer can
>> >> design its CPU to not trap them (see D1-1506 on ARM DDI 0487A.i).
>> >>
>> >> On ARMv7, only unconditional SMC and conditional SMC *which pass the
>> >> condition test* will be trapped. The others will be ignored.
>> >>
>> >> So even if the hypervisor send an event for each SMC trapped, you may not
>> >> receive all the SMCs. This is already unreliable by the architecture.
>> >>
>> >> If you want something reliable, you will have to inject unconditional SMC or
>> >> HVC which are always unconditional.
>> >
>> > Can you tell me how a conditional SMC would look like in memory? Would
>> > it be a variant of the instruction with the condition code mnemonic
>> > embedded in it, or the condition code is like another instruction
>> > following the SMC? From the ARM ARM it's not entirely clear to me
>> > (SMC{cond} #imm4). If it's the latter then we indeed need more work
>> > done during trapping since we would need to be aware of the context of
>> > where we are writing SMC and make sure the following condition check
>> > is also disabled. Otherwise we can just inject unconditional SMCs and
>> > case closed. Either way, we can swallow the SMCs with failed condition
>> > checks, but if it already trapped to the hypervisor, we might as well
>> > forward it to the vm_event subscriber if there is one and let it
>> > decide what it wants to do next (jump over the instruction or crash
>> > the domain being the only paths available).
>> >
>>
>> Never mind, found the info "This condition is encoded in ARM
>> instructions". So yes, we are always injecting unconditional SMCs for
>> monitoring so SMCs with failed condition checks are of no interest. My
>> comment above still stands though, we might as well forward these too
>> if they trapped to the VMM.
>>
>
> Hi,
>
> Forwarding SMC events for SMC insns that didn't pass the condition tests
> doesn't make any sense to me. It'll just make the receivers job harder.
> Why would a receiver want to do anything else than drop these?
> If it actually does look at them it'll be looking at implementation
> defined HW behaviour that may vary between CPU implementations.
If for no other purposes it may be useful to log them to be able to
study the CPU implementation's behavior.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-04 17:40 ` Tamas K Lengyel
@ 2016-06-06 10:07 ` Julien Grall
[not found] ` <CABfawh=tOsUP1dQi9oAZM+iy3rMmCKDW=VByT-L-xYdAMBiMKw@mail.gmail.com>
0 siblings, 1 reply; 46+ messages in thread
From: Julien Grall @ 2016-06-06 10:07 UTC (permalink / raw)
To: Tamas K Lengyel, Edgar E. Iglesias
Cc: Xen-devel, Stefano Stabellini, Steve Capper
Hello,
On 04/06/2016 18:40, Tamas K Lengyel wrote:
> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
>> Forwarding SMC events for SMC insns that didn't pass the condition tests
>> doesn't make any sense to me. It'll just make the receivers job harder.
>> Why would a receiver want to do anything else than drop these?
>> If it actually does look at them it'll be looking at implementation
>> defined HW behaviour that may vary between CPU implementations.
>
> If for no other purposes it may be useful to log them to be able to
> study the CPU implementation's behavior.
I cannot see how you will be able to study ARM CPU implementation's
behavior with VM event. Though I am not familiar with it.
For now, it looks like to me that forwarding conditional SMC even if the
condition check has failed will require a lot of code in each
introspection applications, not to mention that they will need specific
code to distinguish ARMv7 vs ARMv8.
Anyway, I am planning to send a patch to ignore conditional SMCs if the
condition check has failed because this is the right thing to do.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
[not found] ` <CABfawhkSXqky9WWp8NyKEUrH_ZzSJToxAncTeSYeKBg1q63rwg@mail.gmail.com>
@ 2016-06-06 15:24 ` Tamas K Lengyel
2016-06-06 15:54 ` Julien Grall
0 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-06 15:24 UTC (permalink / raw)
To: Julien Grall
Cc: Edgar E. Iglesias, Stefano Stabellini, Xen-devel, Steve Capper
[-- Attachment #1.1: Type: text/plain, Size: 1636 bytes --]
On Jun 6, 2016 04:08, "Julien Grall" <julien.grall@arm.com> wrote:
>
> Hello,
>
>
> On 04/06/2016 18:40, Tamas K Lengyel wrote:
>>
>> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias
>> <edgar.iglesias@gmail.com> wrote:
>>>
>>> Forwarding SMC events for SMC insns that didn't pass the condition tests
>>> doesn't make any sense to me. It'll just make the receivers job harder.
>>> Why would a receiver want to do anything else than drop these?
>>> If it actually does look at them it'll be looking at implementation
>>> defined HW behaviour that may vary between CPU implementations.
>>
>>
>> If for no other purposes it may be useful to log them to be able to
>> study the CPU implementation's behavior.
>
>
> I cannot see how you will be able to study ARM CPU implementation's
behavior with VM event. Though I am not familiar with it.
>
> For now, it looks like to me that forwarding conditional SMC even if the
condition check has failed will require a lot of code in each introspection
applications, not to mention that they will need specific code to
distinguish ARMv7 vs ARMv8.
Why would it require any more code? Right now the only thing the listener
can do is to increment pc to jump over the SMC. That would be the same
regardless of what type it was. As for checking whether it was v7 or v8, if
that is of interest to the app it should implement the appropriate logic
for it. I don't see a problem there either.
>
> Anyway, I am planning to send a patch to ignore conditional SMCs if the
condition check has failed because this is the right thing to do.
As I said I don't really care for these cases so fine by me.
Tamas
[-- Attachment #1.2: Type: text/html, Size: 2128 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-06 15:24 ` Tamas K Lengyel
@ 2016-06-06 15:54 ` Julien Grall
2016-06-06 15:56 ` Tamas K Lengyel
0 siblings, 1 reply; 46+ messages in thread
From: Julien Grall @ 2016-06-06 15:54 UTC (permalink / raw)
To: Tamas K Lengyel
Cc: Edgar E. Iglesias, Stefano Stabellini, Xen-devel, Steve Capper
Hello Tamas,
On 06/06/16 16:24, Tamas K Lengyel wrote:
>
> On Jun 6, 2016 04:08, "Julien Grall" <julien.grall@arm.com
> <mailto:julien.grall@arm.com>> wrote:
> >
> > Hello,
> >
> >
> > On 04/06/2016 18:40, Tamas K Lengyel wrote:
> >>
> >> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias
> >> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
> >>>
> >>> Forwarding SMC events for SMC insns that didn't pass the condition
> tests
> >>> doesn't make any sense to me. It'll just make the receivers job harder.
> >>> Why would a receiver want to do anything else than drop these?
> >>> If it actually does look at them it'll be looking at implementation
> >>> defined HW behaviour that may vary between CPU implementations.
> >>
> >>
> >> If for no other purposes it may be useful to log them to be able to
> >> study the CPU implementation's behavior.
> >
> >
> > I cannot see how you will be able to study ARM CPU implementation's
> behavior with VM event. Though I am not familiar with it.
> >
> > For now, it looks like to me that forwarding conditional SMC even if
> the condition check has failed will require a lot of code in each
> introspection applications, not to mention that they will need specific
> code to distinguish ARMv7 vs ARMv8.
>
> Why would it require any more code? Right now the only thing the
> listener can do is to increment pc to jump over the SMC. That would be
> the same regardless of what type it was. As for checking whether it was
> v7 or v8, if that is of interest to the app it should implement the
> appropriate logic for it. I don't see a problem there either.
A listener can to do more then incrementing pc to jump over the SMC. The
app may want to decode the instruction to get the immediate and then
execute different actions depending on its value (for instance to
emulate some SMC call).
For this kind of app, it will be necessary to find out whether the
condition check has failed or not before executing it.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-06 15:54 ` Julien Grall
@ 2016-06-06 15:56 ` Tamas K Lengyel
2016-06-06 16:14 ` Tamas K Lengyel
0 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-06 15:56 UTC (permalink / raw)
To: Julien Grall
Cc: Edgar E. Iglesias, Stefano Stabellini, Xen-devel, Steve Capper
On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote:
> Hello Tamas,
>
> On 06/06/16 16:24, Tamas K Lengyel wrote:
>>
>>
>> On Jun 6, 2016 04:08, "Julien Grall" <julien.grall@arm.com
>> <mailto:julien.grall@arm.com>> wrote:
>> >
>> > Hello,
>> >
>> >
>> > On 04/06/2016 18:40, Tamas K Lengyel wrote:
>> >>
>> >> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias
>> >> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
>> >>>
>> >>> Forwarding SMC events for SMC insns that didn't pass the condition
>> tests
>> >>> doesn't make any sense to me. It'll just make the receivers job
>> harder.
>> >>> Why would a receiver want to do anything else than drop these?
>> >>> If it actually does look at them it'll be looking at implementation
>> >>> defined HW behaviour that may vary between CPU implementations.
>> >>
>> >>
>> >> If for no other purposes it may be useful to log them to be able to
>> >> study the CPU implementation's behavior.
>> >
>> >
>> > I cannot see how you will be able to study ARM CPU implementation's
>> behavior with VM event. Though I am not familiar with it.
>> >
>> > For now, it looks like to me that forwarding conditional SMC even if
>> the condition check has failed will require a lot of code in each
>> introspection applications, not to mention that they will need specific
>> code to distinguish ARMv7 vs ARMv8.
>>
>> Why would it require any more code? Right now the only thing the
>> listener can do is to increment pc to jump over the SMC. That would be
>> the same regardless of what type it was. As for checking whether it was
>> v7 or v8, if that is of interest to the app it should implement the
>> appropriate logic for it. I don't see a problem there either.
>
>
> A listener can to do more then incrementing pc to jump over the SMC. The app
> may want to decode the instruction to get the immediate and then execute
> different actions depending on its value (for instance to emulate some SMC
> call).
>
> For this kind of app, it will be necessary to find out whether the condition
> check has failed or not before executing it.
>
Sure, and if that need arises the extra information can also be
forwarded. Especially if as you said you are planning to implement the
decoding for Xen to see if it was a failed condition check or not.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-06 15:56 ` Tamas K Lengyel
@ 2016-06-06 16:14 ` Tamas K Lengyel
2016-06-06 16:38 ` Julien Grall
0 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-06 16:14 UTC (permalink / raw)
To: Julien Grall
Cc: Edgar E. Iglesias, Stefano Stabellini, Xen-devel, Steve Capper
On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote:
>> Hello Tamas,
>>
>> On 06/06/16 16:24, Tamas K Lengyel wrote:
>>>
>>>
>>> On Jun 6, 2016 04:08, "Julien Grall" <julien.grall@arm.com
>>> <mailto:julien.grall@arm.com>> wrote:
>>> >
>>> > Hello,
>>> >
>>> >
>>> > On 04/06/2016 18:40, Tamas K Lengyel wrote:
>>> >>
>>> >> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias
>>> >> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote:
>>> >>>
>>> >>> Forwarding SMC events for SMC insns that didn't pass the condition
>>> tests
>>> >>> doesn't make any sense to me. It'll just make the receivers job
>>> harder.
>>> >>> Why would a receiver want to do anything else than drop these?
>>> >>> If it actually does look at them it'll be looking at implementation
>>> >>> defined HW behaviour that may vary between CPU implementations.
>>> >>
>>> >>
>>> >> If for no other purposes it may be useful to log them to be able to
>>> >> study the CPU implementation's behavior.
>>> >
>>> >
>>> > I cannot see how you will be able to study ARM CPU implementation's
>>> behavior with VM event. Though I am not familiar with it.
>>> >
>>> > For now, it looks like to me that forwarding conditional SMC even if
>>> the condition check has failed will require a lot of code in each
>>> introspection applications, not to mention that they will need specific
>>> code to distinguish ARMv7 vs ARMv8.
>>>
>>> Why would it require any more code? Right now the only thing the
>>> listener can do is to increment pc to jump over the SMC. That would be
>>> the same regardless of what type it was. As for checking whether it was
>>> v7 or v8, if that is of interest to the app it should implement the
>>> appropriate logic for it. I don't see a problem there either.
>>
>>
>> A listener can to do more then incrementing pc to jump over the SMC. The app
>> may want to decode the instruction to get the immediate and then execute
>> different actions depending on its value (for instance to emulate some SMC
>> call).
>>
>> For this kind of app, it will be necessary to find out whether the condition
>> check has failed or not before executing it.
>>
>
> Sure, and if that need arises the extra information can also be
> forwarded. Especially if as you said you are planning to implement the
> decoding for Xen to see if it was a failed condition check or not.
>
So either way, I don't see a technical reason why Xen should silently
swallow any SMC trap if the vm_event user specifically asked them to
be forwarded. Other then it being odd that some ARM chips have varying
behavior regarding a subset of SMC instructions, it should not affect
when the vm_event user gets the events. If the user requests that it
wants to get notified any time an SMC is trapped to the VMM, it
should, regardless of whether that makes sense for "us". Depending on
the use-case of the user, indeed it may need extra information if it
wants to do emulation. If that need arises, the interface can easily
be extended to accommodate that usecase. We can also add a comment
saying that the forwarded events may also include ones with failed
condition checks depending on the CPU implementation. Also, it would
also be possible in the future to add a monitor configuration bit
where the user can specify if it wants the failed condition check SMCs
ignored by default or not. At this time however I want to start simple
and just forward all events, adding more bits and pieces only as
needed.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-06 16:14 ` Tamas K Lengyel
@ 2016-06-06 16:38 ` Julien Grall
2016-06-06 17:28 ` Tamas K Lengyel
2016-06-07 7:13 ` Jan Beulich
0 siblings, 2 replies; 46+ messages in thread
From: Julien Grall @ 2016-06-06 16:38 UTC (permalink / raw)
To: Tamas K Lengyel
Cc: Edgar E. Iglesias, Stefano Stabellini, Xen-devel, Steve Capper
Hi Tamas,
On 06/06/16 17:14, Tamas K Lengyel wrote:
> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote:
> So either way, I don't see a technical reason why Xen should silently
> swallow any SMC trap if the vm_event user specifically asked them to
> be forwarded. Other then it being odd that some ARM chips have varying
> behavior regarding a subset of SMC instructions, it should not affect
> when the vm_event user gets the events. If the user requests that it
> wants to get notified any time an SMC is trapped to the VMM, it
> should, regardless of whether that makes sense for "us". Depending on
> the use-case of the user, indeed it may need extra information if it
> wants to do emulation. If that need arises, the interface can easily
> be extended to accommodate that usecase. We can also add a comment
> saying that the forwarded events may also include ones with failed
> condition checks depending on the CPU implementation. Also, it would
> also be possible in the future to add a monitor configuration bit
> where the user can specify if it wants the failed condition check SMCs
> ignored by default or not. At this time however I want to start simple
> and just forward all events, adding more bits and pieces only as
> needed.
We disagree on what is a "starting simple". It easier to relax than
restricting a behavior later one.
Even if we decide to add a bit to ignore some SMC in a later version of
Xen, the introspection app will need to carry the burden mentioned in
lengthly way on the previous mails because they may want to support
older version of Xen.
It would not be that difficult to provide a clean interface from
beginning, which would allow to support more than your usecase.
Anyway, I am not gonna ack this patch for the modification in
arch/arm/traps.c because I don't think this is the right way to go. I
will let Stefano deciding on this one.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-06 16:38 ` Julien Grall
@ 2016-06-06 17:28 ` Tamas K Lengyel
2016-06-07 7:13 ` Jan Beulich
1 sibling, 0 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-06 17:28 UTC (permalink / raw)
To: Julien Grall
Cc: Edgar E. Iglesias, Stefano Stabellini, Xen-devel, Steve Capper
On Mon, Jun 6, 2016 at 10:38 AM, Julien Grall <julien.grall@arm.com> wrote:
> Hi Tamas,
>
> On 06/06/16 17:14, Tamas K Lengyel wrote:
>>
>> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com>
>> wrote:
>>>
>>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com>
>>> wrote:
>>
>> So either way, I don't see a technical reason why Xen should silently
>> swallow any SMC trap if the vm_event user specifically asked them to
>> be forwarded. Other then it being odd that some ARM chips have varying
>> behavior regarding a subset of SMC instructions, it should not affect
>> when the vm_event user gets the events. If the user requests that it
>> wants to get notified any time an SMC is trapped to the VMM, it
>> should, regardless of whether that makes sense for "us". Depending on
>> the use-case of the user, indeed it may need extra information if it
>> wants to do emulation. If that need arises, the interface can easily
>> be extended to accommodate that usecase. We can also add a comment
>> saying that the forwarded events may also include ones with failed
>> condition checks depending on the CPU implementation. Also, it would
>> also be possible in the future to add a monitor configuration bit
>> where the user can specify if it wants the failed condition check SMCs
>> ignored by default or not. At this time however I want to start simple
>> and just forward all events, adding more bits and pieces only as
>> needed.
>
>
> We disagree on what is a "starting simple". It easier to relax than
> restricting a behavior later one.
>
> Even if we decide to add a bit to ignore some SMC in a later version of Xen,
> the introspection app will need to carry the burden mentioned in lengthly
> way on the previous mails because they may want to support older version of
> Xen.
>
> It would not be that difficult to provide a clean interface from beginning,
> which would allow to support more than your usecase.
>
> Anyway, I am not gonna ack this patch for the modification in
> arch/arm/traps.c because I don't think this is the right way to go. I will
> let Stefano deciding on this one.
>
Sounds good to me, my use-case is fine either way so ultimately it's
up to you guys.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-06 16:38 ` Julien Grall
2016-06-06 17:28 ` Tamas K Lengyel
@ 2016-06-07 7:13 ` Jan Beulich
2016-06-07 10:30 ` Stefano Stabellini
1 sibling, 1 reply; 46+ messages in thread
From: Jan Beulich @ 2016-06-07 7:13 UTC (permalink / raw)
To: Julien Grall, Tamas K Lengyel
Cc: Edgar E. Iglesias, Stefano Stabellini, Xen-devel, Steve Capper
>>> On 06.06.16 at 18:38, <julien.grall@arm.com> wrote:
> On 06/06/16 17:14, Tamas K Lengyel wrote:
>> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
>>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote:
>> So either way, I don't see a technical reason why Xen should silently
>> swallow any SMC trap if the vm_event user specifically asked them to
>> be forwarded. Other then it being odd that some ARM chips have varying
>> behavior regarding a subset of SMC instructions, it should not affect
>> when the vm_event user gets the events. If the user requests that it
>> wants to get notified any time an SMC is trapped to the VMM, it
>> should, regardless of whether that makes sense for "us". Depending on
>> the use-case of the user, indeed it may need extra information if it
>> wants to do emulation. If that need arises, the interface can easily
>> be extended to accommodate that usecase. We can also add a comment
>> saying that the forwarded events may also include ones with failed
>> condition checks depending on the CPU implementation. Also, it would
>> also be possible in the future to add a monitor configuration bit
>> where the user can specify if it wants the failed condition check SMCs
>> ignored by default or not. At this time however I want to start simple
>> and just forward all events, adding more bits and pieces only as
>> needed.
>
> We disagree on what is a "starting simple". It easier to relax than
> restricting a behavior later one.
>
> Even if we decide to add a bit to ignore some SMC in a later version of
> Xen, the introspection app will need to carry the burden mentioned in
> lengthly way on the previous mails because they may want to support
> older version of Xen.
FWIW, I'm with Julien here given the information available so far
on this thread. Some of the basic problem is that the original
patch (and namely its modification to the public header) doesn't
really make clear what's intended: To intercept all SMC instruction
uses (aiui that's impossible on some hardware) or to intercept all
privileged calls resulting from their use (in which case instances
with the condition being false wouldn't count).
What you, Tamas, want to get to seems to be some middle
ground, which I don't see what use it would be to the consumer.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-07 7:13 ` Jan Beulich
@ 2016-06-07 10:30 ` Stefano Stabellini
2016-06-07 16:06 ` Tamas K Lengyel
0 siblings, 1 reply; 46+ messages in thread
From: Stefano Stabellini @ 2016-06-07 10:30 UTC (permalink / raw)
To: Jan Beulich
Cc: Tamas K Lengyel, Steve Capper, Julien Grall, Stefano Stabellini,
Xen-devel, Edgar E. Iglesias
On Tue, 7 Jun 2016, Jan Beulich wrote:
> >>> On 06.06.16 at 18:38, <julien.grall@arm.com> wrote:
> > On 06/06/16 17:14, Tamas K Lengyel wrote:
> >> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
> >>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote:
> >> So either way, I don't see a technical reason why Xen should silently
> >> swallow any SMC trap if the vm_event user specifically asked them to
> >> be forwarded. Other then it being odd that some ARM chips have varying
> >> behavior regarding a subset of SMC instructions, it should not affect
> >> when the vm_event user gets the events. If the user requests that it
> >> wants to get notified any time an SMC is trapped to the VMM, it
> >> should, regardless of whether that makes sense for "us". Depending on
> >> the use-case of the user, indeed it may need extra information if it
> >> wants to do emulation. If that need arises, the interface can easily
> >> be extended to accommodate that usecase. We can also add a comment
> >> saying that the forwarded events may also include ones with failed
> >> condition checks depending on the CPU implementation. Also, it would
> >> also be possible in the future to add a monitor configuration bit
> >> where the user can specify if it wants the failed condition check SMCs
> >> ignored by default or not. At this time however I want to start simple
> >> and just forward all events, adding more bits and pieces only as
> >> needed.
> >
> > We disagree on what is a "starting simple". It easier to relax than
> > restricting a behavior later one.
> >
> > Even if we decide to add a bit to ignore some SMC in a later version of
> > Xen, the introspection app will need to carry the burden mentioned in
> > lengthly way on the previous mails because they may want to support
> > older version of Xen.
>
> FWIW, I'm with Julien here given the information available so far
> on this thread. Some of the basic problem is that the original
> patch (and namely its modification to the public header) doesn't
> really make clear what's intended: To intercept all SMC instruction
> uses (aiui that's impossible on some hardware) or to intercept all
> privileged calls resulting from their use (in which case instances
> with the condition being false wouldn't count).
Right. I think that the first thing to do would be to write down in the
public header file what is the intended behavior. Given the scope for
confusion, this is necessary regardless of the chosen behavior.
> What you, Tamas, want to get to seems to be some middle
> ground, which I don't see what use it would be to the consumer.
I think that forwarding SMC events only for unconditional SMCs and SMCs
which succeeded the conditional check would make for a better interface.
This would be my preference.
If you really want to forward SMC events for SMCs which failed the
conditional check, then please add to the SMC event struct all the
necessary information so that the monitoring application can quickly
find out whether the conditional check succeeded or failed without
jumping through hoops.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 5/9] monitor: ARM SMC events
2016-06-07 10:30 ` Stefano Stabellini
@ 2016-06-07 16:06 ` Tamas K Lengyel
0 siblings, 0 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-07 16:06 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Edgar E. Iglesias, Julien Grall, Steve Capper, Jan Beulich, Xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 3619 bytes --]
On Jun 7, 2016 04:30, "Stefano Stabellini" <sstabellini@kernel.org> wrote:
>
> On Tue, 7 Jun 2016, Jan Beulich wrote:
> > >>> On 06.06.16 at 18:38, <julien.grall@arm.com> wrote:
> > > On 06/06/16 17:14, Tamas K Lengyel wrote:
> > >> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com>
wrote:
> > >>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com>
wrote:
> > >> So either way, I don't see a technical reason why Xen should silently
> > >> swallow any SMC trap if the vm_event user specifically asked them to
> > >> be forwarded. Other then it being odd that some ARM chips have
varying
> > >> behavior regarding a subset of SMC instructions, it should not affect
> > >> when the vm_event user gets the events. If the user requests that it
> > >> wants to get notified any time an SMC is trapped to the VMM, it
> > >> should, regardless of whether that makes sense for "us". Depending on
> > >> the use-case of the user, indeed it may need extra information if it
> > >> wants to do emulation. If that need arises, the interface can easily
> > >> be extended to accommodate that usecase. We can also add a comment
> > >> saying that the forwarded events may also include ones with failed
> > >> condition checks depending on the CPU implementation. Also, it would
> > >> also be possible in the future to add a monitor configuration bit
> > >> where the user can specify if it wants the failed condition check
SMCs
> > >> ignored by default or not. At this time however I want to start
simple
> > >> and just forward all events, adding more bits and pieces only as
> > >> needed.
> > >
> > > We disagree on what is a "starting simple". It easier to relax than
> > > restricting a behavior later one.
> > >
> > > Even if we decide to add a bit to ignore some SMC in a later version
of
> > > Xen, the introspection app will need to carry the burden mentioned in
> > > lengthly way on the previous mails because they may want to support
> > > older version of Xen.
> >
> > FWIW, I'm with Julien here given the information available so far
> > on this thread. Some of the basic problem is that the original
> > patch (and namely its modification to the public header) doesn't
> > really make clear what's intended: To intercept all SMC instruction
> > uses (aiui that's impossible on some hardware) or to intercept all
> > privileged calls resulting from their use (in which case instances
> > with the condition being false wouldn't count).
>
> Right. I think that the first thing to do would be to write down in the
> public header file what is the intended behavior. Given the scope for
> confusion, this is necessary regardless of the chosen behavior.
>
>
> > What you, Tamas, want to get to seems to be some middle
> > ground, which I don't see what use it would be to the consumer.
>
> I think that forwarding SMC events only for unconditional SMCs and SMCs
> which succeeded the conditional check would make for a better interface.
> This would be my preference.
>
> If you really want to forward SMC events for SMCs which failed the
> conditional check, then please add to the SMC event struct all the
> necessary information so that the monitoring application can quickly
> find out whether the conditional check succeeded or failed without
> jumping through hoops.
Ack. As I said I have no use for conditional SMCs at all so this is beyond
what I am looking for. From my perspective it is just easier to forward
every trap.
So as for doing the actual filtering, Julien mentioned he is going to add a
patch for that. I'll wait for that and then rebase on top.
Thanks,
Tamas
[-- Attachment #1.2: Type: text/html, Size: 4622 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 2/9] monitor: Rename vm_event_monitor_get_capabilities
2016-06-02 22:52 ` [PATCH v5 2/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
@ 2016-06-17 19:07 ` Tamas K Lengyel
2016-06-21 9:20 ` Julien Grall
1 sibling, 0 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-17 19:07 UTC (permalink / raw)
To: Xen-devel; +Cc: Julien Grall, Tamas K Lengyel, Stefano Stabellini, Jan Beulich
On Thu, Jun 2, 2016 at 4:52 PM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
> The monitor_get_capabilities check actually belongs to the monitor subsystem so
> relocating and renaming it to sanitize the code's name and location. Mechanical
> patch, no code changes introduced.
>
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
Pinging the rest of the maintainers to get an update on this patch.
Thanks,
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
` (9 preceding siblings ...)
2016-06-03 15:54 ` Jan Beulich
@ 2016-06-17 19:09 ` Tamas K Lengyel
2016-06-24 10:58 ` Tian, Kevin
10 siblings, 1 reply; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-17 19:09 UTC (permalink / raw)
To: Xen-devel
Cc: Kevin Tian, Tamas K Lengyel, Jun Nakajima, Razvan Cojocaru,
Andrew Cooper, Jan Beulich
On Thu, Jun 2, 2016 at 4:52 PM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
> The return value has not been clearly defined, with the function
> never returning 0 which seemingly indicated a condition where the
> guest should crash. In this patch we define -rc as error condition
> where the notification was not sent, 0 where the notification was sent
> and the vCPU is not paused and 1 that the notification was sent and
> that the vCPU is paused.
>
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Pinging the rest of the maintainers to get an update on this patch.
Current status is:
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Thanks,
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 3/9] monitor: Rename vm_event_monitor_guest_request
2016-06-02 22:52 ` [PATCH v5 3/9] monitor: Rename vm_event_monitor_guest_request Tamas K Lengyel
@ 2016-06-17 19:10 ` Tamas K Lengyel
2016-06-21 9:18 ` Julien Grall
1 sibling, 0 replies; 46+ messages in thread
From: Tamas K Lengyel @ 2016-06-17 19:10 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Julien Grall, Tamas K Lengyel, Stefano Stabellini
On Thu, Jun 2, 2016 at 4:52 PM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
> Mechanical renaming and relocation to the monitor subsystem.
>
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Pinging the rest of the maintainers to get an update on this patch.
Thanks,
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 3/9] monitor: Rename vm_event_monitor_guest_request
2016-06-02 22:52 ` [PATCH v5 3/9] monitor: Rename vm_event_monitor_guest_request Tamas K Lengyel
2016-06-17 19:10 ` Tamas K Lengyel
@ 2016-06-21 9:18 ` Julien Grall
1 sibling, 0 replies; 46+ messages in thread
From: Julien Grall @ 2016-06-21 9:18 UTC (permalink / raw)
To: Tamas K Lengyel, xen-devel; +Cc: Andrew Cooper, Stefano Stabellini
Hello Tamas,
On 02/06/16 23:52, Tamas K Lengyel wrote:
> Mechanical renaming and relocation to the monitor subsystem.
>
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
For ARM bits:
Acked-by: Julien Grall <julien.grall@arm.com>
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 2/9] monitor: Rename vm_event_monitor_get_capabilities
2016-06-02 22:52 ` [PATCH v5 2/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
2016-06-17 19:07 ` Tamas K Lengyel
@ 2016-06-21 9:20 ` Julien Grall
1 sibling, 0 replies; 46+ messages in thread
From: Julien Grall @ 2016-06-21 9:20 UTC (permalink / raw)
To: Tamas K Lengyel, xen-devel; +Cc: Stefano Stabellini, Jan Beulich
Hello Tamas,
On 02/06/16 23:52, Tamas K Lengyel wrote:
> The monitor_get_capabilities check actually belongs to the monitor subsystem so
> relocating and renaming it to sanitize the code's name and location. Mechanical
> patch, no code changes introduced.
>
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
For the ARM bits:
Acked-by: Julien Grall <julien.grall@arm.com>
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps
2016-06-17 19:09 ` Tamas K Lengyel
@ 2016-06-24 10:58 ` Tian, Kevin
0 siblings, 0 replies; 46+ messages in thread
From: Tian, Kevin @ 2016-06-24 10:58 UTC (permalink / raw)
To: Tamas K Lengyel, Xen-devel
Cc: Andrew Cooper, Jan Beulich, Nakajima, Jun, Razvan Cojocaru
> From: Tamas K Lengyel [mailto:tamas@tklengyel.com]
> Sent: Saturday, June 18, 2016 3:09 AM
>
> On Thu, Jun 2, 2016 at 4:52 PM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
> > The return value has not been clearly defined, with the function
> > never returning 0 which seemingly indicated a condition where the
> > guest should crash. In this patch we define -rc as error condition
> > where the notification was not sent, 0 where the notification was sent
> > and the vCPU is not paused and 1 that the notification was sent and
> > that the vCPU is paused.
> >
> > Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> > ---
> > Cc: Jun Nakajima <jun.nakajima@intel.com>
> > Cc: Kevin Tian <kevin.tian@intel.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Pinging the rest of the maintainers to get an update on this patch.
> Current status is:
>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 46+ messages in thread
end of thread, other threads:[~2016-06-24 10:58 UTC | newest]
Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-02 22:52 [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 2/9] monitor: Rename vm_event_monitor_get_capabilities Tamas K Lengyel
2016-06-17 19:07 ` Tamas K Lengyel
2016-06-21 9:20 ` Julien Grall
2016-06-02 22:52 ` [PATCH v5 3/9] monitor: Rename vm_event_monitor_guest_request Tamas K Lengyel
2016-06-17 19:10 ` Tamas K Lengyel
2016-06-21 9:18 ` Julien Grall
2016-06-02 22:52 ` [PATCH v5 4/9] monitor: Rename hvm/event to hvm/monitor Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 5/9] monitor: ARM SMC events Tamas K Lengyel
2016-06-03 9:49 ` Julien Grall
2016-06-03 13:40 ` Tamas K Lengyel
2016-06-03 14:43 ` Julien Grall
2016-06-03 15:03 ` Tamas K Lengyel
2016-06-03 15:06 ` Julien Grall
2016-06-03 15:42 ` Tamas K Lengyel
2016-06-03 15:27 ` Tamas K Lengyel
2016-06-03 15:34 ` Tamas K Lengyel
2016-06-04 9:03 ` Edgar E. Iglesias
2016-06-04 17:40 ` Tamas K Lengyel
2016-06-06 10:07 ` Julien Grall
[not found] ` <CABfawh=tOsUP1dQi9oAZM+iy3rMmCKDW=VByT-L-xYdAMBiMKw@mail.gmail.com>
[not found] ` <CABfawhkSXqky9WWp8NyKEUrH_ZzSJToxAncTeSYeKBg1q63rwg@mail.gmail.com>
2016-06-06 15:24 ` Tamas K Lengyel
2016-06-06 15:54 ` Julien Grall
2016-06-06 15:56 ` Tamas K Lengyel
2016-06-06 16:14 ` Tamas K Lengyel
2016-06-06 16:38 ` Julien Grall
2016-06-06 17:28 ` Tamas K Lengyel
2016-06-07 7:13 ` Jan Beulich
2016-06-07 10:30 ` Stefano Stabellini
2016-06-07 16:06 ` Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 6/9] arm/vm_event: get/set registers Tamas K Lengyel
2016-06-03 10:34 ` Jan Beulich
2016-06-03 19:27 ` Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 7/9] tools/libxc: add xc_monitor_privileged_call Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 8/9] x86/vm_event: Add HVM debug exception vm_events Tamas K Lengyel
2016-06-03 10:49 ` Jan Beulich
2016-06-03 13:29 ` Tamas K Lengyel
2016-06-03 14:23 ` Jan Beulich
2016-06-03 14:34 ` Tamas K Lengyel
2016-06-03 14:45 ` Jan Beulich
2016-06-03 14:51 ` Tamas K Lengyel
2016-06-02 22:52 ` [PATCH v5 9/9] tools/xen-access: add test-case for ARM SMC Tamas K Lengyel
2016-06-03 7:08 ` [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps Razvan Cojocaru
2016-06-03 15:54 ` Jan Beulich
2016-06-03 16:03 ` Tamas K Lengyel
2016-06-17 19:09 ` Tamas K Lengyel
2016-06-24 10:58 ` Tian, Kevin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).