Re: [Xen-devel] [PATCH 3/4 V2] X86: MPX IA32_BNDCFGS msr handle

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.11.13 at 09:25, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> +static bool_t bndcfgs_invalid(u64 msr_content)
> +{
> +    /* BNDCFGS MSR reserved bits (11 ~ 2) must be zero */
> +    if ( msr_content & 0xffc )
> +        return 1;
> +
> +    /* Canonical address reserved bits must be zero */
> +    if ( hvm_long_mode_enabled(current) )
> +        /* 48b linear address for x86_64 guest */
> +        return !!(msr_content & (~(u64)0 << 48) );

!is_canonical_address()

> +    else
> +        /* 32b linear address for x86_32 (include PAE) guest */
> +        return !!(msr_content & (~(u64)0 << 32) );

!!(msr_content >> 32)

> @@ -3010,6 +3025,12 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t 
> *msr_content)
>          hvm_get_guest_pat(v, msr_content);
>          break;
>  
> +    case MSR_IA32_BNDCFGS:
> +        if ( !cpu_has_mpx )

Wasn't it you who started to properly use hvm_cpuid() for cases like
this? We're not after host capabilities here, but after what the guest
is supposed to (not) use.

> @@ -955,6 +956,34 @@ static int construct_vmcs(struct vcpu *v)
>          vmx_disable_intercept_for_msr(v, MSR_IA32_SYSENTER_EIP, MSR_TYPE_R | 
> MSR_TYPE_W);
>          if ( paging_mode_hap(d) && (!iommu_enabled || iommu_snoop) )
>              vmx_disable_intercept_for_msr(v, MSR_IA32_CR_PAT, MSR_TYPE_R | 
> MSR_TYPE_W);
> +
> +        if ( cpu_has_mpx )
> +        {
> +            /*
> +             * When MPX supported, a new guest-state field for IA32_BNDCFGS
> +             * is added to the VMCS. In addition, two new controls are added:
> +             * - a VM-exit control called "clear BNDCFGS"
> +             * - a VM-entry control called "load BNDCFGS."
> +             * VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS.
> +             */
> +            if ( likely((vmexit_ctl & VM_EXIT_CLEAR_BNDCFGS) &&
> +                        (vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS)) )
> +            {
> +                vmx_disable_intercept_for_msr(v, MSR_IA32_BNDCFGS,
> +                                                 MSR_TYPE_R | MSR_TYPE_W);
> +            }
> +            /* Unlikely, just in case buggy VMX ucode */
> +            else
> +            {
> +                int ret;
> +                ret = vmx_add_guest_msr(MSR_IA32_BNDCFGS);
> +                if ( ret )
> +                    return ret;
> +                ret = vmx_add_host_load_msr(MSR_IA32_BNDCFGS);
> +                if ( ret )
> +                    return ret;

You can't just return here - a vmx_vmcs_exit() is needed on any
exit path in the middle of this function.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel