From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH RFC V2 4/6] xen: Support for VMCALL mem_events Date: Tue, 17 Mar 2015 15:50:52 +0200 Message-ID: <5508313C.3060004@bitdefender.com> References: <1405093418-23481-1-git-send-email-rcojocaru@bitdefender.com> <1405093418-23481-4-git-send-email-rcojocaru@bitdefender.com> <53C01D85.3010205@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53C01D85.3010205@citrix.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: Andrew Cooper , xen-devel@lists.xen.org Cc: mdontu@bitdefender.com, tim@xen.org, JBeulich@suse.com List-Id: xen-devel@lists.xenproject.org On 07/11/2014 08:23 PM, Andrew Cooper wrote: > On 11/07/14 16:43, Razvan Cojocaru wrote: >> Added support for VMCALL events (the memory introspection library >> will have the guest trigger VMCALLs, which will then be sent along >> via the mem_event mechanism). >> >> Changes since V1: >> - Added a #define and an comment explaining a previous magic >> constant. >> - Had MEM_EVENT_REASON_VMCALL explicitly not honour >> HVMPME_onchangeonly. >> >> Signed-off-by: Razvan Cojocaru >> --- >> xen/arch/x86/hvm/hvm.c | 9 +++++++++ >> xen/arch/x86/hvm/vmx/vmx.c | 18 +++++++++++++++++- >> xen/include/asm-x86/hvm/hvm.h | 1 + >> xen/include/public/hvm/params.h | 4 +++- >> xen/include/public/mem_event.h | 5 +++++ >> 5 files changed, 35 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c >> index 89a0382..6e86d7c 100644 >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -5564,6 +5564,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) >> case HVM_PARAM_MEMORY_EVENT_INT3: >> case HVM_PARAM_MEMORY_EVENT_SINGLE_STEP: >> case HVM_PARAM_MEMORY_EVENT_MSR: >> + case HVM_PARAM_MEMORY_EVENT_VMCALL: >> if ( d == current->domain ) >> { >> rc = -EPERM; >> @@ -6199,6 +6200,14 @@ void hvm_memory_event_msr(unsigned long msr, unsigned long value) >> value, ~value, 1, msr); >> } >> >> +void hvm_memory_event_vmcall(unsigned long rip, unsigned long eax) >> +{ >> + hvm_memory_event_traps(current->domain->arch.hvm_domain >> + .params[HVM_PARAM_MEMORY_EVENT_VMCALL], >> + MEM_EVENT_REASON_VMCALL, >> + rip, ~rip, 1, eax); >> +} >> + >> int hvm_memory_event_int3(unsigned long gla) >> { >> uint32_t pfec = PFEC_page_present; >> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c >> index 2caa04a..6c63225 100644 >> --- a/xen/arch/x86/hvm/vmx/vmx.c >> +++ b/xen/arch/x86/hvm/vmx/vmx.c >> @@ -2879,8 +2879,24 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) >> case EXIT_REASON_VMCALL: >> { >> int rc; >> + unsigned long eax = regs->eax; >> + >> HVMTRACE_1D(VMMCALL, regs->eax); >> - rc = hvm_do_hypercall(regs); >> + >> + /* Don't send a VMCALL mem_event unless something >> + * caused the guests's eax register to contain the >> + * VMCALL_EVENT_REQUEST constant. */ >> + if ( regs->eax != VMCALL_EVENT_REQUEST ) >> + { >> + rc = hvm_do_hypercall(regs); >> + } >> + else >> + { >> + hvm_memory_event_vmcall(guest_cpu_user_regs()->eip, eax); >> + update_guest_eip(); >> + break; >> + } > > Thinking more about this, it is really a hypercall pretending not to > be. It would be better to introduce a real HVMOP_send_mem_event. > > From the point of view of your in-guest agent, it would be a vmcall with > rax = 34 (hvmop) rdi = $N (send_mem_event subop) rsi = data or pointer > to struct containing data, depending on how exactly you implement the > hypercall. > > You would have the bonus of being able to detect errors, e.g. -ENOENT > for "mem_event not active", get SVM support for free, and not need magic > numbers, or vendor specific terms like "vmcall" finding their way into > the Xen public API. Actually, this only seems to be the case where mode == 8 in hvm_do_hypercall() (xen/arch/x86/hvm/hvm.c): 4987 : hvm_hypercall64_table)[eax](rdi, rsi, rdx, r10, r8, r9); Otherwise (and this seems to be the case with my Xen build), ebx seems to be used for the subop: 5033 regs->_eax = hvm_hypercall32_table[eax](ebx, ecx, edx, esi, edi, ebp); So, ebx needs to be $N (send_mem_event subop), not rdi. Is this intended (rdi in one case and ebx in the other)? Thanks, Razvan