Skip to contentSkip to conversation

Remove hw_error from target-arm

PATCH Superseded Peter Crosthwaite 2 participants 5 messages Jul 18, 2015

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover Remove hw_error from target-arm

Hi Peter,

This removes the last of the hw_errors from target-arm.

Regards,
Peter

Peter Crosthwaite (2):
arm: cpu: Soften no-EL2 virt IRQ error condition.
arm: Remove hw_error() usages.

 target-arm/cpu.c    | 10 ++++++----
 target-arm/helper.c |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/2 arm: cpu: Soften no-EL2 virt IRQ error condition.

Replace the hw_error for no-EL2 VIRQ with a LOG_UNIMP. This is more accurate and handles the corner case where the user defeatures EL2 using overrides in a system that would otherwise have EL2 connectivity.

---
I'm primarily interested in getting rid of the hw_error() more than anything. If my use-case seems invalid and a stronger error is preffered, it can be redone as an assert and the code will still work for me.

---
 target-arm/cpu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
target-arm/cpu.c +4 -2
index 29b7eca..c4b8448 100644
@@ -310,8 +310,10 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
     case ARM_CPU_VIRQ:
     case ARM_CPU_VFIQ:
         if (!arm_feature(env, ARM_FEATURE_EL2)) {
-            hw_error("%s: Virtual interrupt line %d with no EL2 support\n",
-                     __func__, irq);
+            qemu_log_mask(LOG_UNIMP,
+                          "%s: Virtual interrupt line %d with no EL2 support\n",
+                          __func__, irq);
+            return;
         }
         /* fall through */
     case ARM_CPU_IRQ:
Signature · 1 line
1.9.1
#2 lore raw

Re: [Qemu-devel] [PATCH 1/2] arm: cpu: Soften no-EL2 virt IRQ error condition.

Quoted 20 lines · Peter Crosthwaite

Replace the hw_error for no-EL2 VIRQ with a LOG_UNIMP. This is more accurate and handles the corner case where the user defeatures EL2 using overrides in a system that would otherwise have EL2 connectivity.

---
I'm primarily interested in getting rid of the hw_error() more than anything. If my use-case seems invalid and a stronger error is preffered, it can be redone as an assert and the code will still work for me.

---
 target-arm/cpu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
target-arm/cpu.c +0 -0
index 29b7eca..c4b8448 100644

target-arm/cpu.c static void arm_cpu_set_irq(void *opaque, int irq, int level) · line 316

Quoted 9 lines
target-arm/cpu.c +4 -2
@@ -310,8 +310,10 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
     case ARM_CPU_VIRQ:
     case ARM_CPU_VFIQ:
         if (!arm_feature(env, ARM_FEATURE_EL2)) {
-            hw_error("%s: Virtual interrupt line %d with no EL2 support\n",
-                     __func__, irq);
+            qemu_log_mask(LOG_UNIMP,
+                          "%s: Virtual interrupt line %d with no EL2 support\n",
+                          __func__, irq);
+            return;

My inclination is to say we should assert. If the board model has put together a system where it creates a CPU with no EL2 support and then wires up the VIRQ/VFIQ lines to something, then that sounds like a board model bug to me. LOG_UNIMP is for "this is legitimate guest behaviour but we haven't written the code to support it".

thanks
-- PMM

#3 lore raw

PATCH 2/2 arm: Remove hw_error() usages.

In reply to Peter Crosthwaite

All of these hw_errors are fatal and indicate something wrong with QEMU implementation.

Convert to g_assert_not_reached.

---
Dropped the error messages as they are not hugely useful outside of a debug flow.

---
 target-arm/cpu.c    | 4 ++--
 target-arm/helper.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
target-arm/cpu.c +2 -2
index c4b8448..8b5471a 100644
@@ -325,7 +325,7 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
         }
         break;
     default:
-        hw_error("arm_cpu_set_irq: Bad interrupt line %d\n", irq);
+        g_assert_not_reached();
     }
 }
 
@@ -344,7 +344,7 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level)
         kvm_irq |= KVM_ARM_IRQ_CPU_FIQ;
         break;
     default:
-        hw_error("arm_cpu_kvm_set_irq: Bad interrupt line %d\n", irq);
+        g_assert_not_reached();
     }
     kvm_irq |= cs->cpu_index << KVM_ARM_IRQ_VCPU_SHIFT;
     kvm_set_irq(kvm_state, kvm_irq, level ? 1 : 0);
target-arm/helper.c +1 -1
index b2f3db9..662e2bb 100644
@@ -3997,7 +3997,7 @@ int bank_number(int mode)
     case ARM_CPU_MODE_MON:
         return 7;
     }
-    hw_error("bank number requested for bad CPSR mode value 0x%x\n", mode);
+    g_assert_not_reached();
 }
 
 void switch_mode(CPUARMState *env, int mode)
Signature · 1 line
1.9.1
Reviewed-by on v1 2/2
#4 lore raw

Re: [Qemu-devel] [PATCH 2/2] arm: Remove hw_error() usages.

Quoted 28 lines · Peter Crosthwaite

All of these hw_errors are fatal and indicate something wrong with QEMU implementation.

Convert to g_assert_not_reached.

---
Dropped the error messages as they are not hugely useful outside of a debug flow.

---
 target-arm/cpu.c    | 4 ++--
 target-arm/helper.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
target-arm/cpu.c +1 -1
index c4b8448..8b5471a 100644
@@ -325,7 +325,7 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
         }
         break;
     default:
-        hw_error("arm_cpu_set_irq: Bad interrupt line %d\n", irq);
+        g_assert_not_reached();
     }
 }
 

target-arm/cpu.c static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level) · line 347

Quoted 5 lines
target-arm/cpu.c +1 -1
@@ -344,7 +344,7 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level)
         kvm_irq |= KVM_ARM_IRQ_CPU_FIQ;
         break;
     default:
-        hw_error("arm_cpu_kvm_set_irq: Bad interrupt line %d\n", irq);
+        g_assert_not_reached();

We should be consistent between this and arm_cpu_set_irq() about whether we assert or LOG_UNIMP the VIRQ/VFIQ cases. I'm happy for them all to assert.

Quoted 8 lines
target-arm/cpu.c +0 -0
     }
     kvm_irq |= cs->cpu_index << KVM_ARM_IRQ_VCPU_SHIFT;
     kvm_set_irq(kvm_state, kvm_irq, level ? 1 : 0);
target-arm/helper.c +0 -0
index b2f3db9..662e2bb 100644

target-arm/helper.c int bank_number(int mode) · line 4003

Quoted 8 lines
target-arm/helper.c +1 -1
@@ -3997,7 +3997,7 @@ int bank_number(int mode)
     case ARM_CPU_MODE_MON:
         return 7;
     }
-    hw_error("bank number requested for bad CPSR mode value 0x%x\n", mode);
+    g_assert_not_reached();
 }
 
 void switch_mode(CPUARMState *env, int mode)

-- PMM