Re: [Xen-devel] [PATCH] x86/msr: Fix handling of MSR_AMD_PATCHLEVEL/MSR_IA32_UCODE_REV

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.04.19 at 17:35, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 01/04/2019 15:55, Jan Beulich wrote:
>>
>>>  Secondly, when a guest executes CPUID, this doesn't
>>> typically result in Xen executing a CPUID instruction in practice.
>> Wait - this is true for DomU, but used to be false for (PV) Dom0,
>> until you've switched over from pv_cpuid() to guest_cpuid(). I
>> would assume it was an unintended side effect to, this way,
>> eliminate the actual CPUID access from the special ucode rev
>> reading sequence Dom0 may perform.
> 
> It certainly was an unintended side effect of the cpuid_policy work.  In
> practice, dom0 still doesn't have faulting activated (due to the domain
> builder not having switched to DOMCTL_set_cpu_policy yet), so can still
> execute real CPUID instructions.

Provided it doesn't use the forced-#UD prefix, which I don't think
the Dom0 kernel would make any effort to bypass in this special
case.

> This is not going to stay true for very much longer.

Right.

>> As to DomU, I severely doubt it's a good idea to expose the
>> ucode revision to a guest, at least as long as the guest can be
>> migrated. But yes, we can leave avoiding to do so to the guest
>> itself: We won't mislead it any worse by handing it the real
>> revision than we would by handing it some synthetized one (e.g.
>> constant 0).
> 
> All guests in practice read the current microcode version.

Not really, no. In our XenoLinux forward port I had specifically
suppressed it, for being an undue operation to do by a PV
domain.

>  As it is
> model specific, we can't #GP on read, and faking up 0 is the only other
> option.
> 
> Furthermore, the speculative sidechannels mean that guests in practice
> do need to consult the microcode version for cases where MSR_ARCH_CAPS
> isn't provided.  On Broadwell at least, it is a non-optional part of
> evaluating the safety of retpoline.

But any decisions derived from this information are of limited
applicability.

>>> This patch texturally (but not functionally) interacts with "xen/sched: 
>>> Remove
>>> d->is_pinned" but rebasing either is easy.  It would also be best applied 
>>> with
>>> Sergey's "x86/microcode: always collect_cpu_info() during boot".
>> Well, "best" is probably an understatement, as you'd supply
>> stale data if we never came through that code. So I'd call it a
>> strict prereq.
> 
> Hmm - I've missed part of the explanation.
> 
> a) This behaviour is what happens currently for HVM guests.
> 
> b) In practice, it is only the very original P6 with no microcode which
> may have a fully read/write MSR here without anything to update the
> version.  AFACIT, for all CPUs which Xen will boot on, all that matters
> is that a CPUID instruction has been executed previously, which is
> covered by the AP boot path.
> 
> i.e. I think the logic would be more consistent to follow if this patch
> was behind Sergey's, but there is no functional dependency.

Ah, yes, good point.

Jan



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