Re: [PATCH 5/6] x86/boot: Record MSR_ARCH_CAPS for the Raw and Host CPU policy

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 5/6] x86/boot: Record MSR_ARCH_CAPS for the Raw and Host CPU policy
Date: Tue, 16 May 2023 13:59:07 +0100	[thread overview]
Message-ID: <d3b7e5ce-3a6d-12b3-2ffe-c7b904274070@citrix.com> (raw)
In-Reply-To: <f5f56c2b-f382-b007-3949-2bf4bd37b392@suse.com>

On 16/05/2023 1:53 pm, Jan Beulich wrote:
> On 15.05.2023 16:42, Andrew Cooper wrote:
>> Extend x86_cpu_policy_fill_native() with a read of ARCH_CAPS based on the
>> CPUID information just read, which removes the need handling it specially in
>> calculate_raw_cpu_policy().
>>
>> Extend generic_identify() to read ARCH_CAPS into x86_capability[], which is
>> fed into the Host Policy.  This in turn means there's no need to special case
>> arch_caps in calculate_host_policy().
>>
>> No practical change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

>
> I have a question though, which I think would be nice if the description
> had covered:
>
>> --- a/xen/lib/x86/cpuid.c
>> +++ b/xen/lib/x86/cpuid.c
>> @@ -226,7 +226,12 @@ void x86_cpu_policy_fill_native(struct cpu_policy *p)
>>      p->hv_limit = 0;
>>      p->hv2_limit = 0;
>>  
>> -    /* TODO MSRs */
>> +#ifdef __XEN__
>> +    /* TODO MSR_PLATFORM_INFO */
>> +
>> +    if ( p->feat.arch_caps )
>> +        rdmsrl(MSR_ARCH_CAPABILITIES, p->arch_caps.raw);
>> +#endif
> What about non-Xen environments re-using this code? In particular the
> test harnesses would be nice if they didn't run with the two fields
> all blank at all times.

Right now, I don't have an answer.

In Linux in lockdown mode, there isn't even a way to access this info
userspace, because /proc/cpu/$/msr goes away.

It's only a unit test, and this doesn't break it.

~Andrew