On 11/02/2016 22:25, Lengyel, Tamas wrote: > > > On Thu, Feb 11, 2016 at 2:58 PM, Andrew Cooper > <andrew.coop...@citrix.com <mailto:andrew.coop...@citrix.com>> wrote: > > On 11/02/2016 21:49, Razvan Cojocaru wrote: > > On 02/11/2016 11:35 PM, Andrew Cooper wrote: > >> On 11/02/2016 21:05, Tamas K Lengyel wrote: > >> > >>> diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c > >>> index 08d678a..fa5d154 100644 > >>> --- a/xen/arch/x86/vm_event.c > >>> +++ b/xen/arch/x86/vm_event.c > >>> @@ -122,6 +122,64 @@ void vm_event_set_registers(struct vcpu > *v, vm_event_response_t *rsp) > >>> v->arch.user_regs.eip = rsp->data.regs.x86.rip; > >>> } > >>> > >>> +void vm_event_fill_regs(vm_event_request_t *req) > >>> +{ > >>> + const struct cpu_user_regs *regs = guest_cpu_user_regs(); > >>> + struct segment_register seg; > >>> + struct hvm_hw_cpu ctxt; > >>> + struct vcpu *curr = current; > >>> + > >>> + req->data.regs.x86.rax = regs->eax; > >>> + req->data.regs.x86.rcx = regs->ecx; > >>> + req->data.regs.x86.rdx = regs->edx; > >>> + req->data.regs.x86.rbx = regs->ebx; > >>> + req->data.regs.x86.rsp = regs->esp; > >>> + req->data.regs.x86.rbp = regs->ebp; > >>> + req->data.regs.x86.rsi = regs->esi; > >>> + req->data.regs.x86.rdi = regs->edi; > >>> + > >>> + req->data.regs.x86.r8 = regs->r8; > >>> + req->data.regs.x86.r9 = regs->r9; > >>> + req->data.regs.x86.r10 = regs->r10; > >>> + req->data.regs.x86.r11 = regs->r11; > >>> + req->data.regs.x86.r12 = regs->r12; > >>> + req->data.regs.x86.r13 = regs->r13; > >>> + req->data.regs.x86.r14 = regs->r14; > >>> + req->data.regs.x86.r15 = regs->r15; > >>> + > >>> + req->data.regs.x86.rflags = regs->eflags; > >>> + req->data.regs.x86.rip = regs->eip; > >>> + req->data.regs.x86.dr7 = curr->arch.debugreg[7]; > >> I think there is a %dr7 handling issue here. For an HVM > guests, this > >> field is only valid when you are not in the context of the > guest, as it > >> lives in the vmcs/vmcs. (PV guests keep it synchronously up to > date) > > Would this make it OK to use in p2m_vm_event_fill_regs() but not in > > hvm_event_fill_regs(), as it currently is? Maybe this is the > issue I'm > > remembering. > > Its use in p2m_mem_access_check() looks similarly buggy. That is also > in the context of 'current'. > > I would have thought that the use of hardware debugging facilities > would > be rare in the general case, which probably means that by chance, the > value is right most of the time (as it gets synchronised when a > vcpu is > scheduled on a new pcpu). > > > This is an issue that should be addressed in a separate patch.
Agreed. > It does look like dr7 will need a separate hvm function we can call to > do a __vmread for us on GUEST_DR7. It would be better to modify the existing function to do the right thing, rather than to introduce a brand new one. In some copious free time, I already want to cull some of the redundant hvm_funcs. ~Andrew
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
