Re: [PATCH 4/4] x86/spec-ctrl: Remove opencoded MSR_ARCH_CAPS check

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 4/4] x86/spec-ctrl: Remove opencoded MSR_ARCH_CAPS check
Date: Fri, 19 May 2023 15:38:18 +0100	[thread overview]
Message-ID: <a8013bb5-b0bb-6e42-85de-2e12d7b6f83c@citrix.com> (raw)
In-Reply-To: <fb95d033-7a71-7cc1-bb8f-ee2a74d1c5cf@suse.com>

On 19/05/2023 7:00 am, Jan Beulich wrote:
> On 17.05.2023 18:35, Andrew Cooper wrote:
>> On 17/05/2023 3:47 pm, Jan Beulich wrote:
>>> On 16.05.2023 16:53, Andrew Cooper wrote:
>>>> @@ -401,6 +400,8 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
>>>>          cpuid_count(7, 2, &tmp, &tmp, &tmp, &_7d2);
>>>>      if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
>>>>          cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
>>>> +    if ( cpu_has_arch_caps )
>>>> +        rdmsrl(MSR_ARCH_CAPABILITIES, caps);
>>> Why do you read the MSR again? I would have expected this to come out
>>> of raw_cpu_policy now (and incrementally the CPUID pieces as well,
>>> later on).
>> Consistency with the surrounding logic.
> I view this as relevant only when the code invoking CPUID directly is
> intended to stay.

Quite the contrary.  It stays because this patch, and changing the
semantics of the print block are unrelated things and should not be
mixed together.

>> Also because the raw and host policies don't get sorted until much later
>> in boot.
> identify_cpu(), which invokes init_host_cpu_policies(), is called
> ahead of init_speculation_mitigations(), isn't it?

What is init_host_cpu_policies() ?

I have a plan for what it's going to be if prior MSR work hadn't ground
to a halt, but it's a bit too late for that now.

(To answer the question properly, no the policies aren't set up until
just before building dom0, and that's not something that is trivial to
change.)

~Andrew