Re: [PATCH v3 09/13] target/hexagon: add main arch-specific semihosting operations

["Philippe Mathieu-Daudé" <philmd@oss.qualcomm.com>]

Cc'ing Kohei for WASM

On 20/7/26 19:41, Matheus Tavares Bernardino wrote:
> The Hexagon semihosting ABI extends the arm-compatible set with
> operations like OPEN, WRITECREG, WRITE0, ISTTY, STAT, FSTAT, FTELL,
> SEEK, FTRUNC, ACCESS, and GETCWD. Implement these trap0 handlers so
> that baremetal programs using the standard Hexagon simulator ABI can
> perform file I/O when running on qemu-system-hexagon.
> 
> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
> Signed-off-by: Matheus Tavares Bernardino <matheus.bernardino@oss.qualcomm.com>
> ---
>   include/semihosting/common-semi.h |   1 +
>   semihosting/arm-compat-semi.c     |   2 +-
>   target/hexagon/hexswi.c           | 298 +++++++++++++++++++++++++++++-
>   3 files changed, 299 insertions(+), 2 deletions(-)


> +    case HEX_SYS_STAT:
> +    case HEX_SYS_FSTAT:
> +    {
> +        struct stat st_buf;
> +        uint8_t *st_bufptr = (uint8_t *)&sys_stat;
> +        int rc, err = 0;
> +        char filename[BUFSIZ];
> +        target_ulong physical_filename_addr;
> +        target_ulong statBufferAddr;
> +        hexagon_read_memory(env, swi_info, 4, &physical_filename_addr, retaddr);
> +
> +        if (what_swi == HEX_SYS_STAT) {
> +            int i = 0;
> +            do {
> +                hexagon_read_memory(env, physical_filename_addr + i, 1,
> +                                    &filename[i], retaddr);
> +                i++;
> +            } while ((i < BUFSIZ) && filename[i - 1]);
> +            rc = stat(filename, &st_buf);
> +            err = errno;
> +        } else {
> +            int fd = physical_filename_addr;
> +            GuestFD *gf = get_guestfd(fd);
> +            if (!gf || gf->type != GuestFDHost) {
> +                qemu_log_mask(LOG_UNIMP,
> +                              "fstat semihosting only implemented"
> +                              " for native mode\n");
> +                g_assert_not_reached();
> +            }
> +            rc = fstat(gf->hostfd, &st_buf);
> +            err = errno;
> +        }
> +        if (rc == 0) {
> +            sys_stat.dev   = st_buf.st_dev;
> +            sys_stat.ino   = st_buf.st_ino;
> +            sys_stat.mode  = st_buf.st_mode;
> +            sys_stat.nlink = (uint32_t) st_buf.st_nlink;
> +            sys_stat.rdev  = st_buf.st_rdev;
> +            sys_stat.size  = (uint32_t) st_buf.st_size;
> +#if defined(__linux__)
> +            sys_stat.atime = (uint32_t) st_buf.st_atim.tv_sec;
> +            sys_stat.mtime = (uint32_t) st_buf.st_mtim.tv_sec;
> +            sys_stat.ctime = (uint32_t) st_buf.st_ctim.tv_sec;
> +#elif defined(_WIN32)
> +            sys_stat.atime = st_buf.st_atime;
> +            sys_stat.mtime = st_buf.st_mtime;
> +            sys_stat.ctime = st_buf.st_ctime;
> +#endif

This #if/elif seems bogus in that various hosts are not covered,
in particular *BSD and WASM. We could use #else #error to catch
them, or better implement OS-specific [f]stat() helpers (see
include/system/os-*.h).

> +        }
> +        hexagon_read_memory(env, swi_info + 4, 4, &statBufferAddr, retaddr);
> +
> +        for (int i = 0; i < sizeof(sys_stat); i++) {
> +            hexagon_write_memory(env, statBufferAddr + i, 1, st_bufptr[i],
> +                                 retaddr);
> +        }
> +        common_semi_cb(cs, rc, rc == 0 ? 0 : err);
> +    }
> +    break;