[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
- To: Don Slutz <dslutz@xxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxx
- From: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
- Date: Fri, 12 Sep 2014 09:08:41 -0400
- Cc: Kevin Tian <kevin.tian@xxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, Ian Campbell <ian.campbell@xxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, Eddie Dong <eddie.dong@xxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Aravind Gopalakrishnan <Aravind.Gopalakrishnan@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
- Delivery-date: Fri, 12 Sep 2014 13:08:30 +0000
- List-id: Xen developer discussion <xen-devel.lists.xen.org>
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.
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.
+
+ 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);
+ }
+}
+
...
+
+int vmport_gp_check(struct cpu_user_regs *regs, struct vcpu *v,
+ unsigned long inst_len, unsigned long inst_addr,
+ unsigned long ei1, unsigned long ei2)
+{
+ if ( !v->domain->arch.hvm_domain.is_vmware_port_enabled )
+ return 10;
+
+ if ( inst_len && inst_len <= 2 && get_low_bits(regs->rdx) == BDOOR_PORT &&
+ ei1 == 0 && ei2 == 0 && regs->error_code == 0 &&
+ (uint32_t)regs->rax == BDOOR_MAGIC )
+ {
+ int i = 0;
+ uint32_t val;
+ uint32_t byte_cnt = 4;
+ unsigned char bytes[2];
+ unsigned int fetch_len;
+ int frc;
+ int rc;
+
+ /*
+ * Fetch up to the next page break; we'll fetch from the
+ * next page later if we have to.
+ */
+ fetch_len = min_t(unsigned int, inst_len,
+ PAGE_SIZE - (inst_addr & ~PAGE_MASK));
+ frc = hvm_fetch_from_guest_virt_nofault(bytes, inst_addr, fetch_len,
+ PFEC_page_present);
+ if ( frc != HVMCOPY_okay )
+ {
+ gdprintk(XENLOG_WARNING,
+ "Bad instruction fetch at %#lx (frc=%d il=%lu fl=%u)\n",
+ (unsigned long) inst_addr, frc, inst_len, fetch_len);
+ return 11;
+ }
+ if ( bytes[0] == 0x66 ) /* operand size prefix */
+ {
+ byte_cnt = 2;
+ i = 1;
+ if ( fetch_len != inst_len )
+ {
+ frc = hvm_fetch_from_guest_virt_nofault(&bytes[1],
+ inst_addr + 1, 1,
+ PFEC_page_present);
+ if ( frc != HVMCOPY_okay )
+ {
+ gdprintk(XENLOG_WARNING,
+ "Bad instruction fetch at %#lx + 1 (frc=%d)\n",
+ (unsigned long) inst_addr, frc);
+ return 12;
+ }
+ }
+ }
+ if ( bytes[i] == 0xed ) /* in (%dx),%eax or in (%dx),%ax */
+ {
+ rc = vmport_ioport(IOREQ_READ, BDOOR_PORT, byte_cnt, &val);
+ VMPORT_DBG_LOG(VMPORT_LOG_GP_VMWARE_AFTER,
+ "gp: VMwareIn rc=%d ip=%"PRIx64" byte_cnt=%d ax=%"
+ PRIx64" bx=%"PRIx64" cx=%"PRIx64" dx=%"PRIx64
+ " si=%"PRIx64" di=%"PRIx64, rc,
+ inst_addr, byte_cnt, regs->rax, regs->rbx,
+ regs->rcx, regs->rdx, regs->rsi, regs->rdi);
+ return rc;
+ }
+ else if ( bytes[i] == 0xec ) /* in (%dx),%al */
+ {
+ rc = vmport_ioport(IOREQ_READ, BDOOR_PORT, 1, &val);
+ VMPORT_DBG_LOG(VMPORT_LOG_GP_VMWARE_AFTER,
+ "gp: VMwareIn rc=%d ip=%"PRIx64" byte_cnt=1 ax=%"
+ PRIx64" bx=%"PRIx64" cx=%"PRIx64" dx=%"PRIx64
+ " si=%"PRIx64" di=%"PRIx64, rc,
+ inst_addr, regs->rax, regs->rbx, regs->rcx,
+ regs->rdx, regs->rsi, regs->rdi);
+ return rc;
+ }
+ else if ( bytes[i] == 0xef ) /* out %eax,(%dx) or out %ax,(%dx) */
+ {
+ rc = vmport_ioport(IOREQ_WRITE, BDOOR_PORT, byte_cnt, &val);
+ VMPORT_DBG_LOG(VMPORT_LOG_GP_VMWARE_AFTER,
+ "gp: VMwareOut rc=%d ip=%"PRIx64" byte_cnt=%d ax=%"
+ PRIx64" bx=%"PRIx64" cx=%"PRIx64" dx=%"PRIx64
+ " si=%"PRIx64" di=%"PRIx64, rc,
+ inst_addr, byte_cnt, regs->rax, regs->rbx,
+ regs->rcx, regs->rdx, regs->rsi, regs->rdi);
+ return rc;
+ }
+ else if ( bytes[i] == 0xee ) /* out %al,(%dx) */
+ {
+ rc = vmport_ioport(IOREQ_WRITE, BDOOR_PORT, 1, &val);
+ VMPORT_DBG_LOG(VMPORT_LOG_GP_VMWARE_AFTER,
+ "gp: VMwareOut rc=%d ip=%"PRIx64" byte_cnt=1 ax=%"
+ PRIx64" bx=%"PRIx64" cx=%"PRIx64" dx=%"PRIx64
+ " si=%"PRIx64" di=%"PRIx64, rc,
+ inst_addr, regs->rax, regs->rbx, regs->rcx,
+ regs->rdx, regs->rsi, regs->rdi);
+ return rc;
+ }
+ else
+ {
+ VMPORT_DBG_LOG(VMPORT_LOG_GP_FAIL_RD_INST,
+ "gp: VMware? lip=%"PRIx64"[%d]=>0x%x(%ld) ax=%"
+ PRIx64" bx=%"PRIx64" cx=%"PRIx64" dx=%"PRIx64
+ " si=%"PRIx64" di=%"PRIx64,
+ inst_addr, i, bytes[i], inst_len, regs->rax,
+ regs->rbx, regs->rcx, regs->rdx, regs->rsi,
+ regs->rdi);
+ return 13;
+ }
+ }
+ return 14;
The return values should be defined as macros --- otherwise they look
like some magic integers.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
