diff mbox series

[v2,01/55] hw/core: Make do_unaligned_access noreturn

Message ID 20210803041443.55452-2-richard.henderson@linaro.org (mailing list archive)
State New, archived
Headers show
Series Unaligned access for user-only | expand

Commit Message

Richard Henderson Aug. 3, 2021, 4:13 a.m. UTC
While we may have had some thought of allowing system-mode
to return from this hook, we have no guests that require this.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/hw/core/tcg-cpu-ops.h  | 3 ++-
 target/alpha/cpu.h             | 4 ++--
 target/arm/internals.h         | 3 ++-
 target/microblaze/cpu.h        | 2 +-
 target/mips/tcg/tcg-internal.h | 4 ++--
 target/nios2/cpu.h             | 4 ++--
 target/ppc/internal.h          | 4 ++--
 target/riscv/cpu.h             | 2 +-
 target/s390x/s390x-internal.h  | 4 ++--
 target/sh4/cpu.h               | 4 ++--
 target/xtensa/cpu.h            | 4 ++--
 target/hppa/cpu.c              | 7 ++++---
 12 files changed, 24 insertions(+), 21 deletions(-)

Comments

Philippe Mathieu-Daudé Aug. 3, 2021, 10:01 a.m. UTC | #1
On 8/3/21 6:13 AM, Richard Henderson wrote:
> While we may have had some thought of allowing system-mode
> to return from this hook, we have no guests that require this.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/hw/core/tcg-cpu-ops.h  | 3 ++-
>  target/alpha/cpu.h             | 4 ++--
>  target/arm/internals.h         | 3 ++-
>  target/microblaze/cpu.h        | 2 +-
>  target/mips/tcg/tcg-internal.h | 4 ++--
>  target/nios2/cpu.h             | 4 ++--
>  target/ppc/internal.h          | 4 ++--
>  target/riscv/cpu.h             | 2 +-
>  target/s390x/s390x-internal.h  | 4 ++--
>  target/sh4/cpu.h               | 4 ++--
>  target/xtensa/cpu.h            | 4 ++--
>  target/hppa/cpu.c              | 7 ++++---
>  12 files changed, 24 insertions(+), 21 deletions(-)

> diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
> index 82df108967..6eb3fcc63e 100644
> --- a/target/alpha/cpu.h
> +++ b/target/alpha/cpu.h
> @@ -283,8 +283,8 @@ hwaddr alpha_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>  int alpha_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
>  int alpha_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>  void alpha_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
> -                                   MMUAccessType access_type,
> -                                   int mmu_idx, uintptr_t retaddr);
> +                                   MMUAccessType access_type, int mmu_idx,
> +                                   uintptr_t retaddr) QEMU_NORETURN;
>  
>  #define cpu_list alpha_cpu_list
>  #define cpu_signal_handler cpu_alpha_signal_handler
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index cd2ea8a388..3da9b1c61e 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -594,7 +594,8 @@ bool arm_s1_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx);
>  /* Raise a data fault alignment exception for the specified virtual address */
>  void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
>                                   MMUAccessType access_type,
> -                                 int mmu_idx, uintptr_t retaddr);
> +                                 int mmu_idx, uintptr_t retaddr)
> +    QEMU_NORETURN;

This one ended misaligned, I'd align as:

                                    QEMU_NORETURN;

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Alex Bennée Aug. 3, 2021, 3:47 p.m. UTC | #2
Richard Henderson <richard.henderson@linaro.org> writes:

> While we may have had some thought of allowing system-mode
> to return from this hook, we have no guests that require this.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/hw/core/tcg-cpu-ops.h  | 3 ++-
>  target/alpha/cpu.h             | 4 ++--
>  target/arm/internals.h         | 3 ++-
>  target/microblaze/cpu.h        | 2 +-
>  target/mips/tcg/tcg-internal.h | 4 ++--
>  target/nios2/cpu.h             | 4 ++--
>  target/ppc/internal.h          | 4 ++--
>  target/riscv/cpu.h             | 2 +-
>  target/s390x/s390x-internal.h  | 4 ++--
>  target/sh4/cpu.h               | 4 ++--
>  target/xtensa/cpu.h            | 4 ++--
>  target/hppa/cpu.c              | 7 ++++---
>  12 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index eab27d0c03..ee0795def4 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -72,10 +72,11 @@ struct TCGCPUOps {
>                                    MemTxResult response, uintptr_t retaddr);
>      /**
>       * @do_unaligned_access: Callback for unaligned access handling
> +     * The callback must exit via raising an exception.
>       */
>      void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
>                                  MMUAccessType access_type,
> -                                int mmu_idx, uintptr_t retaddr);
> +                                int mmu_idx, uintptr_t retaddr) QEMU_NORETURN;
>  
>      /**
>       * @adjust_watchpoint_address: hack for cpu_check_watchpoint used by ARM
> diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
> index 82df108967..6eb3fcc63e 100644
> --- a/target/alpha/cpu.h
> +++ b/target/alpha/cpu.h
> @@ -283,8 +283,8 @@ hwaddr alpha_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>  int alpha_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
>  int alpha_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>  void alpha_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
> -                                   MMUAccessType access_type,
> -                                   int mmu_idx, uintptr_t retaddr);
> +                                   MMUAccessType access_type, int mmu_idx,
> +                                   uintptr_t retaddr) QEMU_NORETURN;

These trailing QEMU_NORETURN's seem to be fairly uncommon in the
existing code. Indeed I'd glanced at this code and was about to suggest
one was added. IMHO is scans better when your reading the return type
for a function and you can always do:

  void QEMU_NORETURN
  foo_function(bar args);

if you are worried about over indentation. Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Richard Henderson Aug. 3, 2021, 6:02 p.m. UTC | #3
On 8/3/21 5:47 AM, Alex Bennée wrote:
> These trailing QEMU_NORETURN's seem to be fairly uncommon in the
> existing code.

Showing my age, I suppose.  Once upon a time it was the only place you *could* put it in a 
declaration (as opposed to definition).


r~
diff mbox series

Patch

diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index eab27d0c03..ee0795def4 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -72,10 +72,11 @@  struct TCGCPUOps {
                                   MemTxResult response, uintptr_t retaddr);
     /**
      * @do_unaligned_access: Callback for unaligned access handling
+     * The callback must exit via raising an exception.
      */
     void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
                                 MMUAccessType access_type,
-                                int mmu_idx, uintptr_t retaddr);
+                                int mmu_idx, uintptr_t retaddr) QEMU_NORETURN;
 
     /**
      * @adjust_watchpoint_address: hack for cpu_check_watchpoint used by ARM
diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
index 82df108967..6eb3fcc63e 100644
--- a/target/alpha/cpu.h
+++ b/target/alpha/cpu.h
@@ -283,8 +283,8 @@  hwaddr alpha_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 int alpha_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int alpha_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 void alpha_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
-                                   MMUAccessType access_type,
-                                   int mmu_idx, uintptr_t retaddr);
+                                   MMUAccessType access_type, int mmu_idx,
+                                   uintptr_t retaddr) QEMU_NORETURN;
 
 #define cpu_list alpha_cpu_list
 #define cpu_signal_handler cpu_alpha_signal_handler
diff --git a/target/arm/internals.h b/target/arm/internals.h
index cd2ea8a388..3da9b1c61e 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -594,7 +594,8 @@  bool arm_s1_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx);
 /* Raise a data fault alignment exception for the specified virtual address */
 void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
                                  MMUAccessType access_type,
-                                 int mmu_idx, uintptr_t retaddr);
+                                 int mmu_idx, uintptr_t retaddr)
+    QEMU_NORETURN;
 
 /* arm_cpu_do_transaction_failed: handle a memory system error response
  * (eg "no device/memory present at address") by raising an external abort
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index e4bba8a755..620c3742e1 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -359,7 +359,7 @@  void mb_cpu_do_interrupt(CPUState *cs);
 bool mb_cpu_exec_interrupt(CPUState *cs, int int_req);
 void mb_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
                                 MMUAccessType access_type,
-                                int mmu_idx, uintptr_t retaddr);
+                                int mmu_idx, uintptr_t retaddr) QEMU_NORETURN;
 void mb_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
 hwaddr mb_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
                                         MemTxAttrs *attrs);
diff --git a/target/mips/tcg/tcg-internal.h b/target/mips/tcg/tcg-internal.h
index 81b14eb219..7ac1e578d1 100644
--- a/target/mips/tcg/tcg-internal.h
+++ b/target/mips/tcg/tcg-internal.h
@@ -24,8 +24,8 @@  bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                        MMUAccessType access_type, int mmu_idx,
                        bool probe, uintptr_t retaddr);
 void mips_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
-                                  MMUAccessType access_type,
-                                  int mmu_idx, uintptr_t retaddr);
+                                  MMUAccessType access_type, int mmu_idx,
+                                  uintptr_t retaddr) QEMU_NORETURN;
 
 const char *mips_exception_name(int32_t exception);
 
diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
index 2ab82fdc71..27227b1e88 100644
--- a/target/nios2/cpu.h
+++ b/target/nios2/cpu.h
@@ -198,8 +198,8 @@  void dump_mmu(CPUNios2State *env);
 void nios2_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
 hwaddr nios2_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 void nios2_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
-                                   MMUAccessType access_type,
-                                   int mmu_idx, uintptr_t retaddr);
+                                   MMUAccessType access_type, int mmu_idx,
+                                   uintptr_t retaddr) QEMU_NORETURN;
 
 void do_nios2_semihosting(CPUNios2State *env);
 
diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index f1fd3c8d04..d2163bf5a2 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -213,8 +213,8 @@  void helper_compute_fprf_float128(CPUPPCState *env, float128 arg);
 
 /* Raise a data fault alignment exception for the specified virtual address */
 void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
-                                 MMUAccessType access_type,
-                                 int mmu_idx, uintptr_t retaddr);
+                                 MMUAccessType access_type, int mmu_idx,
+                                 uintptr_t retaddr) QEMU_NORETURN;
 
 /* translate.c */
 
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index bf1c899c00..a5b0047bfd 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -345,7 +345,7 @@  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
 hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
                                     MMUAccessType access_type, int mmu_idx,
-                                    uintptr_t retaddr);
+                                    uintptr_t retaddr) QEMU_NORETURN;
 bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                         MMUAccessType access_type, int mmu_idx,
                         bool probe, uintptr_t retaddr);
diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
index 5506f185e8..96133ac2b6 100644
--- a/target/s390x/s390x-internal.h
+++ b/target/s390x/s390x-internal.h
@@ -274,8 +274,8 @@  bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                        MMUAccessType access_type, int mmu_idx,
                        bool probe, uintptr_t retaddr);
 void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
-                                   MMUAccessType access_type,
-                                   int mmu_idx, uintptr_t retaddr);
+                                   MMUAccessType access_type, int mmu_idx,
+                                   uintptr_t retaddr) QEMU_NORETURN;
 
 
 /* fpu_helper.c */
diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
index 01c4344082..a9191951f8 100644
--- a/target/sh4/cpu.h
+++ b/target/sh4/cpu.h
@@ -211,8 +211,8 @@  hwaddr superh_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 int superh_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int superh_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 void superh_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
-                                    MMUAccessType access_type,
-                                    int mmu_idx, uintptr_t retaddr);
+                                    MMUAccessType access_type, int mmu_idx,
+                                    uintptr_t retaddr) QEMU_NORETURN;
 
 void sh4_translate_init(void);
 int cpu_sh4_signal_handler(int host_signum, void *pinfo,
diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index 2345cb59c7..aa9c77d719 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -579,8 +579,8 @@  void xtensa_count_regs(const XtensaConfig *config,
 int xtensa_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int xtensa_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 void xtensa_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
-                                    MMUAccessType access_type,
-                                    int mmu_idx, uintptr_t retaddr);
+                                    MMUAccessType access_type, int mmu_idx,
+                                    uintptr_t retaddr) QEMU_NORETURN;
 
 #define cpu_signal_handler cpu_xtensa_signal_handler
 #define cpu_list xtensa_cpu_list
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 2eace4ee12..c2c56e7635 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -72,9 +72,10 @@  static void hppa_cpu_disas_set_info(CPUState *cs, disassemble_info *info)
 }
 
 #ifndef CONFIG_USER_ONLY
-static void hppa_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
-                                         MMUAccessType access_type,
-                                         int mmu_idx, uintptr_t retaddr)
+static void QEMU_NORETURN
+hppa_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
+                             MMUAccessType access_type, int mmu_idx,
+                             uintptr_t retaddr)
 {
     HPPACPU *cpu = HPPA_CPU(cs);
     CPUHPPAState *env = &cpu->env;