Re: [PATCH v5 01/31] target/arm: Implement arm_v7m_cpu_has_work()

[Peter Maydell]

On Mon, 20 Sept 2021 at 22:44, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Implement SysemuCPUOps::has_work() handler for the ARM v7M CPU.
>
> See the comments added in commit 7ecdaa4a963 ("armv7m: Fix
> condition check for taking exceptions") which eventually
> forgot to implement this has_work() handler:

Huh? M-profile and A-profile share the same arm_cpu_has_work()
function. Some of the checks the code there does are perhaps
unnecessary for M-profile, but they're harmless.

>   * ARMv7-M interrupt masking works differently than -A or -R.
>   * There is no FIQ/IRQ distinction.
>
> The NVIC signal any pending interrupt by raising ARM_CPU_IRQ
> (see commit 56b7c66f498: "armv7m: QOMify the armv7m container")
> which ends setting the CPU_INTERRUPT_HARD bit in interrupt_request.
>
> Thus arm_v7m_cpu_has_work() implementation is thus quite trivial,
> we simply need to check for this bit.
>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Michael Davidsaver <mdavidsaver@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/arm/cpu_tcg.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
> index 0d5adccf1a7..da348938407 100644
> --- a/target/arm/cpu_tcg.c
> +++ b/target/arm/cpu_tcg.c
> @@ -23,6 +23,11 @@
>  #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
>
>  #if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG)
> +static bool arm_v7m_cpu_has_work(CPUState *cs)
> +{
> +    return cs->interrupt_request & CPU_INTERRUPT_HARD;
> +}

This seems to be missing at least the check on
cpu->power_state and the CPU_INTERRUPT_EXITTB test.

Is there any reason why we shouldn't just continue to
share the same function between A and M profile, and avoid
the extra function and the ifdefs ?

-- PMM