1. Patchew
  2. QEMU
  3. View series

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

Petr Cvek posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/6826113a-d428-401e-b5a3-56ad5d8fbaa4@gmail.com
include/qemu/timer.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] qemu/timer: Don't use RDTSC on i486
Posted by Petr Cvek 1 year ago 
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
Posted by Peter Maydell 1 year ago 
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
Posted by Paolo Bonzini 1 year ago 
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
>
>
Re: [PATCH] qemu/timer: Don't use RDTSC on i486
Posted by Richard Henderson 12 months ago 
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
Posted by Petr Cvek 12 months ago 
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
Posted by Richard Henderson 12 months ago 
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~
Re: [PATCH] qemu/timer: Don't use RDTSC on i486
Posted by Petr Cvek 1 year ago 
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
Posted by Samuel Tardieu 1 year ago 
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
Posted by Petr Cvek 1 year ago 
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
Posted by Samuel Tardieu 1 year ago 
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
Posted by Petr Cvek 1 year ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.