Re: [PATCH v2] KVM: Synthesize G bit for all segments.

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2014-07-08 06:17, Alok Kataria wrote:
> Thanks Jan and Paolo for looking at the change, I have added a comment
> in svm_get_segment. Joerg, please consider this for the next merge.
> 
> --
> 
> From: Jim Mattson <jmattson@vmware.com>
> 
> We have noticed that qemu-kvm hangs early in the BIOS when runnning nested
> under some versions of VMware ESXi.
> 
> The problem we believe is because KVM assumes that the platform preserves
> the 'G' but for any segment register. The SVM specification itemizes the
> segment attribute bits that are observed by the CPU, but the (G)ranularity bit
> is not one of the bits itemized, for any segment. Though current AMD CPUs keep
> track of the (G)ranularity bit for all segment registers other than CS, the
> specification does not require it. VMware's virtual CPU may not track the
> (G)ranularity bit for any segment register.
> 
> Since kvm already synthesizes the (G)ranularity bit for the CS segment. It
> should do so for all segments. The patch below does that, and helps get rid of
> the hangs. Patch applies on top of Linus' tree.
> 
> Signed-off-by: Jim Mattson <jmattson@vmware.com>
> Signed-off-by: Alok N Kataria <akataria@vmware.com>
> 
> Index: linux-2.6/arch/x86/kvm/svm.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kvm/svm.c	2014-07-07 15:32:52.724368183 +0530
> +++ linux-2.6/arch/x86/kvm/svm.c	2014-07-08 09:30:29.124431069 +0530
> @@ -1415,7 +1415,13 @@
>  	var->avl = (s->attrib >> SVM_SELECTOR_AVL_SHIFT) & 1;
>  	var->l = (s->attrib >> SVM_SELECTOR_L_SHIFT) & 1;
>  	var->db = (s->attrib >> SVM_SELECTOR_DB_SHIFT) & 1;
> -	var->g = (s->attrib >> SVM_SELECTOR_G_SHIFT) & 1;
> +
> +	/*
> +	 * SVM spec doesn't require the platform to track the G bit for all
> +	 * segments, so similar to CS, let's synthesize this bit for all
> +	 * segments.

Either I misunderstand the reference to CS or it does no longer apply
once the patch is in. I would suggest to remove that part of the sentence.

Jan

> +	 */
> +	var->g = s->limit > 0xfffff;
>  
>  	/*
>  	 * AMD's VMCB does not have an explicit unusable field, so emulate it
> @@ -1424,14 +1430,6 @@
>  	var->unusable = !var->present || (var->type == 0);
>  
>  	switch (seg) {
> -	case VCPU_SREG_CS:
> -		/*
> -		 * SVM always stores 0 for the 'G' bit in the CS selector in
> -		 * the VMCB on a VMEXIT. This hurts cross-vendor migration:
> -		 * Intel's VMENTRY has a check on the 'G' bit.
> -		 */
> -		var->g = s->limit > 0xfffff;
> -		break;
>  	case VCPU_SREG_TR:
>  		/*
>  		 * Work around a bug where the busy flag in the tr selector
> 
> 
> 

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux