Re: [Xen-devel] [PATCH v9 02/11] x86/cpuid: Handling of IBRS/IBPB, STIBP and IBRS for guests

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.01.18 at 16:46, <andrew.cooper3@xxxxxxxxxx> wrote:
> For guest safety, we treat STIBP as special, always override the toolstack
> choice, and always advertise STIBP if IBRS is available.  This removes the
> corner case where STIBP is not advertised, but the guest is running on
> HT-capable hardware where it does matter.

I guess the answer to my question may live somewhere later in the
series, but since I haven't got there yet: Is this based on the
assumption that on HT-capable hardware they would always be
available together? Otherwise, how do you emulate STIBP for the
guest if all you've got is IBRS/IBPB?

> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -383,6 +383,16 @@ static void __init calculate_pv_max_policy(void)
>      /* Unconditionally claim to be able to set the hypervisor bit. */
>      __set_bit(X86_FEATURE_HYPERVISOR, pv_featureset);
>  
> +    /* On hardware with IBRS/IBPB support, there are further adjustments. */
> +    if ( test_bit(X86_FEATURE_IBRSB, pv_featureset) )
> +    {
> +        /* Offer STIBP unconditionally.  It is a nop on non-HT hardware. */
> +        __set_bit(X86_FEATURE_STIBP, pv_featureset);
> +
> +        /* AMD's IBPB is a subset of IBRS/IBPB. */
> +        __set_bit(X86_FEATURE_IBPB, pv_featureset);
> +    }
> +
>      sanitise_featureset(pv_featureset);
>      cpuid_featureset_to_policy(pv_featureset, p);
>      recalculate_xstate(p);
> @@ -440,6 +450,16 @@ static void __init calculate_hvm_max_policy(void)
>              __clear_bit(X86_FEATURE_XSAVES, hvm_featureset);
>      }
>  
> +    /* On hardware with IBRS/IBPB support, there are further adjustments. */
> +    if ( test_bit(X86_FEATURE_IBRSB, hvm_featureset) )
> +    {
> +        /* Offer STIBP unconditionally.  It is a nop on non-HT hardware. */
> +        __set_bit(X86_FEATURE_STIBP, hvm_featureset);
> +
> +        /* AMD's IBPB is a subset of IBRS/IBPB. */
> +        __set_bit(X86_FEATURE_IBPB, hvm_featureset);
> +    }

As long as we don't expect this logic to grow, having it duplicated
like this is probably fine. Otherwise a helper function might be
better.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel