Re: [PATCH] x86/tsx: Cope with RTM_ALWAYS_ABORT vs RTM mismatch

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 04/04/2024 2:32 pm, Jan Beulich wrote:
> On 04.04.2024 15:22, Andrew Cooper wrote:
>> On 04/04/2024 1:45 pm, Jan Beulich wrote:
>>> For the write-discard property, how was that determined? Does it affect all
>>> writable bits?
>> Marek kindly ran a debugging patch for me last night to try and figure
>> out what was going on.
>>
>> Currently, Xen tries to set 0x2 (TSX_CPUID_CLEAR) and debugging showed
>> it being read back as 0.
>>
>> I didn't check anything else, but I have a strong suspicion that I know
>> exactly what's going wrong here.
> Hmm, at the risk of upsetting you: Is a suspicion really enough for a
> firm statement in a comment?

The statement is all demonstrable properties.

The suspicion is about *why* we've ended up with the properties we have,
and is based on my involvement in the original planning for this.

>> The property the if() condition is mainly looking for is !RTM &&
>> !(MSR_TFA.CPUID_CLEAR) because that's an illegal state in a
>>
>>>> +             * Spot this case, and treat it as if no TSX is available at all.
>>>> +             * This will prevent Xen from thinking it's safe to offer HLE/RTM
>>>> +             * to VMs.
>>>> +             */
>>>> +            if ( val == 0 && cpu_has_rtm_always_abort && !cpu_has_rtm )
>>>> +            {
>>>> +                printk(XENLOG_ERR
>>>> +                       "FIRMWARE BUG: CPU %02x-%02x-%02x, ucode 0x%08x: RTM_ALWAYS_ABORT vs RTM mismatch\n",
>>> This isn't really firmware, is it? At least I wouldn't call microcode
>>> (assuming that's where the bad behavior is rooted) firmware.
>> Microcode is absolutely part of the system firmware.
> The ucode ahead of being loaded into CPUs is, sure. But once in the CPU
> (and there may not be any loading at least in theory), it's not anymore.

You appear to have a very singular impression of what does and does not
constitute firmware.

If you can change Intel and AMD's mind on this matter, feel free to
submit a patch changing the wording here.

~Andrew