Re: [RFC/PATCH 1/3] x86/kvm: Rename VMX's segment access rights defines

[Paolo Bonzini <pbonzini@redhat.com>]

On 13/08/2015 22:18, Andy Lutomirski wrote:
> VMX encodes access rights differently from LAR, and the latter is
> most likely what x86 people think of when they think of "access
> rights".
> 
> Rename them to avoid confusion.

Good idea, I've gone ahead and applied it for 4.3.

> Cc: kvm@vger.kernel.org
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/include/asm/vmx.h | 46 +++++++++++++++++++++++-----------------------
>  arch/x86/kvm/vmx.c         | 14 +++++++-------
>  2 files changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index da772edd19ab..78e243ae1786 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -367,29 +367,29 @@ enum vmcs_field {
>  #define TYPE_PHYSICAL_APIC_EVENT        (10 << 12)
>  #define TYPE_PHYSICAL_APIC_INST         (15 << 12)
>  
> -/* segment AR */
> -#define SEGMENT_AR_L_MASK (1 << 13)
> -
> -#define AR_TYPE_ACCESSES_MASK 1
> -#define AR_TYPE_READABLE_MASK (1 << 1)
> -#define AR_TYPE_WRITEABLE_MASK (1 << 2)
> -#define AR_TYPE_CODE_MASK (1 << 3)
> -#define AR_TYPE_MASK 0x0f
> -#define AR_TYPE_BUSY_64_TSS 11
> -#define AR_TYPE_BUSY_32_TSS 11
> -#define AR_TYPE_BUSY_16_TSS 3
> -#define AR_TYPE_LDT 2
> -
> -#define AR_UNUSABLE_MASK (1 << 16)
> -#define AR_S_MASK (1 << 4)
> -#define AR_P_MASK (1 << 7)
> -#define AR_L_MASK (1 << 13)
> -#define AR_DB_MASK (1 << 14)
> -#define AR_G_MASK (1 << 15)
> -#define AR_DPL_SHIFT 5
> -#define AR_DPL(ar) (((ar) >> AR_DPL_SHIFT) & 3)
> -
> -#define AR_RESERVD_MASK 0xfffe0f00
> +/* segment AR in VMCS -- these are different from what LAR reports */
> +#define VMX_SEGMENT_AR_L_MASK (1 << 13)
> +
> +#define VMX_AR_TYPE_ACCESSES_MASK 1
> +#define VMX_AR_TYPE_READABLE_MASK (1 << 1)
> +#define VMX_AR_TYPE_WRITEABLE_MASK (1 << 2)
> +#define VMX_AR_TYPE_CODE_MASK (1 << 3)
> +#define VMX_AR_TYPE_MASK 0x0f
> +#define VMX_AR_TYPE_BUSY_64_TSS 11
> +#define VMX_AR_TYPE_BUSY_32_TSS 11
> +#define VMX_AR_TYPE_BUSY_16_TSS 3
> +#define VMX_AR_TYPE_LDT 2
> +
> +#define VMX_AR_UNUSABLE_MASK (1 << 16)
> +#define VMX_AR_S_MASK (1 << 4)
> +#define VMX_AR_P_MASK (1 << 7)
> +#define VMX_AR_L_MASK (1 << 13)
> +#define VMX_AR_DB_MASK (1 << 14)
> +#define VMX_AR_G_MASK (1 << 15)
> +#define VMX_AR_DPL_SHIFT 5
> +#define VMX_AR_DPL(ar) (((ar) >> VMX_AR_DPL_SHIFT) & 3)
> +
> +#define VMX_AR_RESERVD_MASK 0xfffe0f00
>  
>  #define TSS_PRIVATE_MEMSLOT			(KVM_USER_MEM_SLOTS + 0)
>  #define APIC_ACCESS_PAGE_PRIVATE_MEMSLOT	(KVM_USER_MEM_SLOTS + 1)
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index e856dd566f4c..d7ff79a5135b 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3423,12 +3423,12 @@ static void enter_lmode(struct kvm_vcpu *vcpu)
>  	vmx_segment_cache_clear(to_vmx(vcpu));
>  
>  	guest_tr_ar = vmcs_read32(GUEST_TR_AR_BYTES);
> -	if ((guest_tr_ar & AR_TYPE_MASK) != AR_TYPE_BUSY_64_TSS) {
> +	if ((guest_tr_ar & VMX_AR_TYPE_MASK) != VMX_AR_TYPE_BUSY_64_TSS) {
>  		pr_debug_ratelimited("%s: tss fixup for long mode. \n",
>  				     __func__);
>  		vmcs_write32(GUEST_TR_AR_BYTES,
> -			     (guest_tr_ar & ~AR_TYPE_MASK)
> -			     | AR_TYPE_BUSY_64_TSS);
> +			     (guest_tr_ar & ~VMX_AR_TYPE_MASK)
> +			     | VMX_AR_TYPE_BUSY_64_TSS);
>  	}
>  	vmx_set_efer(vcpu, vcpu->arch.efer | EFER_LMA);
>  }
> @@ -3719,7 +3719,7 @@ static int vmx_get_cpl(struct kvm_vcpu *vcpu)
>  		return 0;
>  	else {
>  		int ar = vmx_read_guest_seg_ar(vmx, VCPU_SREG_SS);
> -		return AR_DPL(ar);
> +		return VMX_AR_DPL(ar);
>  	}
>  }
>  
> @@ -3847,11 +3847,11 @@ static bool code_segment_valid(struct kvm_vcpu *vcpu)
>  
>  	if (cs.unusable)
>  		return false;
> -	if (~cs.type & (AR_TYPE_CODE_MASK|AR_TYPE_ACCESSES_MASK))
> +	if (~cs.type & (VMX_AR_TYPE_CODE_MASK|VMX_AR_TYPE_ACCESSES_MASK))
>  		return false;
>  	if (!cs.s)
>  		return false;
> -	if (cs.type & AR_TYPE_WRITEABLE_MASK) {
> +	if (cs.type & VMX_AR_TYPE_WRITEABLE_MASK) {
>  		if (cs.dpl > cs_rpl)
>  			return false;
>  	} else {
> @@ -3901,7 +3901,7 @@ static bool data_segment_valid(struct kvm_vcpu *vcpu, int seg)
>  		return false;
>  	if (!var.present)
>  		return false;
> -	if (~var.type & (AR_TYPE_CODE_MASK|AR_TYPE_WRITEABLE_MASK)) {
> +	if (~var.type & (VMX_AR_TYPE_CODE_MASK|VMX_AR_TYPE_WRITEABLE_MASK)) {
>  		if (var.dpl < rpl) /* DPL < RPL */
>  			return false;
>  	}
>