[PATCH v6 3/5] util: [ppc] Use new error_report_fatal() instead of exit()

[PATCH v6 3/5] util: [ppc] Use new error_report_fatal() instead of exit()

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 target-ppc/kvm.c            |    5 +--
 target-ppc/translate.c      |    7 ++--
 target-ppc/translate_init.c |   80 +++++++++++++++++++++----------------------
 3 files changed, 44 insertions(+), 48 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 70ca296..1bb87e6 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -316,9 +316,8 @@ static long gethugepagesize(const char *mem_path)
     } while (ret != 0 && errno == EINTR);
 
     if (ret != 0) {
-        fprintf(stderr, "Couldn't statfs() memory path: %s\n",
-                strerror(errno));
-        exit(1);
+        error_report_fatal("Couldn't statfs() memory path: %s",
+                           strerror(errno));
     }
 
 #define HUGETLBFS_MAGIC       0x958458f6
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 7db3145..546b21d 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -11588,10 +11588,9 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
             break;
         }
         if (tcg_check_temp_count()) {
-            fprintf(stderr, "Opcode %02x %02x %02x (%08x) leaked temporaries\n",
-                    opc1(ctx.opcode), opc2(ctx.opcode), opc3(ctx.opcode),
-                    ctx.opcode);
-            exit(1);
+            error_report_fatal("Opcode %02x %02x %02x (%08x) leaked temporaries",
+                               opc1(ctx.opcode), opc2(ctx.opcode), opc3(ctx.opcode),
+                               ctx.opcode);
         }
     }
     if (tb->cflags & CF_LAST_IO)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index cdd18ac..d66be51 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -620,8 +620,8 @@ static inline void _spr_register(CPUPPCState *env, int num,
         spr->oea_read != NULL || spr->oea_write != NULL ||
 #endif
         spr->uea_read != NULL || spr->uea_write != NULL) {
-        printf("Error: Trying to register SPR %d (%03x) twice !\n", num, num);
-        exit(1);
+        error_report_fatal("Error: Trying to register SPR %d (%03x) twice !",
+                           num, num);
     }
 #if defined(PPC_DEBUG_SPR)
     printf("*** register spr %d (%03x) %s val " TARGET_FMT_lx "\n", num, num,
@@ -1609,8 +1609,7 @@ static void gen_spr_BookE (CPUPPCState *env, uint64_t ivor_mask)
     for (i = 0; i < 64; i++) {
         if (ivor_mask & (1ULL << i)) {
             if (ivor_sprn[i] == SPR_BOOKE_IVORxx) {
-                fprintf(stderr, "ERROR: IVOR %d SPR is not defined\n", i);
-                exit(1);
+                error_report_fatal("ERROR: IVOR %d SPR is not defined", i);
             }
             spr_register(env, ivor_sprn[i], ivor_names[i],
                          SPR_NOACCESS, SPR_NOACCESS,
@@ -8352,14 +8351,14 @@ static void init_ppc_proc(PowerPCCPU *cpu)
         case POWERPC_FLAG_VRE:
             break;
         default:
-            fprintf(stderr, "PowerPC MSR definition inconsistency\n"
-                    "Should define POWERPC_FLAG_SPE or POWERPC_FLAG_VRE\n");
-            exit(1);
+            error_report("PowerPC MSR definition inconsistency");
+            error_report_fatal(
+                "Should define POWERPC_FLAG_SPE or POWERPC_FLAG_VRE");
         }
     } else if (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
-        fprintf(stderr, "PowerPC MSR definition inconsistency\n"
-                "Should not define POWERPC_FLAG_SPE nor POWERPC_FLAG_VRE\n");
-        exit(1);
+        error_report("PowerPC MSR definition inconsistency");
+        error_report_fatal(
+            "Should not define POWERPC_FLAG_SPE nor POWERPC_FLAG_VRE");
     }
     if (env->msr_mask & (1 << 17)) {
         switch (env->flags & (POWERPC_FLAG_TGPR | POWERPC_FLAG_CE)) {
@@ -8367,14 +8366,14 @@ static void init_ppc_proc(PowerPCCPU *cpu)
         case POWERPC_FLAG_CE:
             break;
         default:
-            fprintf(stderr, "PowerPC MSR definition inconsistency\n"
-                    "Should define POWERPC_FLAG_TGPR or POWERPC_FLAG_CE\n");
-            exit(1);
+            error_report("PowerPC MSR definition inconsistency");
+            error_report_fatal(
+                "Should define POWERPC_FLAG_TGPR or POWERPC_FLAG_CE");
         }
     } else if (env->flags & (POWERPC_FLAG_TGPR | POWERPC_FLAG_CE)) {
-        fprintf(stderr, "PowerPC MSR definition inconsistency\n"
-                "Should not define POWERPC_FLAG_TGPR nor POWERPC_FLAG_CE\n");
-        exit(1);
+        error_report("PowerPC MSR definition inconsistency");
+        error_report_fatal(
+            "Should not define POWERPC_FLAG_TGPR nor POWERPC_FLAG_CE");
     }
     if (env->msr_mask & (1 << 10)) {
         switch (env->flags & (POWERPC_FLAG_SE | POWERPC_FLAG_DWE |
@@ -8384,17 +8383,17 @@ static void init_ppc_proc(PowerPCCPU *cpu)
         case POWERPC_FLAG_UBLE:
             break;
         default:
-            fprintf(stderr, "PowerPC MSR definition inconsistency\n"
-                    "Should define POWERPC_FLAG_SE or POWERPC_FLAG_DWE or "
-                    "POWERPC_FLAG_UBLE\n");
-            exit(1);
+            error_report("PowerPC MSR definition inconsistency");
+            error_report_fatal(
+                "Should define POWERPC_FLAG_SE or POWERPC_FLAG_DWE or "
+                "POWERPC_FLAG_UBLE");
         }
     } else if (env->flags & (POWERPC_FLAG_SE | POWERPC_FLAG_DWE |
                              POWERPC_FLAG_UBLE)) {
-        fprintf(stderr, "PowerPC MSR definition inconsistency\n"
-                "Should not define POWERPC_FLAG_SE nor POWERPC_FLAG_DWE nor "
-                "POWERPC_FLAG_UBLE\n");
-            exit(1);
+        error_report("PowerPC MSR definition inconsistency");
+        error_report_fatal(
+            "Should not define POWERPC_FLAG_SE nor POWERPC_FLAG_DWE nor "
+            "POWERPC_FLAG_UBLE");
     }
     if (env->msr_mask & (1 << 9)) {
         switch (env->flags & (POWERPC_FLAG_BE | POWERPC_FLAG_DE)) {
@@ -8402,14 +8401,14 @@ static void init_ppc_proc(PowerPCCPU *cpu)
         case POWERPC_FLAG_DE:
             break;
         default:
-            fprintf(stderr, "PowerPC MSR definition inconsistency\n"
-                    "Should define POWERPC_FLAG_BE or POWERPC_FLAG_DE\n");
-            exit(1);
+            error_report("PowerPC MSR definition inconsistency");
+            error_report_fatal(
+                "Should define POWERPC_FLAG_BE or POWERPC_FLAG_DE");
         }
     } else if (env->flags & (POWERPC_FLAG_BE | POWERPC_FLAG_DE)) {
-        fprintf(stderr, "PowerPC MSR definition inconsistency\n"
-                "Should not define POWERPC_FLAG_BE nor POWERPC_FLAG_DE\n");
-        exit(1);
+        error_report("PowerPC MSR definition inconsistency");
+        error_report_fatal(
+            "Should not define POWERPC_FLAG_BE nor POWERPC_FLAG_DE");
     }
     if (env->msr_mask & (1 << 2)) {
         switch (env->flags & (POWERPC_FLAG_PX | POWERPC_FLAG_PMM)) {
@@ -8417,19 +8416,19 @@ static void init_ppc_proc(PowerPCCPU *cpu)
         case POWERPC_FLAG_PMM:
             break;
         default:
-            fprintf(stderr, "PowerPC MSR definition inconsistency\n"
-                    "Should define POWERPC_FLAG_PX or POWERPC_FLAG_PMM\n");
-            exit(1);
+            error_report("PowerPC MSR definition inconsistency");
+            error_report_fatal(
+                "Should define POWERPC_FLAG_PX or POWERPC_FLAG_PMM");
         }
     } else if (env->flags & (POWERPC_FLAG_PX | POWERPC_FLAG_PMM)) {
-        fprintf(stderr, "PowerPC MSR definition inconsistency\n"
-                "Should not define POWERPC_FLAG_PX nor POWERPC_FLAG_PMM\n");
-        exit(1);
+        error_report("PowerPC MSR definition inconsistency");
+        error_report_fatal(
+            "Should not define POWERPC_FLAG_PX nor POWERPC_FLAG_PMM");
     }
     if ((env->flags & (POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_BUS_CLK)) == 0) {
-        fprintf(stderr, "PowerPC flags inconsistency\n"
-                "Should define the time-base and decrementer clock source\n");
-        exit(1);
+        error_report("PowerPC flags inconsistency");
+        error_report_fatal(
+            "Should define the time-base and decrementer clock source");
     }
     /* Allocate TLBs buffer when needed */
 #if !defined(CONFIG_USER_ONLY)
@@ -9655,8 +9654,7 @@ static void ppc_cpu_reset(CPUState *s)
 #if !defined(TARGET_WORDS_BIGENDIAN)
     msr |= (target_ulong)1 << MSR_LE; /* Little-endian user mode */
     if (!((env->msr_mask >> MSR_LE) & 1)) {
-        fprintf(stderr, "Selected CPU does not support little-endian.\n");
-        exit(1);
+        error_report_fatal("Selected CPU does not support little-endian.");
     }
 #endif
 #endif

[PATCH v6 4/5] util: [ppc] Use new error_report_abort() instead of abort()

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 target-ppc/kvm.c        |    4 ++--
 target-ppc/kvm_ppc.h    |   15 +++++++++------
 target-ppc/mmu-hash32.c |    5 +++--
 target-ppc/mmu_helper.c |    3 +--
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 1bb87e6..b743811 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -587,7 +587,7 @@ static void kvm_get_one_spr(CPUState *cs, uint64_t id, int spr)
 
         default:
             /* Don't handle this size yet */
-            abort();
+            error_report_abort("Unhandled size: %d", id & KVM_REG_SIZE_MASK);
         }
     }
 }
@@ -617,7 +617,7 @@ static void kvm_put_one_spr(CPUState *cs, uint64_t id, int spr)
 
     default:
         /* Don't handle this size yet */
-        abort();
+        error_report_abort("Unhandled size: %d", id & KVM_REG_SIZE_MASK);
     }
 
     ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 62406ce..f82582e 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -9,6 +9,9 @@
 #ifndef __KVM_PPC_H__
 #define __KVM_PPC_H__
 
+#include "qemu/error-report.h"
+
+
 #define TYPE_HOST_POWERPC_CPU "host-" TYPE_POWERPC_CPU
 
 #ifdef CONFIG_KVM
@@ -215,36 +218,36 @@ static inline int kvmppc_get_htab_fd(bool write)
 static inline int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize,
                                    int64_t max_ns)
 {
-    abort();
+    error_report_abort(" ");
 }
 
 static inline int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
                                          uint16_t n_valid, uint16_t n_invalid)
 {
-    abort();
+    error_report_abort(" ");
 }
 
 static inline uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu,
                                                target_ulong pte_index)
 {
-    abort();
+    error_report_abort(" ");
 }
 
 static inline void kvmppc_hash64_free_pteg(uint64_t token)
 {
-    abort();
+    error_report_abort(" ");
 }
 
 static inline void kvmppc_hash64_write_pte(CPUPPCState *env,
                                            target_ulong pte_index,
                                            target_ulong pte0, target_ulong pte1)
 {
-    abort();
+    error_report_abort(" ");
 }
 
 static inline bool kvmppc_has_cap_fixup_hcalls(void)
 {
-    abort();
+    error_report_abort(" ");
 }
 
 static inline int kvmppc_enable_hwrng(void)
diff --git a/target-ppc/mmu-hash32.c b/target-ppc/mmu-hash32.c
index b076d80..75e593a 100644
--- a/target-ppc/mmu-hash32.c
+++ b/target-ppc/mmu-hash32.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/helper-proto.h"
+#include "qemu/error-report.h"
 #include "sysemu/kvm.h"
 #include "kvm_ppc.h"
 #include "mmu-hash32.h"
@@ -56,7 +57,7 @@ static int ppc_hash32_pp_prot(int key, int pp, int nx)
             break;
 
         default:
-            abort();
+            error_report_abort("Unhandled pp: %d", pp);
         }
     } else {
         switch (pp) {
@@ -74,7 +75,7 @@ static int ppc_hash32_pp_prot(int key, int pp, int nx)
             break;
 
         default:
-            abort();
+            error_report_abort("Unhandled pp: %d", pp);
         }
     }
     if (nx == 0) {
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index de4e286..6e50e78 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -1324,8 +1324,7 @@ static inline int check_physical(CPUPPCState *env, mmu_ctx_t *ctx,
 
     default:
         /* Caller's checks mean we should never get here for other models */
-        abort();
-        return -1;
+        error_report_abort("Unhandled MMU model: %d", env->mmu_model);
     }
 
     return ret;

Re: [PATCH v6 4/5] util: [ppc] Use new error_report_abort() instead of abort()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

On 02/02/2016 09:14 AM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  target-ppc/kvm.c        |    4 ++--
>  target-ppc/kvm_ppc.h    |   15 +++++++++------
>  target-ppc/mmu-hash32.c |    5 +++--
>  target-ppc/mmu_helper.c |    3 +--
>  4 files changed, 15 insertions(+), 12 deletions(-)
> 

> @@ -215,36 +218,36 @@ static inline int kvmppc_get_htab_fd(bool write)
>  static inline int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize,
>                                     int64_t max_ns)
>  {
> -    abort();
> +    error_report_abort(" ");
>  }

I still strongly dislike this.  Reporting an error message with nothing
but a trailing whitespace is NOT helpful.  Straight 'abort()' is better
than this; if we can't come up with a useful message, then it is not
worth using the message-handling interface.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [PATCH v6 4/5] util: [ppc] Use new error_report_abort() instead of abort()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2071 bytes --]

On Tue, Feb 02, 2016 at 05:14:15PM +0100, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  target-ppc/kvm.c        |    4 ++--
>  target-ppc/kvm_ppc.h    |   15 +++++++++------
>  target-ppc/mmu-hash32.c |    5 +++--
>  target-ppc/mmu_helper.c |    3 +--
>  4 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 1bb87e6..b743811 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -587,7 +587,7 @@ static void kvm_get_one_spr(CPUState *cs, uint64_t id, int spr)
>  
>          default:
>              /* Don't handle this size yet */
> -            abort();
> +            error_report_abort("Unhandled size: %d", id & KVM_REG_SIZE_MASK);
>          }
>      }
>  }
> @@ -617,7 +617,7 @@ static void kvm_put_one_spr(CPUState *cs, uint64_t id, int spr)
>  
>      default:
>          /* Don't handle this size yet */
> -        abort();
> +        error_report_abort("Unhandled size: %d", id & KVM_REG_SIZE_MASK);
>      }
>  
>      ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 62406ce..f82582e 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -9,6 +9,9 @@
>  #ifndef __KVM_PPC_H__
>  #define __KVM_PPC_H__
>  
> +#include "qemu/error-report.h"
> +
> +
>  #define TYPE_HOST_POWERPC_CPU "host-" TYPE_POWERPC_CPU
>  
>  #ifdef CONFIG_KVM
> @@ -215,36 +218,36 @@ static inline int kvmppc_get_htab_fd(bool write)
>  static inline int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize,
>                                     int64_t max_ns)
>  {
> -    abort();
> +    error_report_abort(" ");

Nack.  The empty message report is just awful.  I don't think we
should feel the need to get rid of every bare abort() or assert().

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]