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

["Alex Bennée" <alex.bennee@linaro.org>]

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