diff mbox series

[kvm-unit-tests,v2,1/4] arm64: remove invalid check from its-trigger test

Message ID 20210525172628.2088-2-alex.bennee@linaro.org (mailing list archive)
State New, archived
Headers show
Series enable LPI and ITS for TCG | expand

Commit Message

Alex Bennée May 25, 2021, 5:26 p.m. UTC
While an IRQ is not "guaranteed to be visible until an appropriate
invalidation" it doesn't stop the actual implementation delivering it
earlier if it wants to. This is the case for QEMU's TCG and as tests
should only be checking architectural compliance this check is
invalid.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Shashi Mallela <shashi.mallela@linaro.org>
---
 arm/gic.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

Comments

Eric Auger May 26, 2021, 3:06 p.m. UTC | #1
Hi Alex,

On 5/25/21 7:26 PM, Alex Bennée wrote:
> While an IRQ is not "guaranteed to be visible until an appropriate
> invalidation" it doesn't stop the actual implementation delivering it
> earlier if it wants to. This is the case for QEMU's TCG and as tests
> should only be checking architectural compliance this check is
> invalid.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Shashi Mallela <shashi.mallela@linaro.org>
> ---
>  arm/gic.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/arm/gic.c b/arm/gic.c
> index 98135ef..bef061a 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -732,21 +732,19 @@ static void test_its_trigger(void)
>  			"dev2/eventid=20 does not trigger any LPI");
>  
>  	/*
> -	 * re-enable the LPI but willingly do not call invall
> -	 * so the change in config is not taken into account.
> -	 * The LPI should not hit
> +	 * re-enable the LPI but willingly do not call invall so the
> +	 * change in config is not taken into account. While "A change
So you may need to remove the above comment, ie. "but willingly do not
call invall so the change in config is not taken into account." as the
conclusion of this thread is it can be taken into account immediatly.

and also concat the comment below, "/* Now call the invall and check the
LPI hits */"
This is an "atomic" test now?

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


Thanks

Eric
> +	 * to the LPI configuration is not guaranteed to be visible
> +	 * until an appropriate invalidation operation has completed"
> +	 * hardware that doesn't implement caches may have delivered
> +	 * the event at any point after the enabling.
>  	 */
>  	gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT);
>  	stats_reset();
>  	cpumask_clear(&mask);
>  	its_send_int(dev2, 20);
> -	wait_for_interrupts(&mask);
> -	report(check_acked(&mask, -1, -1),
> -			"dev2/eventid=20 still does not trigger any LPI");
>  
>  	/* Now call the invall and check the LPI hits */
> -	stats_reset();
> -	cpumask_clear(&mask);
>  	cpumask_set_cpu(3, &mask);
>  	its_send_invall(col3);
>  	wait_for_interrupts(&mask);
>
diff mbox series

Patch

diff --git a/arm/gic.c b/arm/gic.c
index 98135ef..bef061a 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -732,21 +732,19 @@  static void test_its_trigger(void)
 			"dev2/eventid=20 does not trigger any LPI");
 
 	/*
-	 * re-enable the LPI but willingly do not call invall
-	 * so the change in config is not taken into account.
-	 * The LPI should not hit
+	 * re-enable the LPI but willingly do not call invall so the
+	 * change in config is not taken into account. While "A change
+	 * to the LPI configuration is not guaranteed to be visible
+	 * until an appropriate invalidation operation has completed"
+	 * hardware that doesn't implement caches may have delivered
+	 * the event at any point after the enabling.
 	 */
 	gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT);
 	stats_reset();
 	cpumask_clear(&mask);
 	its_send_int(dev2, 20);
-	wait_for_interrupts(&mask);
-	report(check_acked(&mask, -1, -1),
-			"dev2/eventid=20 still does not trigger any LPI");
 
 	/* Now call the invall and check the LPI hits */
-	stats_reset();
-	cpumask_clear(&mask);
 	cpumask_set_cpu(3, &mask);
 	its_send_invall(col3);
 	wait_for_interrupts(&mask);