All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] target/mips: Rewrite UHI errno_mips() to allow building on Haiku OS
@ 2021-07-04 17:07 Philippe Mathieu-Daudé
  2021-07-04 17:07 ` [PATCH 1/4] target/mips: Fix UHI error values Philippe Mathieu-Daudé
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-04 17:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Aleksandar Rikalo, Alexander von Gluck IV,
	Alex Bennée, Richard Henderson, Thomas Huth,
	Philippe Mathieu-Daudé,
	Richard Zak, Leon Alrae, Aurelien Jarno

Richard Zak reported a build failure on Haiku OS which is due
to using POSIX errno as array indexes:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg820587.html

Work around by using a GHashTable.

Since we were reviewing the UHI (semihosting) spec, fix the errno
values and complete to support all the specified errnos.

Regards,

Phil.

Philippe Mathieu-Daudé (4):
  target/mips: Fix UHI error values
  target/mips: Rename UHI err -> host_errno
  target/mips: Rewrite UHI errno_mips() using GHashTable
  target/mips: Complete UHI errno list and log unexpected errors

 target/mips/tcg/sysemu/mips-semi.c | 105 ++++++++++++++++++++++++-----
 1 file changed, 88 insertions(+), 17 deletions(-)

-- 
2.31.1



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] target/mips: Fix UHI error values
  2021-07-04 17:07 [PATCH 0/4] target/mips: Rewrite UHI errno_mips() to allow building on Haiku OS Philippe Mathieu-Daudé
@ 2021-07-04 17:07 ` Philippe Mathieu-Daudé
  2021-07-04 17:07 ` [PATCH 2/4] target/mips: Rename UHI err -> host_errno Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-04 17:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Aleksandar Rikalo, Alexander von Gluck IV,
	Alex Bennée, Richard Henderson, Thomas Huth,
	Philippe Mathieu-Daudé,
	Richard Zak, Leon Alrae, Aurelien Jarno

The UHI support was posted *before* [1] the specification was
released, and merged (commit 2c44b19c199) around the same time
[2], using Linux kernel errno values.
Update to use the spec values.

[1] https://lists.gnu.org/archive/html/qemu-devel/2015-02/msg05387.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg05231.html

Fixes: 2c44b19c199 ("target-mips: convert host to MIPS errno values when required")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/tcg/sysemu/mips-semi.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index 77108b0b1a9..3b1939365c4 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -1,6 +1,8 @@
 /*
  * Unified Hosting Interface syscalls.
  *
+ * Specifications: MD01069 Reference Manual (rev 1.1.6, 06 Jul 2015)
+ *
  * Copyright (c) 2015 Imagination Technologies
  *
  * This library is free software; you can redistribute it and/or
@@ -74,14 +76,17 @@ enum UHIOpenFlags {
     UHIOpen_EXCL   = 0x800
 };
 
-/* Errno values taken from asm-mips/errno.h */
+/*
+ * Unified Hosting Interface (rev 1.1.6)
+ * Appendix A. "Error values"
+ */
 static const uint16_t host_to_mips_errno[] = {
-    [ENAMETOOLONG] = 78,
+    [ENAMETOOLONG] = 91,
 #ifdef EOVERFLOW
-    [EOVERFLOW]    = 79,
+    [EOVERFLOW]    = 139,
 #endif
 #ifdef ELOOP
-    [ELOOP]        = 90,
+    [ELOOP]        = 92,
 #endif
 };
 
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] target/mips: Rename UHI err -> host_errno
  2021-07-04 17:07 [PATCH 0/4] target/mips: Rewrite UHI errno_mips() to allow building on Haiku OS Philippe Mathieu-Daudé
  2021-07-04 17:07 ` [PATCH 1/4] target/mips: Fix UHI error values Philippe Mathieu-Daudé
@ 2021-07-04 17:07 ` Philippe Mathieu-Daudé
  2021-07-04 17:07 ` [PATCH 3/4] target/mips: Rewrite UHI errno_mips() using GHashTable Philippe Mathieu-Daudé
  2021-07-04 17:07 ` [PATCH 4/4] target/mips: Complete UHI errno list and log unexpected errors Philippe Mathieu-Daudé
  3 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-04 17:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Aleksandar Rikalo, Alexander von Gluck IV,
	Alex Bennée, Richard Henderson, Thomas Huth,
	Philippe Mathieu-Daudé,
	Richard Zak, Leon Alrae, Aurelien Jarno

Rename 'err' as 'host_errno' to ease code review.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/tcg/sysemu/mips-semi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index 3b1939365c4..4c924273c1b 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -90,14 +90,14 @@ static const uint16_t host_to_mips_errno[] = {
 #endif
 };
 
-static int errno_mips(int err)
+static int errno_mips(int host_errno)
 {
-    if (err < 0 || err >= ARRAY_SIZE(host_to_mips_errno)) {
+    if (host_errno < 0 || host_errno >= ARRAY_SIZE(host_to_mips_errno)) {
         return EINVAL;
-    } else if (host_to_mips_errno[err]) {
-        return host_to_mips_errno[err];
+    } else if (host_to_mips_errno[host_errno]) {
+        return host_to_mips_errno[host_errno];
     } else {
-        return err;
+        return host_errno;
     }
 }
 
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] target/mips: Rewrite UHI errno_mips() using GHashTable
  2021-07-04 17:07 [PATCH 0/4] target/mips: Rewrite UHI errno_mips() to allow building on Haiku OS Philippe Mathieu-Daudé
  2021-07-04 17:07 ` [PATCH 1/4] target/mips: Fix UHI error values Philippe Mathieu-Daudé
  2021-07-04 17:07 ` [PATCH 2/4] target/mips: Rename UHI err -> host_errno Philippe Mathieu-Daudé
@ 2021-07-04 17:07 ` Philippe Mathieu-Daudé
  2021-07-04 17:25   ` Peter Maydell
  2021-07-04 18:38   ` Thomas Huth
  2021-07-04 17:07 ` [PATCH 4/4] target/mips: Complete UHI errno list and log unexpected errors Philippe Mathieu-Daudé
  3 siblings, 2 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-04 17:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Aleksandar Rikalo, Alexander von Gluck IV,
	Alex Bennée, Richard Henderson, Thomas Huth,
	Philippe Mathieu-Daudé,
	Richard Zak, Leon Alrae, Aurelien Jarno

Linking on Haiku OS fails:

  /boot/system/develop/tools/bin/../lib/gcc/x86_64-unknown-haiku/8.3.0/../../../../x86_64-unknown-haiku/bin/ld:
  error: libqemu-mips-softmmu.fa.p/target_mips_tcg_sysemu_mips-semi.c.o(.rodata) is too large (0xffff405a bytes)
  /boot/system/develop/tools/bin/../lib/gcc/x86_64-unknown-haiku/8.3.0/../../../../x86_64-unknown-haiku/bin/ld:
  final link failed: memory exhausted
  collect2: error: ld returned 1 exit status

This is because the host_to_mips_errno[] uses errno as index,
for example:

  static const uint16_t host_to_mips_errno[] = {
      [ENAMETOOLONG] = 91,
      ...

and Haiku defines [*] ENAMETOOLONG as:

   12 /* Error baselines */
   13 #define B_GENERAL_ERROR_BASE              INT_MIN
   ..
   22 #define B_STORAGE_ERROR_BASE              (B_GENERAL_ERROR_BASE + 0x6000)
  ...
  106 #define B_NAME_TOO_LONG                   (B_STORAGE_ERROR_BASE + 4)
  ...
  211 #define ENAMETOOLONG                      B_TO_POSIX_ERROR(B_NAME_TOO_LONG)

so the array ends up beeing indeed too big.

Since POSIX errno can't be use as indexes on Haiku,
rewrite errno_mips() using a GHashTable.

[*] https://github.com/haiku/haiku/blob/r1beta3/headers/os/support/Errors.h#L130

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/tcg/sysemu/mips-semi.c | 62 ++++++++++++++++++++++--------
 1 file changed, 45 insertions(+), 17 deletions(-)

diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index 4c924273c1b..3e91c9eb76c 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -4,6 +4,7 @@
  * Specifications: MD01069 Reference Manual (rev 1.1.6, 06 Jul 2015)
  *
  * Copyright (c) 2015 Imagination Technologies
+ * Copyright (c) 2021 Philippe Mathieu-Daudé
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -76,29 +77,56 @@ enum UHIOpenFlags {
     UHIOpen_EXCL   = 0x800
 };
 
-/*
- * Unified Hosting Interface (rev 1.1.6)
- * Appendix A. "Error values"
- */
-static const uint16_t host_to_mips_errno[] = {
-    [ENAMETOOLONG] = 91,
-#ifdef EOVERFLOW
-    [EOVERFLOW]    = 139,
-#endif
+static GHashTable *uhi_errno_hash_table;
+
+static void uhi_errno_insert(int host_errno, int uhi_errno)
+{
+    gboolean ret = TRUE;
+
+    assert(uhi_errno_hash_table != NULL);
+    ret = g_hash_table_insert(uhi_errno_hash_table,
+                              GINT_TO_POINTER(host_errno),
+                              GINT_TO_POINTER(uhi_errno));
+    assert(ret == TRUE);
+}
+
+static void uhi_errno_init(void)
+{
+    gboolean ret = TRUE;
+
+    uhi_errno_hash_table = g_hash_table_new(NULL, NULL);
+
+    /*
+     * Unified Hosting Interface (rev 1.1.6)
+     * Appendix A. "Error values"
+     */
+    uhi_errno_insert(ENAMETOOLONG,  91);
 #ifdef ELOOP
-    [ELOOP]        = 92,
+    uhi_errno_insert(ELOOP,         92);
 #endif
-};
+#ifdef EOVERFLOW
+    uhi_errno_insert(EOVERFLOW,     139);
+#endif
+    assert(ret == TRUE);
+}
 
 static int errno_mips(int host_errno)
 {
-    if (host_errno < 0 || host_errno >= ARRAY_SIZE(host_to_mips_errno)) {
-        return EINVAL;
-    } else if (host_to_mips_errno[host_errno]) {
-        return host_to_mips_errno[host_errno];
-    } else {
-        return host_errno;
+    gpointer uhi_errno;
+
+    if (uhi_errno_hash_table == NULL) {
+        uhi_errno_init();
     }
+
+    if (host_errno == 0) {
+        return 0;
+    }
+    if (g_hash_table_lookup_extended(uhi_errno_hash_table,
+                                     GINT_TO_POINTER(host_errno),
+                                     NULL, &uhi_errno)) {
+        return GPOINTER_TO_INT(uhi_errno);
+    }
+    return EINVAL; /* Not reachable per the specification */
 }
 
 static int copy_stat_to_target(CPUMIPSState *env, const struct stat *src,
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] target/mips: Complete UHI errno list and log unexpected errors
  2021-07-04 17:07 [PATCH 0/4] target/mips: Rewrite UHI errno_mips() to allow building on Haiku OS Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2021-07-04 17:07 ` [PATCH 3/4] target/mips: Rewrite UHI errno_mips() using GHashTable Philippe Mathieu-Daudé
@ 2021-07-04 17:07 ` Philippe Mathieu-Daudé
  2021-07-04 17:23   ` Peter Maydell
  3 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-04 17:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Aleksandar Rikalo, Alexander von Gluck IV,
	Alex Bennée, Richard Henderson, Thomas Huth,
	Philippe Mathieu-Daudé,
	Richard Zak, Leon Alrae, Aurelien Jarno

The spec only defines a set of host errno to translate to guest.
Complete the current errno set, and log unexpected errno (they
are currently replaced by EINVAL, which is dubious, but we don't
modify this).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/tcg/sysemu/mips-semi.c | 38 ++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index 3e91c9eb76c..6e6518dd626 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -100,10 +100,46 @@ static void uhi_errno_init(void)
      * Unified Hosting Interface (rev 1.1.6)
      * Appendix A. "Error values"
      */
+    uhi_errno_insert(EPERM,         1);
+    uhi_errno_insert(ENOENT,        2);
+    uhi_errno_insert(EINTR,         4);
+    uhi_errno_insert(EIO,           5);
+    uhi_errno_insert(ENXIO,         6);
+    uhi_errno_insert(EBADF,         9);
+    uhi_errno_insert(EAGAIN,        11);
+    uhi_errno_insert(EWOULDBLOCK,   11);
+    uhi_errno_insert(ENOMEM,        12);
+    uhi_errno_insert(EACCES,        13);
+    uhi_errno_insert(EBUSY,         16);
+    uhi_errno_insert(EEXIST,        17);
+    uhi_errno_insert(EXDEV,         18);
+    uhi_errno_insert(ENOTDIR,       20);
+    uhi_errno_insert(EISDIR,        21);
+    uhi_errno_insert(EINVAL,        22);
+    uhi_errno_insert(ENFILE,        23);
+    uhi_errno_insert(EMFILE,        24);
+#ifdef ETXTBSY
+    uhi_errno_insert(ETXTBSY,       26);
+#endif
+    uhi_errno_insert(EFBIG,         27);
+    uhi_errno_insert(ENOSPC,        28);
+    uhi_errno_insert(ESPIPE,        29);
+    uhi_errno_insert(EROFS,         30);
+    uhi_errno_insert(EMLINK,        31);
+    uhi_errno_insert(EPIPE,         32);
+    uhi_errno_insert(ERANGE,        34);
+    uhi_errno_insert(ENOSR,         63);
+    uhi_errno_insert(EBADMSG,       77);
     uhi_errno_insert(ENAMETOOLONG,  91);
 #ifdef ELOOP
     uhi_errno_insert(ELOOP,         92);
 #endif
+    uhi_errno_insert(ECONNRESET,    104);
+    uhi_errno_insert(ENOBUFS,       105);
+    uhi_errno_insert(ENETUNREACH,   114);
+    uhi_errno_insert(ENETDOWN,      115);
+    uhi_errno_insert(ETIMEDOUT,     116);
+    uhi_errno_insert(ENOTCONN,      128);
 #ifdef EOVERFLOW
     uhi_errno_insert(EOVERFLOW,     139);
 #endif
@@ -126,6 +162,8 @@ static int errno_mips(int host_errno)
                                      NULL, &uhi_errno)) {
         return GPOINTER_TO_INT(uhi_errno);
     }
+    qemu_log("semihosting: Illegal UHI errno: %d\n", host_errno);
+
     return EINVAL; /* Not reachable per the specification */
 }
 
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] target/mips: Complete UHI errno list and log unexpected errors
  2021-07-04 17:07 ` [PATCH 4/4] target/mips: Complete UHI errno list and log unexpected errors Philippe Mathieu-Daudé
@ 2021-07-04 17:23   ` Peter Maydell
  2021-07-04 18:25     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2021-07-04 17:23 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Alexander von Gluck IV, Alex Bennée,
	Richard Henderson, Aleksandar Rikalo, QEMU Developers,
	Richard Zak, Leon Alrae, Aurelien Jarno

On Sun, 4 Jul 2021 at 18:07, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The spec only defines a set of host errno to translate to guest.
> Complete the current errno set, and log unexpected errno (they
> are currently replaced by EINVAL, which is dubious, but we don't
> modify this).
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---

This commit adds the definition of the guest EINVAL:

> +    uhi_errno_insert(EINVAL,        22);
> @@ -126,6 +162,8 @@ static int errno_mips(int host_errno)
>                                       NULL, &uhi_errno)) {
>          return GPOINTER_TO_INT(uhi_errno);
>      }
> +    qemu_log("semihosting: Illegal UHI errno: %d\n", host_errno);
> +
>      return EINVAL; /* Not reachable per the specification */
>  }

...but it leaves the default case returning the host EINVAL.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] target/mips: Rewrite UHI errno_mips() using GHashTable
  2021-07-04 17:07 ` [PATCH 3/4] target/mips: Rewrite UHI errno_mips() using GHashTable Philippe Mathieu-Daudé
@ 2021-07-04 17:25   ` Peter Maydell
  2021-07-04 18:25     ` Philippe Mathieu-Daudé
  2021-07-04 18:38   ` Thomas Huth
  1 sibling, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2021-07-04 17:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Alexander von Gluck IV, Alex Bennée,
	Richard Henderson, Aleksandar Rikalo, QEMU Developers,
	Richard Zak, Leon Alrae, Aurelien Jarno

On Sun, 4 Jul 2021 at 18:07, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Linking on Haiku OS fails:
>
>   /boot/system/develop/tools/bin/../lib/gcc/x86_64-unknown-haiku/8.3.0/../../../../x86_64-unknown-haiku/bin/ld:
>   error: libqemu-mips-softmmu.fa.p/target_mips_tcg_sysemu_mips-semi.c.o(.rodata) is too large (0xffff405a bytes)
>   /boot/system/develop/tools/bin/../lib/gcc/x86_64-unknown-haiku/8.3.0/../../../../x86_64-unknown-haiku/bin/ld:
>   final link failed: memory exhausted
>   collect2: error: ld returned 1 exit status
>
> This is because the host_to_mips_errno[] uses errno as index,
> for example:
>
>   static const uint16_t host_to_mips_errno[] = {
>       [ENAMETOOLONG] = 91,
>       ...
>
> and Haiku defines [*] ENAMETOOLONG as:
>
>    12 /* Error baselines */
>    13 #define B_GENERAL_ERROR_BASE              INT_MIN
>    ..
>    22 #define B_STORAGE_ERROR_BASE              (B_GENERAL_ERROR_BASE + 0x6000)
>   ...
>   106 #define B_NAME_TOO_LONG                   (B_STORAGE_ERROR_BASE + 4)
>   ...
>   211 #define ENAMETOOLONG                      B_TO_POSIX_ERROR(B_NAME_TOO_LONG)
>
> so the array ends up beeing indeed too big.
>
> Since POSIX errno can't be use as indexes on Haiku,
> rewrite errno_mips() using a GHashTable.
>
> [*] https://github.com/haiku/haiku/blob/r1beta3/headers/os/support/Errors.h#L130
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/mips/tcg/sysemu/mips-semi.c | 62 ++++++++++++++++++++++--------
>  1 file changed, 45 insertions(+), 17 deletions(-)

>  static int errno_mips(int host_errno)
>  {
> -    if (host_errno < 0 || host_errno >= ARRAY_SIZE(host_to_mips_errno)) {
> -        return EINVAL;
> -    } else if (host_to_mips_errno[host_errno]) {
> -        return host_to_mips_errno[host_errno];
> -    } else {
> -        return host_errno;
> +    gpointer uhi_errno;
> +
> +    if (uhi_errno_hash_table == NULL) {
> +        uhi_errno_init();
>      }
> +
> +    if (host_errno == 0) {
> +        return 0;
> +    }
> +    if (g_hash_table_lookup_extended(uhi_errno_hash_table,
> +                                     GINT_TO_POINTER(host_errno),
> +                                     NULL, &uhi_errno)) {
> +        return GPOINTER_TO_INT(uhi_errno);
> +    }
> +    return EINVAL; /* Not reachable per the specification */

Per whose specification? This function is passed the errno as set
by various host OS functions like open(), lseek(), read(). POSIX allows
those functions to set errno to any value, so this "we don't know
a guest errno value for that" code is definitely reachable.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] target/mips: Complete UHI errno list and log unexpected errors
  2021-07-04 17:23   ` Peter Maydell
@ 2021-07-04 18:25     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-04 18:25 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Thomas Huth, Alexander von Gluck IV, Leon Alrae,
	Richard Henderson, Aleksandar Rikalo, QEMU Developers,
	Richard Zak, Alex Bennée, Aurelien Jarno

On 7/4/21 7:23 PM, Peter Maydell wrote:
> On Sun, 4 Jul 2021 at 18:07, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> The spec only defines a set of host errno to translate to guest.
>> Complete the current errno set, and log unexpected errno (they
>> are currently replaced by EINVAL, which is dubious, but we don't
>> modify this).
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
> 
> This commit adds the definition of the guest EINVAL:
> 
>> +    uhi_errno_insert(EINVAL,        22);
>> @@ -126,6 +162,8 @@ static int errno_mips(int host_errno)
>>                                       NULL, &uhi_errno)) {
>>          return GPOINTER_TO_INT(uhi_errno);
>>      }
>> +    qemu_log("semihosting: Illegal UHI errno: %d\n", host_errno);
>> +
>>      return EINVAL; /* Not reachable per the specification */
>>  }
> 
> ...but it leaves the default case returning the host EINVAL.

Good catch, thank you!


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] target/mips: Rewrite UHI errno_mips() using GHashTable
  2021-07-04 17:25   ` Peter Maydell
@ 2021-07-04 18:25     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-04 18:25 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Thomas Huth, Alexander von Gluck IV, Leon Alrae,
	Richard Henderson, Aleksandar Rikalo, QEMU Developers,
	Richard Zak, Alex Bennée, Aurelien Jarno

On 7/4/21 7:25 PM, Peter Maydell wrote:
> On Sun, 4 Jul 2021 at 18:07, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> Linking on Haiku OS fails:
>>
>>   /boot/system/develop/tools/bin/../lib/gcc/x86_64-unknown-haiku/8.3.0/../../../../x86_64-unknown-haiku/bin/ld:
>>   error: libqemu-mips-softmmu.fa.p/target_mips_tcg_sysemu_mips-semi.c.o(.rodata) is too large (0xffff405a bytes)
>>   /boot/system/develop/tools/bin/../lib/gcc/x86_64-unknown-haiku/8.3.0/../../../../x86_64-unknown-haiku/bin/ld:
>>   final link failed: memory exhausted
>>   collect2: error: ld returned 1 exit status
>>
>> This is because the host_to_mips_errno[] uses errno as index,
>> for example:
>>
>>   static const uint16_t host_to_mips_errno[] = {
>>       [ENAMETOOLONG] = 91,
>>       ...
>>
>> and Haiku defines [*] ENAMETOOLONG as:
>>
>>    12 /* Error baselines */
>>    13 #define B_GENERAL_ERROR_BASE              INT_MIN
>>    ..
>>    22 #define B_STORAGE_ERROR_BASE              (B_GENERAL_ERROR_BASE + 0x6000)
>>   ...
>>   106 #define B_NAME_TOO_LONG                   (B_STORAGE_ERROR_BASE + 4)
>>   ...
>>   211 #define ENAMETOOLONG                      B_TO_POSIX_ERROR(B_NAME_TOO_LONG)
>>
>> so the array ends up beeing indeed too big.
>>
>> Since POSIX errno can't be use as indexes on Haiku,
>> rewrite errno_mips() using a GHashTable.
>>
>> [*] https://github.com/haiku/haiku/blob/r1beta3/headers/os/support/Errors.h#L130
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  target/mips/tcg/sysemu/mips-semi.c | 62 ++++++++++++++++++++++--------
>>  1 file changed, 45 insertions(+), 17 deletions(-)
> 
>>  static int errno_mips(int host_errno)
>>  {
>> -    if (host_errno < 0 || host_errno >= ARRAY_SIZE(host_to_mips_errno)) {
>> -        return EINVAL;
>> -    } else if (host_to_mips_errno[host_errno]) {
>> -        return host_to_mips_errno[host_errno];
>> -    } else {
>> -        return host_errno;
>> +    gpointer uhi_errno;
>> +
>> +    if (uhi_errno_hash_table == NULL) {
>> +        uhi_errno_init();
>>      }
>> +
>> +    if (host_errno == 0) {
>> +        return 0;
>> +    }
>> +    if (g_hash_table_lookup_extended(uhi_errno_hash_table,
>> +                                     GINT_TO_POINTER(host_errno),
>> +                                     NULL, &uhi_errno)) {
>> +        return GPOINTER_TO_INT(uhi_errno);
>> +    }
>> +    return EINVAL; /* Not reachable per the specification */
> 
> Per whose specification? This function is passed the errno as set
> by various host OS functions like open(), lseek(), read(). POSIX allows
> those functions to set errno to any value, so this "we don't know
> a guest errno value for that" code is definitely reachable.

You are right, it is reachable. What I meant is other errnos are
not expected, and returning EINVAL for them doesn't seem ideal,
but the spec doesn't define a particular errno for unsupported
errnos. I'll reword as "Unsupported errno is not specified, use EINVAL".


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] target/mips: Rewrite UHI errno_mips() using GHashTable
  2021-07-04 17:07 ` [PATCH 3/4] target/mips: Rewrite UHI errno_mips() using GHashTable Philippe Mathieu-Daudé
  2021-07-04 17:25   ` Peter Maydell
@ 2021-07-04 18:38   ` Thomas Huth
  2021-07-04 18:44     ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 11+ messages in thread
From: Thomas Huth @ 2021-07-04 18:38 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Peter Maydell, Aleksandar Rikalo, Alexander von Gluck IV,
	Alex Bennée, Richard Henderson, Richard Zak, Leon Alrae,
	Aurelien Jarno

On 04/07/2021 19.07, Philippe Mathieu-Daudé wrote:
> Linking on Haiku OS fails:
> 
>    /boot/system/develop/tools/bin/../lib/gcc/x86_64-unknown-haiku/8.3.0/../../../../x86_64-unknown-haiku/bin/ld:
>    error: libqemu-mips-softmmu.fa.p/target_mips_tcg_sysemu_mips-semi.c.o(.rodata) is too large (0xffff405a bytes)
>    /boot/system/develop/tools/bin/../lib/gcc/x86_64-unknown-haiku/8.3.0/../../../../x86_64-unknown-haiku/bin/ld:
>    final link failed: memory exhausted
>    collect2: error: ld returned 1 exit status
> 
> This is because the host_to_mips_errno[] uses errno as index,
> for example:
> 
>    static const uint16_t host_to_mips_errno[] = {
>        [ENAMETOOLONG] = 91,
>        ...
> 
> and Haiku defines [*] ENAMETOOLONG as:
> 
>     12 /* Error baselines */
>     13 #define B_GENERAL_ERROR_BASE              INT_MIN
>     ..
>     22 #define B_STORAGE_ERROR_BASE              (B_GENERAL_ERROR_BASE + 0x6000)
>    ...
>    106 #define B_NAME_TOO_LONG                   (B_STORAGE_ERROR_BASE + 4)
>    ...
>    211 #define ENAMETOOLONG                      B_TO_POSIX_ERROR(B_NAME_TOO_LONG)
> 
> so the array ends up beeing indeed too big.
> 
> Since POSIX errno can't be use as indexes on Haiku,
> rewrite errno_mips() using a GHashTable.
> 
> [*] https://github.com/haiku/haiku/blob/r1beta3/headers/os/support/Errors.h#L130
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   target/mips/tcg/sysemu/mips-semi.c | 62 ++++++++++++++++++++++--------
>   1 file changed, 45 insertions(+), 17 deletions(-)
> 
> diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
> index 4c924273c1b..3e91c9eb76c 100644
> --- a/target/mips/tcg/sysemu/mips-semi.c
> +++ b/target/mips/tcg/sysemu/mips-semi.c
> @@ -4,6 +4,7 @@
>    * Specifications: MD01069 Reference Manual (rev 1.1.6, 06 Jul 2015)
>    *
>    * Copyright (c) 2015 Imagination Technologies
> + * Copyright (c) 2021 Philippe Mathieu-Daudé
>    *
>    * This library is free software; you can redistribute it and/or
>    * modify it under the terms of the GNU Lesser General Public
> @@ -76,29 +77,56 @@ enum UHIOpenFlags {
>       UHIOpen_EXCL   = 0x800
>   };
>   
> -/*
> - * Unified Hosting Interface (rev 1.1.6)
> - * Appendix A. "Error values"
> - */
> -static const uint16_t host_to_mips_errno[] = {
> -    [ENAMETOOLONG] = 91,
> -#ifdef EOVERFLOW
> -    [EOVERFLOW]    = 139,
> -#endif
> +static GHashTable *uhi_errno_hash_table;
> +
> +static void uhi_errno_insert(int host_errno, int uhi_errno)
> +{
> +    gboolean ret = TRUE;
> +
> +    assert(uhi_errno_hash_table != NULL);
> +    ret = g_hash_table_insert(uhi_errno_hash_table,
> +                              GINT_TO_POINTER(host_errno),
> +                              GINT_TO_POINTER(uhi_errno));
> +    assert(ret == TRUE);
> +}
> +
> +static void uhi_errno_init(void)
> +{
> +    gboolean ret = TRUE;
> +
> +    uhi_errno_hash_table = g_hash_table_new(NULL, NULL);
> +
> +    /*
> +     * Unified Hosting Interface (rev 1.1.6)
> +     * Appendix A. "Error values"
> +     */
> +    uhi_errno_insert(ENAMETOOLONG,  91);
>   #ifdef ELOOP
> -    [ELOOP]        = 92,
> +    uhi_errno_insert(ELOOP,         92);
>   #endif
> -};
> +#ifdef EOVERFLOW
> +    uhi_errno_insert(EOVERFLOW,     139);
> +#endif
> +    assert(ret == TRUE);
> +}
>   
>   static int errno_mips(int host_errno)
>   {
> -    if (host_errno < 0 || host_errno >= ARRAY_SIZE(host_to_mips_errno)) {
> -        return EINVAL;
> -    } else if (host_to_mips_errno[host_errno]) {
> -        return host_to_mips_errno[host_errno];
> -    } else {
> -        return host_errno;
> +    gpointer uhi_errno;
> +
> +    if (uhi_errno_hash_table == NULL) {
> +        uhi_errno_init();
>       }
> +
> +    if (host_errno == 0) {
> +        return 0;
> +    }
> +    if (g_hash_table_lookup_extended(uhi_errno_hash_table,
> +                                     GINT_TO_POINTER(host_errno),
> +                                     NULL, &uhi_errno)) {
> +        return GPOINTER_TO_INT(uhi_errno);
> +    }
> +    return EINVAL; /* Not reachable per the specification */
>   }

Why not simply use a switch-case statement instead? ... that's simpler and 
still allows to compiler to optimize if the errno values are in a compact range.

  Thomas



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] target/mips: Rewrite UHI errno_mips() using GHashTable
  2021-07-04 18:38   ` Thomas Huth
@ 2021-07-04 18:44     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-04 18:44 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Peter Maydell, Aleksandar Rikalo, Alexander von Gluck IV,
	Leon Alrae, Richard Henderson, Richard Zak, Alex Bennée,
	Aurelien Jarno

On 7/4/21 8:38 PM, Thomas Huth wrote:
> On 04/07/2021 19.07, Philippe Mathieu-Daudé wrote:
>> Linking on Haiku OS fails:
>>
>>   
>> /boot/system/develop/tools/bin/../lib/gcc/x86_64-unknown-haiku/8.3.0/../../../../x86_64-unknown-haiku/bin/ld:
>>
>>    error:
>> libqemu-mips-softmmu.fa.p/target_mips_tcg_sysemu_mips-semi.c.o(.rodata) is
>> too large (0xffff405a bytes)
>>   
>> /boot/system/develop/tools/bin/../lib/gcc/x86_64-unknown-haiku/8.3.0/../../../../x86_64-unknown-haiku/bin/ld:
>>
>>    final link failed: memory exhausted
>>    collect2: error: ld returned 1 exit status
>>
>> This is because the host_to_mips_errno[] uses errno as index,
>> for example:
>>
>>    static const uint16_t host_to_mips_errno[] = {
>>        [ENAMETOOLONG] = 91,
>>        ...
>>
>> and Haiku defines [*] ENAMETOOLONG as:
>>
>>     12 /* Error baselines */
>>     13 #define B_GENERAL_ERROR_BASE              INT_MIN
>>     ..
>>     22 #define B_STORAGE_ERROR_BASE              (B_GENERAL_ERROR_BASE
>> + 0x6000)
>>    ...
>>    106 #define B_NAME_TOO_LONG                   (B_STORAGE_ERROR_BASE
>> + 4)
>>    ...
>>    211 #define ENAMETOOLONG                     
>> B_TO_POSIX_ERROR(B_NAME_TOO_LONG)
>>
>> so the array ends up beeing indeed too big.
>>
>> Since POSIX errno can't be use as indexes on Haiku,
>> rewrite errno_mips() using a GHashTable.
>>
>> [*]
>> https://github.com/haiku/haiku/blob/r1beta3/headers/os/support/Errors.h#L130
>>
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   target/mips/tcg/sysemu/mips-semi.c | 62 ++++++++++++++++++++++--------
>>   1 file changed, 45 insertions(+), 17 deletions(-)

>> +static void uhi_errno_init(void)
>> +{
>> +    gboolean ret = TRUE;
>> +
>> +    uhi_errno_hash_table = g_hash_table_new(NULL, NULL);
>> +
>> +    /*
>> +     * Unified Hosting Interface (rev 1.1.6)
>> +     * Appendix A. "Error values"
>> +     */
>> +    uhi_errno_insert(ENAMETOOLONG,  91);
>>   #ifdef ELOOP
>> -    [ELOOP]        = 92,
>> +    uhi_errno_insert(ELOOP,         92);
>>   #endif
>> -};
>> +#ifdef EOVERFLOW
>> +    uhi_errno_insert(EOVERFLOW,     139);
>> +#endif
>> +    assert(ret == TRUE);
>> +}
>>     static int errno_mips(int host_errno)
>>   {
>> -    if (host_errno < 0 || host_errno >=
>> ARRAY_SIZE(host_to_mips_errno)) {
>> -        return EINVAL;
>> -    } else if (host_to_mips_errno[host_errno]) {
>> -        return host_to_mips_errno[host_errno];
>> -    } else {
>> -        return host_errno;
>> +    gpointer uhi_errno;
>> +
>> +    if (uhi_errno_hash_table == NULL) {
>> +        uhi_errno_init();
>>       }
>> +
>> +    if (host_errno == 0) {
>> +        return 0;
>> +    }
>> +    if (g_hash_table_lookup_extended(uhi_errno_hash_table,
>> +                                     GINT_TO_POINTER(host_errno),
>> +                                     NULL, &uhi_errno)) {
>> +        return GPOINTER_TO_INT(uhi_errno);
>> +    }
>> +    return EINVAL; /* Not reachable per the specification */
>>   }
> 
> Why not simply use a switch-case statement instead? ... that's simpler
> and still allows to compiler to optimize if the errno values are in a
> compact range.

I was expecting the #ifdef'ry to be ugly, but there isn't that many
actually. Sigh :(


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-07-04 18:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-04 17:07 [PATCH 0/4] target/mips: Rewrite UHI errno_mips() to allow building on Haiku OS Philippe Mathieu-Daudé
2021-07-04 17:07 ` [PATCH 1/4] target/mips: Fix UHI error values Philippe Mathieu-Daudé
2021-07-04 17:07 ` [PATCH 2/4] target/mips: Rename UHI err -> host_errno Philippe Mathieu-Daudé
2021-07-04 17:07 ` [PATCH 3/4] target/mips: Rewrite UHI errno_mips() using GHashTable Philippe Mathieu-Daudé
2021-07-04 17:25   ` Peter Maydell
2021-07-04 18:25     ` Philippe Mathieu-Daudé
2021-07-04 18:38   ` Thomas Huth
2021-07-04 18:44     ` Philippe Mathieu-Daudé
2021-07-04 17:07 ` [PATCH 4/4] target/mips: Complete UHI errno list and log unexpected errors Philippe Mathieu-Daudé
2021-07-04 17:23   ` Peter Maydell
2021-07-04 18:25     ` Philippe Mathieu-Daudé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.