RE: [Xen-devel] [PATCH] This patch fixes several issues related to vmxassist

["Li, Xin B" <xin.b.li@xxxxxxxxx>]

>>> I don't know the heritage of that code. I expect someone
>>> decided it was
>>> good enough to be getting on with but maybe now it is time 
>to revisit
>>> and see if we can implement a watertight version which 
>correctly uses
>>> hidden segment descriptor state which is readily available
>>> when running
>>> on VMX.
>>
>> My patch just enhanced the current implementation, and actually it
>> breaks windows, but I have a updated version in hand, and tests show
>> that all the combinations is OK till now.
>>
>> In my mind, the correct way is to identify whether a cpu is 
>in big real
>> mode, but seems this is a little bit hard to do.
>
>Well, it's impossible to know for certain without looking at hidden 
>descriptor state isn't it. Big real mode is only possible because the 
>hidden state can be out of sync with the current execution mode.
>
>If that state can be made available to the address() function 
>then it's 
>implementation, and confidence in its correctness, becomes trivial. Do 
>we have that state available directly in the guest at that point? Or 
>would we need to grab the state from Xen?
>

Keir, do you think this one seems better?
When in real mode, we should always calculate address as ((seg & 0xFFFF)
<< 4) + off, big real mode obey the same rule.
When in real mode to protected mode transition, only the IP address
calculation should use real mode addressing.
-Xin

@@ -50,23 +50,42 @@ static char *rnames[] = { "ax", "cx", "d
 static char *rnames[] = { "ax", "cx", "dx", "bx", "sp", "bp", "si",
"di" };
 #endif /* DEBUG */
 
-unsigned
+static unsigned
 address(struct regs *regs, unsigned seg, unsigned off)
 {
        unsigned long long entry;
-       unsigned addr;
+       unsigned seg_base, seg_limit;
+       unsigned entry_low, entry_high;
 
        if (seg == 0)
-               return off;
-
-       if (seg > oldctx.gdtr_limit)
+       {
+               if (mode == VM86_REAL || mode == VM86_REAL_TO_PROTECTED)
+                       return off;
+               else
+                       panic("segment is zero, but not in real
mode!\n");
+       }
+
+       if (mode == VM86_REAL || seg > oldctx.gdtr_limit ||
+           (mode == VM86_REAL_TO_PROTECTED && regs->cs == seg))
                return ((seg & 0xFFFF) << 4) + off;
 
        entry = ((unsigned long long *) oldctx.gdtr_base)[seg >> 3];
-       addr = (((entry >> (56-24)) & 0xFF000000) |
-               ((entry >> (32-16)) & 0x00FF0000) |
-               ((entry >> (   16)) & 0x0000FFFF)) + off;
-       return addr;
+       entry_high = entry >> 32;
+       entry_low = entry & 0xFFFFFFFF;
+
+       seg_base  = (entry_high & 0xFF000000) | ((entry >> 16) &
0xFFFFFF);
+       seg_limit = (entry_high & 0xF0000) | (entry_low & 0xFFFF);
+
+       if (entry_high & 0x8000 &&
+           ((entry_high & 0x800000 && off >> 12 <= seg_limit) ||
+           (!(entry_high & 0x800000) && off <= seg_limit)))
+               return seg_base + off;
+
+       panic("should never reach here in function address():\n\t"
+             "entry=0x%08x%08x, mode=%d, seg=0x%08x, offset=0x%08x\n",
+             entry_high, entry_low, mode, seg, off);
+
+       return 0;
 }
 
 #ifdef DEBUG

Attachment: fix_vmxassist..1.patch
Description: fix_vmxassist..1.patch

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel