Re: [PATCH 08/11] target/s390x: Restrict system-mode declarations

[Philippe Mathieu-Daudé]

On 5/11/20 12:48 PM, Cornelia Huck wrote:
> On Sat,  9 May 2020 15:09:07 +0200
> Philippe Mathieu-Daudé <address@hidden> wrote:
>
> > As these declarations are restricted to !CONFIG_USER_ONLY in
> > helper.c, only declare them when system-mode emulation is used.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > ---
> >   target/s390x/internal.h | 11 ++++++-----
> >   1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> > index c1678dc6bc..ddc276cdf4 100644
> > --- a/target/s390x/internal.h
> > +++ b/target/s390x/internal.h
> > @@ -236,7 +236,6 @@ int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, 
> > CPUState *cs,
> >   
> >   /* cc_helper.c */
> >   const char *cc_name(enum cc_op cc_op);
> > -void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr);
> >   uint32_t calc_cc(CPUS390XState *env, uint32_t cc_op, uint64_t src, uint64_t 
> > dst,
> >                    uint64_t vr);
> >   
> > @@ -303,18 +302,20 @@ void s390_cpu_gdb_init(CPUState *cs);
> >   
> >   /* helper.c */
> >   void s390_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
> > -hwaddr s390_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> > -hwaddr s390_cpu_get_phys_addr_debug(CPUState *cpu, vaddr addr);
> > +void do_restart_interrupt(CPUS390XState *env);
> > +
> > +#ifndef CONFIG_USER_ONLY
> > +void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr);
>
> load_psw() is in cc_helper.c (and not in helper.c). Rather add the
> #ifndef above, even if it is a bit awkward? Otherwise, the wrong
> comment makes it confusing.

I've been tempted to remove the kinda outdated /* helper.c */ comment...

> >   uint64_t get_psw_mask(CPUS390XState *env);
> >   void s390_cpu_recompute_watchpoints(CPUState *cs);
> >   void s390x_tod_timer(void *opaque);
> >   void s390x_cpu_timer(void *opaque);
> > -void do_restart_interrupt(CPUS390XState *env);
> >   void s390_handle_wait(S390CPU *cpu);
> > +hwaddr s390_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> > +hwaddr s390_cpu_get_phys_addr_debug(CPUState *cpu, vaddr addr);
> >   #define S390_STORE_STATUS_DEF_ADDR offsetof(LowCore, floating_pt_save_area)
> >   int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch);
> >   int s390_store_adtl_status(S390CPU *cpu, hwaddr addr, hwaddr len);
> > -#ifndef CONFIG_USER_ONLY
> >   LowCore *cpu_map_lowcore(CPUS390XState *env);
> >   void cpu_unmap_lowcore(LowCore *lowcore);
> >   #endif /* CONFIG_USER_ONLY */