Re: [Xen-devel] [PATCH v2 4/5] x86/hvm: Add debug exception vm_events

[Tamas K Lengyel]

On May 2, 2016 09:40, "Jan Beulich"  wrote:
>
> >>> On 02.05.16 at 17:35,  wrote:
> > On May 2, 2016 07:12, "Jan Beulich"  wrote:
> >>
> >> >>> On 29.04.16 at 20:07,  wrote:
> >> > @@ -229,8 +231,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];
> >> >  };
> >>
> >> This being an incompatible change - didn't you mean to increment some
> >> version number?
> >
> > I'm not sure. It would still work with clients compiled with the older
> > version of the header as the layout of the debug struct didnt change,
was
> > just appended. The size of the request/response struct didn't change
either
> > so technically this would still be backwards compatible.
>
> But you also need to consider the other direction: Code compiled
> against the new variant, but running on an older hypervisor would
> expect the new fields to be valid, yet they can't be, and the caller
> has no way to know.

Fair point, will incrementnthe version.

Thanks!
Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 4/5] x86/hvm: Add debug exception vm_events

[Jan Beulich]

>>> On 02.05.16 at 17:35,  wrote:
> On May 2, 2016 07:12, "Jan Beulich"  wrote:
>>
>> >>> On 29.04.16 at 20:07,  wrote:
>> > @@ -229,8 +231,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];
>> >  };
>>
>> This being an incompatible change - didn't you mean to increment some
>> version number?
> 
> I'm not sure. It would still work with clients compiled with the older
> version of the header as the layout of the debug struct didnt change, was
> just appended. The size of the request/response struct didn't change either
> so technically this would still be backwards compatible.

But you also need to consider the other direction: Code compiled
against the new variant, but running on an older hypervisor would
expect the new fields to be valid, yet they can't be, and the caller
has no way to know.

Jan


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

Re: [Xen-devel] [PATCH v2 4/5] x86/hvm: Add debug exception vm_events

[Tamas K Lengyel]

On May 2, 2016 07:12, "Jan Beulich"  wrote:
>
> >>> On 29.04.16 at 20:07,  wrote:
> > @@ -229,8 +231,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];
> >  };
>
> This being an incompatible change - didn't you mean to increment some
> version number?
>
> Jan

I'm not sure. It would still work with clients compiled with the older
version of the header as the layout of the debug struct didnt change, was
just appended. The size of the request/response struct didn't change either
so technically this would still be backwards compatible.

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

Re: [Xen-devel] [PATCH v2 4/5] x86/hvm: Add debug exception vm_events

[Jan Beulich]

>>> On 29.04.16 at 20:07,  wrote:
> @@ -229,8 +231,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];
>  };

This being an incompatible change - didn't you mean to increment some
version number?

Jan



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

[Xen-devel] [PATCH v2 4/5] x86/hvm: Add debug exception vm_events

[Tamas K Lengyel]

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 
---
Cc: Razvan Cojocaru 
Cc: Keir Fraser 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Jun Nakajima 
Cc: Kevin Tian 

v2: Rename hvm_monitor_event to hvm_monitor_debug
---
 tools/libxc/include/xenctrl.h   |  3 +-
 tools/libxc/xc_monitor.c| 15 +
 tools/tests/xen-access/xen-access.c | 61 -
 xen/arch/x86/hvm/monitor.c  | 26 ++--
 xen/arch/x86/hvm/vmx/vmx.c  | 26 +---
 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   | 12 +++-
 11 files changed, 156 insertions(+), 21 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 4b75ae4..ff3ba9e 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 072df70..e9b0343 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, );
 }
 
+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, );
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/tests/xen-access/xen-access.c 
b/tools/tests/xen-access/xen-access.c
index 33e8044..ae235e2 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]  write|exec", progname);
 #if defined(__i386__) || defined(__x86_64__)
-fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec");
+fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug");
 #elif defined(__arm__) || defined(__aarch64__)
 fprintf(stderr, "|privcall");
 #endif
@@ -356,11 +360,13 @@ 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 privcall = 0;
 int altp2m = 0;
+int debug = 0;
 uint16_t altp2m_view_id = 0;
 
 char* progname = argv[0];
@@ -394,11 +400,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") )
@@ -409,11 +417,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;
+