| Message ID | 20200509130910.26335-9-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | exec/cpu: Poison 'hwaddr' type in user-mode emulation | expand |
On Sat, 9 May 2020 15:09:07 +0200 Philippe Mathieu-Daudé <f4bug@amsat.org> 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é <f4bug@amsat.org> > --- > 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. > 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 */
On 5/11/20 12:48 PM, Cornelia Huck wrote: > On Sat, 9 May 2020 15:09:07 +0200 > Philippe Mathieu-Daudé <f4bug@amsat.org> 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é <f4bug@amsat.org> >> --- >> 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 */ > >
On Mon, 11 May 2020 14:21:27 +0200 Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > On 5/11/20 12:48 PM, Cornelia Huck wrote: > > On Sat, 9 May 2020 15:09:07 +0200 > > Philippe Mathieu-Daudé <f4bug@amsat.org> 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é <f4bug@amsat.org> > >> --- > >> 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? > > > > >> 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 */ > > > > >
On 5/12/20 8:01 AM, Cornelia Huck wrote: > On Mon, 11 May 2020 14:21:27 +0200 > Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > >> On 5/11/20 12:48 PM, Cornelia Huck wrote: >>> On Sat, 9 May 2020 15:09:07 +0200 >>> Philippe Mathieu-Daudé <f4bug@amsat.org> 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é <f4bug@amsat.org> >>>> --- >>>> 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 */ >>> >>> >> > >
On 12.05.20 08:46, Philippe Mathieu-Daudé wrote: > On 5/12/20 8:01 AM, Cornelia Huck wrote: >> On Mon, 11 May 2020 14:21:27 +0200 >> Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >>> On 5/11/20 12:48 PM, Cornelia Huck wrote: >>>> On Sat, 9 May 2020 15:09:07 +0200 >>>> Philippe Mathieu-Daudé <f4bug@amsat.org> 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é <f4bug@amsat.org> >>>>> --- >>>>> 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 :) I consider these quite valuable to structure the file. But my life would go on without them.
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); 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 */
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é <f4bug@amsat.org> --- target/s390x/internal.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)