diff mbox series

tcg/tci: Fix TCI on hppa host and update TCI test matrix

Message ID ZZpQZ77s2t81mXIT@p100
State New
Headers show
Series tcg/tci: Fix TCI on hppa host and update TCI test matrix | expand

Commit Message

Helge Deller Jan. 7, 2024, 7:19 a.m. UTC
Update the TCI interpreter test matrix for big-endian hosts with
big- (hppa, hppa64) and little-endian (x86,x96-64) targets.
I used native ppc64 and hppa hosts for those tests.

Starting TCI on a hppa host crashed immediately, because hppa is
the only archive left where the stack grows upwards.
Write-protecting the stack guard page at the top of the stack
fixes the crash.

Signed-off-by: Helge Deller <deller@gmx.de>

Comments

Philippe Mathieu-Daudé Jan. 7, 2024, 10:54 a.m. UTC | #1
Cc'ing Akihiko for commit a1eaa6281f.

On 7/1/24 08:19, Helge Deller wrote:
> Update the TCI interpreter test matrix for big-endian hosts with
> big- (hppa, hppa64) and little-endian (x86,x96-64) targets.
> I used native ppc64 and hppa hosts for those tests.
> 
> Starting TCI on a hppa host crashed immediately, because hppa is
> the only archive left where the stack grows upwards.
> Write-protecting the stack guard page at the top of the stack
> fixes the crash.
> 

Fixes: a1eaa6281f ("util: Delete checks for old host definitions")

> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> diff --git a/tcg/tci/README b/tcg/tci/README
> index 4a8b5b5401..0c1e50779e 100644
> --- a/tcg/tci/README
> +++ b/tcg/tci/README
> @@ -72,16 +72,16 @@ host and target with same or different endianness.
>               | host (le)                     host (be)
>               | 32             64             32             64
>   ------------+------------------------------------------------------------
> -target (le) | s0, u0         s1, u1         s?, u?         s?, u?
> +target (le) | s0, u0         s1, u1         s2, u?         s2, u?
>   32 bit      |
>               |
> -target (le) | sc, uc         s1, u1         s?, u?         s?, u?
> +target (le) | sc, uc         s1, u1         s2, u?         s2, u?
>   64 bit      |
>               |
> -target (be) | sc, u0         sc, uc         s?, u?         s?, u?
> +target (be) | sc, u0         sc, uc         s2, u?         s2, u?
>   32 bit      |
>               |
> -target (be) | sc, uc         sc, uc         s?, u?         s?, u?
> +target (be) | sc, uc         sc, uc         s?, u?         s2, u?
>   64 bit      |
>               |
>   
> @@ -110,6 +115,10 @@ u1 = linux-user-test works
>     A cross compiled QEMU for ppc host works at least partially:
>     i386-linux-user/qemu-i386 can run a simple hello-world program
>     (tested in a ppc emulation).
> +  The big-endian tests were run on native hppa (big-endian, 32-bit) and
> +  ppc64 (big-endian, 64-bit) machines. Tested target machines were
> +  x86 and x86-64 (little-endian, debian install ISO) and 32- and 64-bit
> +  big-endian hppa (NetBSD and Debian install ISOs).
>   
>   * Some TCG opcodes are either missing in the code generator and/or
>     in the interpreter. These opcodes raise a runtime exception, so it is
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index e86fd64e09..e378b71641 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -585,11 +585,8 @@ char *qemu_get_pid_name(pid_t pid)
>   
>   void *qemu_alloc_stack(size_t *sz)
>   {
> -    void *ptr;
> +    void *ptr, *ptr2;
>       int flags;
> -#ifdef CONFIG_DEBUG_STACK_USAGE
> -    void *ptr2;
> -#endif
>       size_t pagesz = qemu_real_host_page_size();
>   #ifdef _SC_THREAD_STACK_MIN
>       /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
> @@ -619,7 +616,12 @@ void *qemu_alloc_stack(size_t *sz)
>       }
>   
>       /* Stack grows down -- guard page at the bottom. */
> -    if (mprotect(ptr, pagesz, PROT_NONE) != 0) {
> +    ptr2 = ptr;
> +#if defined(__hppa__)

Is it worth make this generic by declaring some TARGET_STACK_GROWS_UP
definition in target/foo/cpu-param.h?

> +    /* but on hppa the stack grows up, so guard the top page instead */
> +    ptr2 = ptr + *sz - pagesz;
> +#endif
> +    if (mprotect(ptr2, pagesz, PROT_NONE) != 0) {
>           perror("failed to set up stack guard page");
>           abort();
>       }
>
Peter Maydell Jan. 7, 2024, 3:22 p.m. UTC | #2
On Sun, 7 Jan 2024 at 07:20, Helge Deller <deller@kernel.org> wrote:
>
> Update the TCI interpreter test matrix for big-endian hosts with
> big- (hppa, hppa64) and little-endian (x86,x96-64) targets.
> I used native ppc64 and hppa hosts for those tests.
>
> Starting TCI on a hppa host crashed immediately, because hppa is
> the only archive left where the stack grows upwards.
> Write-protecting the stack guard page at the top of the stack
> fixes the crash.

We deliberately dropped support for HPPA hosts, under
commit a1eaa6281f8b and commit b1cef6d02f84bd8.
Do we really care enough about trying to run on these
ancient host CPUs to want to bring it back?

My personal rule of thumb is that if a host CPU is supported
only by TCI then we are better off saying it is entirely
unsupported -- in practice the performance will be so
terrible as to not be something anybody will want to use,
especially for older architectures which are slow to
start with.

thanks
-- PMM
Helge Deller Jan. 7, 2024, 9:40 p.m. UTC | #3
On 1/7/24 16:22, Peter Maydell wrote:
> On Sun, 7 Jan 2024 at 07:20, Helge Deller <deller@kernel.org> wrote:
>>
>> Update the TCI interpreter test matrix for big-endian hosts with
>> big- (hppa, hppa64) and little-endian (x86,x96-64) targets.
>> I used native ppc64 and hppa hosts for those tests.
>>
>> Starting TCI on a hppa host crashed immediately, because hppa is
>> the only archive left where the stack grows upwards.
>> Write-protecting the stack guard page at the top of the stack
>> fixes the crash.
>
> We deliberately dropped support for HPPA hosts, under
> commit a1eaa6281f8b and commit b1cef6d02f84bd8.
> Do we really care enough about trying to run on these
> ancient host CPUs to want to bring it back?
>
> My personal rule of thumb is that if a host CPU is supported
> only by TCI then we are better off saying it is entirely
> unsupported -- in practice the performance will be so
> terrible as to not be something anybody will want to use,
> especially for older architectures which are slow to
> start with.

I can see your point (and the performance is really horrible).
It's not my intention to make hppa a supported TCI platform,
but for me it's a good candidate to at least test TCI on
a big-endian machine, mostly because I have access to some of
such machines.
And, this patch is all what's needed and it's pretty trivial, so
it would be great if it could be accepted.

Helge
Akihiko Odaki Jan. 8, 2024, 7:24 a.m. UTC | #4
On 2024/01/07 19:54, Philippe Mathieu-Daudé wrote:
> Cc'ing Akihiko for commit a1eaa6281f.
> 
> On 7/1/24 08:19, Helge Deller wrote:
>> Update the TCI interpreter test matrix for big-endian hosts with
>> big- (hppa, hppa64) and little-endian (x86,x96-64) targets.
>> I used native ppc64 and hppa hosts for those tests.
>>
>> Starting TCI on a hppa host crashed immediately, because hppa is
>> the only archive left where the stack grows upwards.
>> Write-protecting the stack guard page at the top of the stack
>> fixes the crash.
>>
> 
> Fixes: a1eaa6281f ("util: Delete checks for old host definitions")

The change is intentional. If you need HP-PA host support, please 
explain why you need to run latest QEMU on HP-PA. You may also look at 
commit b1cef6d02f ("Drop remaining bits of ia64 host support"), which is 
mentioned in commit a1eaa6281f ("util: Delete checks for old host 
definitions"), if you are going to restore HP-PA host support.

Regards,
Akihiko Odaki
Philippe Mathieu-Daudé Jan. 8, 2024, 11:50 a.m. UTC | #5
On 7/1/24 22:40, Helge Deller wrote:
> On 1/7/24 16:22, Peter Maydell wrote:
>> On Sun, 7 Jan 2024 at 07:20, Helge Deller <deller@kernel.org> wrote:
>>>
>>> Update the TCI interpreter test matrix for big-endian hosts with
>>> big- (hppa, hppa64) and little-endian (x86,x96-64) targets.
>>> I used native ppc64 and hppa hosts for those tests.
>>>
>>> Starting TCI on a hppa host crashed immediately, because hppa is
>>> the only archive left where the stack grows upwards.
>>> Write-protecting the stack guard page at the top of the stack
>>> fixes the crash.
>>
>> We deliberately dropped support for HPPA hosts, under
>> commit a1eaa6281f8b and commit b1cef6d02f84bd8.
>> Do we really care enough about trying to run on these
>> ancient host CPUs to want to bring it back?
>>
>> My personal rule of thumb is that if a host CPU is supported
>> only by TCI then we are better off saying it is entirely
>> unsupported -- in practice the performance will be so
>> terrible as to not be something anybody will want to use,
>> especially for older architectures which are slow to
>> start with.
> 
> I can see your point (and the performance is really horrible).
> It's not my intention to make hppa a supported TCI platform,
> but for me it's a good candidate to at least test TCI on
> a big-endian machine, mostly because I have access to some of
> such machines.
> And, this patch is all what's needed and it's pretty trivial, so
> it would be great if it could be accepted.

I was also running linux-user tests under HPPA/TCI and Cc'ed
Helge for HPPA host removal:
https://lore.kernel.org/qemu-devel/f61ff7ff-44a2-14c3-da08-755c290c75b7@linaro.org/
where he seemed to agree with the change so I didn't insist:
https://lore.kernel.org/qemu-devel/9a5f4a80-0700-4261-f26d-ad86ad4209cd@gmx.de/

Peter, in 2021 I had a go at implementing it (back then there
were few more HW still alive). I hit 2 issues:

- in compat32-bit user mode, signals are broken, see in
   arch/parisc/kernel/signal32.h:

   /* ELF32 signal handling */

   /* In a deft move of uber-hackery, we decide to carry the top half of all
    * 64-bit registers in a non-portable, non-ABI, hidden structure.
    * Userspace can read the hidden structure if it *wants* but is never
    * guaranteed to be in the same place. In fact the uc_sigmask from the
    * ucontext_t structure may push the hidden register file downwards
    */
   struct compat_regfile {
           /* Upper half of all the 64-bit registers that were truncated
              on a copy to a 32-bit userspace */
           compat_int_t rf_gr[32];
           compat_int_t rf_iasq[2];
           compat_int_t rf_iaoq[2];
           compat_int_t rf_sar;
   };

- 64-bit userland is missing glibc (missing L3 TLB support for user
   applications). John was working on it.

Regards,

Phil.
diff mbox series

Patch

diff --git a/tcg/tci/README b/tcg/tci/README
index 4a8b5b5401..0c1e50779e 100644
--- a/tcg/tci/README
+++ b/tcg/tci/README
@@ -72,16 +72,16 @@  host and target with same or different endianness.
             | host (le)                     host (be)
             | 32             64             32             64
 ------------+------------------------------------------------------------
-target (le) | s0, u0         s1, u1         s?, u?         s?, u?
+target (le) | s0, u0         s1, u1         s2, u?         s2, u?
 32 bit      |
             |
-target (le) | sc, uc         s1, u1         s?, u?         s?, u?
+target (le) | sc, uc         s1, u1         s2, u?         s2, u?
 64 bit      |
             |
-target (be) | sc, u0         sc, uc         s?, u?         s?, u?
+target (be) | sc, u0         sc, uc         s2, u?         s2, u?
 32 bit      |
             |
-target (be) | sc, uc         sc, uc         s?, u?         s?, u?
+target (be) | sc, uc         sc, uc         s?, u?         s2, u?
 64 bit      |
             |
 
@@ -110,6 +115,10 @@  u1 = linux-user-test works
   A cross compiled QEMU for ppc host works at least partially:
   i386-linux-user/qemu-i386 can run a simple hello-world program
   (tested in a ppc emulation).
+  The big-endian tests were run on native hppa (big-endian, 32-bit) and
+  ppc64 (big-endian, 64-bit) machines. Tested target machines were
+  x86 and x86-64 (little-endian, debian install ISO) and 32- and 64-bit
+  big-endian hppa (NetBSD and Debian install ISOs).
 
 * Some TCG opcodes are either missing in the code generator and/or
   in the interpreter. These opcodes raise a runtime exception, so it is
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e86fd64e09..e378b71641 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -585,11 +585,8 @@  char *qemu_get_pid_name(pid_t pid)
 
 void *qemu_alloc_stack(size_t *sz)
 {
-    void *ptr;
+    void *ptr, *ptr2;
     int flags;
-#ifdef CONFIG_DEBUG_STACK_USAGE
-    void *ptr2;
-#endif
     size_t pagesz = qemu_real_host_page_size();
 #ifdef _SC_THREAD_STACK_MIN
     /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
@@ -619,7 +616,12 @@  void *qemu_alloc_stack(size_t *sz)
     }
 
     /* Stack grows down -- guard page at the bottom. */
-    if (mprotect(ptr, pagesz, PROT_NONE) != 0) {
+    ptr2 = ptr;
+#if defined(__hppa__)
+    /* but on hppa the stack grows up, so guard the top page instead */
+    ptr2 = ptr + *sz - pagesz;
+#endif
+    if (mprotect(ptr2, pagesz, PROT_NONE) != 0) {
         perror("failed to set up stack guard page");
         abort();
     }