[PATCH] qemu/timer: Don't use RDTSC on i486

[PATCH] qemu/timer: Don't use RDTSC on i486

[Petr Cvek]

GCC defines __i386__ for i386 and i486, which both lack RDTSC instruction.
The i386 seems to be impossible to distinguish, but i486 can be identified
by checking for undefined __i486__.

Signed-off-by: Petr Cvek <petrcvekcz@gmail.com>
---
 include/qemu/timer.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 9a366e551f..7baa5d1d41 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -872,7 +872,7 @@ static inline int64_t cpu_get_host_ticks(void)
     return retval;
 }
 
-#elif defined(__i386__)
+#elif defined(__i386__) && !defined(__i486__)
 
 static inline int64_t cpu_get_host_ticks(void)
 {
-- 
2.43.0

Re: [PATCH] qemu/timer: Don't use RDTSC on i486

[Samuel Tardieu]

Petr Cvek <petrcvekcz@gmail.com> writes:

> GCC defines __i386__ for i386 and i486, which both lack RDTSC 
> instruction.
> The i386 seems to be impossible to distinguish, but i486 can be 
> identified
> by checking for undefined __i486__.

Couldn't you check for an undefined __tune_i386__, which would be 
set by both GCC and LLVM when using -march=i386.

  Sam
-- 
Samuel Tardieu
Télécom Paris - Institut Polytechnique de Paris

Re: [PATCH] qemu/timer: Don't use RDTSC on i486

[Petr Cvek]

Actually I was thinking about mentioning it in the commit message also, but I wasn't able
to find any specification for that (if all compilers use it).

Other problem is the __tune_i386__ is also set when -mtune=i386 (but with -march=i686).

But if the general idea of changing the code for 486 is OK it can be added also.

Petr

Dne 26. 11. 23 v 13:37 Samuel Tardieu napsal(a):
> 
> Petr Cvek <petrcvekcz@gmail.com> writes:
> 
>> GCC defines __i386__ for i386 and i486, which both lack RDTSC instruction.
>> The i386 seems to be impossible to distinguish, but i486 can be identified
>> by checking for undefined __i486__.
> 
> Couldn't you check for an undefined __tune_i386__, which would be set by both GCC and LLVM when using -march=i386.
> 
>  Sam

Re: [PATCH] qemu/timer: Don't use RDTSC on i486

[Samuel Tardieu]

Petr Cvek <petrcvekcz@gmail.com> writes:

> Actually I was thinking about mentioning it in the commit 
> message also, but I wasn't able
> to find any specification for that (if all compilers use it).

Note that this change would be safe: at worst, some compilers 
don't use __tune_i386__ and the situation would be the same as 
today without the patch.

> Other problem is the __tune_i386__ is also set when -mtune=i386 
> (but with -march=i686).

Indeed, this is the case for GCC (not clang).

  Sam
-- 
Samuel Tardieu
Télécom Paris - Institut Polytechnique de Paris

Re: [PATCH] qemu/timer: Don't use RDTSC on i486

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 935 bytes --]

Il sab 25 nov 2023, 13:23 Petr Cvek <petrcvekcz@gmail.com> ha scritto:

> GCC defines __i386__ for i386 and i486, which both lack RDTSC instruction.
> The i386 seems to be impossible to distinguish, but i486 can be identified
> by checking for undefined __i486__.
>

As far as I know QEMU cannot be run on i486 anyway since TCG assumes the
presence of CPUID. Have you actually tried?

Paolo


> Signed-off-by: Petr Cvek <petrcvekcz@gmail.com>
> ---
>  include/qemu/timer.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index 9a366e551f..7baa5d1d41 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -872,7 +872,7 @@ static inline int64_t cpu_get_host_ticks(void)
>      return retval;
>  }
>
> -#elif defined(__i386__)
> +#elif defined(__i386__) && !defined(__i486__)
>
>  static inline int64_t cpu_get_host_ticks(void)
>  {
> --
> 2.43.0
>
>

[-- Attachment #2: Type: text/html, Size: 1674 bytes --]

Re: [PATCH] qemu/timer: Don't use RDTSC on i486

[Petr Cvek]

Dne 26. 11. 23 v 14:03 Samuel Tardieu napsal(a):
> 
> Petr Cvek <petrcvekcz@gmail.com> writes:
> 
>> Actually I was thinking about mentioning it in the commit message also, but I wasn't able
>> to find any specification for that (if all compilers use it).
> 
> Note that this change would be safe: at worst, some compilers don't use __tune_i386__ and the situation would be the same as today without the patch.

Good remark. You're right.

> 
>> Other problem is the __tune_i386__ is also set when -mtune=i386 (but with -march=i686).
> 
> Indeed, this is the case for GCC (not clang).
> 
>  Sam

Re: [PATCH] qemu/timer: Don't use RDTSC on i486

[Petr Cvek]

Dne 26. 11. 23 v 16:56 Paolo Bonzini napsal(a):
> 
> 
> Il sab 25 nov 2023, 13:23 Petr Cvek <petrcvekcz@gmail.com <mailto:petrcvekcz@gmail.com>> ha scritto:
> 
>     GCC defines __i386__ for i386 and i486, which both lack RDTSC instruction.
>     The i386 seems to be impossible to distinguish, but i486 can be identified
>     by checking for undefined __i486__.
> 
> 
> As far as I know QEMU cannot be run on i486 anyway since TCG assumes the presence of CPUID. Have you actually tried?
> 

Yes I tried running x86_64 mesa3d glxgears on amd 5x86. It worked with about 5 fps :). Latest 486 CPUs supports CPUID btw.

> Paolo
> 
> 
>     Signed-off-by: Petr Cvek <petrcvekcz@gmail.com <mailto:petrcvekcz@gmail.com>>
>     ---
>      include/qemu/timer.h | 2 +-
>      1 file changed, 1 insertion(+), 1 deletion(-)
> 
>     diff --git a/include/qemu/timer.h b/include/qemu/timer.h
>     index 9a366e551f..7baa5d1d41 100644
>     --- a/include/qemu/timer.h
>     +++ b/include/qemu/timer.h
>     @@ -872,7 +872,7 @@ static inline int64_t cpu_get_host_ticks(void)
>          return retval;
>      }
> 
>     -#elif defined(__i386__)
>     +#elif defined(__i386__) && !defined(__i486__)
> 
>      static inline int64_t cpu_get_host_ticks(void)
>      {
>     -- 
>     2.43.0
> 

Re: [PATCH] qemu/timer: Don't use RDTSC on i486

[Peter Maydell]

On Sat, 25 Nov 2023 at 12:24, Petr Cvek <petrcvekcz@gmail.com> wrote:
>
> GCC defines __i386__ for i386 and i486, which both lack RDTSC instruction.
> The i386 seems to be impossible to distinguish, but i486 can be identified
> by checking for undefined __i486__.
>
> Signed-off-by: Petr Cvek <petrcvekcz@gmail.com>

Last time this came up (over a decade ago!) we dropped the idea
because we couldn't find a consistent way of identifying
the no-RDTSC CPUs:
https://patchwork.ozlabs.org/project/qemu-devel/patch/1353683570-30525-1-git-send-email-peter.maydell@linaro.org/

We have already deprecated 32-bit x86 host support for
system emulation mode, incidentally.

thanks
-- PMM

Re: [PATCH] qemu/timer: Don't use RDTSC on i486

[Richard Henderson]

On 11/26/23 09:56, Paolo Bonzini wrote:
> 
> 
> Il sab 25 nov 2023, 13:23 Petr Cvek <petrcvekcz@gmail.com <mailto:petrcvekcz@gmail.com>> 
> ha scritto:
> 
>     GCC defines __i386__ for i386 and i486, which both lack RDTSC instruction.
>     The i386 seems to be impossible to distinguish, but i486 can be identified
>     by checking for undefined __i486__.
> 
> 
> As far as I know QEMU cannot be run on i486 anyway since TCG assumes the presence of 
> CPUID. Have you actually tried?

TCG does not assume CPUID.

We might have problems without cmpxchg8b, but if so that's in accel/tcg/ not tcg/.


r~

Re: [PATCH] qemu/timer: Don't use RDTSC on i486

[Petr Cvek]

I can agree that binary compiled for i486 doesn't contain cmpxchg8b and works OK with exception of setjmp bug I described in another thread [1]. glxgears which doesn't use signals works without problem. 64bit atomic operations seems to be emulated in util/atomic64.c.

However I've found out the compilation for i386 seems to fail during configure due to the lack of i386 atomic instructions in my GCC 13.2.0 version. If this is normal behavior, I guess __tune_i386__ check doesn't make sense to add. 

[1] [BUG] accel/tcg: cpu_exec_longjmp_cleanup: assertion failed: (cpu == current_cpu)

Petr

Dne 28. 11. 23 v 16:52 Richard Henderson napsal(a):
> On 11/26/23 09:56, Paolo Bonzini wrote:
>>
>>
>> Il sab 25 nov 2023, 13:23 Petr Cvek <petrcvekcz@gmail.com <mailto:petrcvekcz@gmail.com>> ha scritto:
>>
>>     GCC defines __i386__ for i386 and i486, which both lack RDTSC instruction.
>>     The i386 seems to be impossible to distinguish, but i486 can be identified
>>     by checking for undefined __i486__.
>>
>>
>> As far as I know QEMU cannot be run on i486 anyway since TCG assumes the presence of CPUID. Have you actually tried?
> 
> TCG does not assume CPUID.
> 
> We might have problems without cmpxchg8b, but if so that's in accel/tcg/ not tcg/.
> 
> 
> r~

Re: [PATCH] qemu/timer: Don't use RDTSC on i486

[Richard Henderson]

On 11/29/23 07:50, Petr Cvek wrote:
> I can agree that binary compiled for i486 doesn't contain cmpxchg8b and works OK with exception of setjmp bug I described in another thread [1]. glxgears which doesn't use signals works without problem. 64bit atomic operations seems to be emulated in util/atomic64.c.
> 
> However I've found out the compilation for i386 seems to fail during configure due to the lack of i386 atomic instructions in my GCC 13.2.0 version. If this is normal behavior, I guess __tune_i386__ check doesn't make sense to add.

Correct, we cannot operate at all without the i486 atomic insns.


r~