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

Re: [Xen-devel] [RFC PATCH 08/10] connect vmport up



On 12/12/13 19:51, Andrew Cooper wrote:
On 12/12/2013 19:15, Don Slutz wrote:
From: Don Slutz <dslutz@xxxxxxxxxxx>

Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
---
  xen/arch/x86/hvm/io.c       |   4 ++
  xen/arch/x86/hvm/svm/svm.c  | 104 ++++++++++++++++++++++++++++++++++++
  xen/arch/x86/hvm/svm/vmcb.c |   1 +
  xen/arch/x86/hvm/vmx/vmcs.c |   1 +
  xen/arch/x86/hvm/vmx/vmx.c  | 125 ++++++++++++++++++++++++++++++++++++++++++++
  xen/arch/x86/hvm/vmx/vvmx.c |  13 +++++
  xen/include/public/trace.h  |   1 +
  7 files changed, 249 insertions(+)

diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index bf6309d..4bc4716 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -42,6 +42,7 @@
  #include <asm/hvm/vlapic.h>
  #include <asm/hvm/trace.h>
  #include <asm/hvm/emulate.h>
+#include <asm/hvm/vmport.h>
  #include <public/sched.h>
  #include <xen/iocap.h>
  #include <public/hvm/ioreq.h>
@@ -236,6 +237,9 @@ int handle_pio(uint16_t port, unsigned int size, int dir)
      if ( dir == IOREQ_WRITE )
          data = guest_cpu_user_regs()->eax;
+ if ( port == VMPORT_PORT )
+        return vmport_ioport(dir, size, data, guest_cpu_user_regs());
+
Use register_portio_handler(), which is the already-existing
infrastructure for intercepting ports.

Will try it out.


      rc = hvmemul_do_pio(port, &reps, size, 0, dir, 0, &data);
switch ( rc )
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 406d394..80cf2bf 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -56,6 +56,7 @@
  #include <asm/hvm/svm/nestedsvm.h>
  #include <asm/hvm/nestedhvm.h>
  #include <asm/x86_emulate.h>
+#include <asm/hvm/vmport.h>
  #include <public/sched.h>
  #include <asm/hvm/vpt.h>
  #include <asm/hvm/trace.h>
@@ -1904,6 +1905,105 @@ svm_vmexit_do_vmsave(struct vmcb_struct *vmcb,
      return;
  }
+static void svm_vmexit_gp_intercept(struct cpu_user_regs *regs, struct vcpu *v)
+{
+    struct hvm_domain *hd = &v->domain->arch.hvm_domain;
+    struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
+    unsigned long inst_len, bytes_len;
+    int frc;
+    unsigned char bytes[15];
+
+    regs->error_code = vmcb->exitinfo1;
+    if ( !cpu_has_svm_nrips || (vmcb->nextrip <= vmcb->rip) )
+        inst_len = 0;
+    else
+        inst_len = vmcb->nextrip - vmcb->rip;
+    bytes_len = 2 /* inst_len < 15 ? inst_len > 1 ? inst_len : 2 : 15 */;
+    frc = hvm_fetch_from_guest_virt_nofault(bytes, regs->eip,
+                                            bytes_len,
+                                            PFEC_page_present);
+
+    if ( hvm_long_mode_enabled(v) )
+        HVMTRACE_LONG_4D(TRAP, TRAP_gp_fault, inst_len,
+                         regs->error_code,
+                         TRC_PAR_LONG(vmcb->exitinfo2) );
+    else
+        HVMTRACE_4D(TRAP, TRAP_gp_fault, inst_len,
+                    regs->error_code, vmcb->exitinfo2 );
+
+    if (hd->params[HVM_PARAM_VMPORT_LOGMASK] & 0x400000 /* LOG_GP_FAIL_RD_INST 
*/)
+        printk("[HVM:%d.%d] <%s> "
+               "gp: e2=%lx ec=%lx ip=%lx=>0x%x 0x%x(%ld,%ld,%d) 
nip(%d)=%lx(%d,%d(0x%x) 0x%x 0x%x)"
+               "\n",
+               current->domain->domain_id, current->vcpu_id, __func__,
+               (unsigned long)vmcb->exitinfo2,
+               (unsigned long)regs->error_code,
+               (unsigned long)regs->eip, (unsigned int)bytes[0],
+               (unsigned int)bytes[1], bytes_len, inst_len, frc,
+               cpu_has_svm_nrips, (unsigned long)vmcb->nextrip,
+               cpu_has_svm_decode, vmcb->guest_ins_len & 0xf, 
vmcb->guest_ins_len,
+               vmcb->guest_ins[0], vmcb->guest_ins[1]);
+
+    if ( !frc && bytes[0] == 0xed && (regs->edx & 0xffff) == VMPORT_PORT &&
+         vmcb->exitinfo2 == 0 && regs->error_code == 0 )
+    {
+        /*  in (%dx),%eax */
+        uint32_t magic = regs->eax;
+
+        if ( magic == VMPORT_MAGIC ) {
+            __update_guest_eip(regs, 1);
+            vmport_ioport(IOREQ_READ, 4, 0, regs);
This appears to be intercepting an L2 guest doing vmport magic IO to the
L1 hypervisor.

Is this sane/sensible/wise?

If I am reading this right; my answer is yes. This is how VMware defined it's backdoor port. What might be worse is that many "commands" work in ring 3.

L1 hypervisor is Xen.
L2 guest is both dom0 and domU.

I.E. you are not talking about nested (xen on xen) case.

+            if (hd->params[HVM_PARAM_VMPORT_LOGMASK] & 0x800000 /* 
LOG_GP_VMWARE_AFTER */)
+                printk("[HVM:%d.%d] <%s> "
+                       "gp: VMware ip=%lx ax=%lx bx=%lx cx=%lx dx=%lx si=%lx 
di=%lx"
+                       "\n",
+                       current->domain->domain_id, current->vcpu_id, __func__,
+                       (unsigned long)regs->eip,
+                       (unsigned long)regs->eax, (unsigned long)regs->ebx,
+                       (unsigned long)regs->ecx, (unsigned long)regs->edx,
+                       (unsigned long)regs->esi, (unsigned long)regs->edi);
+            return;
+        } else {
+            if (hd->params[HVM_PARAM_VMPORT_LOGMASK] & 0x200000 /* 
LOG_GP_NOT_VMWARE */)
+                printk("[HVM:%d.%d] <%s> "
+                       "gp: ip=%lx ax=%lx bx=%lx cx=%lx dx=%lx si=%lx di=%lx"
+                       "\n",
+                       current->domain->domain_id, current->vcpu_id, __func__,
+                       (unsigned long)regs->eip,
+                       (unsigned long)regs->eax, (unsigned long)regs->ebx,
+                       (unsigned long)regs->ecx, (unsigned long)regs->edx,
+                       (unsigned long)regs->esi, (unsigned long)regs->edi);
+            hvm_inject_hw_exception(TRAP_gp_fault, regs->error_code);
+        }
+    } else if (!frc && regs->error_code == 0
+               && bytes[0] == 0x0f && bytes[1] == 0x33 && regs->ecx == 0x10000)
+    {
+        /* "rdpmc 0x10000" */
+        /* Not a very good emulation!  But just not faulting is good enough
+         * to get NetApp booting. */
+        regs->edx = regs->eax = 0;
This doesn't look like it is logically part of "connecting vmport up"

You are right. This code should not be part of this patch set. Will drop it.


~Andrew


[snip]

diff --git a/xen/include/public/trace.h b/xen/include/public/trace.h
index e2f60a6..32489f0 100644
--- a/xen/include/public/trace.h
+++ b/xen/include/public/trace.h
@@ -223,6 +223,7 @@
  #define TRC_HVM_NPF             (TRC_HVM_HANDLER + 0x21)
  #define TRC_HVM_REALMODE_EMULATE (TRC_HVM_HANDLER + 0x22)
  #define TRC_HVM_TRAP             (TRC_HVM_HANDLER + 0x23)
+#define TRC_HVM_TRAP64           (TRC_HVM_HANDLER + TRC_64_FLAG + 0x23)
Haven't you already defined this in a previous patch?

Nope, just a related one.  Will factor both out into their own patch.

   -Don Slutz

  #define TRC_HVM_TRAP_DEBUG       (TRC_HVM_HANDLER + 0x24)
  #define TRC_HVM_VLAPIC           (TRC_HVM_HANDLER + 0x25)


_______________________________________________
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®.