Re: [PATCH] hw/timer/a9gtimer: Handle QTest mode in a9_gtimer_get_current_cpu

[Peter Maydell]

On Tue, 18 Jun 2024 at 15:51, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 18/6/24 16:40, Zheyu Ma wrote:
> > This commit updates the a9_gtimer_get_current_cpu() function to handle
> > cases where QTest is enabled. When QTest is used, it returns 0 instead
> > of dereferencing the current_cpu, which can be NULL. This prevents the
> > program from crashing during QTest runs.
> >
> > Reproducer:
> > cat << EOF | qemu-system-aarch64 -display \
> > none -machine accel=qtest, -m 512M -machine npcm750-evb -qtest stdio
> > writel 0xf03fe20c 0x26d7468c
> > EOF
> >
> > Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
> > ---
> >   hw/timer/a9gtimer.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c
> > index a2ac5bdfb9..64d80cdf6a 100644
> > --- a/hw/timer/a9gtimer.c
> > +++ b/hw/timer/a9gtimer.c
> > @@ -32,6 +32,7 @@
> >   #include "qemu/log.h"
> >   #include "qemu/module.h"
> >   #include "hw/core/cpu.h"
> > +#include "sysemu/qtest.h"
> >
> >   #ifndef A9_GTIMER_ERR_DEBUG
> >   #define A9_GTIMER_ERR_DEBUG 0
> > @@ -48,6 +49,10 @@
> >
> >   static inline int a9_gtimer_get_current_cpu(A9GTimerState *s)
> >   {
> > +    if (qtest_enabled()) {
> > +        return 0;
>
> Indeed this is how we fixed hw/intc/arm_gic in commit 09bbdb89bc,
> so:
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> > +    }
> > +
> >       if (current_cpu->cpu_index >= s->num_cpu) {
>
> That said, such accesses of @current_cpu from hw/ are dubious.

True, but I'm not sure we ever settled on the right way to avoid
them, did we?

Anyway, I've applied this patch to target-arm.next.

-- PMM