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

[Philippe Mathieu-Daudé]

On 5/12/20 8:01 AM, Cornelia Huck wrote:
> On Mon, 11 May 2020 14:21:27 +0200
> Philippe Mathieu-Daudé <address@hidden> wrote:
>
> > 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...
>
> I don't think they're really outdated, but not sure how useful they are
> to people. I'm not personally attached to them, other opinions?

Since the project switched to git, developers can now use git-grep :)

> > >    
> > > >    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 */
> > >
> > >