| Message ID | 1496137567-6574-3-git-send-email-ppircalabu@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
>>> 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
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
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); }
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(-)