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
