Hi Andre,
On 12/18/20 6:36 PM, André Przywara wrote:
> 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).
Had another look, and you're totally right: its_post_commands() already has a
wmb() in writeq(), thank you for pointing it out. This makes the wmb() which I've
added pointless, I'll drop the patch since it doesn't add useful.
Thanks,
Alex
>
> 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;
>>
