Current recommended style is to log a guest error on bad register
accesses, not kill the whole system with hw_error(). Change the
hw_error() calls to log as LOG_GUEST_ERROR or LOG_UNIMP or use
g_assert_not_reached() as appropriate.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/stellaris.c | 60 +++++++++++++++++++++++++++++++++---------------------
1 file changed, 37 insertions(+), 23 deletions(-)
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index 9edcd49..ea7a809 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -108,7 +108,10 @@ static void gptm_reload(gptm_state *s, int n, int reset)
} else if (s->mode[n] == 0xa) {
/* PWM mode. Not implemented. */
} else {
- hw_error("TODO: 16-bit timer mode 0x%x\n", s->mode[n]);
+ qemu_log_mask(LOG_UNIMP,
+ "GPTM: 16-bit timer mode unimplemented: 0x%x\n",
+ s->mode[n]);
+ return;
}
s->tick[n] = tick;
timer_mod(s->timer[n], tick);
@@ -149,7 +152,9 @@ static void gptm_tick(void *opaque)
} else if (s->mode[n] == 0xa) {
/* PWM mode. Not implemented. */
} else {
- hw_error("TODO: 16-bit timer mode 0x%x\n", s->mode[n]);
+ qemu_log_mask(LOG_UNIMP,
+ "GPTM: 16-bit timer mode unimplemented: 0x%x\n",
+ s->mode[n]);
}
gptm_update_irq(s);
}
@@ -286,7 +291,8 @@ static void gptm_write(void *opaque, hwaddr offset,
s->match_prescale[0] = value;
break;
default:
- hw_error("gptm_write: Bad offset 0x%x\n", (int)offset);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "GPTM: read at bad offset 0x%x\n", (int)offset);
}
gptm_update_irq(s);
}
@@ -425,7 +431,10 @@ static int ssys_board_class(const ssys_state *s)
}
/* for unknown classes, fall through */
default:
- hw_error("ssys_board_class: Unknown class 0x%08x\n", did0);
+ /* This can only happen if the hardwired constant did0 value
+ * in this board's stellaris_board_info struct is wrong.
+ */
+ g_assert_not_reached();
}
}
@@ -479,8 +488,7 @@ static uint64_t ssys_read(void *opaque, hwaddr offset,
case DID0_CLASS_SANDSTORM:
return pllcfg_sandstorm[xtal];
default:
- hw_error("ssys_read: Unhandled class for PLLCFG read.\n");
- return 0;
+ g_assert_not_reached();
}
}
case 0x070: /* RCC2 */
@@ -512,7 +520,8 @@ static uint64_t ssys_read(void *opaque, hwaddr offset,
case 0x1e4: /* USER1 */
return s->user1;
default:
- hw_error("ssys_read: Bad offset 0x%x\n", (int)offset);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "SSYS: read at bad offset 0x%x\n", (int)offset);
return 0;
}
}
@@ -614,7 +623,8 @@ static void ssys_write(void *opaque, hwaddr offset,
s->ldoarst = value;
break;
default:
- hw_error("ssys_write: Bad offset 0x%x\n", (int)offset);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "SSYS: write at bad offset 0x%x\n", (int)offset);
}
ssys_update(s);
}
@@ -748,7 +758,8 @@ static uint64_t stellaris_i2c_read(void *opaque, hwaddr offset,
case 0x20: /* MCR */
return s->mcr;
default:
- hw_error("strllaris_i2c_read: Bad offset 0x%x\n", (int)offset);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "stellaris_i2c: read at bad offset 0x%x\n", (int)offset);
return 0;
}
}
@@ -823,17 +834,18 @@ static void stellaris_i2c_write(void *opaque, hwaddr offset,
s->mris &= ~value;
break;
case 0x20: /* MCR */
- if (value & 1)
- hw_error(
- "stellaris_i2c_write: Loopback not implemented\n");
- if (value & 0x20)
- hw_error(
- "stellaris_i2c_write: Slave mode not implemented\n");
+ if (value & 1) {
+ qemu_log_mask(LOG_UNIMP, "stellaris_i2c: Loopback not implemented");
+ }
+ if (value & 0x20) {
+ qemu_log_mask(LOG_UNIMP,
+ "stellaris_i2c: Slave mode not implemented");
+ }
s->mcr = value & 0x31;
break;
default:
- hw_error("stellaris_i2c_write: Bad offset 0x%x\n",
- (int)offset);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "stellaris_i2c: write at bad offset 0x%x\n", (int)offset);
}
stellaris_i2c_update(s);
}
@@ -1057,8 +1069,8 @@ static uint64_t stellaris_adc_read(void *opaque, hwaddr offset,
case 0x30: /* SAC */
return s->sac;
default:
- hw_error("strllaris_adc_read: Bad offset 0x%x\n",
- (int)offset);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "stellaris_adc: read at bad offset 0x%x\n", (int)offset);
return 0;
}
}
@@ -1078,8 +1090,9 @@ static void stellaris_adc_write(void *opaque, hwaddr offset,
return;
case 0x04: /* SSCTL */
if (value != 6) {
- hw_error("ADC: Unimplemented sequence %" PRIx64 "\n",
- value);
+ qemu_log_mask(LOG_UNIMP,
+ "ADC: Unimplemented sequence %" PRIx64 "\n",
+ value);
}
s->ssctl[n] = value;
return;
@@ -1110,13 +1123,14 @@ static void stellaris_adc_write(void *opaque, hwaddr offset,
s->sspri = value;
break;
case 0x28: /* PSSI */
- hw_error("Not implemented: ADC sample initiate\n");
+ qemu_log_mask(LOG_UNIMP, "ADC: sample initiate unimplemented");
break;
case 0x30: /* SAC */
s->sac = value;
break;
default:
- hw_error("stellaris_adc_write: Bad offset 0x%x\n", (int)offset);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "stellaris_adc: write at bad offset 0x%x\n", (int)offset);
}
stellaris_adc_update(s);
}
--
2.7.4
Hi Peter,
On 04/06/2017 10:45 AM, Peter Maydell wrote:
> Current recommended style is to log a guest error on bad register
> accesses, not kill the whole system with hw_error(). Change the
> hw_error() calls to log as LOG_GUEST_ERROR or LOG_UNIMP or use
> g_assert_not_reached() as appropriate.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/stellaris.c | 60 +++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 37 insertions(+), 23 deletions(-)
>
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index 9edcd49..ea7a809 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -108,7 +108,10 @@ static void gptm_reload(gptm_state *s, int n, int reset)
> } else if (s->mode[n] == 0xa) {
> /* PWM mode. Not implemented. */
> } else {
> - hw_error("TODO: 16-bit timer mode 0x%x\n", s->mode[n]);
> + qemu_log_mask(LOG_UNIMP,
> + "GPTM: 16-bit timer mode unimplemented: 0x%x\n",
> + s->mode[n]);
> + return;
> }
> s->tick[n] = tick;
> timer_mod(s->timer[n], tick);
> @@ -149,7 +152,9 @@ static void gptm_tick(void *opaque)
> } else if (s->mode[n] == 0xa) {
> /* PWM mode. Not implemented. */
> } else {
> - hw_error("TODO: 16-bit timer mode 0x%x\n", s->mode[n]);
> + qemu_log_mask(LOG_UNIMP,
> + "GPTM: 16-bit timer mode unimplemented: 0x%x\n",
> + s->mode[n]);
> }
> gptm_update_irq(s);
> }
> @@ -286,7 +291,8 @@ static void gptm_write(void *opaque, hwaddr offset,
> s->match_prescale[0] = value;
> break;
> default:
> - hw_error("gptm_write: Bad offset 0x%x\n", (int)offset);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "GPTM: read at bad offset 0x%x\n", (int)offset);
use HWADDR_PRIx to remove this unnecessary casts here in following changes?
ie: "GPTM: read at bad offset 0x%" HWADDR_PRIx "\n", offset);
either way:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> }
> gptm_update_irq(s);
> }
> @@ -425,7 +431,10 @@ static int ssys_board_class(const ssys_state *s)
> }
> /* for unknown classes, fall through */
> default:
> - hw_error("ssys_board_class: Unknown class 0x%08x\n", did0);
> + /* This can only happen if the hardwired constant did0 value
> + * in this board's stellaris_board_info struct is wrong.
> + */
> + g_assert_not_reached();
> }
> }
>
> @@ -479,8 +488,7 @@ static uint64_t ssys_read(void *opaque, hwaddr offset,
> case DID0_CLASS_SANDSTORM:
> return pllcfg_sandstorm[xtal];
> default:
> - hw_error("ssys_read: Unhandled class for PLLCFG read.\n");
> - return 0;
> + g_assert_not_reached();
> }
> }
> case 0x070: /* RCC2 */
> @@ -512,7 +520,8 @@ static uint64_t ssys_read(void *opaque, hwaddr offset,
> case 0x1e4: /* USER1 */
> return s->user1;
> default:
> - hw_error("ssys_read: Bad offset 0x%x\n", (int)offset);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "SSYS: read at bad offset 0x%x\n", (int)offset);
> return 0;
> }
> }
> @@ -614,7 +623,8 @@ static void ssys_write(void *opaque, hwaddr offset,
> s->ldoarst = value;
> break;
> default:
> - hw_error("ssys_write: Bad offset 0x%x\n", (int)offset);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "SSYS: write at bad offset 0x%x\n", (int)offset);
> }
> ssys_update(s);
> }
> @@ -748,7 +758,8 @@ static uint64_t stellaris_i2c_read(void *opaque, hwaddr offset,
> case 0x20: /* MCR */
> return s->mcr;
> default:
> - hw_error("strllaris_i2c_read: Bad offset 0x%x\n", (int)offset);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "stellaris_i2c: read at bad offset 0x%x\n", (int)offset);
> return 0;
> }
> }
> @@ -823,17 +834,18 @@ static void stellaris_i2c_write(void *opaque, hwaddr offset,
> s->mris &= ~value;
> break;
> case 0x20: /* MCR */
> - if (value & 1)
> - hw_error(
> - "stellaris_i2c_write: Loopback not implemented\n");
> - if (value & 0x20)
> - hw_error(
> - "stellaris_i2c_write: Slave mode not implemented\n");
> + if (value & 1) {
> + qemu_log_mask(LOG_UNIMP, "stellaris_i2c: Loopback not implemented");
> + }
> + if (value & 0x20) {
> + qemu_log_mask(LOG_UNIMP,
> + "stellaris_i2c: Slave mode not implemented");
> + }
> s->mcr = value & 0x31;
> break;
> default:
> - hw_error("stellaris_i2c_write: Bad offset 0x%x\n",
> - (int)offset);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "stellaris_i2c: write at bad offset 0x%x\n", (int)offset);
> }
> stellaris_i2c_update(s);
> }
> @@ -1057,8 +1069,8 @@ static uint64_t stellaris_adc_read(void *opaque, hwaddr offset,
> case 0x30: /* SAC */
> return s->sac;
> default:
> - hw_error("strllaris_adc_read: Bad offset 0x%x\n",
> - (int)offset);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "stellaris_adc: read at bad offset 0x%x\n", (int)offset);
> return 0;
> }
> }
> @@ -1078,8 +1090,9 @@ static void stellaris_adc_write(void *opaque, hwaddr offset,
> return;
> case 0x04: /* SSCTL */
> if (value != 6) {
> - hw_error("ADC: Unimplemented sequence %" PRIx64 "\n",
> - value);
> + qemu_log_mask(LOG_UNIMP,
> + "ADC: Unimplemented sequence %" PRIx64 "\n",
> + value);
> }
> s->ssctl[n] = value;
> return;
> @@ -1110,13 +1123,14 @@ static void stellaris_adc_write(void *opaque, hwaddr offset,
> s->sspri = value;
> break;
> case 0x28: /* PSSI */
> - hw_error("Not implemented: ADC sample initiate\n");
> + qemu_log_mask(LOG_UNIMP, "ADC: sample initiate unimplemented");
> break;
> case 0x30: /* SAC */
> s->sac = value;
> break;
> default:
> - hw_error("stellaris_adc_write: Bad offset 0x%x\n", (int)offset);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "stellaris_adc: write at bad offset 0x%x\n", (int)offset);
> }
> stellaris_adc_update(s);
> }
>
On 6 April 2017 at 17:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi Peter,
>
>
> On 04/06/2017 10:45 AM, Peter Maydell wrote:
>> default:
>> - hw_error("gptm_write: Bad offset 0x%x\n", (int)offset);
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "GPTM: read at bad offset 0x%x\n", (int)offset);
>
>
> use HWADDR_PRIx to remove this unnecessary casts here in following changes?
>
> ie: "GPTM: read at bad offset 0x%" HWADDR_PRIx "\n", offset);
I don't think either of the two is clearly better for this
sort of case where the offset is known to be small, so I opted
to leave the code the way it was already written.
> either way:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Thanks.
-- PMM
© 2016 - 2024 Red Hat, Inc.