[kvm-unit-tests PATCH v2 0/4] enable LPI and ITS for TCG

[kvm-unit-tests PATCH v2 0/4] enable LPI and ITS for TCG

[Alex Bennée]

This is a companion to Shashi's series enabling LPI and ITS features
for QEMU's TCG emulation. This is part of our push for a sbsa-ref
platform which needs a more modern set of features. Since the last
posting this is now at v3:

  From: Shashi Mallela <shashi.mallela@linaro.org>
  Subject: [PATCH v3 0/8] GICv3 LPI and ITS feature implementation
  Date: Thu, 29 Apr 2021 19:41:53 -0400
  Message-Id: <20210429234201.125565-1-shashi.mallela@linaro.org>

The only real change from the last version was to drop the IMPDEF
behaviour check instead of trying to work around it for the TCG case.

Please review.

Alex Bennée (4):
  arm64: remove invalid check from its-trigger test
  scripts/arch-run: don't use deprecated server/nowait options
  arm64: enable its-migration tests for TCG
  arm64: split its-migrate-unmapped-collection into KVM and TCG variants

 scripts/arch-run.bash |  4 ++--
 arm/gic.c             | 22 +++++++++++++---------
 arm/unittests.cfg     | 12 +++++++++---
 3 files changed, 24 insertions(+), 14 deletions(-)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Alex Bennée]

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
+	 * 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);
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Auger Eric]

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);
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[kvm-unit-tests PATCH v2 2/4] scripts/arch-run: don't use deprecated server/nowait options

[Alex Bennée]

The very fact that QEMU drops the deprecation warning while running is
enough to confuse the its-migration test into failing. The boolean
options server and wait have accepted the long form options for a long
time.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Shashi Mallela <shashi.mallela@linaro.org>
---
 scripts/arch-run.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 5997e38..70693f2 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -122,14 +122,14 @@ run_migration ()
 	trap 'kill 0; exit 2' INT TERM
 	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
 
-	eval "$@" -chardev socket,id=mon1,path=${qmp1},server,nowait \
+	eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
 		-mon chardev=mon1,mode=control | tee ${migout1} &
 
 	# We have to use cat to open the named FIFO, because named FIFO's, unlike
 	# pipes, will block on open() until the other end is also opened, and that
 	# totally breaks QEMU...
 	mkfifo ${fifo}
-	eval "$@" -chardev socket,id=mon2,path=${qmp2},server,nowait \
+	eval "$@" -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
 		-mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat ${fifo}) &
 	incoming_pid=`jobs -l %+ | awk '{print$2}'`
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kvm-unit-tests PATCH v2 2/4] scripts/arch-run: don't use deprecated server/nowait options

[Zenghui Yu]

On 2021/5/26 1:26, Alex Bennée wrote:
> The very fact that QEMU drops the deprecation warning while running is
> enough to confuse the its-migration test into failing. The boolean
> options server and wait have accepted the long form options for a long
> time.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Shashi Mallela <shashi.mallela@linaro.org>

This works for me.

Tested-by: Zenghui Yu <yuzenghui@huawei.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kvm-unit-tests PATCH v2 2/4] scripts/arch-run: don't use deprecated server/nowait options

[Auger Eric]

Hi Alex,

On 5/25/21 7:26 PM, Alex Bennée wrote:
> The very fact that QEMU drops the deprecation warning while running is
> enough to confuse the its-migration test into failing. The boolean
> options server and wait have accepted the long form options for a long
> time.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Shashi Mallela <shashi.mallela@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

> ---
>  scripts/arch-run.bash | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 5997e38..70693f2 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -122,14 +122,14 @@ run_migration ()
>  	trap 'kill 0; exit 2' INT TERM
>  	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
>  
> -	eval "$@" -chardev socket,id=mon1,path=${qmp1},server,nowait \
> +	eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
>  		-mon chardev=mon1,mode=control | tee ${migout1} &
>  
>  	# We have to use cat to open the named FIFO, because named FIFO's, unlike
>  	# pipes, will block on open() until the other end is also opened, and that
>  	# totally breaks QEMU...
>  	mkfifo ${fifo}
> -	eval "$@" -chardev socket,id=mon2,path=${qmp2},server,nowait \
> +	eval "$@" -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
>  		-mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat ${fifo}) &
>  	incoming_pid=`jobs -l %+ | awk '{print$2}'`
>  
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kvm-unit-tests PATCH v2 2/4] scripts/arch-run: don't use deprecated server/nowait options

[Andrew Jones]

On Tue, May 25, 2021 at 06:26:26PM +0100, Alex Bennée wrote:
> The very fact that QEMU drops the deprecation warning while running is
> enough to confuse the its-migration test into failing. The boolean
> options server and wait have accepted the long form options for a long
> time.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Shashi Mallela <shashi.mallela@linaro.org>
> ---
>  scripts/arch-run.bash | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Andrew Jones <drjones@redhat.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kvm-unit-tests PATCH v2 2/4] scripts/arch-run: don't use deprecated server/nowait options

[Andrew Jones]

On Tue, May 25, 2021 at 06:26:26PM +0100, Alex Bennée wrote:
> The very fact that QEMU drops the deprecation warning while running is
> enough to confuse the its-migration test into failing. The boolean
> options server and wait have accepted the long form options for a long
> time.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Shashi Mallela <shashi.mallela@linaro.org>
> ---
>  scripts/arch-run.bash | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 5997e38..70693f2 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -122,14 +122,14 @@ run_migration ()
>  	trap 'kill 0; exit 2' INT TERM
>  	trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT
>  
> -	eval "$@" -chardev socket,id=mon1,path=${qmp1},server,nowait \
> +	eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \
>  		-mon chardev=mon1,mode=control | tee ${migout1} &
>  
>  	# We have to use cat to open the named FIFO, because named FIFO's, unlike
>  	# pipes, will block on open() until the other end is also opened, and that
>  	# totally breaks QEMU...
>  	mkfifo ${fifo}
> -	eval "$@" -chardev socket,id=mon2,path=${qmp2},server,nowait \
> +	eval "$@" -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \
>  		-mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat ${fifo}) &
>  	incoming_pid=`jobs -l %+ | awk '{print$2}'`
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Applied to misc/queue

https://gitlab.com/rhdrjones/kvm-unit-tests/-/commits/misc/queue

Thanks,
drew


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[kvm-unit-tests PATCH v2 3/4] arm64: enable its-migration tests for TCG

[Alex Bennée]

With the support for TCG emulated GIC we can also test these now.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Shashi Mallela <shashi.mallela@linaro.org>
---
 arm/unittests.cfg | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index f776b66..1a39428 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -194,7 +194,6 @@ arch = arm64
 [its-migration]
 file = gic.flat
 smp = $MAX_SMP
-accel = kvm
 extra_params = -machine gic-version=3 -append 'its-migration'
 groups = its migration
 arch = arm64
@@ -202,7 +201,6 @@ arch = arm64
 [its-pending-migration]
 file = gic.flat
 smp = $MAX_SMP
-accel = kvm
 extra_params = -machine gic-version=3 -append 'its-pending-migration'
 groups = its migration
 arch = arm64
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kvm-unit-tests PATCH v2 3/4] arm64: enable its-migration tests for TCG

[Auger Eric]

Hi Alex,

On 5/25/21 7:26 PM, Alex Bennée wrote:
> With the support for TCG emulated GIC we can also test these now.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Shashi Mallela <shashi.mallela@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

> ---
>  arm/unittests.cfg | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index f776b66..1a39428 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -194,7 +194,6 @@ arch = arm64
>  [its-migration]
>  file = gic.flat
>  smp = $MAX_SMP
> -accel = kvm
>  extra_params = -machine gic-version=3 -append 'its-migration'
>  groups = its migration
>  arch = arm64
> @@ -202,7 +201,6 @@ arch = arm64
>  [its-pending-migration]
>  file = gic.flat
>  smp = $MAX_SMP
> -accel = kvm
>  extra_params = -machine gic-version=3 -append 'its-pending-migration'
>  groups = its migration
>  arch = arm64
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kvm-unit-tests PATCH v2 3/4] arm64: enable its-migration tests for TCG

[Andrew Jones]

On Tue, May 25, 2021 at 06:26:27PM +0100, Alex Bennée wrote:
> With the support for TCG emulated GIC we can also test these now.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Shashi Mallela <shashi.mallela@linaro.org>
> ---
>  arm/unittests.cfg | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index f776b66..1a39428 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -194,7 +194,6 @@ arch = arm64
>  [its-migration]
>  file = gic.flat
>  smp = $MAX_SMP
> -accel = kvm
>  extra_params = -machine gic-version=3 -append 'its-migration'
>  groups = its migration
>  arch = arm64
> @@ -202,7 +201,6 @@ arch = arm64
>  [its-pending-migration]
>  file = gic.flat
>  smp = $MAX_SMP
> -accel = kvm
>  extra_params = -machine gic-version=3 -append 'its-pending-migration'
>  groups = its migration
>  arch = arm64
> -- 
> 2.20.1
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Reviewed-by: Andrew Jones <drjones@redhat.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[kvm-unit-tests PATCH v2 4/4] arm64: split its-migrate-unmapped-collection into KVM and TCG variants

[Alex Bennée]

When running the test in TCG we are basically running on bare metal so
don't rely on having a particular kernel errata applied.

You might wonder why we handle this with a totally new test name
instead of adjusting the append to take an extra parameter? Well the
run_migration shell script uses eval "$@" which unwraps the -append
leading to any second parameter being split and leaving QEMU very
confused and the test hanging. This seemed simpler than re-writing all
the test running logic in something sane ;-)

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Shashi Mallela <shashi.mallela@linaro.org>
---
 arm/gic.c         |  8 +++++++-
 arm/unittests.cfg | 10 +++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arm/gic.c b/arm/gic.c
index bef061a..0fce2a4 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -36,6 +36,7 @@ static struct gic *gic;
 static int acked[NR_CPUS], spurious[NR_CPUS];
 static int irq_sender[NR_CPUS], irq_number[NR_CPUS];
 static cpumask_t ready;
+static bool under_tcg;
 
 static void nr_cpu_check(int nr)
 {
@@ -834,7 +835,7 @@ static void test_migrate_unmapped_collection(void)
 		goto do_migrate;
 	}
 
-	if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
+	if (!errata(ERRATA_UNMAPPED_COLLECTIONS) && !under_tcg) {
 		report_skip("Skipping test, as this test hangs without the fix. "
 			    "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
 		test_skipped = true;
@@ -1005,6 +1006,11 @@ int main(int argc, char **argv)
 		report_prefix_push(argv[1]);
 		test_migrate_unmapped_collection();
 		report_prefix_pop();
+	} else if (!strcmp(argv[1], "its-migrate-unmapped-collection-tcg")) {
+		under_tcg = true;
+		report_prefix_push(argv[1]);
+		test_migrate_unmapped_collection();
+		report_prefix_pop();
 	} else if (strcmp(argv[1], "its-introspection") == 0) {
 		report_prefix_push(argv[1]);
 		test_its_introspection();
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 1a39428..adc1bbf 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -205,7 +205,7 @@ extra_params = -machine gic-version=3 -append 'its-pending-migration'
 groups = its migration
 arch = arm64
 
-[its-migrate-unmapped-collection]
+[its-migrate-unmapped-collection-kvm]
 file = gic.flat
 smp = $MAX_SMP
 accel = kvm
@@ -213,6 +213,14 @@ extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection'
 groups = its migration
 arch = arm64
 
+[its-migrate-unmapped-collection-tcg]
+file = gic.flat
+smp = $MAX_SMP
+accel = tcg
+extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection-tcg'
+groups = its migration
+arch = arm64
+
 # Test PSCI emulation
 [psci]
 file = psci.flat
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kvm-unit-tests PATCH v2 4/4] arm64: split its-migrate-unmapped-collection into KVM and TCG variants

[Auger Eric]

Hi Alex,

On 5/25/21 7:26 PM, Alex Bennée wrote:
> When running the test in TCG we are basically running on bare metal so
> don't rely on having a particular kernel errata applied.
> 
> You might wonder why we handle this with a totally new test name
> instead of adjusting the append to take an extra parameter? Well the
> run_migration shell script uses eval "$@" which unwraps the -append
> leading to any second parameter being split and leaving QEMU very
> confused and the test hanging. This seemed simpler than re-writing all
> the test running logic in something sane ;-)

there is
lib/s390x/vm.h:bool vm_is_tcg(void)

but I don't see any particular ID we could use to differentiate both the
KVM and the TCG mode, do you?

without a more elegant solution,
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric


> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Shashi Mallela <shashi.mallela@linaro.org>
> ---
>  arm/gic.c         |  8 +++++++-
>  arm/unittests.cfg | 10 +++++++++-
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/arm/gic.c b/arm/gic.c
> index bef061a..0fce2a4 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -36,6 +36,7 @@ static struct gic *gic;
>  static int acked[NR_CPUS], spurious[NR_CPUS];
>  static int irq_sender[NR_CPUS], irq_number[NR_CPUS];
>  static cpumask_t ready;
> +static bool under_tcg;
>  
>  static void nr_cpu_check(int nr)
>  {
> @@ -834,7 +835,7 @@ static void test_migrate_unmapped_collection(void)
>  		goto do_migrate;
>  	}
>  
> -	if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
> +	if (!errata(ERRATA_UNMAPPED_COLLECTIONS) && !under_tcg) {
>  		report_skip("Skipping test, as this test hangs without the fix. "
>  			    "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
>  		test_skipped = true;
> @@ -1005,6 +1006,11 @@ int main(int argc, char **argv)
>  		report_prefix_push(argv[1]);
>  		test_migrate_unmapped_collection();
>  		report_prefix_pop();
> +	} else if (!strcmp(argv[1], "its-migrate-unmapped-collection-tcg")) {
> +		under_tcg = true;
> +		report_prefix_push(argv[1]);
> +		test_migrate_unmapped_collection();
> +		report_prefix_pop();
>  	} else if (strcmp(argv[1], "its-introspection") == 0) {
>  		report_prefix_push(argv[1]);
>  		test_its_introspection();
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index 1a39428..adc1bbf 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -205,7 +205,7 @@ extra_params = -machine gic-version=3 -append 'its-pending-migration'
>  groups = its migration
>  arch = arm64
>  
> -[its-migrate-unmapped-collection]
> +[its-migrate-unmapped-collection-kvm]
>  file = gic.flat
>  smp = $MAX_SMP
>  accel = kvm
> @@ -213,6 +213,14 @@ extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection'
>  groups = its migration
>  arch = arm64
>  
> +[its-migrate-unmapped-collection-tcg]
> +file = gic.flat
> +smp = $MAX_SMP
> +accel = tcg
> +extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection-tcg'
> +groups = its migration
> +arch = arm64
> +
>  # Test PSCI emulation
>  [psci]
>  file = psci.flat
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kvm-unit-tests PATCH v2 4/4] arm64: split its-migrate-unmapped-collection into KVM and TCG variants

[Alex Bennée]

Auger Eric <eric.auger@redhat.com> writes:

> Hi Alex,
>
> On 5/25/21 7:26 PM, Alex Bennée wrote:
>> When running the test in TCG we are basically running on bare metal so
>> don't rely on having a particular kernel errata applied.
>> 
>> You might wonder why we handle this with a totally new test name
>> instead of adjusting the append to take an extra parameter? Well the
>> run_migration shell script uses eval "$@" which unwraps the -append
>> leading to any second parameter being split and leaving QEMU very
>> confused and the test hanging. This seemed simpler than re-writing all
>> the test running logic in something sane ;-)
>
> there is
> lib/s390x/vm.h:bool vm_is_tcg(void)
>
> but I don't see any particular ID we could use to differentiate both the
> KVM and the TCG mode, do you?

For -cpu max we do:

        /*
         * Reset MIDR so the guest doesn't mistake our 'max' CPU type for a real
         * one and try to apply errata workarounds or use impdef features we
         * don't provide.
         * An IMPLEMENTER field of 0 means "reserved for software use";
         * ARCHITECTURE must be 0xf indicating "v7 or later, check ID registers
         * to see which features are present";
         * the VARIANT, PARTNUM and REVISION fields are all implementation
         * defined and we choose to define PARTNUM just in case guest
         * code needs to distinguish this QEMU CPU from other software
         * implementations, though this shouldn't be needed.
         */
        t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
        t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
        t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
        t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
        t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
        cpu->midr = t;

However for the default -cpu cortex-a57 we aim to look just like the
real thing - only without any annoying micro-architecture bugs ;-)

>
> without a more elegant solution,

I'll look into the suggestion made by Richard.

> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>
> Thanks
>
> Eric
>
>
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Shashi Mallela <shashi.mallela@linaro.org>
>> ---
>>  arm/gic.c         |  8 +++++++-
>>  arm/unittests.cfg | 10 +++++++++-
>>  2 files changed, 16 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arm/gic.c b/arm/gic.c
>> index bef061a..0fce2a4 100644
>> --- a/arm/gic.c
>> +++ b/arm/gic.c
>> @@ -36,6 +36,7 @@ static struct gic *gic;
>>  static int acked[NR_CPUS], spurious[NR_CPUS];
>>  static int irq_sender[NR_CPUS], irq_number[NR_CPUS];
>>  static cpumask_t ready;
>> +static bool under_tcg;
>>  
>>  static void nr_cpu_check(int nr)
>>  {
>> @@ -834,7 +835,7 @@ static void test_migrate_unmapped_collection(void)
>>  		goto do_migrate;
>>  	}
>>  
>> -	if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
>> +	if (!errata(ERRATA_UNMAPPED_COLLECTIONS) && !under_tcg) {
>>  		report_skip("Skipping test, as this test hangs without the fix. "
>>  			    "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
>>  		test_skipped = true;
>> @@ -1005,6 +1006,11 @@ int main(int argc, char **argv)
>>  		report_prefix_push(argv[1]);
>>  		test_migrate_unmapped_collection();
>>  		report_prefix_pop();
>> +	} else if (!strcmp(argv[1], "its-migrate-unmapped-collection-tcg")) {
>> +		under_tcg = true;
>> +		report_prefix_push(argv[1]);
>> +		test_migrate_unmapped_collection();
>> +		report_prefix_pop();
>>  	} else if (strcmp(argv[1], "its-introspection") == 0) {
>>  		report_prefix_push(argv[1]);
>>  		test_its_introspection();
>> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
>> index 1a39428..adc1bbf 100644
>> --- a/arm/unittests.cfg
>> +++ b/arm/unittests.cfg
>> @@ -205,7 +205,7 @@ extra_params = -machine gic-version=3 -append 'its-pending-migration'
>>  groups = its migration
>>  arch = arm64
>>  
>> -[its-migrate-unmapped-collection]
>> +[its-migrate-unmapped-collection-kvm]
>>  file = gic.flat
>>  smp = $MAX_SMP
>>  accel = kvm
>> @@ -213,6 +213,14 @@ extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection'
>>  groups = its migration
>>  arch = arm64
>>  
>> +[its-migrate-unmapped-collection-tcg]
>> +file = gic.flat
>> +smp = $MAX_SMP
>> +accel = tcg
>> +extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection-tcg'
>> +groups = its migration
>> +arch = arm64
>> +
>>  # Test PSCI emulation
>>  [psci]
>>  file = psci.flat
>> 


-- 
Alex Bennée

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kvm-unit-tests PATCH v2 4/4] arm64: split its-migrate-unmapped-collection into KVM and TCG variants

[Andrew Jones]

On Tue, Jun 01, 2021 at 05:49:01PM +0100, Alex Bennée wrote:
> 
> Auger Eric <eric.auger@redhat.com> writes:
> 
> > Hi Alex,
> >
> > On 5/25/21 7:26 PM, Alex Bennée wrote:
> >> When running the test in TCG we are basically running on bare metal so
> >> don't rely on having a particular kernel errata applied.
> >> 
> >> You might wonder why we handle this with a totally new test name
> >> instead of adjusting the append to take an extra parameter? Well the
> >> run_migration shell script uses eval "$@" which unwraps the -append
> >> leading to any second parameter being split and leaving QEMU very
> >> confused and the test hanging. This seemed simpler than re-writing all
> >> the test running logic in something sane ;-)
> >
> > there is
> > lib/s390x/vm.h:bool vm_is_tcg(void)
> >
> > but I don't see any particular ID we could use to differentiate both the
> > KVM and the TCG mode, do you?
> 
> For -cpu max we do:
> 
>         /*
>          * Reset MIDR so the guest doesn't mistake our 'max' CPU type for a real
>          * one and try to apply errata workarounds or use impdef features we
>          * don't provide.
>          * An IMPLEMENTER field of 0 means "reserved for software use";
>          * ARCHITECTURE must be 0xf indicating "v7 or later, check ID registers
>          * to see which features are present";
>          * the VARIANT, PARTNUM and REVISION fields are all implementation
>          * defined and we choose to define PARTNUM just in case guest
>          * code needs to distinguish this QEMU CPU from other software
>          * implementations, though this shouldn't be needed.
>          */
>         t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
>         t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
>         t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
>         t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
>         t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
>         cpu->midr = t;
> 
> However for the default -cpu cortex-a57 we aim to look just like the
> real thing - only without any annoying micro-architecture bugs ;-)
> 
> >
> > without a more elegant solution,
> 
> I'll look into the suggestion made by Richard.

Where did Richard make a suggestion? And what is it?

Thanks,
drew

> 
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> >
> > Thanks
> >
> > Eric
> >
> >
> >> 
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> Cc: Shashi Mallela <shashi.mallela@linaro.org>
> >> ---
> >>  arm/gic.c         |  8 +++++++-
> >>  arm/unittests.cfg | 10 +++++++++-
> >>  2 files changed, 16 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/arm/gic.c b/arm/gic.c
> >> index bef061a..0fce2a4 100644
> >> --- a/arm/gic.c
> >> +++ b/arm/gic.c
> >> @@ -36,6 +36,7 @@ static struct gic *gic;
> >>  static int acked[NR_CPUS], spurious[NR_CPUS];
> >>  static int irq_sender[NR_CPUS], irq_number[NR_CPUS];
> >>  static cpumask_t ready;
> >> +static bool under_tcg;
> >>  
> >>  static void nr_cpu_check(int nr)
> >>  {
> >> @@ -834,7 +835,7 @@ static void test_migrate_unmapped_collection(void)
> >>  		goto do_migrate;
> >>  	}
> >>  
> >> -	if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
> >> +	if (!errata(ERRATA_UNMAPPED_COLLECTIONS) && !under_tcg) {
> >>  		report_skip("Skipping test, as this test hangs without the fix. "
> >>  			    "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
> >>  		test_skipped = true;
> >> @@ -1005,6 +1006,11 @@ int main(int argc, char **argv)
> >>  		report_prefix_push(argv[1]);
> >>  		test_migrate_unmapped_collection();
> >>  		report_prefix_pop();
> >> +	} else if (!strcmp(argv[1], "its-migrate-unmapped-collection-tcg")) {
> >> +		under_tcg = true;
> >> +		report_prefix_push(argv[1]);
> >> +		test_migrate_unmapped_collection();
> >> +		report_prefix_pop();
> >>  	} else if (strcmp(argv[1], "its-introspection") == 0) {
> >>  		report_prefix_push(argv[1]);
> >>  		test_its_introspection();
> >> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> >> index 1a39428..adc1bbf 100644
> >> --- a/arm/unittests.cfg
> >> +++ b/arm/unittests.cfg
> >> @@ -205,7 +205,7 @@ extra_params = -machine gic-version=3 -append 'its-pending-migration'
> >>  groups = its migration
> >>  arch = arm64
> >>  
> >> -[its-migrate-unmapped-collection]
> >> +[its-migrate-unmapped-collection-kvm]
> >>  file = gic.flat
> >>  smp = $MAX_SMP
> >>  accel = kvm
> >> @@ -213,6 +213,14 @@ extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection'
> >>  groups = its migration
> >>  arch = arm64
> >>  
> >> +[its-migrate-unmapped-collection-tcg]
> >> +file = gic.flat
> >> +smp = $MAX_SMP
> >> +accel = tcg
> >> +extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection-tcg'
> >> +groups = its migration
> >> +arch = arm64
> >> +
> >>  # Test PSCI emulation
> >>  [psci]
> >>  file = psci.flat
> >> 
> 
> 
> -- 
> Alex Bennée
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kvm-unit-tests PATCH v2 4/4] arm64: split its-migrate-unmapped-collection into KVM and TCG variants

[Alex Bennée]

Andrew Jones <drjones@redhat.com> writes:

> On Tue, Jun 01, 2021 at 05:49:01PM +0100, Alex Bennée wrote:
>> 
>> Auger Eric <eric.auger@redhat.com> writes:
>> 
>> > Hi Alex,
>> >
>> > On 5/25/21 7:26 PM, Alex Bennée wrote:
>> >> When running the test in TCG we are basically running on bare metal so
>> >> don't rely on having a particular kernel errata applied.
>> >> 
>> >> You might wonder why we handle this with a totally new test name
>> >> instead of adjusting the append to take an extra parameter? Well the
>> >> run_migration shell script uses eval "$@" which unwraps the -append
>> >> leading to any second parameter being split and leaving QEMU very
>> >> confused and the test hanging. This seemed simpler than re-writing all
>> >> the test running logic in something sane ;-)
>> >
>> > there is
>> > lib/s390x/vm.h:bool vm_is_tcg(void)
>> >
>> > but I don't see any particular ID we could use to differentiate both the
>> > KVM and the TCG mode, do you?
>> 
>> For -cpu max we do:
>> 
>>         /*
>>          * Reset MIDR so the guest doesn't mistake our 'max' CPU type for a real
>>          * one and try to apply errata workarounds or use impdef features we
>>          * don't provide.
>>          * An IMPLEMENTER field of 0 means "reserved for software use";
>>          * ARCHITECTURE must be 0xf indicating "v7 or later, check ID registers
>>          * to see which features are present";
>>          * the VARIANT, PARTNUM and REVISION fields are all implementation
>>          * defined and we choose to define PARTNUM just in case guest
>>          * code needs to distinguish this QEMU CPU from other software
>>          * implementations, though this shouldn't be needed.
>>          */
>>         t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
>>         t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
>>         t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
>>         t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
>>         t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
>>         cpu->midr = t;
>> 
>> However for the default -cpu cortex-a57 we aim to look just like the
>> real thing - only without any annoying micro-architecture bugs ;-)
>> 
>> >
>> > without a more elegant solution,
>> 
>> I'll look into the suggestion made by Richard.
>
> Where did Richard make a suggestion? And what is it?

Sorry - I had a brain fart, I was of course referring to your
ERRATA_FORCE suggestion.

>
> Thanks,
> drew
>
>> 
>> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
>> >
>> > Thanks
>> >
>> > Eric
>> >
>> >
>> >> 
>> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> >> Cc: Shashi Mallela <shashi.mallela@linaro.org>
>> >> ---
>> >>  arm/gic.c         |  8 +++++++-
>> >>  arm/unittests.cfg | 10 +++++++++-
>> >>  2 files changed, 16 insertions(+), 2 deletions(-)
>> >> 
>> >> diff --git a/arm/gic.c b/arm/gic.c
>> >> index bef061a..0fce2a4 100644
>> >> --- a/arm/gic.c
>> >> +++ b/arm/gic.c
>> >> @@ -36,6 +36,7 @@ static struct gic *gic;
>> >>  static int acked[NR_CPUS], spurious[NR_CPUS];
>> >>  static int irq_sender[NR_CPUS], irq_number[NR_CPUS];
>> >>  static cpumask_t ready;
>> >> +static bool under_tcg;
>> >>  
>> >>  static void nr_cpu_check(int nr)
>> >>  {
>> >> @@ -834,7 +835,7 @@ static void test_migrate_unmapped_collection(void)
>> >>  		goto do_migrate;
>> >>  	}
>> >>  
>> >> -	if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
>> >> +	if (!errata(ERRATA_UNMAPPED_COLLECTIONS) && !under_tcg) {
>> >>  		report_skip("Skipping test, as this test hangs without the fix. "
>> >>  			    "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
>> >>  		test_skipped = true;
>> >> @@ -1005,6 +1006,11 @@ int main(int argc, char **argv)
>> >>  		report_prefix_push(argv[1]);
>> >>  		test_migrate_unmapped_collection();
>> >>  		report_prefix_pop();
>> >> +	} else if (!strcmp(argv[1], "its-migrate-unmapped-collection-tcg")) {
>> >> +		under_tcg = true;
>> >> +		report_prefix_push(argv[1]);
>> >> +		test_migrate_unmapped_collection();
>> >> +		report_prefix_pop();
>> >>  	} else if (strcmp(argv[1], "its-introspection") == 0) {
>> >>  		report_prefix_push(argv[1]);
>> >>  		test_its_introspection();
>> >> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
>> >> index 1a39428..adc1bbf 100644
>> >> --- a/arm/unittests.cfg
>> >> +++ b/arm/unittests.cfg
>> >> @@ -205,7 +205,7 @@ extra_params = -machine gic-version=3 -append 'its-pending-migration'
>> >>  groups = its migration
>> >>  arch = arm64
>> >>  
>> >> -[its-migrate-unmapped-collection]
>> >> +[its-migrate-unmapped-collection-kvm]
>> >>  file = gic.flat
>> >>  smp = $MAX_SMP
>> >>  accel = kvm
>> >> @@ -213,6 +213,14 @@ extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection'
>> >>  groups = its migration
>> >>  arch = arm64
>> >>  
>> >> +[its-migrate-unmapped-collection-tcg]
>> >> +file = gic.flat
>> >> +smp = $MAX_SMP
>> >> +accel = tcg
>> >> +extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection-tcg'
>> >> +groups = its migration
>> >> +arch = arm64
>> >> +
>> >>  # Test PSCI emulation
>> >>  [psci]
>> >>  file = psci.flat
>> >> 
>> 
>> 
>> -- 
>> Alex Bennée
>> _______________________________________________
>> kvmarm mailing list
>> kvmarm@lists.cs.columbia.edu
>> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


-- 
Alex Bennée

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kvm-unit-tests PATCH v2 4/4] arm64: split its-migrate-unmapped-collection into KVM and TCG variants

[Andrew Jones]

On Tue, May 25, 2021 at 06:26:28PM +0100, Alex Bennée wrote:
> When running the test in TCG we are basically running on bare metal so
> don't rely on having a particular kernel errata applied.
> 
> You might wonder why we handle this with a totally new test name
> instead of adjusting the append to take an extra parameter? Well the
> run_migration shell script uses eval "$@" which unwraps the -append
> leading to any second parameter being split and leaving QEMU very
> confused and the test hanging. This seemed simpler than re-writing all
> the test running logic in something sane ;-)

Yes, bash is a pain for this. I may try to get migration with more than
one parameter to work at some point though. But, for generally determining
if a unit test is running with tcg or with kvm, we have the QEMU_ACCEL
environment variable. So you could just do getenv("QEMU_ACCEL") in the
unit test. However, I wouldn't use it for this case, since the purpose is
just to force errata to be ignored. We have the "ERRATA_FORCE" environment
variable for that already. You can set it yourself, e.g.

 $ ERRATA_FORCE=y tests/its-migration

or, if you plan to run all tests, then with

 $ ./run_tests.sh -a

but that also runs nodefault tests. Maybe we should teach run_tests.sh
to always set ERRATA_FORCE=y when running with TCG?

Thanks,
drew


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel