Re: [patch 3/3] KVM: VMX: handle segment limit granularity special case in software

Avi Kivity
2008-07-17T13:20:40Z

Marcelo Tosatti wrote:
> On Thu, Jul 17, 2008 at 01:03:57PM +0300, Avi Kivity wrote:
>
>> Marcelo Tosatti wrote:
>>
>>> As the comment in the diff mentions, VMX does not accept any bit in
>>> the range 11:0 of ES,CS,FS,GS,SS segment registers limit field to be
>>> zero with the granulity bit set to one.
>>>
>>> So clear granularity and adjust the limit accordingly.
>>>
>>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>>
>>> Index: kvm/arch/x86/kvm/vmx.c
>>> ===================================================================
>>> --- kvm.orig/arch/x86/kvm/vmx.c
>>> +++ kvm/arch/x86/kvm/vmx.c
>>> @@ -1665,6 +1665,22 @@ static void vmx_set_segment(struct kvm_v
>>> return;
>>> }
>>> vmcs_writel(sf->base, var->base);
>>> +
>>> + /*
>>> + * section 22.3.1.2:
>>> + * - If any bit in the limit field in the range 11:0 is 0, G must be 0.
>>> + * - If any bit in the limit field in the range 31:20 is 1, G must be 1.
>>> + */
>>> + if (!vcpu->arch.rmode.active && !var->unusable &&
>>> + seg != VCPU_SREG_TR && seg != VCPU_SREG_LDTR) {
>>> +#define SEG_MASK ((1 << 12)-1)
>>> + if (var->g && (var->limit & SEG_MASK) != SEG_MASK) {
>>> + var->g = 0;
>>> + var->limit <<= 12;
>>> + var->limit |= SEG_MASK;
>>> + }
>>> + }
>>> +
>>>
>> Both kvm_segment::limit and vmx's GUEST_xS_LIMIT are normalized (always
>> in bytes), so I don't see why you are modifying var->limit (which is an
>> input parameter!)
>>
>
>
> The problem is the Windows new TSS's FS segment:
>
> unhandled vm exit: 0x80000021 vcpu_id 2
> rax 0000000000000000 rbx 0000000000000000 rcx 0000000000000000 rdx
> 0000000000000000
> rsi 0000000000000000 rdi 0000000000000000 rsp 00000000fd6b73c0 rbp
> 0000000000000000
> r8 0000000000000000 r9 0000000000000000 r10 0000000000000000 r11
> 0000000000000000
> r12 0000000000000000 r13 0000000000000000 r14 0000000000000000 r15
> 0000000000000000
> rip 000000008088ab72 rflags 00004002
> cs 0008 (00000000/000fffff p 1 dpl 0 db 1 s 1 type b l 0 g 1 avl 0)
> ds 0023 (00000000/000fffff p 1 dpl 3 db 1 s 1 type 3 l 0 g 1 avl 0)
> es 0023 (00000000/000fffff p 1 dpl 3 db 1 s 1 type 3 l 0 g 1 avl 0)
> ss 0010 (00000000/000fffff p 1 dpl 0 db 1 s 1 type 3 l 0 g 1 avl 0)
> fs 0030 (fffffffffd6b1000/00000001 p 1 dpl 0 db 1 s 1 type 3 l 0 g 1 avl
> ^^^^^^^ ^^^
>
> "section 22.3.1.2:
> - If any bit in the limit field in the range 11:0 is 0, G must be 0."
>
> So this patch fixes that particular issue by setting G to 0 (G=1 ignores
> the 12 least significant bits of the offset when comparing the address
> against the segment limit), then shifts left the limit by 12, and sets
> those 12 bits.
>
> I don't understand what you mean by "vmx's GUEST_sX_LIMIT are
> normalized".
>
>

I meant, they are always in bytes. In a descriptor, the limit is in
bytes or pages, depending on the g bit.

> Do you have a better suggestion on how to deal with this? Or is it
> supposed to by handled somewhere already?

I think the problem is in seg_desct_to_kvm_desct() (besides the extra
T's). It copies the limit from the descriptor directly to the
kvm_segment structure.

Most likely a simple

if (seg_desc->g)
kvm_desct->limit <<= 12;

will suffice.

--
error compiling committee.c: too many arguments to function