| Message ID | 20170404095757.9064-1-apop@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/04/2017 12:57 PM, Adrian Pop wrote: > Adds monitor support for descriptor access events (reads & writes of > IDTR/GDTR/LDTR/TR) for the x86 architecture (VMX and SVM). > > Signed-off-by: Adrian Pop <apop@bitdefender.com> > --- > changes in v2: > - use two sub-structs (vmx/svm) for vm_event_desc_access (Tamas K > Lengyel) > - more compact version of the descriptor VM-Exit handling on svm (Boris > Ostrovsky) > - use bool instead of bool_t (Jan Beulich) > - use curr, currd for the struct vcpu and struct domain local variables > (Jan Beulich) > - move hvm_emulate_ctxt into a narrower scope (Jan Beulich) > - remove leftover dead code (Jan Beulich) > - order the monitor events numerically (Jan Beulich) > - remove VM_EVENT_DESC_INVALID (Jan Beulich) > - crash the domain if the descriptor access instruction can't be > emulated (Jan Beulich) > - call hvm_inject_event without checking for pending events (Andrew > Cooper) > - introduce structures for the VM-Exit instruction information used for > the descriptor instructions and use fewer magic numbers (Andrew > Cooper, Jan Beulich) You've forgotten Wei Liu's ack for the first version of the patch. For the vm_event bits: Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com> Thanks, Razvan
On Tue, Apr 04, 2017 at 01:52:24PM +0300, Razvan Cojocaru wrote: > You've forgotten Wei Liu's ack for the first version of the patch. > > For the vm_event bits: > > Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com> Okay, will include them next time. Thanks!
On 04/04/2017 05:57 AM, Adrian Pop wrote: > Adds monitor support for descriptor access events (reads & writes of > IDTR/GDTR/LDTR/TR) for the x86 architecture (VMX and SVM). > > Signed-off-by: Adrian Pop <apop@bitdefender.com> > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -856,6 +856,23 @@ static void svm_set_rdtsc_exiting(struct vcpu *v, bool_t enable) > vmcb_set_general2_intercepts(vmcb, general2_intercepts); > } > > +static void svm_set_descriptor_access_exiting(struct vcpu *v, bool enable) > +{ > + struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; > + u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb); > + u32 mask = GENERAL1_INTERCEPT_IDTR_READ | GENERAL1_INTERCEPT_GDTR_READ \ > + | GENERAL1_INTERCEPT_LDTR_READ | GENERAL1_INTERCEPT_TR_READ \ > + | GENERAL1_INTERCEPT_IDTR_WRITE | GENERAL1_INTERCEPT_GDTR_WRITE \ > + | GENERAL1_INTERCEPT_LDTR_WRITE | GENERAL1_INTERCEPT_TR_WRITE; I didn't notice this last time --- there is no need for line continuation here. With that fixed, Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>eason = %#"PRIx64", "
On Tue, Apr 04, 2017 at 09:23:28AM -0400, Boris Ostrovsky wrote: > On 04/04/2017 05:57 AM, Adrian Pop wrote: > > Adds monitor support for descriptor access events (reads & writes of > > IDTR/GDTR/LDTR/TR) for the x86 architecture (VMX and SVM). > > > > Signed-off-by: Adrian Pop <apop@bitdefender.com> > > > > --- a/xen/arch/x86/hvm/svm/svm.c > > +++ b/xen/arch/x86/hvm/svm/svm.c > > @@ -856,6 +856,23 @@ static void svm_set_rdtsc_exiting(struct vcpu *v, bool_t enable) > > vmcb_set_general2_intercepts(vmcb, general2_intercepts); > > } > > > > +static void svm_set_descriptor_access_exiting(struct vcpu *v, bool enable) > > +{ > > + struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; > > + u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb); > > + u32 mask = GENERAL1_INTERCEPT_IDTR_READ | GENERAL1_INTERCEPT_GDTR_READ \ > > + | GENERAL1_INTERCEPT_LDTR_READ | GENERAL1_INTERCEPT_TR_READ \ > > + | GENERAL1_INTERCEPT_IDTR_WRITE | GENERAL1_INTERCEPT_GDTR_WRITE \ > > + | GENERAL1_INTERCEPT_LDTR_WRITE | GENERAL1_INTERCEPT_TR_WRITE; > > I didn't notice this last time --- there is no need for line > continuation here. > > With that fixed, > > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> Oh, yes. I'll remove the escapes for the next version. Thank you!
On 04/04/17 10:57, Adrian Pop wrote: > diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c > index f5cd245771..d60e4afd0c 100644 > --- a/xen/arch/x86/hvm/monitor.c > +++ b/xen/arch/x86/hvm/monitor.c > @@ -72,6 +72,28 @@ void hvm_monitor_msr(unsigned int msr, uint64_t value) > } > } > > +void hvm_monitor_descriptor_access(uint64_t exit_info, > + uint64_t vmx_exit_qualification, > + uint8_t descriptor, bool is_write) > +{ > + struct vcpu *curr = current; > + vm_event_request_t req = { > + .reason = VM_EVENT_REASON_DESCRIPTOR_ACCESS, > + .u.desc_access.descriptor = descriptor, > + .u.desc_access.is_write = is_write, > + }; Newline here > + if ( cpu_has_vmx ) > + { > + req.u.desc_access.arch.vmx.instr_info = exit_info; > + req.u.desc_access.arch.vmx.exit_qualification = vmx_exit_qualification; > + } > + else > + { > + req.u.desc_access.arch.svm.exitinfo = exit_info; > + } And here please. > + monitor_traps(curr, 1, &req); > +} > + > static inline unsigned long gfn_of_rip(unsigned long rip) > { > struct vcpu *curr = current; > diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h > index 2b781ab120..b00ba52443 100644 > --- a/xen/include/asm-x86/hvm/vmx/vmx.h > +++ b/xen/include/asm-x86/hvm/vmx/vmx.h > @@ -628,4 +628,46 @@ typedef struct { > u16 eptp_index; > } ve_info_t; > > +/* VM-Exit instruction info for LIDT, LGDT, SIDT, SGDT */ > +typedef union idt_or_gdt_instr_info { > + unsigned long raw; > + struct { > + unsigned long scaling :2, /* bits 0:1 - Scaling */ > + :5, /* bits 6:2 - Undefined */ > + addr_size :3, /* bits 9:7 - Address size */ > + :1, /* bit 10 - Cleared to 0 */ > + operand_size :1, /* bit 11 - Operand size */ > + :3, /* bits 14:12 - Undefined */ > + segment_reg :3, /* bits 17:15 - Segment register */ > + index_reg :4, /* bits 21:18 - Index register */ > + index_reg_invalid :1, /* bit 22 - Index register invalid */ > + base_reg :4, /* bits 26:23 - Base register */ > + base_reg_invalid :1, /* bit 27 - Base register invalid */ > + instr_identity :1, /* bit 28 - 0:GDT, 1:IDT */ > + instr_write :1, /* bit 29 - 0:store, 1:load */ > + :2; /* bits 30:31 - Undefined */ I think you need a :32 /* undefined */ in each of these, to avoid breaking the Clang build, which cares that each half of the union have the same bit size. All of these can be fixed on commit if there are no other issues. Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
On Tue, Apr 04, 2017 at 04:26:24PM +0100, Andrew Cooper wrote: > On 04/04/17 10:57, Adrian Pop wrote: > > diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c > > index f5cd245771..d60e4afd0c 100644 > > --- a/xen/arch/x86/hvm/monitor.c > > +++ b/xen/arch/x86/hvm/monitor.c > > @@ -72,6 +72,28 @@ void hvm_monitor_msr(unsigned int msr, uint64_t value) > > } > > } > > > > +void hvm_monitor_descriptor_access(uint64_t exit_info, > > + uint64_t vmx_exit_qualification, > > + uint8_t descriptor, bool is_write) > > +{ > > + struct vcpu *curr = current; > > + vm_event_request_t req = { > > + .reason = VM_EVENT_REASON_DESCRIPTOR_ACCESS, > > + .u.desc_access.descriptor = descriptor, > > + .u.desc_access.is_write = is_write, > > + }; > > Newline here Ok. > > + if ( cpu_has_vmx ) > > + { > > + req.u.desc_access.arch.vmx.instr_info = exit_info; > > + req.u.desc_access.arch.vmx.exit_qualification = vmx_exit_qualification; > > + } > > + else > > + { > > + req.u.desc_access.arch.svm.exitinfo = exit_info; > > + } > > And here please. Ok. > > + monitor_traps(curr, 1, &req); > > +} > > + > > static inline unsigned long gfn_of_rip(unsigned long rip) > > { > > struct vcpu *curr = current; > > diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h > > index 2b781ab120..b00ba52443 100644 > > --- a/xen/include/asm-x86/hvm/vmx/vmx.h > > +++ b/xen/include/asm-x86/hvm/vmx/vmx.h > > @@ -628,4 +628,46 @@ typedef struct { > > u16 eptp_index; > > } ve_info_t; > > > > +/* VM-Exit instruction info for LIDT, LGDT, SIDT, SGDT */ > > +typedef union idt_or_gdt_instr_info { > > + unsigned long raw; > > + struct { > > + unsigned long scaling :2, /* bits 0:1 - Scaling */ > > + :5, /* bits 6:2 - Undefined */ > > + addr_size :3, /* bits 9:7 - Address size */ > > + :1, /* bit 10 - Cleared to 0 */ > > + operand_size :1, /* bit 11 - Operand size */ > > + :3, /* bits 14:12 - Undefined */ > > + segment_reg :3, /* bits 17:15 - Segment register */ > > + index_reg :4, /* bits 21:18 - Index register */ > > + index_reg_invalid :1, /* bit 22 - Index register invalid */ > > + base_reg :4, /* bits 26:23 - Base register */ > > + base_reg_invalid :1, /* bit 27 - Base register invalid */ > > + instr_identity :1, /* bit 28 - 0:GDT, 1:IDT */ > > + instr_write :1, /* bit 29 - 0:store, 1:load */ > > + :2; /* bits 30:31 - Undefined */ > > I think you need a :32 /* undefined */ in each of these, to avoid > breaking the Clang build, which cares that each half of the union have > the same bit size. All right. > All of these can be fixed on commit if there are no other issues. > Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Thanks!
> From: Adrian Pop [mailto:apop@bitdefender.com] > Sent: Tuesday, April 4, 2017 5:58 PM > > Adds monitor support for descriptor access events (reads & writes of > IDTR/GDTR/LDTR/TR) for the x86 architecture (VMX and SVM). > > Signed-off-by: Adrian Pop <apop@bitdefender.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
>>> On 04.04.17 at 11:57, <apop@bitdefender.com> wrote: > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -3572,6 +3572,43 @@ gp_fault: > return X86EMUL_EXCEPTION; > } > > +int hvm_descriptor_access_intercept(uint64_t exit_info, > + uint64_t vmx_exit_qualification, > + uint8_t descriptor, bool is_write) Why uint8_t? > +{ > + struct vcpu *curr = current; > + struct domain *currd = curr->domain; > + int rc; > + > + if ( currd->arch.monitor.descriptor_access_enabled ) > + { > + ASSERT(curr->arch.vm_event); > + hvm_monitor_descriptor_access(exit_info, vmx_exit_qualification, > + descriptor, is_write); > + } > + else > + { > + struct hvm_emulate_ctxt ctxt = {}; > + > + hvm_emulate_init_once(&ctxt, NULL, guest_cpu_user_regs()); > + rc = hvm_emulate_one(&ctxt); > + switch ( rc ) You don't really need to go through a local variable here. > --- a/xen/arch/x86/hvm/monitor.c > +++ b/xen/arch/x86/hvm/monitor.c > @@ -72,6 +72,28 @@ void hvm_monitor_msr(unsigned int msr, uint64_t value) > } > } > > +void hvm_monitor_descriptor_access(uint64_t exit_info, > + uint64_t vmx_exit_qualification, > + uint8_t descriptor, bool is_write) > +{ > + struct vcpu *curr = current; > + vm_event_request_t req = { > + .reason = VM_EVENT_REASON_DESCRIPTOR_ACCESS, > + .u.desc_access.descriptor = descriptor, > + .u.desc_access.is_write = is_write, > + }; > + if ( cpu_has_vmx ) > + { > + req.u.desc_access.arch.vmx.instr_info = exit_info; > + req.u.desc_access.arch.vmx.exit_qualification = vmx_exit_qualification; > + } > + else > + { > + req.u.desc_access.arch.svm.exitinfo = exit_info; > + } > + monitor_traps(curr, 1, &req); true > @@ -3361,6 +3376,40 @@ static void vmx_handle_xrstors(void) > domain_crash(current->domain); > } > > +static void vmx_handle_idt_or_gdt(idt_or_gdt_instr_info_t instr_info, > + uint64_t exit_qualification) > +{ > + uint8_t descriptor = instr_info.instr_identity > + ? VM_EVENT_DESC_IDTR : VM_EVENT_DESC_GDTR; > + > + hvm_descriptor_access_intercept(instr_info.raw, exit_qualification, > + descriptor, instr_info.instr_write); > +} > + > +static void vmx_handle_ldt_or_tr(ldt_or_tr_instr_info_t instr_info, > + uint64_t exit_qualification) > +{ > + uint8_t descriptor = instr_info.instr_identity > + ? VM_EVENT_DESC_TR : VM_EVENT_DESC_LDTR; > + > + hvm_descriptor_access_intercept(instr_info.raw, exit_qualification, > + descriptor, instr_info.instr_write); > +} I think these should be folded into their only caller (at once eliminating the need to make those unions transparent ones). And again - why uint8_t? Jan
Hello, On Wed, Apr 05, 2017 at 08:26:27AM -0600, Jan Beulich wrote: > >>> On 04.04.17 at 11:57, <apop@bitdefender.com> wrote: > > --- a/xen/arch/x86/hvm/hvm.c > > +++ b/xen/arch/x86/hvm/hvm.c > > @@ -3572,6 +3572,43 @@ gp_fault: > > return X86EMUL_EXCEPTION; > > } > > > > +int hvm_descriptor_access_intercept(uint64_t exit_info, > > + uint64_t vmx_exit_qualification, > > + uint8_t descriptor, bool is_write) > > Why uint8_t? The descriptor type from struct vm_event_desc_access is uint8_t since there are only 4 possible descriptors: > > +#define VM_EVENT_DESC_IDTR 1 > > +#define VM_EVENT_DESC_GDTR 2 > > +#define VM_EVENT_DESC_LDTR 3 > > +#define VM_EVENT_DESC_TR 4 Should it be something else? > > +{ > > + struct vcpu *curr = current; > > + struct domain *currd = curr->domain; > > + int rc; > > + > > + if ( currd->arch.monitor.descriptor_access_enabled ) > > + { > > + ASSERT(curr->arch.vm_event); > > + hvm_monitor_descriptor_access(exit_info, vmx_exit_qualification, > > + descriptor, is_write); > > + } > > + else > > + { > > + struct hvm_emulate_ctxt ctxt = {}; > > + > > + hvm_emulate_init_once(&ctxt, NULL, guest_cpu_user_regs()); > > + rc = hvm_emulate_one(&ctxt); > > + switch ( rc ) > > You don't really need to go through a local variable here. Ok. > > --- a/xen/arch/x86/hvm/monitor.c > > +++ b/xen/arch/x86/hvm/monitor.c > > @@ -72,6 +72,28 @@ void hvm_monitor_msr(unsigned int msr, uint64_t value) > > } > > } > > > > +void hvm_monitor_descriptor_access(uint64_t exit_info, > > + uint64_t vmx_exit_qualification, > > + uint8_t descriptor, bool is_write) > > +{ > > + struct vcpu *curr = current; > > + vm_event_request_t req = { > > + .reason = VM_EVENT_REASON_DESCRIPTOR_ACCESS, > > + .u.desc_access.descriptor = descriptor, > > + .u.desc_access.is_write = is_write, > > + }; > > + if ( cpu_has_vmx ) > > + { > > + req.u.desc_access.arch.vmx.instr_info = exit_info; > > + req.u.desc_access.arch.vmx.exit_qualification = vmx_exit_qualification; > > + } > > + else > > + { > > + req.u.desc_access.arch.svm.exitinfo = exit_info; > > + } > > + monitor_traps(curr, 1, &req); > > true Ok. > > @@ -3361,6 +3376,40 @@ static void vmx_handle_xrstors(void) > > domain_crash(current->domain); > > } > > > > +static void vmx_handle_idt_or_gdt(idt_or_gdt_instr_info_t instr_info, > > + uint64_t exit_qualification) > > +{ > > + uint8_t descriptor = instr_info.instr_identity > > + ? VM_EVENT_DESC_IDTR : VM_EVENT_DESC_GDTR; > > + > > + hvm_descriptor_access_intercept(instr_info.raw, exit_qualification, > > + descriptor, instr_info.instr_write); > > +} > > + > > +static void vmx_handle_ldt_or_tr(ldt_or_tr_instr_info_t instr_info, > > + uint64_t exit_qualification) > > +{ > > + uint8_t descriptor = instr_info.instr_identity > > + ? VM_EVENT_DESC_TR : VM_EVENT_DESC_LDTR; > > + > > + hvm_descriptor_access_intercept(instr_info.raw, exit_qualification, > > + descriptor, instr_info.instr_write); > > +} > > I think these should be folded into their only caller (at once > eliminating the need to make those unions transparent ones). Ok. > And again - why uint8_t? Same as above. > Jan Thank you!
>>> On 06.04.17 at 10:59, <apop@bitdefender.com> wrote: > On Wed, Apr 05, 2017 at 08:26:27AM -0600, Jan Beulich wrote: >> >>> On 04.04.17 at 11:57, <apop@bitdefender.com> wrote: >> > --- a/xen/arch/x86/hvm/hvm.c >> > +++ b/xen/arch/x86/hvm/hvm.c >> > @@ -3572,6 +3572,43 @@ gp_fault: >> > return X86EMUL_EXCEPTION; >> > } >> > >> > +int hvm_descriptor_access_intercept(uint64_t exit_info, >> > + uint64_t vmx_exit_qualification, >> > + uint8_t descriptor, bool is_write) >> >> Why uint8_t? > > The descriptor type from struct vm_event_desc_access is uint8_t since > there are only 4 possible descriptors: > >> > +#define VM_EVENT_DESC_IDTR 1 >> > +#define VM_EVENT_DESC_GDTR 2 >> > +#define VM_EVENT_DESC_LDTR 3 >> > +#define VM_EVENT_DESC_TR 4 > > Should it be something else? Well, you should avoid fixed width types where they're not really needed (their use should signal a true dependency on the specified width). "unsigned int" would be quite fine here afaict. Jan
On Thu, Apr 06, 2017 at 03:20:21AM -0600, Jan Beulich wrote: > >>> On 06.04.17 at 10:59, <apop@bitdefender.com> wrote: > > On Wed, Apr 05, 2017 at 08:26:27AM -0600, Jan Beulich wrote: > >> >>> On 04.04.17 at 11:57, <apop@bitdefender.com> wrote: > >> > --- a/xen/arch/x86/hvm/hvm.c > >> > +++ b/xen/arch/x86/hvm/hvm.c > >> > @@ -3572,6 +3572,43 @@ gp_fault: > >> > return X86EMUL_EXCEPTION; > >> > } > >> > > >> > +int hvm_descriptor_access_intercept(uint64_t exit_info, > >> > + uint64_t vmx_exit_qualification, > >> > + uint8_t descriptor, bool is_write) > >> > >> Why uint8_t? > > > > The descriptor type from struct vm_event_desc_access is uint8_t since > > there are only 4 possible descriptors: > > > >> > +#define VM_EVENT_DESC_IDTR 1 > >> > +#define VM_EVENT_DESC_GDTR 2 > >> > +#define VM_EVENT_DESC_LDTR 3 > >> > +#define VM_EVENT_DESC_TR 4 > > > > Should it be something else? > > Well, you should avoid fixed width types where they're not really > needed (their use should signal a true dependency on the specified > width). "unsigned int" would be quite fine here afaict. So should it be changed in the struct definition as well? > >> > +struct vm_event_desc_access { > >> > + union { > >> > + struct { > >> > + uint32_t instr_info; /* VMX: VMCS Instruction-Information */ > >> > + uint32_t _pad1; > >> > + uint64_t exit_qualification; /* VMX: VMCS Exit Qualification */ > >> > + } vmx; > >> > + struct { > >> > + uint64_t exitinfo; /* SVM: VMCB EXITINFO */ > >> > + uint64_t _pad2; > >> > + } svm; > >> > + } arch; > >> > + uint8_t descriptor; /* VM_EVENT_DESC_* */ > >> > + uint8_t is_write; > >> > + uint8_t _pad[6]; > >> > +};
>>> On 06.04.17 at 11:37, <apop@bitdefender.com> wrote: > On Thu, Apr 06, 2017 at 03:20:21AM -0600, Jan Beulich wrote: >> >>> On 06.04.17 at 10:59, <apop@bitdefender.com> wrote: >> > On Wed, Apr 05, 2017 at 08:26:27AM -0600, Jan Beulich wrote: >> >> >>> On 04.04.17 at 11:57, <apop@bitdefender.com> wrote: >> >> > --- a/xen/arch/x86/hvm/hvm.c >> >> > +++ b/xen/arch/x86/hvm/hvm.c >> >> > @@ -3572,6 +3572,43 @@ gp_fault: >> >> > return X86EMUL_EXCEPTION; >> >> > } >> >> > >> >> > +int hvm_descriptor_access_intercept(uint64_t exit_info, >> >> > + uint64_t vmx_exit_qualification, >> >> > + uint8_t descriptor, bool is_write) >> >> >> >> Why uint8_t? >> > >> > The descriptor type from struct vm_event_desc_access is uint8_t since >> > there are only 4 possible descriptors: >> > >> >> > +#define VM_EVENT_DESC_IDTR 1 >> >> > +#define VM_EVENT_DESC_GDTR 2 >> >> > +#define VM_EVENT_DESC_LDTR 3 >> >> > +#define VM_EVENT_DESC_TR 4 >> > >> > Should it be something else? >> >> Well, you should avoid fixed width types where they're not really >> needed (their use should signal a true dependency on the specified >> width). "unsigned int" would be quite fine here afaict. > > So should it be changed in the struct definition as well? You mean in the public interface? No, there you _need_ to use fixed width types. Jan
On Thu, Apr 06, 2017 at 08:09:23AM -0600, Jan Beulich wrote: > >>> On 06.04.17 at 11:37, <apop@bitdefender.com> wrote: > > On Thu, Apr 06, 2017 at 03:20:21AM -0600, Jan Beulich wrote: > >> >>> On 06.04.17 at 10:59, <apop@bitdefender.com> wrote: > >> > On Wed, Apr 05, 2017 at 08:26:27AM -0600, Jan Beulich wrote: > >> >> >>> On 04.04.17 at 11:57, <apop@bitdefender.com> wrote: > >> >> > --- a/xen/arch/x86/hvm/hvm.c > >> >> > +++ b/xen/arch/x86/hvm/hvm.c > >> >> > @@ -3572,6 +3572,43 @@ gp_fault: > >> >> > return X86EMUL_EXCEPTION; > >> >> > } > >> >> > > >> >> > +int hvm_descriptor_access_intercept(uint64_t exit_info, > >> >> > + uint64_t vmx_exit_qualification, > >> >> > + uint8_t descriptor, bool is_write) > >> >> > >> >> Why uint8_t? > >> > > >> > The descriptor type from struct vm_event_desc_access is uint8_t since > >> > there are only 4 possible descriptors: > >> > > >> >> > +#define VM_EVENT_DESC_IDTR 1 > >> >> > +#define VM_EVENT_DESC_GDTR 2 > >> >> > +#define VM_EVENT_DESC_LDTR 3 > >> >> > +#define VM_EVENT_DESC_TR 4 > >> > > >> > Should it be something else? > >> > >> Well, you should avoid fixed width types where they're not really > >> needed (their use should signal a true dependency on the specified > >> width). "unsigned int" would be quite fine here afaict. > > > > So should it be changed in the struct definition as well? > > You mean in the public interface? No, there you _need_ to use fixed > width types. Ok, I'll make the changes and resend.
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 2d97d36c38..2248041c5a 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -2007,6 +2007,8 @@ int xc_monitor_mov_to_msr(xc_interface *xch, domid_t domain_id, uint32_t msr, int xc_monitor_singlestep(xc_interface *xch, domid_t domain_id, bool enable); int xc_monitor_software_breakpoint(xc_interface *xch, domid_t domain_id, bool enable); +int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id, + bool enable); int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable, bool sync); int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id, diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c index 15a7c32d52..f99b6e3a33 100644 --- a/tools/libxc/xc_monitor.c +++ b/tools/libxc/xc_monitor.c @@ -129,6 +129,20 @@ int xc_monitor_singlestep(xc_interface *xch, domid_t domain_id, return do_domctl(xch, &domctl); } +int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id, + bool enable) +{ + DECLARE_DOMCTL; + + domctl.cmd = XEN_DOMCTL_monitor_op; + domctl.domain = domain_id; + domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE + : XEN_DOMCTL_MONITOR_OP_DISABLE; + domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS; + + return do_domctl(xch, &domctl); +} + int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable, bool sync) { diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c index 9d4f95756b..ff4d289b45 100644 --- a/tools/tests/xen-access/xen-access.c +++ b/tools/tests/xen-access/xen-access.c @@ -337,7 +337,7 @@ void usage(char* progname) { fprintf(stderr, "Usage: %s [-m] <domain_id> write|exec", progname); #if defined(__i386__) || defined(__x86_64__) - fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid"); + fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid|desc_access"); #elif defined(__arm__) || defined(__aarch64__) fprintf(stderr, "|privcall"); #endif @@ -368,6 +368,7 @@ int main(int argc, char *argv[]) int altp2m = 0; int debug = 0; int cpuid = 0; + int desc_access = 0; uint16_t altp2m_view_id = 0; char* progname = argv[0]; @@ -434,6 +435,10 @@ int main(int argc, char *argv[]) { cpuid = 1; } + else if ( !strcmp(argv[0], "desc_access") ) + { + desc_access = 1; + } #elif defined(__arm__) || defined(__aarch64__) else if ( !strcmp(argv[0], "privcall") ) { @@ -571,6 +576,16 @@ int main(int argc, char *argv[]) } } + if ( desc_access ) + { + rc = xc_monitor_descriptor_access(xch, domain_id, 1); + if ( rc < 0 ) + { + ERROR("Error %d setting descriptor access listener with vm_event\n", rc); + goto exit; + } + } + if ( privcall ) { rc = xc_monitor_privileged_call(xch, domain_id, 1); @@ -595,6 +610,8 @@ int main(int argc, char *argv[]) rc = xc_monitor_debug_exceptions(xch, domain_id, 0, 0); if ( cpuid ) rc = xc_monitor_cpuid(xch, domain_id, 0); + if ( desc_access ) + rc = xc_monitor_descriptor_access(xch, domain_id, 0); if ( privcall ) rc = xc_monitor_privileged_call(xch, domain_id, 0); @@ -779,6 +796,16 @@ int main(int argc, char *argv[]) rsp.data = req.data; rsp.data.regs.x86.rip += req.u.cpuid.insn_length; break; + case VM_EVENT_REASON_DESCRIPTOR_ACCESS: + printf("Descriptor access: rip=%016"PRIx64", vcpu %d: "\ + "VMExit info=0x%"PRIx32", descriptor=%d, is write=%d\n", + req.data.regs.x86.rip, + req.vcpu_id, + req.u.desc_access.arch.vmx.instr_info, + req.u.desc_access.descriptor, + req.u.desc_access.is_write); + rsp.flags |= VM_EVENT_FLAG_EMULATE; + break; default: fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason); } diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 0282986738..61383cb72f 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3572,6 +3572,43 @@ gp_fault: return X86EMUL_EXCEPTION; } +int hvm_descriptor_access_intercept(uint64_t exit_info, + uint64_t vmx_exit_qualification, + uint8_t descriptor, bool is_write) +{ + struct vcpu *curr = current; + struct domain *currd = curr->domain; + int rc; + + if ( currd->arch.monitor.descriptor_access_enabled ) + { + ASSERT(curr->arch.vm_event); + hvm_monitor_descriptor_access(exit_info, vmx_exit_qualification, + descriptor, is_write); + } + else + { + struct hvm_emulate_ctxt ctxt = {}; + + hvm_emulate_init_once(&ctxt, NULL, guest_cpu_user_regs()); + rc = hvm_emulate_one(&ctxt); + switch ( rc ) + { + case X86EMUL_UNHANDLEABLE: + domain_crash(currd); + return X86EMUL_UNHANDLEABLE; + case X86EMUL_EXCEPTION: + hvm_inject_event(&ctxt.ctxt.event); + /* fall through */ + default: + hvm_emulate_writeback(&ctxt); + break; + } + } + + return X86EMUL_OKAY; +} + static bool is_cross_vendor(const struct x86_emulate_state *state, const struct x86_emulate_ctxt *ctxt) { diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c index f5cd245771..d60e4afd0c 100644 --- a/xen/arch/x86/hvm/monitor.c +++ b/xen/arch/x86/hvm/monitor.c @@ -72,6 +72,28 @@ void hvm_monitor_msr(unsigned int msr, uint64_t value) } } +void hvm_monitor_descriptor_access(uint64_t exit_info, + uint64_t vmx_exit_qualification, + uint8_t descriptor, bool is_write) +{ + struct vcpu *curr = current; + vm_event_request_t req = { + .reason = VM_EVENT_REASON_DESCRIPTOR_ACCESS, + .u.desc_access.descriptor = descriptor, + .u.desc_access.is_write = is_write, + }; + if ( cpu_has_vmx ) + { + req.u.desc_access.arch.vmx.instr_info = exit_info; + req.u.desc_access.arch.vmx.exit_qualification = vmx_exit_qualification; + } + else + { + req.u.desc_access.arch.svm.exitinfo = exit_info; + } + monitor_traps(curr, 1, &req); +} + static inline unsigned long gfn_of_rip(unsigned long rip) { struct vcpu *curr = current; diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index b69789b35c..f78dee6249 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -856,6 +856,23 @@ static void svm_set_rdtsc_exiting(struct vcpu *v, bool_t enable) vmcb_set_general2_intercepts(vmcb, general2_intercepts); } +static void svm_set_descriptor_access_exiting(struct vcpu *v, bool enable) +{ + struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; + u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb); + u32 mask = GENERAL1_INTERCEPT_IDTR_READ | GENERAL1_INTERCEPT_GDTR_READ \ + | GENERAL1_INTERCEPT_LDTR_READ | GENERAL1_INTERCEPT_TR_READ \ + | GENERAL1_INTERCEPT_IDTR_WRITE | GENERAL1_INTERCEPT_GDTR_WRITE \ + | GENERAL1_INTERCEPT_LDTR_WRITE | GENERAL1_INTERCEPT_TR_WRITE; + + if ( enable ) + general1_intercepts |= mask; + else + general1_intercepts &= ~mask; + + vmcb_set_general1_intercepts(vmcb, general1_intercepts); +} + static unsigned int svm_get_insn_bytes(struct vcpu *v, uint8_t *buf) { struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; @@ -2225,6 +2242,7 @@ static struct hvm_function_table __initdata svm_function_table = { .msr_read_intercept = svm_msr_read_intercept, .msr_write_intercept = svm_msr_write_intercept, .set_rdtsc_exiting = svm_set_rdtsc_exiting, + .set_descriptor_access_exiting = svm_set_descriptor_access_exiting, .get_insn_bytes = svm_get_insn_bytes, .nhvm_vcpu_initialise = nsvm_vcpu_initialise, @@ -2644,6 +2662,30 @@ void svm_vmexit_handler(struct cpu_user_regs *regs) svm_vmexit_do_pause(regs); break; + case VMEXIT_IDTR_READ: + case VMEXIT_IDTR_WRITE: + hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, + VM_EVENT_DESC_IDTR, exit_reason == VMEXIT_IDTR_WRITE); + break; + + case VMEXIT_GDTR_READ: + case VMEXIT_GDTR_WRITE: + hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, + VM_EVENT_DESC_GDTR, exit_reason == VMEXIT_GDTR_WRITE); + break; + + case VMEXIT_LDTR_READ: + case VMEXIT_LDTR_WRITE: + hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, + VM_EVENT_DESC_LDTR, exit_reason == VMEXIT_LDTR_WRITE); + break; + + case VMEXIT_TR_READ: + case VMEXIT_TR_WRITE: + hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, + VM_EVENT_DESC_TR, exit_reason == VMEXIT_TR_WRITE); + break; + default: unexpected_exit_type: gdprintk(XENLOG_ERR, "unexpected VMEXIT: exit reason = %#"PRIx64", " diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index d201956c91..89b29576f4 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -1343,6 +1343,20 @@ static void vmx_set_rdtsc_exiting(struct vcpu *v, bool_t enable) vmx_vmcs_exit(v); } +static void vmx_set_descriptor_access_exiting(struct vcpu *v, bool enable) +{ + if ( enable ) + v->arch.hvm_vmx.secondary_exec_control |= + SECONDARY_EXEC_DESCRIPTOR_TABLE_EXITING; + else + v->arch.hvm_vmx.secondary_exec_control &= + ~SECONDARY_EXEC_DESCRIPTOR_TABLE_EXITING; + + vmx_vmcs_enter(v); + vmx_update_secondary_exec_control(v); + vmx_vmcs_exit(v); +} + static void vmx_init_hypercall_page(struct domain *d, void *hypercall_page) { char *p; @@ -2230,6 +2244,7 @@ static struct hvm_function_table __initdata vmx_function_table = { .handle_cd = vmx_handle_cd, .set_info_guest = vmx_set_info_guest, .set_rdtsc_exiting = vmx_set_rdtsc_exiting, + .set_descriptor_access_exiting = vmx_set_descriptor_access_exiting, .nhvm_vcpu_initialise = nvmx_vcpu_initialise, .nhvm_vcpu_destroy = nvmx_vcpu_destroy, .nhvm_vcpu_reset = nvmx_vcpu_reset, @@ -3361,6 +3376,40 @@ static void vmx_handle_xrstors(void) domain_crash(current->domain); } +static void vmx_handle_idt_or_gdt(idt_or_gdt_instr_info_t instr_info, + uint64_t exit_qualification) +{ + uint8_t descriptor = instr_info.instr_identity + ? VM_EVENT_DESC_IDTR : VM_EVENT_DESC_GDTR; + + hvm_descriptor_access_intercept(instr_info.raw, exit_qualification, + descriptor, instr_info.instr_write); +} + +static void vmx_handle_ldt_or_tr(ldt_or_tr_instr_info_t instr_info, + uint64_t exit_qualification) +{ + uint8_t descriptor = instr_info.instr_identity + ? VM_EVENT_DESC_TR : VM_EVENT_DESC_LDTR; + + hvm_descriptor_access_intercept(instr_info.raw, exit_qualification, + descriptor, instr_info.instr_write); +} + +static void vmx_handle_descriptor_access(uint32_t exit_reason) +{ + uint64_t instr_info; + uint64_t exit_qualification; + + __vmread(EXIT_QUALIFICATION, &exit_qualification); + __vmread(VMX_INSTRUCTION_INFO, &instr_info); + + if ( exit_reason == EXIT_REASON_ACCESS_GDTR_OR_IDTR ) + vmx_handle_idt_or_gdt(instr_info, exit_qualification); + else + vmx_handle_ldt_or_tr(instr_info, exit_qualification); +} + static int vmx_handle_apic_write(void) { unsigned long exit_qualification; @@ -3972,6 +4021,9 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) case EXIT_REASON_ACCESS_GDTR_OR_IDTR: case EXIT_REASON_ACCESS_LDTR_OR_TR: + vmx_handle_descriptor_access(exit_reason); + break; + case EXIT_REASON_VMX_PREEMPTION_TIMER_EXPIRED: case EXIT_REASON_INVPCID: /* fall through */ diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c index 5f60743c22..eeb67f5b09 100644 --- a/xen/arch/x86/monitor.c +++ b/xen/arch/x86/monitor.c @@ -211,6 +211,24 @@ int arch_monitor_domctl_event(struct domain *d, break; } + case XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS: + { + bool old_status = ad->monitor.descriptor_access_enabled; + struct vcpu *v; + + if ( unlikely(old_status == requested_status) ) + return -EEXIST; + + domain_pause(d); + ad->monitor.descriptor_access_enabled = requested_status; + + for_each_vcpu ( d, v ) + hvm_funcs.set_descriptor_access_exiting(v, requested_status); + + domain_unpause(d); + break; + } + case XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT: { bool_t old_status = ad->monitor.software_breakpoint_enabled; diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c index a7ce6ca194..502ccc2d44 100644 --- a/xen/arch/x86/vm_event.c +++ b/xen/arch/x86/vm_event.c @@ -229,6 +229,12 @@ void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp) } break; + case VM_EVENT_REASON_DESCRIPTOR_ACCESS: + if ( rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA ) + v->arch.vm_event->emul.read = rsp->data.emul.read; + v->arch.vm_event->emulate_flags = rsp->flags; + break; + default: break; }; diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index ec14cce81f..841e376bd8 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -404,6 +404,7 @@ struct arch_domain unsigned int debug_exception_enabled : 1; unsigned int debug_exception_sync : 1; unsigned int cpuid_enabled : 1; + unsigned int descriptor_access_enabled : 1; struct monitor_msr_bitmap *msr_bitmap; } monitor; diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index c854183a2e..d908e4aaba 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -173,6 +173,7 @@ struct hvm_function_table { void (*handle_cd)(struct vcpu *v, unsigned long value); void (*set_info_guest)(struct vcpu *v); void (*set_rdtsc_exiting)(struct vcpu *v, bool_t); + void (*set_descriptor_access_exiting)(struct vcpu *v, bool); /* Nested HVM */ int (*nhvm_vcpu_initialise)(struct vcpu *v); diff --git a/xen/include/asm-x86/hvm/monitor.h b/xen/include/asm-x86/hvm/monitor.h index 85ca678d3f..d9efb3505e 100644 --- a/xen/include/asm-x86/hvm/monitor.h +++ b/xen/include/asm-x86/hvm/monitor.h @@ -38,6 +38,9 @@ bool_t hvm_monitor_cr(unsigned int index, unsigned long value, #define hvm_monitor_crX(cr, new, old) \ hvm_monitor_cr(VM_EVENT_X86_##cr, new, old) void hvm_monitor_msr(unsigned int msr, uint64_t value); +void hvm_monitor_descriptor_access(uint64_t exit_info, + uint64_t vmx_exit_qualification, + uint8_t descriptor, bool is_write); int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type, unsigned long trap_type, unsigned long insn_length); int hvm_monitor_cpuid(unsigned long insn_length, unsigned int leaf, diff --git a/xen/include/asm-x86/hvm/support.h b/xen/include/asm-x86/hvm/support.h index 632eb90f74..b99ff2a330 100644 --- a/xen/include/asm-x86/hvm/support.h +++ b/xen/include/asm-x86/hvm/support.h @@ -128,6 +128,9 @@ int hvm_set_efer(uint64_t value); int hvm_set_cr0(unsigned long value, bool_t may_defer); int hvm_set_cr3(unsigned long value, bool_t may_defer); int hvm_set_cr4(unsigned long value, bool_t may_defer); +int hvm_descriptor_access_intercept(uint64_t exit_info, + uint64_t vmx_exit_qualification, + uint8_t descriptor, bool is_write); int hvm_mov_to_cr(unsigned int cr, unsigned int gpr); int hvm_mov_from_cr(unsigned int cr, unsigned int gpr); void hvm_ud_intercept(struct cpu_user_regs *); diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h index 2b781ab120..b00ba52443 100644 --- a/xen/include/asm-x86/hvm/vmx/vmx.h +++ b/xen/include/asm-x86/hvm/vmx/vmx.h @@ -628,4 +628,46 @@ typedef struct { u16 eptp_index; } ve_info_t; +/* VM-Exit instruction info for LIDT, LGDT, SIDT, SGDT */ +typedef union idt_or_gdt_instr_info { + unsigned long raw; + struct { + unsigned long scaling :2, /* bits 0:1 - Scaling */ + :5, /* bits 6:2 - Undefined */ + addr_size :3, /* bits 9:7 - Address size */ + :1, /* bit 10 - Cleared to 0 */ + operand_size :1, /* bit 11 - Operand size */ + :3, /* bits 14:12 - Undefined */ + segment_reg :3, /* bits 17:15 - Segment register */ + index_reg :4, /* bits 21:18 - Index register */ + index_reg_invalid :1, /* bit 22 - Index register invalid */ + base_reg :4, /* bits 26:23 - Base register */ + base_reg_invalid :1, /* bit 27 - Base register invalid */ + instr_identity :1, /* bit 28 - 0:GDT, 1:IDT */ + instr_write :1, /* bit 29 - 0:store, 1:load */ + :2; /* bits 30:31 - Undefined */ + }; +} __transparent__ idt_or_gdt_instr_info_t; + +/* VM-Exit instruction info for LLDT, LTR, SLDT, STR */ +typedef union ldt_or_tr_instr_info { + unsigned long raw; + struct { + unsigned long scaling :2, /* bits 0:1 - Scaling */ + :1, /* bit 2 - Undefined */ + reg1 :4, /* bits 6:3 - Reg1 */ + addr_size :3, /* bits 9:7 - Address size */ + mem_reg :1, /* bit 10 - Mem/Reg */ + :4, /* bits 14:11 - Undefined */ + segment_reg :3, /* bits 17:15 - Segment register */ + index_reg :4, /* bits 21:18 - Index register */ + index_reg_invalid :1, /* bit 22 - Index register invalid */ + base_reg :4, /* bits 26:23 - Base register */ + base_reg_invalid :1, /* bit 27 - Base register invalid */ + instr_identity :1, /* bit 28 - 0:LDT, 1:TR */ + instr_write :1, /* bit 29 - 0:store, 1:load */ + :2; /* bits 31:30 - Undefined */ + }; +} __transparent__ ldt_or_tr_instr_info_t; + #endif /* __ASM_X86_HVM_VMX_VMX_H__ */ diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h index e4093732d3..c3d2699103 100644 --- a/xen/include/asm-x86/monitor.h +++ b/xen/include/asm-x86/monitor.h @@ -77,7 +77,8 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d) (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) | (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) | (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) | - (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT); + (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) | + (1U << XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS); /* Since we know this is on VMX, we can just call the hvm func */ if ( hvm_is_singlestep_supported() ) diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 9e3ce21f71..e3ed4002d9 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -1081,6 +1081,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t); #define XEN_DOMCTL_MONITOR_EVENT_CPUID 6 #define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 7 #define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT 8 +#define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS 9 struct xen_domctl_monitor_op { uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */ diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h index b7487a12f3..f01e47159d 100644 --- a/xen/include/public/vm_event.h +++ b/xen/include/public/vm_event.h @@ -146,6 +146,8 @@ #define VM_EVENT_REASON_PRIVILEGED_CALL 11 /* An interrupt has been delivered. */ #define VM_EVENT_REASON_INTERRUPT 12 +/* A descriptor table register was accessed. */ +#define VM_EVENT_REASON_DESCRIPTOR_ACCESS 13 /* Supported values for the vm_event_write_ctrlreg index. */ #define VM_EVENT_X86_CR0 0 @@ -259,6 +261,28 @@ struct vm_event_mov_to_msr { uint64_t value; }; +#define VM_EVENT_DESC_IDTR 1 +#define VM_EVENT_DESC_GDTR 2 +#define VM_EVENT_DESC_LDTR 3 +#define VM_EVENT_DESC_TR 4 + +struct vm_event_desc_access { + union { + struct { + uint32_t instr_info; /* VMX: VMCS Instruction-Information */ + uint32_t _pad1; + uint64_t exit_qualification; /* VMX: VMCS Exit Qualification */ + } vmx; + struct { + uint64_t exitinfo; /* SVM: VMCB EXITINFO */ + uint64_t _pad2; + } svm; + } arch; + uint8_t descriptor; /* VM_EVENT_DESC_* */ + uint8_t is_write; + uint8_t _pad[6]; +}; + struct vm_event_cpuid { uint32_t insn_length; uint32_t leaf; @@ -313,6 +337,7 @@ typedef struct vm_event_st { struct vm_event_mem_access mem_access; struct vm_event_write_ctrlreg write_ctrlreg; struct vm_event_mov_to_msr mov_to_msr; + struct vm_event_desc_access desc_access; struct vm_event_singlestep singlestep; struct vm_event_debug software_breakpoint; struct vm_event_debug debug_exception;
Adds monitor support for descriptor access events (reads & writes of IDTR/GDTR/LDTR/TR) for the x86 architecture (VMX and SVM). Signed-off-by: Adrian Pop <apop@bitdefender.com> --- changes in v2: - use two sub-structs (vmx/svm) for vm_event_desc_access (Tamas K Lengyel) - more compact version of the descriptor VM-Exit handling on svm (Boris Ostrovsky) - use bool instead of bool_t (Jan Beulich) - use curr, currd for the struct vcpu and struct domain local variables (Jan Beulich) - move hvm_emulate_ctxt into a narrower scope (Jan Beulich) - remove leftover dead code (Jan Beulich) - order the monitor events numerically (Jan Beulich) - remove VM_EVENT_DESC_INVALID (Jan Beulich) - crash the domain if the descriptor access instruction can't be emulated (Jan Beulich) - call hvm_inject_event without checking for pending events (Andrew Cooper) - introduce structures for the VM-Exit instruction information used for the descriptor instructions and use fewer magic numbers (Andrew Cooper, Jan Beulich) --- tools/libxc/include/xenctrl.h | 2 ++ tools/libxc/xc_monitor.c | 14 ++++++++++ tools/tests/xen-access/xen-access.c | 29 ++++++++++++++++++++- xen/arch/x86/hvm/hvm.c | 37 ++++++++++++++++++++++++++ xen/arch/x86/hvm/monitor.c | 22 ++++++++++++++++ xen/arch/x86/hvm/svm/svm.c | 42 ++++++++++++++++++++++++++++++ xen/arch/x86/hvm/vmx/vmx.c | 52 +++++++++++++++++++++++++++++++++++++ xen/arch/x86/monitor.c | 18 +++++++++++++ xen/arch/x86/vm_event.c | 6 +++++ xen/include/asm-x86/domain.h | 1 + xen/include/asm-x86/hvm/hvm.h | 1 + xen/include/asm-x86/hvm/monitor.h | 3 +++ xen/include/asm-x86/hvm/support.h | 3 +++ xen/include/asm-x86/hvm/vmx/vmx.h | 42 ++++++++++++++++++++++++++++++ xen/include/asm-x86/monitor.h | 3 ++- xen/include/public/domctl.h | 1 + xen/include/public/vm_event.h | 25 ++++++++++++++++++ 17 files changed, 299 insertions(+), 2 deletions(-)