Re: [PULL 35/47] target/arm: Send interrupts on PMU counter overflow

[Peter Maydell]

On Fri, 1 Feb 2019 at 16:07, Peter Maydell <address@hidden> wrote:
>
> From: Aaron Lindsay OS <address@hidden>
>
> Whenever we notice that a counter overflow has occurred, send an
> interrupt. This is made more reliable with the addition of a timer in a
> follow-on commit.
>
> Signed-off-by: Aaron Lindsay <address@hidden>
> Reviewed-by: Richard Henderson <address@hidden>
> Message-id: address@hidden
> Signed-off-by: Peter Maydell <address@hidden>

Hi Aaron -- I've just noticed a problem with this patch that
went into QEMU recently. The problem is that we can end up
calling pmu_update_irq(), which injects an interrupt, from
a raw-write function for some of the PMU registers. This is
bad because when we're using KVM the raw-write functions are
called as part of syncing state to and from the kernel. In
particular, if using '-cpu host,pmu=off' we don't set up the
PMU interrupt because we don't want to provide the guest a
PMU but then we can still find ourselves in this function,
and then we assert because we try to set a bogus interrupt.
Here's the backtrace:

#1  0x0000fffff6be68b4 in __GI_abort () at abort.c:79
#2  0x0000aaaaaae20820 in kvm_set_irq (s=0xaaaaabf8a020, irq=33554455, level=0)
    at /home/pm/qemu-bisect/accel/kvm/kvm-all.c:1277
#3  0x0000aaaaaafb7890 in kvm_arm_set_irq (cpu=0, irqtype=2, irq=23, level=0)
    at /home/pm/qemu-bisect/target/arm/kvm.c:897
#4  0x0000aaaaaae729dc in kvm_arm_gic_set_irq (num_irq=288, irq=23, level=0)
    at /home/pm/qemu-bisect/hw/intc/arm_gic_kvm.c:75
#5  0x0000aaaaaae72a1c in kvm_arm_gicv2_set_irq
(opaque=0xaaaaac169ff0, irq=279, level=0)
    at /home/pm/qemu-bisect/hw/intc/arm_gic_kvm.c:82
#6  0x0000aaaaab1ba15c in qemu_set_irq (irq=0xaaaaac186090, level=0)
    at /home/pm/qemu-bisect/hw/core/irq.c:44
#7  0x0000aaaaaaf86050 in pmu_update_irq (env=0xaaaaac0fa470)
    at /home/pm/qemu-bisect/target/arm/helper.c:1412
#8  0x0000aaaaaaf8747c in pmintenclr_write (env=0xaaaaac0fa470,
ri=0xaaaaac12c3e0, value=2154950974777589790) at
/home/pm/qemu-bisect/target/arm/helper.c:1903
#9  0x0000aaaaaaf83e68 in write_raw_cp_reg (env=0xaaaaac0fa470,
ri=0xaaaaac12c3e0, v=2154950976315703518) at
/home/pm/qemu-bisect/target/arm/helper.c:206
#10 0x0000aaaaaaf840d4 in write_cpustate_to_list (cpu=0xaaaaac0f0b90,
kvm_sync=true)
    at /home/pm/qemu-bisect/target/arm/helper.c:290
#11 0x0000aaaaaafbb1ac in kvm_arch_put_registers (cs=0xaaaaac0f0b90, level=3)
    at /home/pm/qemu-bisect/target/arm/kvm64.c:1108
#12 0x0000aaaaaae22ea0 in do_kvm_cpu_synchronize_post_init
(cpu=0xaaaaac0f0b90, arg=...)
    at /home/pm/qemu-bisect/accel/kvm/kvm-all.c:2223
#13 0x0000aaaaab107fa0 in process_queued_cpu_work (cpu=0xaaaaac0f0b90)
    at /home/pm/qemu-bisect/cpus-common.c:338
#14 0x0000aaaaaadf4ff4 in qemu_wait_io_event_common (cpu=0xaaaaac0f0b90)
    at /home/pm/qemu-bisect/cpus.c:1175
#15 0x0000aaaaaadf51a8 in qemu_wait_io_event (cpu=0xaaaaac0f0b90)
    at /home/pm/qemu-bisect/cpus.c:1215
#16 0x0000aaaaaadf52cc in qemu_kvm_cpu_thread_fn (arg=0xaaaaac0f0b90)
    at /home/pm/qemu-bisect/cpus.c:1251
#17 0x0000aaaaab690268 in qemu_thread_start (args=0xaaaaac14b1d0)
    at /home/pm/qemu-bisect/util/qemu-thread-posix.c:519


The point of the 'raw_read/write' accessors is that they're supposed
to not have side effects but just to be usable to read and write
any underlying register state. If the regdef doesn't define them
we fall back to the usual readfn/writefn on the assumption that
they're side-effect-free. So I think the fix here would be to
provide a raw_writefn everywhere where we've made
the normal writefn have a "sets an interrupt" side effect.

thanks
-- PMM