Leendert van Doorn
2005-May-27 02:33 UTC
[Xen-devel] [PATCH] More accurate address decoding for VMX string I/O
This patch includes two patches. The major patch is the improved address
decoding for ins/outs instructions. In the previous version, segment
overrides were ignored which caused a lot of problems for parts of the
ROMBIOS code where they are used heavily. Address decoding has been
made slightly easier now that GUEST_LINEAR_ADDRESS is documented. We
still need to work around a bug in the current stepping and make sure
that in protected mode the selector is not null.
The second patch removes the ASSERT(cr3). cr3==0 is a perfectly legal
value and Xen should not crash when its read.
Signed-Off-By: Leendert van Doorn <leendert@watson.ibm.com>
--- xeno-unstable.orig/xen/arch/x86/vmx.c 2005-05-25 07:48:57.000000000 -0400
+++ xeno-unstable.mine/xen/arch/x86/vmx.c 2005-05-26 22:36:15.000000000 -0400
@@ -318,6 +318,55 @@
shadow_invlpg(ed, va);
}
+static int check_for_null_selector(unsigned long eip)
+{
+ unsigned char inst[MAX_INST_LEN];
+ unsigned long sel;
+ int i, inst_len;
+ int inst_copy_from_guest(unsigned char *, unsigned long, int);
+
+ __vmread(INSTRUCTION_LEN, &inst_len);
+ memset(inst, 0, MAX_INST_LEN);
+ if (inst_copy_from_guest(inst, eip, inst_len) != inst_len) {
+ printf("check_for_null_selector: get guest instruction
failed\n");
+ domain_crash_synchronous();
+ }
+
+ for (i = 0; i < inst_len; i++) {
+ switch (inst[i]) {
+ case 0xf3: /* REPZ */
+ case 0xf2: /* REPNZ */
+ case 0xf0: /* LOCK */
+ case 0x66: /* data32 */
+ case 0x67: /* addr32 */
+ continue;
+ case 0x2e: /* CS */
+ __vmread(GUEST_CS_SELECTOR, &sel);
+ break;
+ case 0x36: /* SS */
+ __vmread(GUEST_SS_SELECTOR, &sel);
+ break;
+ case 0x26: /* ES */
+ __vmread(GUEST_ES_SELECTOR, &sel);
+ break;
+ case 0x64: /* FS */
+ __vmread(GUEST_FS_SELECTOR, &sel);
+ break;
+ case 0x65: /* GS */
+ __vmread(GUEST_GS_SELECTOR, &sel);
+ break;
+ case 0x3e: /* DS */
+ /* FALLTHROUGH */
+ default:
+ /* DS is the default */
+ __vmread(GUEST_DS_SELECTOR, &sel);
+ }
+ return sel == 0 ? 1 : 0;
+ }
+
+ return 0;
+}
+
static void vmx_io_instruction(struct cpu_user_regs *regs,
unsigned long exit_qualification, unsigned long inst_len)
{
@@ -354,40 +403,44 @@
domain_crash_synchronous();
}
p = &vio->vp_ioreq;
- p->dir = test_bit(3, &exit_qualification);
+ p->dir = test_bit(3, &exit_qualification); /* direction */
p->pdata_valid = 0;
p->count = 1;
p->size = (exit_qualification & 7) + 1;
- if (test_bit(4, &exit_qualification)) {
- p->df = (eflags & X86_EFLAGS_DF) ? 1 : 0;
- p->pdata_valid = 1;
+ if (test_bit(4, &exit_qualification)) { /* string instruction */
+ unsigned long laddr;
- if (vm86) {
- unsigned long seg;
- if (p->dir == IOREQ_WRITE) {
- __vmread(GUEST_DS_SELECTOR, &seg);
- p->u.pdata = (void *)
- ((seg << 4) + (regs->esi & 0xFFFF));
- } else {
- __vmread(GUEST_ES_SELECTOR, &seg);
- p->u.pdata = (void *)
- ((seg << 4) + (regs->edi & 0xFFFF));
- }
- } else {
- p->u.pdata = (void *) ((p->dir == IOREQ_WRITE) ?
- regs->esi : regs->edi);
+ __vmread(GUEST_LINEAR_ADDRESS, &laddr);
+ /*
+ * In protected mode, guest linear address is invalid if the
+ * selector is null.
+ */
+ if (!vm86 && check_for_null_selector(eip)) {
+ printf("String I/O with null selector
(cs:eip=0x%lx:0x%lx)\n",
+ cs, eip);
+ laddr = (p->dir == IOREQ_WRITE) ? regs->esi : regs->edi;
}
- p->u.pdata = (void *) gva_to_gpa(p->u.data);
+ p->pdata_valid = 1;
+ p->u.pdata = (void *) gva_to_gpa(laddr);
+ p->df = (eflags & X86_EFLAGS_DF) ? 1 : 0;
- if (test_bit(5, &exit_qualification))
+ if (test_bit(5, &exit_qualification)) /* "rep" prefix */
p->count = vm86 ? regs->ecx & 0xFFFF : regs->ecx;
+
+ /*
+ * Split up string I/O operations that cross page boundaries.
Don''t
+ * advance %eip so that "rep insb" will restart at the next
page.
+ */
if ((p->u.data & PAGE_MASK) !=
- ((p->u.data + p->count * p->size - 1) & PAGE_MASK)) {
- printk("stringio crosses page boundary!\n");
+ ((p->u.data + p->count * p->size - 1) & PAGE_MASK)) {
+ VMX_DBG_LOG(DBG_LEVEL_2,
+ "String I/O crosses page boundary (cs:eip=0x%lx:0x%lx)\n",
+ cs, eip);
if (p->u.data & (p->size - 1)) {
- printk("Not aligned I/O!\n");
+ printf("Unaligned string I/O operation (cs:eip=0x%lx:0x%lx)\n",
+ cs, eip);
domain_crash_synchronous();
}
p->count = (PAGE_SIZE - (p->u.data & ~PAGE_MASK)) /
p->size;
@@ -885,7 +938,6 @@
__vmx_bug(regs);
value = (unsigned long) d->arch.arch_vmx.cpu_cr3;
- ASSERT(value);
switch (gp) {
CASE_SET_REG(EAX, eax);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel