Re: [PATCH for-5.1? 0/3] Fix AIRCR.SYSRESETREQ for most M-profile boards

[Alistair Francis]

On Tue, Jul 28, 2020 at 3:38 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> QEMU's NVIC device provides an outbound qemu_irq "SYSRESETREQ" which
> it signals when the guest sets the SYSRESETREQ bit in the AIRCR
> register.  This matches the hardware design (where the CPU has a
> signal of this name and it is up to the SoC to connect that up to an
> actual reset mechanism), but in QEMU it mostly results in duplicated
> code in SoC objects and bugs where SoC model implementors forget to
> wire up the SYSRESETREQ line.
>
> This patchseries provides a default behaviour for the case where
> SYSRESETREQ is not actually connected to anything: use
> qemu_system_reset_request() to perform a system reset. This is a
> much more plausible default than "do nothing". It allows us to
> allow us to remove the implementations of SYSRESETREQ handling from
> the boards which currently hook up a reset function that just
> does that (stellaris, emcraft-sf2), and also fixes the bugs
> in these board models which forgot to wire up the signal:
>
>  * microbit
>  * mps2-an385
>  * mps2-an505
>  * mps2-an511
>  * mps2-an521
>  * musca-a
>  * musca-b1
>  * netduino
>  * netduinoplus2
>
> [I admit that I have not specifically checked for datasheets
> and errata notes for all these boards to confirm that AIRCR.SYSRESETREQ
> really does reset the system or to look for more complex
> reset-control logic... As an example, though, the SSE-200 used in
> the mps2-an521 and the musca boards has a RESET_MASK register in the
> system control block that allows a guest to suppress reset requests from
> one or both CPUs. Since we don't model that register, "always reset"
> is still closer to reasonable behaviour than "do nothing".]
>
> We still allow the board to wire up the signal if it needs to, in case
> we need to model more complicated reset controller logic (eg if we
> wanted to get that SSE-200 corner case right in future) or to model
> buggy SoC hardware which forgot to wire up the line itself. But
> defaulting to "reset the system" is going to be correct much more
> often than "do nothing".
>
> Patch 1 introduces a new API for "check whether my qemu_irq is
> actually connected to anything" (the test is trivial but the
> encapsulation seems worthwhile); patch 2 provides the new default
> and patch 3 removes the now-unnecessary SYSRESETREQ handlers in
> board/SoC code.

I checked the STM Reference Manual and this matches what they expect.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

>
> thanks
> -- PMM
>
> Peter Maydell (3):
>   include/hw/irq.h: New function qemu_irq_is_connected()
>   hw/intc/armv7m_nvic: Provide default "reset the system" behaviour for
>     SYSRESETREQ
>   msf2-soc, stellaris: Don't wire up SYSRESETREQ
>
>  include/hw/arm/armv7m.h |  4 +++-
>  include/hw/irq.h        | 18 ++++++++++++++++++
>  hw/arm/msf2-soc.c       | 11 -----------
>  hw/arm/stellaris.c      | 12 ------------
>  hw/intc/armv7m_nvic.c   | 17 ++++++++++++++++-
>  5 files changed, 37 insertions(+), 25 deletions(-)
>
> --
> 2.20.1
>
>