On Thu, Feb 11, 2016 at 3:30 PM, Andrew Cooper <andrew.coop...@citrix.com>
wrote:
> On 11/02/2016 22:25, Lengyel, Tamas wrote:
>
>
>
> On Thu, Feb 11, 2016 at 2:58 PM, Andrew Cooper <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.
>
Sure, adding an extra vmread in there would be simple enough.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
