Re: [Xen-devel] [PATCH v3 13/28] xen/x86: Improvements to in-hypervisor cpuid sanity checks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.03.16 at 16:35, <andrew.cooper3@xxxxxxxxxx> wrote:
>      case 0x80000001:
> -        /* We expose RDTSCP feature to guest only when
> -           tsc_mode == TSC_MODE_DEFAULT and host_tsc_is_safe() returns 1 */
> -        if ( d->arch.tsc_mode != TSC_MODE_DEFAULT ||
> -             !host_tsc_is_safe() )
> -            *edx &= ~cpufeat_mask(X86_FEATURE_RDTSCP);
> -        /* Hide 1GB-superpage feature if we can't emulate it. */
> -        if (!hvm_pse1gb_supported(d))
> -            *edx &= ~cpufeat_mask(X86_FEATURE_PAGE1GB);
> -        /* Only provide PSE36 when guest runs in 32bit PAE or in long mode */
> -        if ( !(hvm_pae_enabled(v) || hvm_long_mode_enabled(v)) )
> -            *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
> -        /* Hide data breakpoint extensions if the hardware has no support. */
> -        if ( !boot_cpu_has(X86_FEATURE_DBEXT) )
> -            *ecx &= ~cpufeat_mask(X86_FEATURE_DBEXT);
> +        *ecx &= hvm_featureset[FEATURESET_e1c];
> +        *edx &= hvm_featureset[FEATURESET_e1d];
> +
> +        /* If emulating Intel, clear the duplicated features in e1d. */
> +        if ( d->arch.x86_vendor == X86_VENDOR_INTEL )
> +            *edx &= ~CPUID_COMMON_1D_FEATURES;

I think this would better be != X86_VENDOR_AMD, to also cover
VIA.

> +        /* OSXSAVE cleared by pv_featureset.  Fast-forward CR4 back in. */
> +        if ( (is_pv_domain(currd) && guest_kernel_mode(curr, regs) &&
> +              (this_cpu(cr4) & X86_CR4_OSXSAVE)) ||
> +             (curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_OSXSAVE) )
> +            c |= cpufeat_mask(X86_FEATURE_OSXSAVE);
> +
> +        c |= cpufeat_mask(X86_FEATURE_HYPERVISOR);
>          break;

Is this correct for PVH (which calls here out of vmx.c)? At least
the ->arch.pv_vcpu use unlikely is.

> +            /* OSPKE cleared by pv_featureset.  Fast-forward CR4 back in. */
> +            if ( curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_PKE )
> +                c |= cpufeat_mask(X86_FEATURE_OSPKE);

That's kind of pointless for PV, and similarly to the above one likely
wrong for PVH.

Jan


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