[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 04/16] xen: Add is_vmware_port_enabled




On 09/16/2014 08:08 AM, Slutz, Donald Christopher wrote:
On 09/12/14 09:08, Boris Ostrovsky wrote:
On 09/11/2014 02:36 PM, Don Slutz wrote:
   int __get_instruction_length_from_list(struct vcpu *v,
-        const enum instruction_index *list, unsigned int list_count)
+                                       const enum instruction_index
*list,
+                                       unsigned int list_count,
+                                       bool_t err_rpt)
   {
       struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
       unsigned int i, j, inst_len = 0;
@@ -211,10 +222,13 @@ int __get_instruction_length_from_list(struct
vcpu *v,
       mismatch: ;
       }
   -    gdprintk(XENLOG_WARNING,
-             "%s: Mismatch between expected and actual instruction
bytes: "
-             "eip = %lx\n",  __func__, (unsigned long)vmcb->rip);
-    hvm_inject_hw_exception(TRAP_gp_fault, 0);
+    if ( err_rpt )
+    {
+        gdprintk(XENLOG_WARNING,
+                 "%s: Mismatch between expected and actual
instruction bytes: "
+                 "eip = %lx\n",  __func__, (unsigned long)vmcb->rip);
+        hvm_inject_hw_exception(TRAP_gp_fault, 0);
+    }
       return 0;
      done:
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index b5188e6..9e14d2a 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -59,6 +59,7 @@
   #include <public/sched.h>
   #include <asm/hvm/vpt.h>
   #include <asm/hvm/trace.h>
+#include <asm/hvm/vmport.h>
   #include <asm/hap.h>
   #include <asm/apic.h>
   #include <asm/debugger.h>
@@ -2065,6 +2066,38 @@ svm_vmexit_do_vmsave(struct vmcb_struct *vmcb,
       return;
   }
   +static void svm_vmexit_gp_intercept(struct cpu_user_regs *regs,
+                                    struct vcpu *v)
+{
+    struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
+    unsigned long inst_len;
+    unsigned long inst_addr = svm_rip2pointer(v);
+    int rc;
+    static const enum instruction_index list[] = {
+        INSTR_INL_DX, INSTR_INB_DX, INSTR_OUTL_DX, INSTR_OUTB_DX
+    };
+
+    inst_len = __get_instruction_length_from_list(
+        v, list, ARRAY_SIZE(list), 0);
I should have asked earlier but I don't think I understand why the
last argument here is 0 (and therefore why you have this last argument
at all).

Because whether or not you are warning in
__get_instruction_length_from_list() it will still return 0. And that,
in turn, will cause vmport_gp_check() to return an error. And then you
will print another warning in VMPORT_LOG. So there is a warning anyway.

A key part that you appear to have missed is that
__get_instruction_length_from_list() uses gdprintk(XENLOG_WARNING,...
but VMPORT_DBG_LOG is only available in debug=y builds.  So the new
last argument is used to control this.  Since this change includes
enabling #GP vmexits, it is now possible for ring 3 users to generate at
large volume of these which with gdprintk() can flood the console.

Would it be possible to decide where and whether to print the warning inside __get_instruction_length_from_list() as opposed to passing a new parameter? E.g. if vmware_port_enabled is set and list includes IN/OUT and possibly something else?

-boris



Second, since this handler appears to be handling #GP only for VMware
guest we should make sure that it is not executed for any other guest.
You do now condition intercept got #GP for such guests only but I
still think having a check here is worth doing. Maybe a BUG() or
ASSERT()?

The same comments are applicable to VMX code, I suspect.

I will change the check in vmport_gp_check on is_vmware_port_enabled
into an ASSERT() so both SVM and VMX will be covered.

+
+    rc = vmport_gp_check(regs, v, inst_len, inst_addr,
+                         vmcb->exitinfo1, vmcb->exitinfo2);
+    if ( !rc )
+        __update_guest_eip(regs, inst_len);
+    else
+    {
+        VMPORT_DBG_LOG(VMPORT_LOG_GP_UNKNOWN,
+                       "gp: rc=%d ei1=0x%lx ei2=0x%lx ip=%"PRIx64
+                       " (0x%lx,%ld) ax=%"PRIx64" bx=%"PRIx64"
cx=%"PRIx64
+                       " dx=%"PRIx64" si=%"PRIx64" di=%"PRIx64, rc,
+                       (unsigned long)vmcb->exitinfo1,
+                       (unsigned long)vmcb->exitinfo2, regs->rip,
inst_addr,
+                       inst_len, regs->rax, regs->rbx, regs->rcx,
regs->rdx,
+                       regs->rsi, regs->rdi);
+        hvm_inject_hw_exception(TRAP_gp_fault, regs->error_code);
+    }
+}
+
.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.