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

[Avi Kivity <avi@qumranet.com>]

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