On Tue, 18 Jun 2024 at 15:51, Philippe Mathieu-Daudé <phi...@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 <zheyum...@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é <phi...@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
