* write_ctrlreg event masking
@ 2017-05-26 13:41 Petre Pircalabu
2017-05-26 13:41 ` [PATCH 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
` (2 more replies)
0 siblings, 3 replies; 30+ messages in thread
From: Petre Pircalabu @ 2017-05-26 13:41 UTC (permalink / raw)
To: xen-devel
Cc: tamas, wei.liu2, rcojocaru, andrew.cooper3, ian.jackson, jbeulich
Hello,
This patchset enables masking the reception of write_ctrlreg events depending
on the value of certain bits in that register.
The most representative example is filtering out events when the CR4.PGE
bit is being flipped (global TLB flushes)
Best regards,
Petre
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-05-26 13:41 write_ctrlreg event masking Petre Pircalabu
@ 2017-05-26 13:41 ` Petre Pircalabu
2017-05-29 15:01 ` Jan Beulich
2017-05-26 13:41 ` [PATCH 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
2017-05-30 9:46 ` write_ctrlreg event masking Petre Pircalabu
2 siblings, 1 reply; 30+ messages in thread
From: Petre Pircalabu @ 2017-05-26 13:41 UTC (permalink / raw)
To: xen-devel
Cc: Petre Pircalabu, tamas, wei.liu2, rcojocaru, andrew.cooper3,
ian.jackson, jbeulich
Add support for filtering out the write_ctrlreg monitor events if they
are generated only by changing certains bits.
A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
function in order to mask the event generation if the changed bits are
set.
Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
---
tools/libxc/include/xenctrl.h | 2 +-
tools/libxc/xc_monitor.c | 3 ++-
xen/arch/x86/hvm/monitor.c | 3 ++-
xen/arch/x86/monitor.c | 6 ++++++
xen/include/asm-x86/domain.h | 1 +
xen/include/public/domctl.h | 6 +++++-
xen/include/public/vm_event.h | 9 +++++++++
7 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 1629f41..8c26cb4 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1999,7 +1999,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
uint32_t *capabilities);
int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
uint16_t index, bool enable, bool sync,
- bool onchangeonly);
+ uint64_t bitmask, bool onchangeonly);
/*
* A list of MSR indices can usually be found in /usr/include/asm/msr-index.h.
* Please consult the Intel/AMD manuals for more information on
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index f99b6e3..70648d7 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -70,7 +70,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
uint16_t index, bool enable, bool sync,
- bool onchangeonly)
+ uint64_t bitmask, bool onchangeonly)
{
DECLARE_DOMCTL;
@@ -82,6 +82,7 @@ int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
domctl.u.monitor_op.u.mov_to_cr.index = index;
domctl.u.monitor_op.u.mov_to_cr.sync = sync;
domctl.u.monitor_op.u.mov_to_cr.onchangeonly = onchangeonly;
+ domctl.u.monitor_op.u.mov_to_cr.bitmask = bitmask;
return do_domctl(xch, &domctl);
}
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index bde5fd0..a7ccfc4 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -38,7 +38,8 @@ bool_t hvm_monitor_cr(unsigned int index, unsigned long value, unsigned long old
if ( (ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask) &&
(!(ad->monitor.write_ctrlreg_onchangeonly & ctrlreg_bitmask) ||
- value != old) )
+ value != old) &&
+ (!((value ^ old) & ad->monitor.write_ctrlreg_mask[index])) )
{
bool_t sync = !!(ad->monitor.write_ctrlreg_sync & ctrlreg_bitmask);
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 449c64c..d02d2ea 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -155,9 +155,15 @@ int arch_monitor_domctl_event(struct domain *d,
ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask;
if ( requested_status )
+ {
+ ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = mop->u.mov_to_cr.bitmask;
ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
+ }
else
+ {
+ ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 0;
ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask;
+ }
if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index )
{
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 924caac..27d80ee 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -406,6 +406,7 @@ struct arch_domain
unsigned int cpuid_enabled : 1;
unsigned int descriptor_access_enabled : 1;
struct monitor_msr_bitmap *msr_bitmap;
+ uint64_t write_ctrlreg_mask[4];
} monitor;
/* Mem_access emulation control */
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index e6cf211..2bf5c5b 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -37,7 +37,7 @@
#include "hvm/save.h"
#include "memory.h"
-#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000d
+#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000e
/*
* NB. xen_domctl.domain is an IN/OUT parameter for this operation.
@@ -1107,6 +1107,10 @@ struct xen_domctl_monitor_op {
uint8_t sync;
/* Send event only on a change of value */
uint8_t onchangeonly;
+ /* Send event only if the changed bit in the control register
+ * is not masked
+ */
+ unsigned long bitmask;
} mov_to_cr;
struct {
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index f01e471..8a77d4d 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -155,6 +155,15 @@
#define VM_EVENT_X86_CR4 2
#define VM_EVENT_X86_XCR0 3
+/* vm_event_write_ctrlreg default bit mask
+ * If the changed bit in the control register is masked the event is
+ * not triggered
+ */
+#define VM_EVENT_X86_CR0_DEFAULT_MASK 0x00000000
+#define VM_EVENT_X86_CR3_DEFAULT_MASK 0x00000000
+#define VM_EVENT_X86_CR4_DEFAULT_MASK 0x00000080
+#define VM_EVENT_X86_XCR0_DEFAULT_MASK 0x00000000
+
/*
* 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.
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 2/2] xen-access: write_ctrlreg_c4 test
2017-05-26 13:41 write_ctrlreg event masking Petre Pircalabu
2017-05-26 13:41 ` [PATCH 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
@ 2017-05-26 13:41 ` Petre Pircalabu
2017-05-30 9:46 ` write_ctrlreg event masking Petre Pircalabu
2 siblings, 0 replies; 30+ messages in thread
From: Petre Pircalabu @ 2017-05-26 13:41 UTC (permalink / raw)
To: xen-devel
Cc: Petre Pircalabu, tamas, wei.liu2, rcojocaru, andrew.cooper3,
ian.jackson, jbeulich
Add test for write_ctrlreg event handling.
Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
---
tools/tests/xen-access/xen-access.c | 43 ++++++++++++++++++++++++++++++++++++-
1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 238011e..0a1873e 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -314,6 +314,22 @@ static void get_request(vm_event_t *vm_event, vm_event_request_t *req)
}
/*
+ * X86 control register names
+ */
+static const char* get_x86_ctrl_reg_name(uint32_t index)
+{
+ static const char* names[] = {
+ "CR0",
+ "CR3",
+ "CR4",
+ "XCR0",
+ };
+
+ return (index > 3) ? "" : names[index];
+}
+
+
+/*
* Note that this function is not thread safe.
*/
static void put_response(vm_event_t *vm_event, vm_event_response_t *rsp)
@@ -337,7 +353,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|debug|cpuid|desc_access");
+ fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid|desc_access|write_ctrlreg_cr4");
#elif defined(__arm__) || defined(__aarch64__)
fprintf(stderr, "|privcall");
#endif
@@ -369,6 +385,7 @@ int main(int argc, char *argv[])
int debug = 0;
int cpuid = 0;
int desc_access = 0;
+ int write_ctrlreg_cr4 = 1;
uint16_t altp2m_view_id = 0;
char* progname = argv[0];
@@ -439,6 +456,10 @@ int main(int argc, char *argv[])
{
desc_access = 1;
}
+ else if ( !strcmp(argv[0], "write_ctrlreg_cr4") )
+ {
+ write_ctrlreg_cr4 = 1;
+ }
#elif defined(__arm__) || defined(__aarch64__)
else if ( !strcmp(argv[0], "privcall") )
{
@@ -596,6 +617,17 @@ int main(int argc, char *argv[])
}
}
+ if ( write_ctrlreg_cr4 )
+ {
+ rc = xc_monitor_write_ctrlreg(xch, domain_id, VM_EVENT_X86_CR4, 1, 1,
+ VM_EVENT_X86_CR4_DEFAULT_MASK, 1);
+ if ( rc < 0 )
+ {
+ ERROR("Error %d setting write control register trapping with vm_event\n", rc);
+ goto exit;
+ }
+ }
+
/* Wait for access */
for (;;)
{
@@ -806,6 +838,15 @@ int main(int argc, char *argv[])
req.u.desc_access.is_write);
rsp.flags |= VM_EVENT_FLAG_EMULATE;
break;
+ case VM_EVENT_REASON_WRITE_CTRLREG:
+ printf("Control register written: rip=%016"PRIx64", vcpu %d: "
+ "reg=%s, old_value=%016"PRIx64", new_value=%016"PRIx64"\n",
+ req.data.regs.x86.rip,
+ req.vcpu_id,
+ get_x86_ctrl_reg_name(req.u.write_ctrlreg.index),
+ req.u.write_ctrlreg.old_value,
+ req.u.write_ctrlreg.new_value);
+ break;
default:
fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
}
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-05-26 13:41 ` [PATCH 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
@ 2017-05-29 15:01 ` Jan Beulich
2017-05-30 9:38 ` Petre PIRCALABU
0 siblings, 1 reply; 30+ messages in thread
From: Jan Beulich @ 2017-05-29 15:01 UTC (permalink / raw)
To: Petre Pircalabu
Cc: tamas, wei.liu2, rcojocaru, andrew.cooper3, ian.jackson, xen-devel
>>> On 26.05.17 at 15:41, <ppircalabu@bitdefender.com> wrote:
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -37,7 +37,7 @@
> #include "hvm/save.h"
> #include "memory.h"
>
> -#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000d
> +#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000e
>
> /*
> * NB. xen_domctl.domain is an IN/OUT parameter for this operation.
> @@ -1107,6 +1107,10 @@ struct xen_domctl_monitor_op {
> uint8_t sync;
> /* Send event only on a change of value */
> uint8_t onchangeonly;
> + /* Send event only if the changed bit in the control register
> + * is not masked
> + */
> + unsigned long bitmask;
You can't use "unsigned long" in public headers. Also beware of
alignment issues between 32-bit and 64-bit callers (I didn't check
whether there actually is any, I'm merely hinting towards the
use of uint64_aligned_t).
> --- a/xen/include/public/vm_event.h
> +++ b/xen/include/public/vm_event.h
> @@ -155,6 +155,15 @@
> #define VM_EVENT_X86_CR4 2
> #define VM_EVENT_X86_XCR0 3
>
> +/* vm_event_write_ctrlreg default bit mask
> + * If the changed bit in the control register is masked the event is
> + * not triggered
> + */
Comment style.
> +#define VM_EVENT_X86_CR0_DEFAULT_MASK 0x00000000
> +#define VM_EVENT_X86_CR3_DEFAULT_MASK 0x00000000
> +#define VM_EVENT_X86_CR4_DEFAULT_MASK 0x00000080
> +#define VM_EVENT_X86_XCR0_DEFAULT_MASK 0x00000000
These are unused - what are they good for? And why would
you want to special case CR4.PGE (and even without any
comment)?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-05-29 15:01 ` Jan Beulich
@ 2017-05-30 9:38 ` Petre PIRCALABU
0 siblings, 0 replies; 30+ messages in thread
From: Petre PIRCALABU @ 2017-05-30 9:38 UTC (permalink / raw)
To: JBeulich
Cc: tamas, wei.liu2, rcojocaru, andrew.cooper3, ian.jackson, xen-devel
On Lu, 2017-05-29 at 09:01 -0600, Jan Beulich wrote:
> >
> > >
> > > >
> > > > On 26.05.17 at 15:41, <ppircalabu@bitdefender.com> wrote:
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -37,7 +37,7 @@
> > #include "hvm/save.h"
> > #include "memory.h"
> >
> > -#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000d
> > +#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000e
> >
> > /*
> > * NB. xen_domctl.domain is an IN/OUT parameter for this
> > operation.
> > @@ -1107,6 +1107,10 @@ struct xen_domctl_monitor_op {
> > uint8_t sync;
> > /* Send event only on a change of value */
> > uint8_t onchangeonly;
> > + /* Send event only if the changed bit in the control
> > register
> > + * is not masked
> > + */
> > + unsigned long bitmask;
> You can't use "unsigned long" in public headers. Also beware of
> alignment issues between 32-bit and 64-bit callers (I didn't check
> whether there actually is any, I'm merely hinting towards the
> use of uint64_aligned_t).
>
I will change it to uint64_aligned_t and resubmit the patch.
> >
> > --- a/xen/include/public/vm_event.h
> > +++ b/xen/include/public/vm_event.h
> > @@ -155,6 +155,15 @@
> > #define VM_EVENT_X86_CR4 2
> > #define VM_EVENT_X86_XCR0 3
> >
> > +/* vm_event_write_ctrlreg default bit mask
> > + * If the changed bit in the control register is masked the event
> > is
> > + * not triggered
> > + */
> Comment style.
>
> >
> > +#define VM_EVENT_X86_CR0_DEFAULT_MASK 0x00000000
> > +#define VM_EVENT_X86_CR3_DEFAULT_MASK 0x00000000
> > +#define VM_EVENT_X86_CR4_DEFAULT_MASK 0x00000080
> > +#define VM_EVENT_X86_XCR0_DEFAULT_MASK 0x00000000
> These are unused - what are they good for? And why would
> you want to special case CR4.PGE (and even without any
> comment)?
>
> Jan
>
The defaults were added because when the introspection engine requests
CR4 events it will get too many as CR4.PGE is used for global TLB
flushes. However, every client can set-up it's own event masking policy
so these macros are not needed globally.
>
> ________________________
> This email was scanned by Bitdefender
Many thanks for your comments,
Petre
________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* write_ctrlreg event masking
2017-05-26 13:41 write_ctrlreg event masking Petre Pircalabu
2017-05-26 13:41 ` [PATCH 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
2017-05-26 13:41 ` [PATCH 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
@ 2017-05-30 9:46 ` Petre Pircalabu
2017-05-30 9:46 ` [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
` (4 more replies)
2 siblings, 5 replies; 30+ messages in thread
From: Petre Pircalabu @ 2017-05-30 9:46 UTC (permalink / raw)
To: xen-devel
Cc: tamas, wei.liu2, rcojocaru, andrew.cooper3, ian.jackson, jbeulich
This patchset enables masking the reception of write_ctrlreg events depending
on the value of certain bits in that register.
The most representative example is filtering out events when the CR4.PGE
bit is being flipped (global TLB flushes)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-05-30 9:46 ` write_ctrlreg event masking Petre Pircalabu
@ 2017-05-30 9:46 ` Petre Pircalabu
2017-06-16 14:26 ` Tamas K Lengyel
2017-05-30 9:46 ` [PATCH v2 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
` (3 subsequent siblings)
4 siblings, 1 reply; 30+ messages in thread
From: Petre Pircalabu @ 2017-05-30 9:46 UTC (permalink / raw)
To: xen-devel
Cc: Petre Pircalabu, tamas, wei.liu2, rcojocaru, andrew.cooper3,
ian.jackson, jbeulich
Add support for filtering out the write_ctrlreg monitor events if they
are generated only by changing certains bits.
A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
function in order to mask the event generation if the changed bits are
set.
Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
---
tools/libxc/include/xenctrl.h | 2 +-
tools/libxc/xc_monitor.c | 3 ++-
xen/arch/x86/hvm/monitor.c | 3 ++-
xen/arch/x86/monitor.c | 6 ++++++
xen/include/asm-x86/domain.h | 1 +
xen/include/public/domctl.h | 7 ++++++-
6 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 1629f41..8c26cb4 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1999,7 +1999,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
uint32_t *capabilities);
int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
uint16_t index, bool enable, bool sync,
- bool onchangeonly);
+ uint64_t bitmask, bool onchangeonly);
/*
* A list of MSR indices can usually be found in /usr/include/asm/msr-index.h.
* Please consult the Intel/AMD manuals for more information on
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index f99b6e3..70648d7 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -70,7 +70,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
uint16_t index, bool enable, bool sync,
- bool onchangeonly)
+ uint64_t bitmask, bool onchangeonly)
{
DECLARE_DOMCTL;
@@ -82,6 +82,7 @@ int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
domctl.u.monitor_op.u.mov_to_cr.index = index;
domctl.u.monitor_op.u.mov_to_cr.sync = sync;
domctl.u.monitor_op.u.mov_to_cr.onchangeonly = onchangeonly;
+ domctl.u.monitor_op.u.mov_to_cr.bitmask = bitmask;
return do_domctl(xch, &domctl);
}
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index bde5fd0..a7ccfc4 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -38,7 +38,8 @@ bool_t hvm_monitor_cr(unsigned int index, unsigned long value, unsigned long old
if ( (ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask) &&
(!(ad->monitor.write_ctrlreg_onchangeonly & ctrlreg_bitmask) ||
- value != old) )
+ value != old) &&
+ (!((value ^ old) & ad->monitor.write_ctrlreg_mask[index])) )
{
bool_t sync = !!(ad->monitor.write_ctrlreg_sync & ctrlreg_bitmask);
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 449c64c..d02d2ea 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -155,9 +155,15 @@ int arch_monitor_domctl_event(struct domain *d,
ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask;
if ( requested_status )
+ {
+ ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = mop->u.mov_to_cr.bitmask;
ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
+ }
else
+ {
+ ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 0;
ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask;
+ }
if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index )
{
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 924caac..27d80ee 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -406,6 +406,7 @@ struct arch_domain
unsigned int cpuid_enabled : 1;
unsigned int descriptor_access_enabled : 1;
struct monitor_msr_bitmap *msr_bitmap;
+ uint64_t write_ctrlreg_mask[4];
} monitor;
/* Mem_access emulation control */
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index e6cf211..652fb17 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -37,7 +37,7 @@
#include "hvm/save.h"
#include "memory.h"
-#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000d
+#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000e
/*
* NB. xen_domctl.domain is an IN/OUT parameter for this operation.
@@ -1107,6 +1107,11 @@ struct xen_domctl_monitor_op {
uint8_t sync;
/* Send event only on a change of value */
uint8_t onchangeonly;
+ /*
+ * Send event only if the changed bit in the control register
+ * is not masked.
+ */
+ uint64_aligned_t bitmask;
} mov_to_cr;
struct {
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 2/2] xen-access: write_ctrlreg_c4 test
2017-05-30 9:46 ` write_ctrlreg event masking Petre Pircalabu
2017-05-30 9:46 ` [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
@ 2017-05-30 9:46 ` Petre Pircalabu
2017-06-16 14:32 ` Tamas K Lengyel
2017-06-16 14:09 ` write_ctrlreg event masking Petre Ovidiu PIRCALABU
` (2 subsequent siblings)
4 siblings, 1 reply; 30+ messages in thread
From: Petre Pircalabu @ 2017-05-30 9:46 UTC (permalink / raw)
To: xen-devel
Cc: Petre Pircalabu, tamas, wei.liu2, rcojocaru, andrew.cooper3,
ian.jackson, jbeulich
Add test for write_ctrlreg event handling.
Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
---
tools/tests/xen-access/xen-access.c | 47 ++++++++++++++++++++++++++++++++++++-
1 file changed, 46 insertions(+), 1 deletion(-)
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 238011e..29b60e8 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -57,6 +57,9 @@
#define X86_TRAP_DEBUG 1
#define X86_TRAP_INT3 3
+/* From xen/include/asm-x86/x86-defns.h */
+#define X86_CR4_PGE 0x00000080 /* enable global pages */
+
typedef struct vm_event {
domid_t domain_id;
xenevtchn_handle *xce_handle;
@@ -314,6 +317,22 @@ static void get_request(vm_event_t *vm_event, vm_event_request_t *req)
}
/*
+ * X86 control register names
+ */
+static const char* get_x86_ctrl_reg_name(uint32_t index)
+{
+ static const char* names[] = {
+ "CR0",
+ "CR3",
+ "CR4",
+ "XCR0",
+ };
+
+ return (index > 3) ? "" : names[index];
+}
+
+
+/*
* Note that this function is not thread safe.
*/
static void put_response(vm_event_t *vm_event, vm_event_response_t *rsp)
@@ -337,7 +356,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|debug|cpuid|desc_access");
+ fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid|desc_access|write_ctrlreg_cr4");
#elif defined(__arm__) || defined(__aarch64__)
fprintf(stderr, "|privcall");
#endif
@@ -369,6 +388,7 @@ int main(int argc, char *argv[])
int debug = 0;
int cpuid = 0;
int desc_access = 0;
+ int write_ctrlreg_cr4 = 1;
uint16_t altp2m_view_id = 0;
char* progname = argv[0];
@@ -439,6 +459,10 @@ int main(int argc, char *argv[])
{
desc_access = 1;
}
+ else if ( !strcmp(argv[0], "write_ctrlreg_cr4") )
+ {
+ write_ctrlreg_cr4 = 1;
+ }
#elif defined(__arm__) || defined(__aarch64__)
else if ( !strcmp(argv[0], "privcall") )
{
@@ -596,6 +620,18 @@ int main(int argc, char *argv[])
}
}
+ if ( write_ctrlreg_cr4 )
+ {
+ /* Mask the CR4.PGE bit so no events will be generated for global TLB flushes. */
+ rc = xc_monitor_write_ctrlreg(xch, domain_id, VM_EVENT_X86_CR4, 1, 1,
+ X86_CR4_PGE, 1);
+ if ( rc < 0 )
+ {
+ ERROR("Error %d setting write control register trapping with vm_event\n", rc);
+ goto exit;
+ }
+ }
+
/* Wait for access */
for (;;)
{
@@ -806,6 +842,15 @@ int main(int argc, char *argv[])
req.u.desc_access.is_write);
rsp.flags |= VM_EVENT_FLAG_EMULATE;
break;
+ case VM_EVENT_REASON_WRITE_CTRLREG:
+ printf("Control register written: rip=%016"PRIx64", vcpu %d: "
+ "reg=%s, old_value=%016"PRIx64", new_value=%016"PRIx64"\n",
+ req.data.regs.x86.rip,
+ req.vcpu_id,
+ get_x86_ctrl_reg_name(req.u.write_ctrlreg.index),
+ req.u.write_ctrlreg.old_value,
+ req.u.write_ctrlreg.new_value);
+ break;
default:
fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
}
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: write_ctrlreg event masking
2017-05-30 9:46 ` write_ctrlreg event masking Petre Pircalabu
2017-05-30 9:46 ` [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
2017-05-30 9:46 ` [PATCH v2 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
@ 2017-06-16 14:09 ` Petre Ovidiu PIRCALABU
2017-06-16 19:20 ` [PATCH v3 0/2] " Petre Pircalabu
2017-06-19 12:24 ` [PATCH v4 0/2] write_ctrlreg event masking Petre Pircalabu
4 siblings, 0 replies; 30+ messages in thread
From: Petre Ovidiu PIRCALABU @ 2017-06-16 14:09 UTC (permalink / raw)
To: xen-devel
Cc: tamas, wei.liu2, rcojocaru, andrew.cooper3, ian.jackson, jbeulich
Hello,
Any comments for this series?
Thanks,
Petre
On Ma, 2017-05-30 at 12:46 +0300, Petre Pircalabu wrote:
> This patchset enables masking the reception of write_ctrlreg events
> depending
> on the value of certain bits in that register.
> The most representative example is filtering out events when the
> CR4.PGE
> bit is being flipped (global TLB flushes)
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
>
> ________________________
> This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-05-30 9:46 ` [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
@ 2017-06-16 14:26 ` Tamas K Lengyel
2017-06-16 15:15 ` Jan Beulich
0 siblings, 1 reply; 30+ messages in thread
From: Tamas K Lengyel @ 2017-06-16 14:26 UTC (permalink / raw)
To: Petre Pircalabu
Cc: wei.liu2, Razvan Cojocaru, Andrew Cooper, Ian Jackson, Xen-devel,
Jan Beulich
On Tue, May 30, 2017 at 3:46 AM, Petre Pircalabu
<ppircalabu@bitdefender.com> wrote:
> Add support for filtering out the write_ctrlreg monitor events if they
> are generated only by changing certains bits.
> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
> function in order to mask the event generation if the changed bits are
> set.
>
> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> tools/libxc/include/xenctrl.h | 2 +-
> tools/libxc/xc_monitor.c | 3 ++-
> xen/arch/x86/hvm/monitor.c | 3 ++-
> xen/arch/x86/monitor.c | 6 ++++++
> xen/include/asm-x86/domain.h | 1 +
> xen/include/public/domctl.h | 7 ++++++-
> 6 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 1629f41..8c26cb4 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1999,7 +1999,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
> uint32_t *capabilities);
> int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
> uint16_t index, bool enable, bool sync,
> - bool onchangeonly);
> + uint64_t bitmask, bool onchangeonly);
> /*
> * A list of MSR indices can usually be found in /usr/include/asm/msr-index.h.
> * Please consult the Intel/AMD manuals for more information on
> diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
> index f99b6e3..70648d7 100644
> --- a/tools/libxc/xc_monitor.c
> +++ b/tools/libxc/xc_monitor.c
> @@ -70,7 +70,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
>
> int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
> uint16_t index, bool enable, bool sync,
> - bool onchangeonly)
> + uint64_t bitmask, bool onchangeonly)
> {
> DECLARE_DOMCTL;
>
> @@ -82,6 +82,7 @@ int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
> domctl.u.monitor_op.u.mov_to_cr.index = index;
> domctl.u.monitor_op.u.mov_to_cr.sync = sync;
> domctl.u.monitor_op.u.mov_to_cr.onchangeonly = onchangeonly;
> + domctl.u.monitor_op.u.mov_to_cr.bitmask = bitmask;
>
> return do_domctl(xch, &domctl);
> }
> diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
> index bde5fd0..a7ccfc4 100644
> --- a/xen/arch/x86/hvm/monitor.c
> +++ b/xen/arch/x86/hvm/monitor.c
> @@ -38,7 +38,8 @@ bool_t hvm_monitor_cr(unsigned int index, unsigned long value, unsigned long old
>
> if ( (ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask) &&
> (!(ad->monitor.write_ctrlreg_onchangeonly & ctrlreg_bitmask) ||
> - value != old) )
> + value != old) &&
> + (!((value ^ old) & ad->monitor.write_ctrlreg_mask[index])) )
> {
> bool_t sync = !!(ad->monitor.write_ctrlreg_sync & ctrlreg_bitmask);
>
> diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
> index 449c64c..d02d2ea 100644
> --- a/xen/arch/x86/monitor.c
> +++ b/xen/arch/x86/monitor.c
> @@ -155,9 +155,15 @@ int arch_monitor_domctl_event(struct domain *d,
> ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask;
>
> if ( requested_status )
> + {
> + ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = mop->u.mov_to_cr.bitmask;
> ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
> + }
> else
> + {
> + ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 0;
> ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask;
> + }
>
> if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index )
> {
> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
> index 924caac..27d80ee 100644
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -406,6 +406,7 @@ struct arch_domain
> unsigned int cpuid_enabled : 1;
> unsigned int descriptor_access_enabled : 1;
> struct monitor_msr_bitmap *msr_bitmap;
> + uint64_t write_ctrlreg_mask[4];
> } monitor;
>
> /* Mem_access emulation control */
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index e6cf211..652fb17 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -37,7 +37,7 @@
> #include "hvm/save.h"
> #include "memory.h"
>
> -#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000d
> +#define XEN_DOMCTL_INTERFACE_VERSION 0x0000000e
>
> /*
> * NB. xen_domctl.domain is an IN/OUT parameter for this operation.
> @@ -1107,6 +1107,11 @@ struct xen_domctl_monitor_op {
> uint8_t sync;
> /* Send event only on a change of value */
> uint8_t onchangeonly;
> + /*
> + * Send event only if the changed bit in the control register
> + * is not masked.
> + */
> + uint64_aligned_t bitmask;
> } mov_to_cr;
>
> struct {
> --
> 2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/2] xen-access: write_ctrlreg_c4 test
2017-05-30 9:46 ` [PATCH v2 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
@ 2017-06-16 14:32 ` Tamas K Lengyel
2017-06-16 15:12 ` Jan Beulich
0 siblings, 1 reply; 30+ messages in thread
From: Tamas K Lengyel @ 2017-06-16 14:32 UTC (permalink / raw)
To: Petre Pircalabu
Cc: wei.liu2, Razvan Cojocaru, Andrew Cooper, Ian Jackson, Xen-devel,
Jan Beulich
On Tue, May 30, 2017 at 3:46 AM, Petre Pircalabu
<ppircalabu@bitdefender.com> wrote:
> Add test for write_ctrlreg event handling.
>
> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
> ---
> tools/tests/xen-access/xen-access.c | 47 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
> index 238011e..29b60e8 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -57,6 +57,9 @@
> #define X86_TRAP_DEBUG 1
> #define X86_TRAP_INT3 3
>
> +/* From xen/include/asm-x86/x86-defns.h */
> +#define X86_CR4_PGE 0x00000080 /* enable global pages */
> +
> typedef struct vm_event {
> domid_t domain_id;
> xenevtchn_handle *xce_handle;
> @@ -314,6 +317,22 @@ static void get_request(vm_event_t *vm_event, vm_event_request_t *req)
> }
>
> /*
> + * X86 control register names
> + */
> +static const char* get_x86_ctrl_reg_name(uint32_t index)
> +{
> + static const char* names[] = {
I would prefer to see this being defined in the following form so that
it is clear where the indexes come from:
[VM_EVENT_X86_CR0] = "CR0",
...
> + "CR0",
> + "CR3",
> + "CR4",
> + "XCR0",
> + };
And this check to be index > VM_EVENT_X86_XCR0
> + return (index > 3) ? "" : names[index];
> +}
> +
> +
> +/*
> * Note that this function is not thread safe.
> */
> static void put_response(vm_event_t *vm_event, vm_event_response_t *rsp)
> @@ -337,7 +356,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|debug|cpuid|desc_access");
> + fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid|desc_access|write_ctrlreg_cr4");
> #elif defined(__arm__) || defined(__aarch64__)
> fprintf(stderr, "|privcall");
> #endif
> @@ -369,6 +388,7 @@ int main(int argc, char *argv[])
> int debug = 0;
> int cpuid = 0;
> int desc_access = 0;
> + int write_ctrlreg_cr4 = 1;
> uint16_t altp2m_view_id = 0;
>
> char* progname = argv[0];
> @@ -439,6 +459,10 @@ int main(int argc, char *argv[])
> {
> desc_access = 1;
> }
> + else if ( !strcmp(argv[0], "write_ctrlreg_cr4") )
> + {
> + write_ctrlreg_cr4 = 1;
> + }
> #elif defined(__arm__) || defined(__aarch64__)
> else if ( !strcmp(argv[0], "privcall") )
> {
> @@ -596,6 +620,18 @@ int main(int argc, char *argv[])
> }
> }
>
> + if ( write_ctrlreg_cr4 )
> + {
> + /* Mask the CR4.PGE bit so no events will be generated for global TLB flushes. */
> + rc = xc_monitor_write_ctrlreg(xch, domain_id, VM_EVENT_X86_CR4, 1, 1,
> + X86_CR4_PGE, 1);
> + if ( rc < 0 )
> + {
> + ERROR("Error %d setting write control register trapping with vm_event\n", rc);
> + goto exit;
> + }
> + }
> +
> /* Wait for access */
> for (;;)
> {
> @@ -806,6 +842,15 @@ int main(int argc, char *argv[])
> req.u.desc_access.is_write);
> rsp.flags |= VM_EVENT_FLAG_EMULATE;
> break;
> + case VM_EVENT_REASON_WRITE_CTRLREG:
> + printf("Control register written: rip=%016"PRIx64", vcpu %d: "
> + "reg=%s, old_value=%016"PRIx64", new_value=%016"PRIx64"\n",
> + req.data.regs.x86.rip,
> + req.vcpu_id,
> + get_x86_ctrl_reg_name(req.u.write_ctrlreg.index),
> + req.u.write_ctrlreg.old_value,
> + req.u.write_ctrlreg.new_value);
> + break;
> default:
> fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
> }
> --
> 2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/2] xen-access: write_ctrlreg_c4 test
2017-06-16 14:32 ` Tamas K Lengyel
@ 2017-06-16 15:12 ` Jan Beulich
2017-06-16 15:24 ` Tamas K Lengyel
0 siblings, 1 reply; 30+ messages in thread
From: Jan Beulich @ 2017-06-16 15:12 UTC (permalink / raw)
To: Petre Pircalabu, Tamas K Lengyel
Cc: Andrew Cooper, wei.liu2, Ian Jackson, Razvan Cojocaru, Xen-devel
>>> On 16.06.17 at 16:32, <tamas@tklengyel.com> wrote:
> On Tue, May 30, 2017 at 3:46 AM, Petre Pircalabu <ppircalabu@bitdefender.com> wrote:
>> @@ -314,6 +317,22 @@ static void get_request(vm_event_t *vm_event, vm_event_request_t *req)
>> }
>>
>> /*
>> + * X86 control register names
>> + */
>> +static const char* get_x86_ctrl_reg_name(uint32_t index)
>> +{
>> + static const char* names[] = {
>
> I would prefer to see this being defined in the following form so that
> it is clear where the indexes come from:
> [VM_EVENT_X86_CR0] = "CR0",
> ...
>
>> + "CR0",
>> + "CR3",
>> + "CR4",
>> + "XCR0",
>> + };
>
> And this check to be index > VM_EVENT_X86_XCR0
Or perhaps even better >= ARRAY_SIZE()?
>> + return (index > 3) ? "" : names[index];
>> +}
>> +
>> +
>> +/*
I also notice there are two successive blank lines here, which we
generally try to avoid.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-06-16 14:26 ` Tamas K Lengyel
@ 2017-06-16 15:15 ` Jan Beulich
2017-06-16 15:28 ` Tamas K Lengyel
0 siblings, 1 reply; 30+ messages in thread
From: Jan Beulich @ 2017-06-16 15:15 UTC (permalink / raw)
To: Tamas K Lengyel
Cc: Petre Pircalabu, wei.liu2, Razvan Cojocaru, Andrew Cooper,
Ian Jackson, Xen-devel
>>> On 16.06.17 at 16:26, <tamas@tklengyel.com> wrote:
> On Tue, May 30, 2017 at 3:46 AM, Petre Pircalabu
> <ppircalabu@bitdefender.com> wrote:
>> Add support for filtering out the write_ctrlreg monitor events if they
>> are generated only by changing certains bits.
>> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
>> function in order to mask the event generation if the changed bits are
>> set.
>>
>> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
>
> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Are you btw in agreement with ...
>> @@ -1107,6 +1107,11 @@ struct xen_domctl_monitor_op {
>> uint8_t sync;
>> /* Send event only on a change of value */
>> uint8_t onchangeonly;
>> + /*
>> + * Send event only if the changed bit in the control register
>> + * is not masked.
>> + */
>> + uint64_aligned_t bitmask;
... the 5-byte gap being introduced here, using of which won't
be possible without bumping XEN_DOMCTL_INTERFACE_VERSION
again? Generally we aim at making padding explicit and checking
it to be zero on input / providing zero on output.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/2] xen-access: write_ctrlreg_c4 test
2017-06-16 15:12 ` Jan Beulich
@ 2017-06-16 15:24 ` Tamas K Lengyel
0 siblings, 0 replies; 30+ messages in thread
From: Tamas K Lengyel @ 2017-06-16 15:24 UTC (permalink / raw)
To: Jan Beulich
Cc: Petre Pircalabu, wei.liu2, Razvan Cojocaru, Andrew Cooper,
Ian Jackson, Xen-devel
On Fri, Jun 16, 2017 at 9:12 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 16.06.17 at 16:32, <tamas@tklengyel.com> wrote:
>> On Tue, May 30, 2017 at 3:46 AM, Petre Pircalabu <ppircalabu@bitdefender.com> wrote:
>>> @@ -314,6 +317,22 @@ static void get_request(vm_event_t *vm_event, vm_event_request_t *req)
>>> }
>>>
>>> /*
>>> + * X86 control register names
>>> + */
>>> +static const char* get_x86_ctrl_reg_name(uint32_t index)
>>> +{
>>> + static const char* names[] = {
>>
>> I would prefer to see this being defined in the following form so that
>> it is clear where the indexes come from:
>> [VM_EVENT_X86_CR0] = "CR0",
>> ...
>>
>>> + "CR0",
>>> + "CR3",
>>> + "CR4",
>>> + "XCR0",
>>> + };
>>
>> And this check to be index > VM_EVENT_X86_XCR0
>
> Or perhaps even better >= ARRAY_SIZE()?
Yeap, even better.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-06-16 15:15 ` Jan Beulich
@ 2017-06-16 15:28 ` Tamas K Lengyel
2017-06-16 15:33 ` Jan Beulich
0 siblings, 1 reply; 30+ messages in thread
From: Tamas K Lengyel @ 2017-06-16 15:28 UTC (permalink / raw)
To: Jan Beulich
Cc: Petre Pircalabu, wei.liu2, Razvan Cojocaru, Andrew Cooper,
Ian Jackson, Xen-devel
On Fri, Jun 16, 2017 at 9:15 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 16.06.17 at 16:26, <tamas@tklengyel.com> wrote:
>> On Tue, May 30, 2017 at 3:46 AM, Petre Pircalabu
>> <ppircalabu@bitdefender.com> wrote:
>>> Add support for filtering out the write_ctrlreg monitor events if they
>>> are generated only by changing certains bits.
>>> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
>>> function in order to mask the event generation if the changed bits are
>>> set.
>>>
>>> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
>>
>> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
>
> Are you btw in agreement with ...
>
>>> @@ -1107,6 +1107,11 @@ struct xen_domctl_monitor_op {
>>> uint8_t sync;
>>> /* Send event only on a change of value */
>>> uint8_t onchangeonly;
>>> + /*
>>> + * Send event only if the changed bit in the control register
>>> + * is not masked.
>>> + */
>>> + uint64_aligned_t bitmask;
>
> ... the 5-byte gap being introduced here, using of which won't
> be possible without bumping XEN_DOMCTL_INTERFACE_VERSION
> again? Generally we aim at making padding explicit and checking
> it to be zero on input / providing zero on output.
Yes, making the padding explicit would be better (but I'm not the
maintainer of the domctl interface so my ack doesn't cover that).
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-06-16 15:28 ` Tamas K Lengyel
@ 2017-06-16 15:33 ` Jan Beulich
2017-06-16 16:24 ` Tamas K Lengyel
0 siblings, 1 reply; 30+ messages in thread
From: Jan Beulich @ 2017-06-16 15:33 UTC (permalink / raw)
To: Tamas K Lengyel
Cc: Petre Pircalabu, wei.liu2, Razvan Cojocaru, Andrew Cooper,
Ian Jackson, Xen-devel
>>> On 16.06.17 at 17:28, <tamas@tklengyel.com> wrote:
> On Fri, Jun 16, 2017 at 9:15 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 16.06.17 at 16:26, <tamas@tklengyel.com> wrote:
>>> On Tue, May 30, 2017 at 3:46 AM, Petre Pircalabu
>>> <ppircalabu@bitdefender.com> wrote:
>>>> Add support for filtering out the write_ctrlreg monitor events if they
>>>> are generated only by changing certains bits.
>>>> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
>>>> function in order to mask the event generation if the changed bits are
>>>> set.
>>>>
>>>> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
>>>
>>> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
>>
>> Are you btw in agreement with ...
>>
>>>> @@ -1107,6 +1107,11 @@ struct xen_domctl_monitor_op {
>>>> uint8_t sync;
>>>> /* Send event only on a change of value */
>>>> uint8_t onchangeonly;
>>>> + /*
>>>> + * Send event only if the changed bit in the control register
>>>> + * is not masked.
>>>> + */
>>>> + uint64_aligned_t bitmask;
>>
>> ... the 5-byte gap being introduced here, using of which won't
>> be possible without bumping XEN_DOMCTL_INTERFACE_VERSION
>> again? Generally we aim at making padding explicit and checking
>> it to be zero on input / providing zero on output.
>
> Yes, making the padding explicit would be better (but I'm not the
> maintainer of the domctl interface so my ack doesn't cover that).
Well, I have been waiting for a monitor maintainer ack to perhaps
give my own for the little bit of it where a REST maintainer one
would be needed. Irrespective of file level maintainership I think
the public interface parts still belong to their subsystems, and the
REST maintainer ack should normally be a purely mechanical last
step.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-06-16 15:33 ` Jan Beulich
@ 2017-06-16 16:24 ` Tamas K Lengyel
0 siblings, 0 replies; 30+ messages in thread
From: Tamas K Lengyel @ 2017-06-16 16:24 UTC (permalink / raw)
To: Jan Beulich
Cc: Petre Pircalabu, wei.liu2, Razvan Cojocaru, Andrew Cooper,
Ian Jackson, Xen-devel
On Fri, Jun 16, 2017 at 9:33 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 16.06.17 at 17:28, <tamas@tklengyel.com> wrote:
>> On Fri, Jun 16, 2017 at 9:15 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 16.06.17 at 16:26, <tamas@tklengyel.com> wrote:
>>>> On Tue, May 30, 2017 at 3:46 AM, Petre Pircalabu
>>>> <ppircalabu@bitdefender.com> wrote:
>>>>> Add support for filtering out the write_ctrlreg monitor events if they
>>>>> are generated only by changing certains bits.
>>>>> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
>>>>> function in order to mask the event generation if the changed bits are
>>>>> set.
>>>>>
>>>>> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
>>>>
>>>> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
>>>
>>> Are you btw in agreement with ...
>>>
>>>>> @@ -1107,6 +1107,11 @@ struct xen_domctl_monitor_op {
>>>>> uint8_t sync;
>>>>> /* Send event only on a change of value */
>>>>> uint8_t onchangeonly;
>>>>> + /*
>>>>> + * Send event only if the changed bit in the control register
>>>>> + * is not masked.
>>>>> + */
>>>>> + uint64_aligned_t bitmask;
>>>
>>> ... the 5-byte gap being introduced here, using of which won't
>>> be possible without bumping XEN_DOMCTL_INTERFACE_VERSION
>>> again? Generally we aim at making padding explicit and checking
>>> it to be zero on input / providing zero on output.
>>
>> Yes, making the padding explicit would be better (but I'm not the
>> maintainer of the domctl interface so my ack doesn't cover that).
>
> Well, I have been waiting for a monitor maintainer ack to perhaps
> give my own for the little bit of it where a REST maintainer one
> would be needed. Irrespective of file level maintainership I think
> the public interface parts still belong to their subsystems, and the
> REST maintainer ack should normally be a purely mechanical last
> step.
I can see the logic behind that. However, I would feel better if it
wasn't just a mechanical last step as for example I'm not as well
versed in the ABI side of things. Like in here, the padding bits have
completely went by me without me noticing.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 0/2] write_ctrlreg event masking
2017-05-30 9:46 ` write_ctrlreg event masking Petre Pircalabu
` (2 preceding siblings ...)
2017-06-16 14:09 ` write_ctrlreg event masking Petre Ovidiu PIRCALABU
@ 2017-06-16 19:20 ` Petre Pircalabu
2017-06-16 19:20 ` [PATCH v3 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
2017-06-16 19:20 ` [PATCH v3 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
2017-06-19 12:24 ` [PATCH v4 0/2] write_ctrlreg event masking Petre Pircalabu
4 siblings, 2 replies; 30+ messages in thread
From: Petre Pircalabu @ 2017-06-16 19:20 UTC (permalink / raw)
To: xen-devel
Cc: tamas, wei.liu2, rcojocaru, andrew.cooper3, ian.jackson, jbeulich
This patchset enables masking the reception of write_ctrlreg events depending
on the value of certain bits in that register.
The most representative example is filtering out events when the CR4.PGE
bit is being flipped (global TLB flushes)
---
Changed since v2
* fix coding style.
* use ARRAY_SIZE and named indexes for x86 ctrl register resolution.
* add allignment padding for xen_domctl_monitor_op.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-06-16 19:20 ` [PATCH v3 0/2] " Petre Pircalabu
@ 2017-06-16 19:20 ` Petre Pircalabu
2017-06-16 19:20 ` [PATCH v3 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
1 sibling, 0 replies; 30+ messages in thread
From: Petre Pircalabu @ 2017-06-16 19:20 UTC (permalink / raw)
To: xen-devel
Cc: Petre Pircalabu, tamas, wei.liu2, rcojocaru, andrew.cooper3,
ian.jackson, jbeulich
Add support for filtering out the write_ctrlreg monitor events if they
are generated only by changing certains bits.
A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
function in order to mask the event generation if the changed bits are
set.
Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
---
tools/libxc/include/xenctrl.h | 2 +-
tools/libxc/xc_monitor.c | 5 ++++-
xen/arch/x86/hvm/monitor.c | 3 ++-
xen/arch/x86/monitor.c | 9 +++++++++
xen/include/asm-x86/domain.h | 1 +
xen/include/public/domctl.h | 8 ++++++++
6 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 1629f41..8c26cb4 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1999,7 +1999,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
uint32_t *capabilities);
int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
uint16_t index, bool enable, bool sync,
- bool onchangeonly);
+ uint64_t bitmask, bool onchangeonly);
/*
* A list of MSR indices can usually be found in /usr/include/asm/msr-index.h.
* Please consult the Intel/AMD manuals for more information on
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index f99b6e3..b44ce93 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -70,7 +70,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
uint16_t index, bool enable, bool sync,
- bool onchangeonly)
+ uint64_t bitmask, bool onchangeonly)
{
DECLARE_DOMCTL;
@@ -82,6 +82,9 @@ int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
domctl.u.monitor_op.u.mov_to_cr.index = index;
domctl.u.monitor_op.u.mov_to_cr.sync = sync;
domctl.u.monitor_op.u.mov_to_cr.onchangeonly = onchangeonly;
+ domctl.u.monitor_op.u.mov_to_cr.bitmask = bitmask;
+ domctl.u.monitor_op.u.mov_to_cr.pad1 = 0;
+ domctl.u.monitor_op.u.mov_to_cr.pad2 = 0;
return do_domctl(xch, &domctl);
}
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index bde5fd0..a7ccfc4 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -38,7 +38,8 @@ bool_t hvm_monitor_cr(unsigned int index, unsigned long value, unsigned long old
if ( (ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask) &&
(!(ad->monitor.write_ctrlreg_onchangeonly & ctrlreg_bitmask) ||
- value != old) )
+ value != old) &&
+ (!((value ^ old) & ad->monitor.write_ctrlreg_mask[index])) )
{
bool_t sync = !!(ad->monitor.write_ctrlreg_sync & ctrlreg_bitmask);
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 449c64c..bedf13c 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -136,6 +136,9 @@ int arch_monitor_domctl_event(struct domain *d,
if ( unlikely(mop->u.mov_to_cr.index > 31) )
return -EINVAL;
+ if ( unlikely(mop->u.mov_to_cr.pad1 || mop->u.mov_to_cr.pad2) )
+ return -EINVAL;
+
ctrlreg_bitmask = monitor_ctrlreg_bitmask(mop->u.mov_to_cr.index);
old_status = !!(ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask);
@@ -155,9 +158,15 @@ int arch_monitor_domctl_event(struct domain *d,
ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask;
if ( requested_status )
+ {
+ ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = mop->u.mov_to_cr.bitmask;
ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
+ }
else
+ {
+ ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 0;
ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask;
+ }
if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index )
{
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 924caac..27d80ee 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -406,6 +406,7 @@ struct arch_domain
unsigned int cpuid_enabled : 1;
unsigned int descriptor_access_enabled : 1;
struct monitor_msr_bitmap *msr_bitmap;
+ uint64_t write_ctrlreg_mask[4];
} monitor;
/* Mem_access emulation control */
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index f7cbc0a..ff39762 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1107,6 +1107,14 @@ struct xen_domctl_monitor_op {
uint8_t sync;
/* Send event only on a change of value */
uint8_t onchangeonly;
+ /* Allignment padding */
+ uint8_t pad1;
+ uint32_t pad2;
+ /*
+ * Send event only if the changed bit in the control register
+ * is not masked.
+ */
+ uint64_aligned_t bitmask;
} mov_to_cr;
struct {
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 2/2] xen-access: write_ctrlreg_c4 test
2017-06-16 19:20 ` [PATCH v3 0/2] " Petre Pircalabu
2017-06-16 19:20 ` [PATCH v3 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
@ 2017-06-16 19:20 ` Petre Pircalabu
2017-06-16 19:45 ` Razvan Cojocaru
1 sibling, 1 reply; 30+ messages in thread
From: Petre Pircalabu @ 2017-06-16 19:20 UTC (permalink / raw)
To: xen-devel
Cc: Petre Pircalabu, tamas, wei.liu2, rcojocaru, andrew.cooper3,
ian.jackson, jbeulich
Add test for write_ctrlreg event handling.
Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
---
tools/tests/xen-access/xen-access.c | 53 ++++++++++++++++++++++++++++++++++++-
1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 238011e..bbf5047 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -57,6 +57,13 @@
#define X86_TRAP_DEBUG 1
#define X86_TRAP_INT3 3
+/* From xen/include/asm-x86/x86-defns.h */
+#define X86_CR4_PGE 0x00000080 /* enable global pages */
+
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+#endif
+
typedef struct vm_event {
domid_t domain_id;
xenevtchn_handle *xce_handle;
@@ -314,6 +321,24 @@ static void get_request(vm_event_t *vm_event, vm_event_request_t *req)
}
/*
+ * X86 control register names
+ */
+static const char* get_x86_ctrl_reg_name(uint32_t index)
+{
+ static const char* names[] = {
+ [VM_EVENT_X86_CR0] = "CR0",
+ [VM_EVENT_X86_CR3] = "CR3",
+ [VM_EVENT_X86_CR4] = "CR4",
+ [VM_EVENT_X86_XCR0] = "XCR0",
+ };
+
+ if ( index > ARRAY_SIZE(names) || names[index] == NULL )
+ return "";
+
+ return names[index];
+}
+
+/*
* Note that this function is not thread safe.
*/
static void put_response(vm_event_t *vm_event, vm_event_response_t *rsp)
@@ -337,7 +362,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|debug|cpuid|desc_access");
+ fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid|desc_access|write_ctrlreg_cr4");
#elif defined(__arm__) || defined(__aarch64__)
fprintf(stderr, "|privcall");
#endif
@@ -369,6 +394,7 @@ int main(int argc, char *argv[])
int debug = 0;
int cpuid = 0;
int desc_access = 0;
+ int write_ctrlreg_cr4 = 1;
uint16_t altp2m_view_id = 0;
char* progname = argv[0];
@@ -439,6 +465,10 @@ int main(int argc, char *argv[])
{
desc_access = 1;
}
+ else if ( !strcmp(argv[0], "write_ctrlreg_cr4") )
+ {
+ write_ctrlreg_cr4 = 1;
+ }
#elif defined(__arm__) || defined(__aarch64__)
else if ( !strcmp(argv[0], "privcall") )
{
@@ -596,6 +626,18 @@ int main(int argc, char *argv[])
}
}
+ if ( write_ctrlreg_cr4 )
+ {
+ /* Mask the CR4.PGE bit so no events will be generated for global TLB flushes. */
+ rc = xc_monitor_write_ctrlreg(xch, domain_id, VM_EVENT_X86_CR4, 1, 1,
+ X86_CR4_PGE, 1);
+ if ( rc < 0 )
+ {
+ ERROR("Error %d setting write control register trapping with vm_event\n", rc);
+ goto exit;
+ }
+ }
+
/* Wait for access */
for (;;)
{
@@ -806,6 +848,15 @@ int main(int argc, char *argv[])
req.u.desc_access.is_write);
rsp.flags |= VM_EVENT_FLAG_EMULATE;
break;
+ case VM_EVENT_REASON_WRITE_CTRLREG:
+ printf("Control register written: rip=%016"PRIx64", vcpu %d: "
+ "reg=%s, old_value=%016"PRIx64", new_value=%016"PRIx64"\n",
+ req.data.regs.x86.rip,
+ req.vcpu_id,
+ get_x86_ctrl_reg_name(req.u.write_ctrlreg.index),
+ req.u.write_ctrlreg.old_value,
+ req.u.write_ctrlreg.new_value);
+ break;
default:
fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
}
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v3 2/2] xen-access: write_ctrlreg_c4 test
2017-06-16 19:20 ` [PATCH v3 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
@ 2017-06-16 19:45 ` Razvan Cojocaru
0 siblings, 0 replies; 30+ messages in thread
From: Razvan Cojocaru @ 2017-06-16 19:45 UTC (permalink / raw)
To: Petre Pircalabu, xen-devel
Cc: andrew.cooper3, tamas, wei.liu2, ian.jackson, jbeulich
On 06/16/2017 10:20 PM, Petre Pircalabu wrote:
> Add test for write_ctrlreg event handling.
>
> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
> ---
> tools/tests/xen-access/xen-access.c | 53 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
> index 238011e..bbf5047 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -57,6 +57,13 @@
> #define X86_TRAP_DEBUG 1
> #define X86_TRAP_INT3 3
>
> +/* From xen/include/asm-x86/x86-defns.h */
> +#define X86_CR4_PGE 0x00000080 /* enable global pages */
> +
> +#ifndef ARRAY_SIZE
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> +#endif
> +
> typedef struct vm_event {
> domid_t domain_id;
> xenevtchn_handle *xce_handle;
> @@ -314,6 +321,24 @@ static void get_request(vm_event_t *vm_event, vm_event_request_t *req)
> }
>
> /*
> + * X86 control register names
> + */
> +static const char* get_x86_ctrl_reg_name(uint32_t index)
> +{
> + static const char* names[] = {
> + [VM_EVENT_X86_CR0] = "CR0",
> + [VM_EVENT_X86_CR3] = "CR3",
> + [VM_EVENT_X86_CR4] = "CR4",
> + [VM_EVENT_X86_XCR0] = "XCR0",
> + };
> +
> + if ( index > ARRAY_SIZE(names) || names[index] == NULL )
I think this probably wants to be index >= ARRAY_SIZE(names).
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 0/2] write_ctrlreg event masking
2017-05-30 9:46 ` write_ctrlreg event masking Petre Pircalabu
` (3 preceding siblings ...)
2017-06-16 19:20 ` [PATCH v3 0/2] " Petre Pircalabu
@ 2017-06-19 12:24 ` Petre Pircalabu
2017-06-19 12:24 ` [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
2017-06-19 12:24 ` [PATCH v4 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
4 siblings, 2 replies; 30+ messages in thread
From: Petre Pircalabu @ 2017-06-19 12:24 UTC (permalink / raw)
To: xen-devel
Cc: tamas, wei.liu2, rcojocaru, andrew.cooper3, ian.jackson, jbeulich
This patchset enables masking the reception of write_ctrlreg events depending
on the value of certain bits in that register.
The most representative example is filtering out events when the CR4.PGE
bit is being flipped (global TLB flushes)
---
Changed since v2
* fix coding style
* use ARRAY_SIZE and named indexes for x86 ctrl register resolution
* add allignment padding for xen_domctl_monitor_op
Changed since v3
* Fix index condition in get_x86_ctrl_reg_name
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-06-19 12:24 ` [PATCH v4 0/2] write_ctrlreg event masking Petre Pircalabu
@ 2017-06-19 12:24 ` Petre Pircalabu
2017-06-20 12:42 ` Wei Liu
2017-06-21 13:58 ` Wei Liu
2017-06-19 12:24 ` [PATCH v4 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
1 sibling, 2 replies; 30+ messages in thread
From: Petre Pircalabu @ 2017-06-19 12:24 UTC (permalink / raw)
To: xen-devel
Cc: Petre Pircalabu, tamas, wei.liu2, rcojocaru, andrew.cooper3,
ian.jackson, jbeulich
Add support for filtering out the write_ctrlreg monitor events if they
are generated only by changing certains bits.
A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
function in order to mask the event generation if the changed bits are
set.
Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
---
tools/libxc/include/xenctrl.h | 2 +-
tools/libxc/xc_monitor.c | 5 ++++-
xen/arch/x86/hvm/monitor.c | 3 ++-
xen/arch/x86/monitor.c | 9 +++++++++
xen/include/asm-x86/domain.h | 1 +
xen/include/public/domctl.h | 8 ++++++++
6 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 1629f41..8c26cb4 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1999,7 +1999,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
uint32_t *capabilities);
int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
uint16_t index, bool enable, bool sync,
- bool onchangeonly);
+ uint64_t bitmask, bool onchangeonly);
/*
* A list of MSR indices can usually be found in /usr/include/asm/msr-index.h.
* Please consult the Intel/AMD manuals for more information on
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index f99b6e3..b44ce93 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -70,7 +70,7 @@ int xc_monitor_get_capabilities(xc_interface *xch, domid_t domain_id,
int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
uint16_t index, bool enable, bool sync,
- bool onchangeonly)
+ uint64_t bitmask, bool onchangeonly)
{
DECLARE_DOMCTL;
@@ -82,6 +82,9 @@ int xc_monitor_write_ctrlreg(xc_interface *xch, domid_t domain_id,
domctl.u.monitor_op.u.mov_to_cr.index = index;
domctl.u.monitor_op.u.mov_to_cr.sync = sync;
domctl.u.monitor_op.u.mov_to_cr.onchangeonly = onchangeonly;
+ domctl.u.monitor_op.u.mov_to_cr.bitmask = bitmask;
+ domctl.u.monitor_op.u.mov_to_cr.pad1 = 0;
+ domctl.u.monitor_op.u.mov_to_cr.pad2 = 0;
return do_domctl(xch, &domctl);
}
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index bde5fd0..a7ccfc4 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -38,7 +38,8 @@ bool_t hvm_monitor_cr(unsigned int index, unsigned long value, unsigned long old
if ( (ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask) &&
(!(ad->monitor.write_ctrlreg_onchangeonly & ctrlreg_bitmask) ||
- value != old) )
+ value != old) &&
+ (!((value ^ old) & ad->monitor.write_ctrlreg_mask[index])) )
{
bool_t sync = !!(ad->monitor.write_ctrlreg_sync & ctrlreg_bitmask);
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 449c64c..bedf13c 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -136,6 +136,9 @@ int arch_monitor_domctl_event(struct domain *d,
if ( unlikely(mop->u.mov_to_cr.index > 31) )
return -EINVAL;
+ if ( unlikely(mop->u.mov_to_cr.pad1 || mop->u.mov_to_cr.pad2) )
+ return -EINVAL;
+
ctrlreg_bitmask = monitor_ctrlreg_bitmask(mop->u.mov_to_cr.index);
old_status = !!(ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask);
@@ -155,9 +158,15 @@ int arch_monitor_domctl_event(struct domain *d,
ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask;
if ( requested_status )
+ {
+ ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = mop->u.mov_to_cr.bitmask;
ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
+ }
else
+ {
+ ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 0;
ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask;
+ }
if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index )
{
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 924caac..27d80ee 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -406,6 +406,7 @@ struct arch_domain
unsigned int cpuid_enabled : 1;
unsigned int descriptor_access_enabled : 1;
struct monitor_msr_bitmap *msr_bitmap;
+ uint64_t write_ctrlreg_mask[4];
} monitor;
/* Mem_access emulation control */
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index f7cbc0a..ff39762 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1107,6 +1107,14 @@ struct xen_domctl_monitor_op {
uint8_t sync;
/* Send event only on a change of value */
uint8_t onchangeonly;
+ /* Allignment padding */
+ uint8_t pad1;
+ uint32_t pad2;
+ /*
+ * Send event only if the changed bit in the control register
+ * is not masked.
+ */
+ uint64_aligned_t bitmask;
} mov_to_cr;
struct {
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 2/2] xen-access: write_ctrlreg_c4 test
2017-06-19 12:24 ` [PATCH v4 0/2] write_ctrlreg event masking Petre Pircalabu
2017-06-19 12:24 ` [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
@ 2017-06-19 12:24 ` Petre Pircalabu
2017-06-19 14:51 ` Tamas K Lengyel
1 sibling, 1 reply; 30+ messages in thread
From: Petre Pircalabu @ 2017-06-19 12:24 UTC (permalink / raw)
To: xen-devel
Cc: Petre Pircalabu, tamas, wei.liu2, rcojocaru, andrew.cooper3,
ian.jackson, jbeulich
Add test for write_ctrlreg event handling.
Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
---
tools/tests/xen-access/xen-access.c | 53 ++++++++++++++++++++++++++++++++++++-
1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 238011e..1e69e25 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -57,6 +57,13 @@
#define X86_TRAP_DEBUG 1
#define X86_TRAP_INT3 3
+/* From xen/include/asm-x86/x86-defns.h */
+#define X86_CR4_PGE 0x00000080 /* enable global pages */
+
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+#endif
+
typedef struct vm_event {
domid_t domain_id;
xenevtchn_handle *xce_handle;
@@ -314,6 +321,24 @@ static void get_request(vm_event_t *vm_event, vm_event_request_t *req)
}
/*
+ * X86 control register names
+ */
+static const char* get_x86_ctrl_reg_name(uint32_t index)
+{
+ static const char* names[] = {
+ [VM_EVENT_X86_CR0] = "CR0",
+ [VM_EVENT_X86_CR3] = "CR3",
+ [VM_EVENT_X86_CR4] = "CR4",
+ [VM_EVENT_X86_XCR0] = "XCR0",
+ };
+
+ if ( index >= ARRAY_SIZE(names) || names[index] == NULL )
+ return "";
+
+ return names[index];
+}
+
+/*
* Note that this function is not thread safe.
*/
static void put_response(vm_event_t *vm_event, vm_event_response_t *rsp)
@@ -337,7 +362,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|debug|cpuid|desc_access");
+ fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid|desc_access|write_ctrlreg_cr4");
#elif defined(__arm__) || defined(__aarch64__)
fprintf(stderr, "|privcall");
#endif
@@ -369,6 +394,7 @@ int main(int argc, char *argv[])
int debug = 0;
int cpuid = 0;
int desc_access = 0;
+ int write_ctrlreg_cr4 = 1;
uint16_t altp2m_view_id = 0;
char* progname = argv[0];
@@ -439,6 +465,10 @@ int main(int argc, char *argv[])
{
desc_access = 1;
}
+ else if ( !strcmp(argv[0], "write_ctrlreg_cr4") )
+ {
+ write_ctrlreg_cr4 = 1;
+ }
#elif defined(__arm__) || defined(__aarch64__)
else if ( !strcmp(argv[0], "privcall") )
{
@@ -596,6 +626,18 @@ int main(int argc, char *argv[])
}
}
+ if ( write_ctrlreg_cr4 )
+ {
+ /* Mask the CR4.PGE bit so no events will be generated for global TLB flushes. */
+ rc = xc_monitor_write_ctrlreg(xch, domain_id, VM_EVENT_X86_CR4, 1, 1,
+ X86_CR4_PGE, 1);
+ if ( rc < 0 )
+ {
+ ERROR("Error %d setting write control register trapping with vm_event\n", rc);
+ goto exit;
+ }
+ }
+
/* Wait for access */
for (;;)
{
@@ -806,6 +848,15 @@ int main(int argc, char *argv[])
req.u.desc_access.is_write);
rsp.flags |= VM_EVENT_FLAG_EMULATE;
break;
+ case VM_EVENT_REASON_WRITE_CTRLREG:
+ printf("Control register written: rip=%016"PRIx64", vcpu %d: "
+ "reg=%s, old_value=%016"PRIx64", new_value=%016"PRIx64"\n",
+ req.data.regs.x86.rip,
+ req.vcpu_id,
+ get_x86_ctrl_reg_name(req.u.write_ctrlreg.index),
+ req.u.write_ctrlreg.old_value,
+ req.u.write_ctrlreg.new_value);
+ break;
default:
fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
}
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4 2/2] xen-access: write_ctrlreg_c4 test
2017-06-19 12:24 ` [PATCH v4 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
@ 2017-06-19 14:51 ` Tamas K Lengyel
0 siblings, 0 replies; 30+ messages in thread
From: Tamas K Lengyel @ 2017-06-19 14:51 UTC (permalink / raw)
To: Petre Pircalabu
Cc: wei.liu2, Razvan Cojocaru, Andrew Cooper, Ian Jackson, Xen-devel,
Jan Beulich
On Mon, Jun 19, 2017 at 6:24 AM, Petre Pircalabu
<ppircalabu@bitdefender.com> wrote:
> Add test for write_ctrlreg event handling.
>
> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> tools/tests/xen-access/xen-access.c | 53 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
> index 238011e..1e69e25 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -57,6 +57,13 @@
> #define X86_TRAP_DEBUG 1
> #define X86_TRAP_INT3 3
>
> +/* From xen/include/asm-x86/x86-defns.h */
> +#define X86_CR4_PGE 0x00000080 /* enable global pages */
> +
> +#ifndef ARRAY_SIZE
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> +#endif
> +
> typedef struct vm_event {
> domid_t domain_id;
> xenevtchn_handle *xce_handle;
> @@ -314,6 +321,24 @@ static void get_request(vm_event_t *vm_event, vm_event_request_t *req)
> }
>
> /*
> + * X86 control register names
> + */
> +static const char* get_x86_ctrl_reg_name(uint32_t index)
> +{
> + static const char* names[] = {
> + [VM_EVENT_X86_CR0] = "CR0",
> + [VM_EVENT_X86_CR3] = "CR3",
> + [VM_EVENT_X86_CR4] = "CR4",
> + [VM_EVENT_X86_XCR0] = "XCR0",
> + };
> +
> + if ( index >= ARRAY_SIZE(names) || names[index] == NULL )
> + return "";
> +
> + return names[index];
> +}
> +
> +/*
> * Note that this function is not thread safe.
> */
> static void put_response(vm_event_t *vm_event, vm_event_response_t *rsp)
> @@ -337,7 +362,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|debug|cpuid|desc_access");
> + fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid|desc_access|write_ctrlreg_cr4");
> #elif defined(__arm__) || defined(__aarch64__)
> fprintf(stderr, "|privcall");
> #endif
> @@ -369,6 +394,7 @@ int main(int argc, char *argv[])
> int debug = 0;
> int cpuid = 0;
> int desc_access = 0;
> + int write_ctrlreg_cr4 = 1;
> uint16_t altp2m_view_id = 0;
>
> char* progname = argv[0];
> @@ -439,6 +465,10 @@ int main(int argc, char *argv[])
> {
> desc_access = 1;
> }
> + else if ( !strcmp(argv[0], "write_ctrlreg_cr4") )
> + {
> + write_ctrlreg_cr4 = 1;
> + }
> #elif defined(__arm__) || defined(__aarch64__)
> else if ( !strcmp(argv[0], "privcall") )
> {
> @@ -596,6 +626,18 @@ int main(int argc, char *argv[])
> }
> }
>
> + if ( write_ctrlreg_cr4 )
> + {
> + /* Mask the CR4.PGE bit so no events will be generated for global TLB flushes. */
> + rc = xc_monitor_write_ctrlreg(xch, domain_id, VM_EVENT_X86_CR4, 1, 1,
> + X86_CR4_PGE, 1);
> + if ( rc < 0 )
> + {
> + ERROR("Error %d setting write control register trapping with vm_event\n", rc);
> + goto exit;
> + }
> + }
> +
> /* Wait for access */
> for (;;)
> {
> @@ -806,6 +848,15 @@ int main(int argc, char *argv[])
> req.u.desc_access.is_write);
> rsp.flags |= VM_EVENT_FLAG_EMULATE;
> break;
> + case VM_EVENT_REASON_WRITE_CTRLREG:
> + printf("Control register written: rip=%016"PRIx64", vcpu %d: "
> + "reg=%s, old_value=%016"PRIx64", new_value=%016"PRIx64"\n",
> + req.data.regs.x86.rip,
> + req.vcpu_id,
> + get_x86_ctrl_reg_name(req.u.write_ctrlreg.index),
> + req.u.write_ctrlreg.old_value,
> + req.u.write_ctrlreg.new_value);
> + break;
> default:
> fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
> }
> --
> 2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-06-19 12:24 ` [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
@ 2017-06-20 12:42 ` Wei Liu
2017-06-21 13:58 ` Wei Liu
1 sibling, 0 replies; 30+ messages in thread
From: Wei Liu @ 2017-06-20 12:42 UTC (permalink / raw)
To: Petre Pircalabu
Cc: tamas, wei.liu2, rcojocaru, andrew.cooper3, ian.jackson,
xen-devel, jbeulich
On Mon, Jun 19, 2017 at 03:24:38PM +0300, Petre Pircalabu wrote:
> Add support for filtering out the write_ctrlreg monitor events if they
> are generated only by changing certains bits.
> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
> function in order to mask the event generation if the changed bits are
> set.
>
> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> tools/libxc/include/xenctrl.h | 2 +-
> tools/libxc/xc_monitor.c | 5 ++++-
Acked-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-06-19 12:24 ` [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
2017-06-20 12:42 ` Wei Liu
@ 2017-06-21 13:58 ` Wei Liu
2017-06-21 14:11 ` Tamas K Lengyel
2017-06-21 14:13 ` Razvan Cojocaru
1 sibling, 2 replies; 30+ messages in thread
From: Wei Liu @ 2017-06-21 13:58 UTC (permalink / raw)
To: Petre Pircalabu
Cc: tamas, wei.liu2, rcojocaru, andrew.cooper3, ian.jackson,
xen-devel, jbeulich
On Mon, Jun 19, 2017 at 03:24:38PM +0300, Petre Pircalabu wrote:
> Add support for filtering out the write_ctrlreg monitor events if they
> are generated only by changing certains bits.
> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
> function in order to mask the event generation if the changed bits are
> set.
>
> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Coverity isn't happy with this patch.
It seems to me there is indeed a risk to overrun the buffer (4 in size) because
the caller can specify index up to 31.
** CID 1412966: Memory - corruptions (OVERRUN)
/xen/arch/x86/monitor.c: 162 in arch_monitor_domctl_event()
________________________________________________________________________________________________________
*** CID 1412966: Memory - corruptions (OVERRUN)
/xen/arch/x86/monitor.c: 162 in arch_monitor_domctl_event()
156 ad->monitor.write_ctrlreg_onchangeonly |= ctrlreg_bitmask;
157 else
158 ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask;
159
160 if ( requested_status )
161 {
>>> CID 1412966: Memory - corruptions (OVERRUN)
>>> Overrunning array "ad->monitor.write_ctrlreg_mask" of 4 8-byte elements at element index 31 (byte offset 248) using index "mop->u.mov_to_cr.index"
(which evaluates to 31).
162 ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = mop->u.mov_to_cr.bitmask;
163 ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
164 }
165 else
166 {
167 ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 0;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-06-21 13:58 ` Wei Liu
@ 2017-06-21 14:11 ` Tamas K Lengyel
2017-06-21 14:13 ` Razvan Cojocaru
1 sibling, 0 replies; 30+ messages in thread
From: Tamas K Lengyel @ 2017-06-21 14:11 UTC (permalink / raw)
To: Wei Liu
Cc: Petre Pircalabu, Razvan Cojocaru, Andrew Cooper, Ian Jackson,
Xen-devel, Jan Beulich
On Wed, Jun 21, 2017 at 7:58 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Mon, Jun 19, 2017 at 03:24:38PM +0300, Petre Pircalabu wrote:
>> Add support for filtering out the write_ctrlreg monitor events if they
>> are generated only by changing certains bits.
>> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
>> function in order to mask the event generation if the changed bits are
>> set.
>>
>> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
>> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
>
> Coverity isn't happy with this patch.
>
> It seems to me there is indeed a risk to overrun the buffer (4 in size) because
> the caller can specify index up to 31.
Indeed. We have a sanity check earlier in here that checks whether
index > 31 but it would make more sense to check it against the max
valid value of index to begin with (which at the moment is
VM_EVENT_X86_XCR0 = 3).
>
> ** CID 1412966: Memory - corruptions (OVERRUN)
> /xen/arch/x86/monitor.c: 162 in arch_monitor_domctl_event()
>
>
> ________________________________________________________________________________________________________
> *** CID 1412966: Memory - corruptions (OVERRUN)
> /xen/arch/x86/monitor.c: 162 in arch_monitor_domctl_event()
> 156 ad->monitor.write_ctrlreg_onchangeonly |= ctrlreg_bitmask;
> 157 else
> 158 ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask;
> 159
> 160 if ( requested_status )
> 161 {
>>>> CID 1412966: Memory - corruptions (OVERRUN)
>>>> Overrunning array "ad->monitor.write_ctrlreg_mask" of 4 8-byte elements at element index 31 (byte offset 248) using index "mop->u.mov_to_cr.index"
> (which evaluates to 31).
> 162 ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = mop->u.mov_to_cr.bitmask;
> 163 ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
> 164 }
> 165 else
> 166 {
> 167 ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 0;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-06-21 13:58 ` Wei Liu
2017-06-21 14:11 ` Tamas K Lengyel
@ 2017-06-21 14:13 ` Razvan Cojocaru
2017-06-21 14:19 ` Wei Liu
1 sibling, 1 reply; 30+ messages in thread
From: Razvan Cojocaru @ 2017-06-21 14:13 UTC (permalink / raw)
To: Wei Liu, Petre Pircalabu
Cc: andrew.cooper3, tamas, ian.jackson, jbeulich, xen-devel
On 06/21/2017 04:58 PM, Wei Liu wrote:
> On Mon, Jun 19, 2017 at 03:24:38PM +0300, Petre Pircalabu wrote:
>> Add support for filtering out the write_ctrlreg monitor events if they
>> are generated only by changing certains bits.
>> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
>> function in order to mask the event generation if the changed bits are
>> set.
>>
>> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
>> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
>
> Coverity isn't happy with this patch.
>
> It seems to me there is indeed a risk to overrun the buffer (4 in size) because
> the caller can specify index up to 31.
>
> ** CID 1412966: Memory - corruptions (OVERRUN)
> /xen/arch/x86/monitor.c: 162 in arch_monitor_domctl_event()
>
>
> ________________________________________________________________________________________________________
> *** CID 1412966: Memory - corruptions (OVERRUN)
> /xen/arch/x86/monitor.c: 162 in arch_monitor_domctl_event()
> 156 ad->monitor.write_ctrlreg_onchangeonly |= ctrlreg_bitmask;
> 157 else
> 158 ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask;
> 159
> 160 if ( requested_status )
> 161 {
>>>> CID 1412966: Memory - corruptions (OVERRUN)
>>>> Overrunning array "ad->monitor.write_ctrlreg_mask" of 4 8-byte elements at element index 31 (byte offset 248) using index "mop->u.mov_to_cr.index"
> (which evaluates to 31).
> 162 ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = mop->u.mov_to_cr.bitmask;
> 163 ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
> 164 }
> 165 else
> 166 {
> 167 ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 0;
I vaguely remember that 31 was introduced simply as a "reserved"
precaution - we can probably safely please Coverity by simply patching
that code to not go over 3 as an index.
To Petre's credit, he did notice and propose that we change this value
but I've suggested that we keep the check as-is for the future. My bad. :)
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events
2017-06-21 14:13 ` Razvan Cojocaru
@ 2017-06-21 14:19 ` Wei Liu
0 siblings, 0 replies; 30+ messages in thread
From: Wei Liu @ 2017-06-21 14:19 UTC (permalink / raw)
To: Razvan Cojocaru
Cc: Petre Pircalabu, tamas, Wei Liu, andrew.cooper3, ian.jackson,
xen-devel, jbeulich
On Wed, Jun 21, 2017 at 05:13:29PM +0300, Razvan Cojocaru wrote:
> On 06/21/2017 04:58 PM, Wei Liu wrote:
> > On Mon, Jun 19, 2017 at 03:24:38PM +0300, Petre Pircalabu wrote:
> >> Add support for filtering out the write_ctrlreg monitor events if they
> >> are generated only by changing certains bits.
> >> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
> >> function in order to mask the event generation if the changed bits are
> >> set.
> >>
> >> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
> >> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
> >
> > Coverity isn't happy with this patch.
> >
> > It seems to me there is indeed a risk to overrun the buffer (4 in size) because
> > the caller can specify index up to 31.
> >
> > ** CID 1412966: Memory - corruptions (OVERRUN)
> > /xen/arch/x86/monitor.c: 162 in arch_monitor_domctl_event()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 1412966: Memory - corruptions (OVERRUN)
> > /xen/arch/x86/monitor.c: 162 in arch_monitor_domctl_event()
> > 156 ad->monitor.write_ctrlreg_onchangeonly |= ctrlreg_bitmask;
> > 157 else
> > 158 ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask;
> > 159
> > 160 if ( requested_status )
> > 161 {
> >>>> CID 1412966: Memory - corruptions (OVERRUN)
> >>>> Overrunning array "ad->monitor.write_ctrlreg_mask" of 4 8-byte elements at element index 31 (byte offset 248) using index "mop->u.mov_to_cr.index"
> > (which evaluates to 31).
> > 162 ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = mop->u.mov_to_cr.bitmask;
> > 163 ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
> > 164 }
> > 165 else
> > 166 {
> > 167 ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 0;
>
> I vaguely remember that 31 was introduced simply as a "reserved"
> precaution - we can probably safely please Coverity by simply patching
> that code to not go over 3 as an index.
>
> To Petre's credit, he did notice and propose that we change this value
> but I've suggested that we keep the check as-is for the future. My bad. :)
>
OK. Please submit a patch to fix this. Using 3 should be fine. Please
include
Coverity-ID: 1412966
in the commit message.
>
> Thanks,
> Razvan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2017-06-21 14:19 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-26 13:41 write_ctrlreg event masking Petre Pircalabu
2017-05-26 13:41 ` [PATCH 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
2017-05-29 15:01 ` Jan Beulich
2017-05-30 9:38 ` Petre PIRCALABU
2017-05-26 13:41 ` [PATCH 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
2017-05-30 9:46 ` write_ctrlreg event masking Petre Pircalabu
2017-05-30 9:46 ` [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
2017-06-16 14:26 ` Tamas K Lengyel
2017-06-16 15:15 ` Jan Beulich
2017-06-16 15:28 ` Tamas K Lengyel
2017-06-16 15:33 ` Jan Beulich
2017-06-16 16:24 ` Tamas K Lengyel
2017-05-30 9:46 ` [PATCH v2 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
2017-06-16 14:32 ` Tamas K Lengyel
2017-06-16 15:12 ` Jan Beulich
2017-06-16 15:24 ` Tamas K Lengyel
2017-06-16 14:09 ` write_ctrlreg event masking Petre Ovidiu PIRCALABU
2017-06-16 19:20 ` [PATCH v3 0/2] " Petre Pircalabu
2017-06-16 19:20 ` [PATCH v3 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
2017-06-16 19:20 ` [PATCH v3 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
2017-06-16 19:45 ` Razvan Cojocaru
2017-06-19 12:24 ` [PATCH v4 0/2] write_ctrlreg event masking Petre Pircalabu
2017-06-19 12:24 ` [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events Petre Pircalabu
2017-06-20 12:42 ` Wei Liu
2017-06-21 13:58 ` Wei Liu
2017-06-21 14:11 ` Tamas K Lengyel
2017-06-21 14:13 ` Razvan Cojocaru
2017-06-21 14:19 ` Wei Liu
2017-06-19 12:24 ` [PATCH v4 2/2] xen-access: write_ctrlreg_c4 test Petre Pircalabu
2017-06-19 14:51 ` Tamas K Lengyel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.