Re: [Xen-devel] [PATCH] x86/cpuid: Don't use volatile asm statements

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.05.19 at 17:26, <andrew.cooper3@xxxxxxxxxx> wrote:
> Common use of the CPUID instruction operates without side effects.  Let the
> compiler better optimise code by dropping the volatile qualifier.
> 
> The only place where order matters is for Intel microcode loading, where
> executing a CPUID instruction is used for its side effect of updating
> MSR_IA32_UCODE_REV.
> 
> The existing logic is buggy because GCC has been seen to reorder independent
> asm volatile statements.  Opencode the two cases, with a compiler barrier to
> enforce the correct ordering of operations.

I'm afraid I don't see how a "compiler barrier" helps here. Recall
that we only call it this way, in reality it's a memory clobber, and
that is all it means to the compiler. As a result, I don't think it'll
help order against the earlier WRMSR. If you want to enforce
order reliably, I think your only options are a single asm() doing
everything or the second asm() consuming an output (perhaps
a fake one) of the first one. I think that's also what in essence
the gcc doc says.

> While here, fix the comment, which isn't correct.  The SDM doesn't state that
> a read of leaf 1 is required - just that a CPUID instruction is required.
> Using leaf 0 results in better code generation, following the write of 0 to
> MSR_IA32_UCODE_REV.

Well, the question is - are we sure this is working as expected
nevertheless, on all models? The example in the SDM certainly
uses leaf 1.

> Suggested-by: Jan Beulich <JBeulich@xxxxxxxx>

Did I? Must have been quite some time back, as I don't really recall.

Jan



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