diff mbox series

[kvm-unit-tests,07/10] arm/arm64: gic: Wait for writes to acked or spurious to complete

Message ID 20201125155113.192079-8-alexandru.elisei@arm.com (mailing list archive)
State New, archived
Headers show
Series GIC fixes and improvements | expand

Commit Message

Alexandru Elisei Nov. 25, 2020, 3:51 p.m. UTC
The IPI test has two parts: in the first part, it tests that the sender CPU
can send an IPI to itself (ipi_test_self()), and in the second part it
sends interrupts to even-numbered CPUs (ipi_test_smp()). When acknowledging
an interrupt, if we read back a spurious interrupt ID (1023), the handler
increments the index in the static array spurious corresponding to the CPU
ID that the handler is running on; if we get the expected interrupt ID, we
increment the same index in the acked array.

Reads of the spurious and acked arrays are synchronized with writes
performed before sending the IPI. The synchronization is done either in the
IPI sender function (GICv3), either by creating a data dependency (GICv2).

At the end of the test, the sender CPU reads from the acked and spurious
arrays to check against the expected behaviour. We need to make sure the
that writes in ipi_handler() are observable by the sender CPU. Use a DSB
ISHST to make sure that the writes have completed.

One might rightfully argue that there are no guarantees regarding when the
DSB instruction completes, just like there are no guarantees regarding when
the value is observed by the other CPUs. However, let's do our best and
instruct the CPU to complete the memory access when we know that it will be
needed.

We still need to follow the message passing pattern for the acked,
respectively bad_irq and bad_sender, because DSB guarantees that all memory
accesses that come before the barrier have completed, not that they have
completed in program order.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/gic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Auger Eric Dec. 3, 2020, 1:21 p.m. UTC | #1
Hi Alexandru,

On 11/25/20 4:51 PM, Alexandru Elisei wrote:
> The IPI test has two parts: in the first part, it tests that the sender CPU
> can send an IPI to itself (ipi_test_self()), and in the second part it
> sends interrupts to even-numbered CPUs (ipi_test_smp()). When acknowledging
> an interrupt, if we read back a spurious interrupt ID (1023), the handler
> increments the index in the static array spurious corresponding to the CPU
> ID that the handler is running on; if we get the expected interrupt ID, we
> increment the same index in the acked array.
> 
> Reads of the spurious and acked arrays are synchronized with writes
> performed before sending the IPI. The synchronization is done either in the
> IPI sender function (GICv3), either by creating a data dependency (GICv2).
> 
> At the end of the test, the sender CPU reads from the acked and spurious
> arrays to check against the expected behaviour. We need to make sure the
> that writes in ipi_handler() are observable by the sender CPU. Use a DSB
> ISHST to make sure that the writes have completed.
> 
> One might rightfully argue that there are no guarantees regarding when the
> DSB instruction completes, just like there are no guarantees regarding when
> the value is observed by the other CPUs. However, let's do our best and
> instruct the CPU to complete the memory access when we know that it will be
> needed.
> 
> We still need to follow the message passing pattern for the acked,
> respectively bad_irq and bad_sender, because DSB guarantees that all memory
> accesses that come before the barrier have completed, not that they have
> completed in program order.
I guess the removal of the smp_rmb in check_spurious should belong to
that patch?
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Besides, AFAIU

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  arm/gic.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/gic.c b/arm/gic.c
> index 5727d72a0ef3..544c283f5f47 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -161,8 +161,10 @@ static void ipi_handler(struct pt_regs *regs __unused)
>  		++acked[smp_processor_id()];
>  	} else {
>  		++spurious[smp_processor_id()];
> -		smp_wmb();
>  	}
> +
> +	/* Wait for writes to acked/spurious to complete */
> +	dsb(ishst);
>  }
>  
>  static void setup_irq(irq_handler_fn handler)
>
diff mbox series

Patch

diff --git a/arm/gic.c b/arm/gic.c
index 5727d72a0ef3..544c283f5f47 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -161,8 +161,10 @@  static void ipi_handler(struct pt_regs *regs __unused)
 		++acked[smp_processor_id()];
 	} else {
 		++spurious[smp_processor_id()];
-		smp_wmb();
 	}
+
+	/* Wait for writes to acked/spurious to complete */
+	dsb(ishst);
 }
 
 static void setup_irq(irq_handler_fn handler)