Re: [RFC PATCH] x86/amd: Add support for AMD TCE

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 03/04/2025 1:58 pm, Jan Beulich wrote:
> On 03.04.2025 14:44, Teddy Astie wrote:
>> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
>> ---
>> RFC:
>>  - is this change actually safe ?
> Well, before getting here with reading I was already about to ask this very
> question. It's really you who needs to prove it.
>
>>  - should we add a tce/no-tce option to opt-in/out this feature ?
> Unless we're entirely certain we got this right and didn't overlook any
> corner case, perhaps better to do so.

To bring across a quote of mine from Mattermost:

"I'm reasonably sure our TLB handling algorithm is safe for it,
following the PCID work we did for Meltdown"

But, proving this is hard.

Some history: INVLPG flushing the entire paging structure cache
(non-leaf mappings) was a last-minute "fix" to keep Windows working on
the Pentium(?), where it had started using INVLPG from the 486(?) but
with a logical error.

AMD's TCE feature is "that's a hefty hit to keep around, so here's an
option for the behaviour one would more reasonably expect from INVLPG".

Anyway.  I have a suspicion that Intel's INVPCID no longer followed the
INVLPG behaviour anyway, and that we were forced to account for that. 
However, I'm struggling to find confirmation one way or another in the SDM.

Another mitigating factor is that, because we use recursive pagetables,
we have to upgrade an INVLPG into a full flush anyway if we edited
non-leaf entries.


As to a cmdline option, there's cpuid=no-tce if we really really need
it, but I don't think we want a dedicated TCE option.


>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -372,6 +372,9 @@ void asmlinkage start_secondary(void *unused)
>>  
>>      microcode_update_one();
>>  
>> +    if ( boot_cpu_has(X86_FEATURE_TCE) )
>> +        write_efer(read_efer() | EFER_TCE);
> Same here. But I wonder if you couldn't set the bit in trampoline_efer.

Yes, do set it in trampoline_efer, and drop this hunk.

~Andrew