From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH v2 2/2] vm_event: consolidate hvm_event_fill_regs and p2m_vm_event_fill_regs Date: Fri, 12 Feb 2016 12:19:06 +0200 Message-ID: <56BDB19A.6050603@bitdefender.com> References: <1455236525-14866-1-git-send-email-tlengyel@novetta.com> <1455236525-14866-2-git-send-email-tlengyel@novetta.com> <56BDBA7D02000078000D1558@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aUAog-0005JR-Tq for xen-devel@lists.xenproject.org; Fri, 12 Feb 2016 10:19:11 +0000 Received: from smtp01.buh.bitdefender.com (unknown [10.17.80.75]) by mx-sr.buh.bitdefender.com (Postfix) with ESMTP id 4985D7F8BE for ; Fri, 12 Feb 2016 12:19:06 +0200 (EET) In-Reply-To: <56BDBA7D02000078000D1558@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Tamas K Lengyel Cc: George Dunlap , Andrew Cooper , Keir Fraser , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 02/12/2016 11:57 AM, Jan Beulich wrote: >>>> On 12.02.16 at 01:22, wrote: >> --- a/xen/arch/x86/vm_event.c >> +++ b/xen/arch/x86/vm_event.c >> @@ -122,6 +122,65 @@ 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; >> + >> + if ( !is_hvm_domain(curr->domain) ) >> + return; > > No such check existed in either of the two original functions. Why is > it needed all of the sudden? And if it is needed, why do the other > fields not get filled (as far as possible at least) for PV guests? I can't speak for Tamas, but I suspect the check has been placed there because calls to hvm_funcs.save_cpu_ctxt(curr, &ctxt) and hvm_get_segment_register(curr, x86_seg_fs, &seg) follow, and he's put vm_event_fill_regs() in xen/arch/x86/vm_event.c (a previous function was called hvm_event_fill_regs(), in arch/x86/hvm/event.c, so no checking for HVM was needed). I don't think the check is needed for the current codepaths, but since the code has been moved to xen/arch/x86/ the question about future PV events is fair. Thanks, Razvan