Re: [Xen-devel] [PATCH 2/2] x86/vmx: conditionally disable LBR support due to TSX format quirk

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.08.16 at 08:59, <msw@xxxxxxxx> wrote:
> Systems that support LBR formats that include TSX information but do
> not support TSX require special handling when saving and restoring MSR
> values. For example, see the Linux kernel quirks[1, 2] in the MSR
> context switching code. As a wrmsr with certain values under these
> conditions causes a #GP, VM entry will fail due to MSR loading (see
> last bullet of SDM 26.4 "Loading MSRS").

I had recently noticed this as an area needing work too, so I'm glad
you (hopefully) eliminate this item from my todo list. However, I'm
sad to see that you really just disable LBR use in the problem case.
I'd prefer to take such a change only as an immediate workaround,
with the understanding that a proper one will be made available not
too much later.

Jun, Kevin - workarounds for hardware quirks like this one are really
what I would think should come out of Intel, and preferably not
after the first problem report on Xen, but soon after the quirk has
got identified in general (which according to the Linux patches must
have been at least 2 months ago).

> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2576,8 +2576,22 @@ static const struct lbr_info *last_branch_msr_get(void)
>          /* Haswell */
>          case 60: case 63: case 69: case 70:
>          /* Broadwell */
> -        case 61: case 71: case 79: case 86:
> +        case 61: case 71: case 79: case 86: {
> +            u64 caps;
> +            bool_t tsx_support = boot_cpu_has(X86_FEATURE_HLE) ||
> +                                 boot_cpu_has(X86_FEATURE_RTM);
> +
> +            rdmsrl(MSR_IA32_PERF_CAPABILITIES, caps);

This is guarded by a X86_FEATURE_PDCM check in Linux - why
would we not need the same here? Also I think this RDMSR should
be performed once at boot, not every time we come here.

> +            /*
> +             * Unimplemented fixups are required if the processor
> +             * supports an LBR format that includes TSX information,
> +             * but not TSX. Disable LBR save/load on such platforms.
> +             */
> +            if ( !tsx_support && (caps & 4) )

As I understand it the low 6 bits of the MSR contain an enumeration
like value, so just checking bit 2 can't be right (and would wrongly
trigger on already defined format types 5 and 6 - even if those were
known to be impossible on the models currently covered, this would
be a latent trap for someone to fall into when adding further model
values).

Jan


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