Re: [PATCH qemu] linux-user: Emulate /proc/cpuinfo output for riscv

[Palmer Dabbelt]

On Sun, 05 Mar 2023 06:34:37 PST (-0800), abordado@git.sr.ht wrote:
> From: Afonso Bordado <afonsobordado@gmail.com>
>
> RISC-V does not expose all extensions via hwcaps, thus some userspace
> applications may want to query these via /proc/cpuinfo.
>
> Currently when querying this file the host's file is shown instead
> which is slightly confusing. Emulate a basic /proc/cpuinfo file
> with mmu info and an ISA sting.

Kind of an orthogonal issue, but maybe we should default to having no 
/proc/cpuinfo (or an empty one, or something that can be detected as 
obviously QEMU) on systems that don't otherwise provide one (maybe when 
target != host)?  I'd bet that there's a lot of subtle oddness when 
trying to parse other systems' /proc/cpuinfo all over the software 
stack, and defaulting to something obvious could be easier for folks to 
debug.

That doesn't need to block this one, though.

> Signed-off-by: Afonso Bordado <afonsobordado@gmail.com>
> ---
>  linux-user/syscall.c              | 32 +++++++++++++++++++++++++++++--
>  tests/tcg/riscv64/Makefile.target |  1 +
>  tests/tcg/riscv64/cpuinfo.c       | 30 +++++++++++++++++++++++++++++
>  3 files changed, 61 insertions(+), 2 deletions(-)
>  create mode 100644 tests/tcg/riscv64/cpuinfo.c
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index a6c426d73c..eda2bc5df0 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8183,7 +8183,8 @@ void target_exception_dump(CPUArchState *env, const char 
> *fmt, int code)
>  }
>  
>  #if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN || \
> -    defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA)
> +    defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA) || \
> +    defined(TARGET_RISCV)
>  static int is_proc(const char *filename, const char *entry)
>  {
>      return strcmp(filename, entry) == 0;
> @@ -8261,6 +8262,33 @@ static int open_cpuinfo(CPUArchState *cpu_env, int fd)
>  }
>  #endif
>  
> +#if defined(TARGET_RISCV)
> +static int open_cpuinfo(CPUArchState *cpu_env, int fd)
> +{
> +    int i;
> +    int num_cpus = sysconf(_SC_NPROCESSORS_ONLN);
> +    RISCVCPU *cpu = env_archcpu(cpu_env);
> +    char *isa_string = riscv_isa_string(cpu);
> +    bool is_32_bit = riscv_cpu_xlen(&cpu->env) == 32;
> +    const char *mmu;
> +
> +    if (cpu->cfg.mmu) {
> +        mmu = (is_32_bit) ? "sv32" : "sv48";
> +    } else {
> +        mmu = "none";
> +    }
> +
> +    for (i = 0; i < num_cpus; i++) {
> +        dprintf(fd, "processor\t: %d\n", i);
> +        dprintf(fd, "hart\t\t: %d\n", i);
> +        dprintf(fd, "isa\t\t: %s\n", isa_string);
> +        dprintf(fd, "mmu\t\t: %s\n", mmu);
> +        dprintf(fd, "uarch\t\t: qemu\n\n");
> +    }
> +    return 0;
> +}
> +#endif
> +
>  #if defined(TARGET_M68K)
>  static int open_hardware(CPUArchState *cpu_env, int fd)
>  {
> @@ -8285,7 +8313,7 @@ static int do_openat(CPUArchState *cpu_env, int dirfd, 
> const char *pathname, int
>  #if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
>          { "/proc/net/route", open_net_route, is_proc },
>  #endif
> -#if defined(TARGET_SPARC) || defined(TARGET_HPPA)
> +#if defined(TARGET_SPARC) || defined(TARGET_HPPA) || defined(TARGET_RISCV)
>          { "/proc/cpuinfo", open_cpuinfo, is_proc },
>  #endif
>  #if defined(TARGET_M68K)
> diff --git a/tests/tcg/riscv64/Makefile.target 
> b/tests/tcg/riscv64/Makefile.target
> index cc3ed65ffd..df93a2ce1f 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -4,6 +4,7 @@
>  VPATH += $(SRC_PATH)/tests/tcg/riscv64
>  TESTS += test-div
>  TESTS += noexec
> +TESTS += cpuinfo
>  
>  # Disable compressed instructions for test-noc
>  TESTS += test-noc
> diff --git a/tests/tcg/riscv64/cpuinfo.c b/tests/tcg/riscv64/cpuinfo.c
> new file mode 100644
> index 0000000000..296abd0a8c
> --- /dev/null
> +++ b/tests/tcg/riscv64/cpuinfo.c
> @@ -0,0 +1,30 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <assert.h>
> +
> +#define BUFFER_SIZE 1024
> +
> +int main(void)
> +{
> +    char buffer[BUFFER_SIZE];
> +    FILE *fp = fopen("/proc/cpuinfo", "r");
> +    assert(fp != NULL);
> +
> +    while (fgets(buffer, BUFFER_SIZE, fp) != NULL) {
> +        if (strstr(buffer, "processor") != NULL) {
> +            assert(strstr(buffer, "processor\t: ") == buffer);
> +        } else if (strstr(buffer, "hart") != NULL) {
> +            assert(strstr(buffer, "hart\t\t: ") == buffer);
> +        } else if (strstr(buffer, "isa") != NULL) {
> +            assert(strcmp(buffer, "isa\t\t: rv64imafdc_zicsr_zifencei\n") == 
> 0);
> +        } else if (strstr(buffer, "mmu") != NULL) {
> +            assert(strcmp(buffer, "mmu\t\t: sv48\n") == 0);
> +        } else if (strstr(buffer, "uarch") != NULL) {
> +            assert(strcmp(buffer, "uarch\t\t: qemu\n") == 0);
> +        }
> +    }
> +
> +    fclose(fp);
> +    return 0;
> +}
> --
> 2.34.7

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

I know this might sound weird as I'm aiming to still take those ACPI 
patches, but I'm going to call it too late for 8.0 for this one for me.  
I'm fine if the linux-user folks want to pick this up, though, as IIRC 
these usually go through that tree anyway so that's fine with me.

I'm also not opposed to calling this a bug fix, but that's kind of 
pushing it...

Thanks!