Re: [kvm-unit-tests PATCH v2 11/12] lib: arm64: gic-v3-its: Add wmb() barrier before INT command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;
>>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm




[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux