On Sun, 3 Mar 2024 at 14:08, Arnaud Minier <arnaud.min...@telecom-paris.fr> wrote: > > Update the RCC state and propagate frequency changes when writing to the > RCC registers. Currently, ICSCR, CIER, the reset registers and the stop > mode registers are not implemented. > > Some fields have not been implemented due to uncertainty about > how to handle them (Like the clock security system or bypassing > mecanisms). > > Signed-off-by: Arnaud Minier <arnaud.min...@telecom-paris.fr> > Signed-off-by: Inès Varhol <ines.var...@telecom-paris.fr>
Hi; somebody has reported a bug in this change, which they found using a fuzzer: https://gitlab.com/qemu-project/qemu/-/issues/2356 > +static void rcc_update_cfgr_register(Stm32l4x5RccState *s) > +{ > + uint32_t val; > + /* MCOPRE */ > + val = FIELD_EX32(s->cfgr, CFGR, MCOPRE); > + assert(val <= 0b100); You can't assert() things about guest register values, because then if the guest writes that value QEMU will fall over. For "this is something the spec says is invalid", the right thing to do in a device model is to qemu_log_mask(LOG_GUEST_ERROR, ...) the situation, and proceed as best you can (eg treat the value as if it was some valid one, or disable the clock entirely). > + clock_mux_set_factor(&s->clock_muxes[RCC_CLOCK_MUX_MCO], > + 1, 1 << val); > + > + /* MCOSEL */ > + val = FIELD_EX32(s->cfgr, CFGR, MCOSEL); > + assert(val <= 0b111); Similarly here. (The obvious behaviour for "invalid clock source selected" would be "treat as clock disabled".) > + 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); > + } thanks -- PMM
