| Message ID | 20231103192915.2209393-3-coltonlewis@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add arch_timer_edge_cases selftest | expand |
Hi Colton, On Fri, Nov 03, 2023 at 07:29:14PM +0000, Colton Lewis wrote: > Break up the asm instructions poking daifclr and daifset to handle > interrupts. R_RBZYL specifies pending interrupts will be handle after > context synchronization events such as an ISB. > > Introduce a function wrapper for the WFI instruction. > > Signed-off-by: Colton Lewis <coltonlewis@google.com> What's missing from the changelog is that you've spotted an actual bug, and this is really a bugfix. > --- > tools/testing/selftests/kvm/aarch64/vgic_irq.c | 12 ++++++------ > tools/testing/selftests/kvm/include/aarch64/gic.h | 3 +++ > tools/testing/selftests/kvm/lib/aarch64/gic.c | 5 +++++ > 3 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c > index d3bf584d2cc1..85f182704d79 100644 > --- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c > +++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c > @@ -269,13 +269,13 @@ static void guest_inject(struct test_args *args, > KVM_INJECT_MULTI(cmd, first_intid, num); > > while (irq_handled < num) { > - asm volatile("wfi\n" > - "msr daifclr, #2\n" > - /* handle IRQ */ > - "msr daifset, #2\n" > - : : : "memory"); > + gic_wfi(); > + local_irq_enable(); > + isb(); > + /* handle IRQ */ nit: this comment should appear above the isb() > + local_irq_disable(); > } > - asm volatile("msr daifclr, #2" : : : "memory"); > + local_irq_enable(); > > GUEST_ASSERT_EQ(irq_handled, num); > for (i = first_intid; i < num + first_intid; i++) > diff --git a/tools/testing/selftests/kvm/include/aarch64/gic.h b/tools/testing/selftests/kvm/include/aarch64/gic.h > index 9043eaef1076..f474714e4cb2 100644 > --- a/tools/testing/selftests/kvm/include/aarch64/gic.h > +++ b/tools/testing/selftests/kvm/include/aarch64/gic.h > @@ -47,4 +47,7 @@ void gic_irq_clear_pending(unsigned int intid); > bool gic_irq_get_pending(unsigned int intid); > void gic_irq_set_config(unsigned int intid, bool is_edge); > > +/* Execute a Wait For Interrupt instruction. */ > +void gic_wfi(void); > + WFIs have nothing to do with the GIC, they're an instruction supported by the CPU. I'd just merge the definition and declaration into the header while you're at it. So, could you throw something like this in aarch64/processor.h? static inline void wfi(void) { asm volatile("wfi"); }
diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c index d3bf584d2cc1..85f182704d79 100644 --- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c +++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c @@ -269,13 +269,13 @@ static void guest_inject(struct test_args *args, KVM_INJECT_MULTI(cmd, first_intid, num); while (irq_handled < num) { - asm volatile("wfi\n" - "msr daifclr, #2\n" - /* handle IRQ */ - "msr daifset, #2\n" - : : : "memory"); + gic_wfi(); + local_irq_enable(); + isb(); + /* handle IRQ */ + local_irq_disable(); } - asm volatile("msr daifclr, #2" : : : "memory"); + local_irq_enable(); GUEST_ASSERT_EQ(irq_handled, num); for (i = first_intid; i < num + first_intid; i++) diff --git a/tools/testing/selftests/kvm/include/aarch64/gic.h b/tools/testing/selftests/kvm/include/aarch64/gic.h index 9043eaef1076..f474714e4cb2 100644 --- a/tools/testing/selftests/kvm/include/aarch64/gic.h +++ b/tools/testing/selftests/kvm/include/aarch64/gic.h @@ -47,4 +47,7 @@ void gic_irq_clear_pending(unsigned int intid); bool gic_irq_get_pending(unsigned int intid); void gic_irq_set_config(unsigned int intid, bool is_edge); +/* Execute a Wait For Interrupt instruction. */ +void gic_wfi(void); + #endif /* SELFTEST_KVM_GIC_H */ diff --git a/tools/testing/selftests/kvm/lib/aarch64/gic.c b/tools/testing/selftests/kvm/lib/aarch64/gic.c index 9d15598d4e34..392e3f581ae0 100644 --- a/tools/testing/selftests/kvm/lib/aarch64/gic.c +++ b/tools/testing/selftests/kvm/lib/aarch64/gic.c @@ -164,3 +164,8 @@ void gic_irq_set_config(unsigned int intid, bool is_edge) GUEST_ASSERT(gic_common_ops); gic_common_ops->gic_irq_set_config(intid, is_edge); } + +void gic_wfi(void) +{ + asm volatile("wfi"); +}
Break up the asm instructions poking daifclr and daifset to handle interrupts. R_RBZYL specifies pending interrupts will be handle after context synchronization events such as an ISB. Introduce a function wrapper for the WFI instruction. Signed-off-by: Colton Lewis <coltonlewis@google.com> --- tools/testing/selftests/kvm/aarch64/vgic_irq.c | 12 ++++++------ tools/testing/selftests/kvm/include/aarch64/gic.h | 3 +++ tools/testing/selftests/kvm/lib/aarch64/gic.c | 5 +++++ 3 files changed, 14 insertions(+), 6 deletions(-) -- 2.42.0.869.gea05f2083d-goog