Re: [PATCH 4/4] linux-user: Emulate /proc/cpuinfo on s390x

[Ilya Leoshkevich]

On Fri, 2023-06-02 at 14:34 +0200, David Hildenbrand wrote:
> On 01.06.23 18:25, Ilya Leoshkevich wrote:
> > Some s390x userspace programs are confused when seeing a foreign
> > /proc/cpuinfo [1]. Add the emulation for s390x; follow the
> > respective
> > kernel code structure where possible.
> > 
> > [1] https://bugzilla.redhat.com/show_bug.cgi?id=2211472
> > 
> 
> Would be nice to have an example output here.
> 
> > Reported-by: Tulio Magno Quites Machado Filho <tuliom@redhat.com>
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >   linux-user/syscall.c | 106
> > ++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 105 insertions(+), 1 deletion(-)
> > 
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 3e6ed51ce62..d142e09dc70 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -8339,6 +8339,109 @@ static int open_cpuinfo(CPUArchState
> > *cpu_env, int fd)
> >   }
> >   #endif
> >   
> 
> [...]
> 
> > +
> > +static void show_cpuinfo(CPUArchState *cpu_env, int fd, unsigned
> > long n)
> > +{
> > +    if (n == 0) {
> > +        show_cpu_summary(cpu_env, fd);
> > +    }
> > +    dprintf(fd, "\ncpu number      : %ld\n", n);
> > +    show_cpu_ids(cpu_env, fd, n);
> > +}
> 
> Considering the n==0 special case, it might be cleaner to just place 
> that code directly into open_cpuinfo and get rid of show_cpuinfo().

Will do.

This was copied from the kernel, where a similar check actually made
sense, but in the QEMU context we don't really need it.

> > +
> > +static int open_cpuinfo(CPUArchState *cpu_env, int fd)
> > +{
> > +    int num_cpus = sysconf(_SC_NPROCESSORS_ONLN);
> > +    int i;
> > +
> > +    for (i = 0; i < num_cpus; i++) {
> > +        show_cpuinfo(cpu_env, fd, i);
> > +    }
> > +    return 0;
> > +}
> > +#endif
> > +
> >   #if defined(TARGET_M68K)
> >   static int open_hardware(CPUArchState *cpu_env, int fd)
> >   {
> > @@ -8364,7 +8467,8 @@ int do_guest_openat(CPUArchState *cpu_env,
> > int dirfd, const char *pathname,
> >   #if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
> >           { "/proc/net/route", open_net_route, is_proc },
> >   #endif
> > -#if defined(TARGET_SPARC) || defined(TARGET_HPPA) ||
> > defined(TARGET_RISCV)
> > +#if defined(TARGET_SPARC) || defined(TARGET_HPPA) || \
> > +    defined(TARGET_RISCV) || defined(TARGET_S390X)
> >           { "/proc/cpuinfo", open_cpuinfo, is_proc },
> >   #endif
> >   #if defined(TARGET_M68K)
> 
> In general, looks very good to me, thanks! With the fixup:
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
>