On 18/6/24 15:55, Zheyu Ma wrote:
This commit adds validation checks for the MCOPRE and MCOSEL values in
the rcc_update_cfgr_register function. If the MCOPRE value exceeds
0b100 or the MCOSEL value exceeds 0b111, an error is logged and the
corresponding clock mux is disabled. This helps in identifying and
handling invalid configurations in the RCC registers.
Reproducer:
cat << EOF | qemu-system-aarch64 -display \
none -machine accel=qtest, -m 512M -machine b-l475e-iot01a -qtest \
stdio
writeq 0x40021008 0xffffffff
EOF
Signed-off-by: Zheyu Ma <[email protected]>
---
hw/misc/stm32l4x5_rcc.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/hw/misc/stm32l4x5_rcc.c b/hw/misc/stm32l4x5_rcc.c
index 417bd5e85f..59d428fa66 100644
--- a/hw/misc/stm32l4x5_rcc.c
+++ b/hw/misc/stm32l4x5_rcc.c
@@ -543,19 +543,31 @@ static void rcc_update_cfgr_register(Stm32l4x5RccState *s)
uint32_t val;
/* MCOPRE */
val = FIELD_EX32(s->cfgr, CFGR, MCOPRE);
- assert(val <= 0b100);
- clock_mux_set_factor(&s->clock_muxes[RCC_CLOCK_MUX_MCO],
- 1, 1 << val);
+ if (val > 0b100) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Invalid MCOPRE value: 0x%"PRIx32"\n",
+ __func__, val);
How the hardware handles that? I suppose it just ignores the
value, so I'd simply:
return;
instead of disabling the clock...
Inès, Arnaud, what do you think?
+ clock_mux_set_enable(&s->clock_muxes[RCC_CLOCK_MUX_MCO], false);
+ } else {
+ clock_mux_set_factor(&s->clock_muxes[RCC_CLOCK_MUX_MCO],
+ 1, 1 << val);
+ }
/* MCOSEL */
val = FIELD_EX32(s->cfgr, CFGR, MCOSEL);
MCOSEL is 3-bit wide, so ...
- assert(val <= 0b111);
... this condition is always true (and can be removed) ...
- if (val == 0) {
+ if (val > 0b111) {
... and this path isn't reachable IIUC. So you added dead code.
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Invalid MCOSEL value: 0x%"PRIx32"\n",
+ __func__, val);
clock_mux_set_enable(&s->clock_muxes[RCC_CLOCK_MUX_MCO], false);
} else {
- clock_mux_set_enable(&s->clock_muxes[RCC_CLOCK_MUX_MCO], true);
- clock_mux_set_source(&s->clock_muxes[RCC_CLOCK_MUX_MCO],
- val - 1);
+ if (val == 0) {
+ clock_mux_set_enable(&s->clock_muxes[RCC_CLOCK_MUX_MCO], false);
+ } else {
+ clock_mux_set_enable(&s->clock_muxes[RCC_CLOCK_MUX_MCO], true);
+ clock_mux_set_source(&s->clock_muxes[RCC_CLOCK_MUX_MCO],
+ val - 1);
+ }
}
/* STOPWUCK */
