Re: Direct rdtsc call side-effect

[Thomas Gleixner]

On Thu, Jun 01 2023 at 19:07, Steven Noonan wrote:
> On Thursday, June 1st, 2023 at 11:20 AM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>> Here is an example where it falls flat on its nose.
>> 
>> One of the early Ryzen laptops had a broken BIOS which came up with
>> unsynchronized TSCs. I tried to fix that up, but couldn't get it to sync
>> on all CPUs because for some stupid reason the TSC write got
>> arbritrarily delayed (assumably by SMI/SMM).
>
> Hah, I remember that. That was actually my laptop. A Lenovo ThinkPad
> A485 with a Ryzen 2700U. I've seen the problem since then occasionally
> on newer Ryzen laptops (and even desktops). Without the awful
> "tsc=directsync" patch I wrote, which I've been carrying for years now
> in my own kernel builds, it just falls back to HPET. It's not
> pleasant, but at least it's a stable clock.

Well, yours seem at least to sync. The silly box I tried refused due to
SMM value add magic.

> Agreed, TSC_ADJUST is the ultimate solution for any of these kinds of
> issues. But last I heard from AMD, it's still several years out in
> silicon, and there's plenty of hardware to maintain compatibility
> with. Ugh.

Yes.

> A software solution would be preferable in the meantime, but I don't
> know what options are left at this point.

Not that many.

> The trap-and-emulate via SIGSEGV approach proposed earlier in the
> thread is unfortunately not likely to be practical, assuming I
> implemented it properly.

That's why I said we need to ask Microsoft to do the same so that the
applications get fixed. :)

> Most Windows games that use this instruction directly are doing so
> under the assumption that the TSC is faster to read than any of the
> native Windows API clock sources.

The recommended interface QueryPerformanceCounter() is actually not much
slower and safe. But sure performance first and correctness is overrated.

So back to the options:

   1) Kernel

      If at all then this needs to be disabled by default and enabled by
      a command line option along with a big fat warning that it might
      disable TSC for timekeeping and bug reports related to this are
      going to be ignored.

      Honestly I'm not too interested in this. It's yet another piece of
      art which needs to be maintained and kept alive for a long time.

      The fact that we need to check for synchronized TSCs in the first
      place is hillarious already. TSC_ADJUST makes the resynchronization
      attempt at least halfways sensible.

      Without it, it's just a pile of never going to be correct
      heuristics with a flood of "this fixes it for my machine (and
      breaks the rest)" patches.


   2) Binary patching

      Unfortunately RDTSC is only a two byte instruction, but there are
      enough advanced binary patching tools to deal with that.

      It might be a completely crazy idea, but I wouldn't dismiss it
      before trying.

Thanks,

        tglx