On 17/12/2020 14:13, Alexandru Elisei wrote:
> The ITS tests use the INT command like an SGI. The its_send_int() function
> kicks a CPU and then the test checks that the interrupt was observed as
> expected in check_lpi_stats(). This is done by using lpi_stats.observed and
> lpi_stats.expected, where the target CPU only writes to lpi_stats.observed,
> and the source CPU reads it and compares the values with
> lpi_stats.expected.
>
> The fact that the target CPU doesn't read data written by the source CPU
> means that we don't need to do inter-processor memory synchronization
> for that between the two at the moment.
>
> The acked array is used by its-pending-migration test, but the reset value
> for acked (zero) is the same as the initialization value for static
> variables, so memory synchronization is again not needed.
>
> However, that is all about to change when we modify all ITS tests to use
> the same functions as the IPI tests. Add a write memory barrier to
> its_send_int(), similar to the gicv3_ipi_send_mask(), which has similar
> semantics.
I agree to the requirement for having the barrier, but am not sure this
is the right place. Wouldn't it be better to have the barrier in the
callers?
Besides: This command is written to the command queue (in normal
memory), then we notify the ITS via an MMIO writeq. And this one has a
"wmb" barrier already (though for other reasons).
Cheers,
Andre
>
> Suggested-by: Auger Eric <eric.auger@xxxxxxxxxx>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
> ---
> lib/arm64/gic-v3-its-cmd.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/lib/arm64/gic-v3-its-cmd.c b/lib/arm64/gic-v3-its-cmd.c
> index 34574f71d171..32703147ee85 100644
> --- a/lib/arm64/gic-v3-its-cmd.c
> +++ b/lib/arm64/gic-v3-its-cmd.c
> @@ -385,6 +385,12 @@ void __its_send_int(struct its_device *dev, u32 event_id, bool verbose)
> {
> struct its_cmd_desc desc;
>
> + /*
> + * The INT command is used by tests as an IPI. Ensure stores to Normal
> + * memory are visible to other CPUs before sending the LPI.
> + */
> + wmb();
> +
> desc.its_int_cmd.dev = dev;
> desc.its_int_cmd.event_id = event_id;
> desc.verbose = verbose;
>
