diff mbox series

[kvm-unit-tests] fixup! arm/arm64: ITS: pending table migration test

Message ID 20200402180148.490026-1-drjones@redhat.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests] fixup! arm/arm64: ITS: pending table migration test | expand

Commit Message

Andrew Jones April 2, 2020, 6:01 p.m. UTC 
[ Without the fix this test would hang, as timeouts don't work with
  the migration scripts (yet). Use errata to skip instead of hang. ]
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/gic.c  | 18 ++++++++++++++++--
 errata.txt |  1 +
 2 files changed, 17 insertions(+), 2 deletions(-)

Comments

Eric Auger April 3, 2020, 5:07 a.m. UTC | #1 
Hi Drew,

On 4/2/20 8:01 PM, Andrew Jones wrote:
> [ Without the fix this test would hang, as timeouts don't work with
>   the migration scripts (yet). Use errata to skip instead of hang. ]
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  arm/gic.c  | 18 ++++++++++++++++--
>  errata.txt |  1 +
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/arm/gic.c b/arm/gic.c
> index ddf0f9d09b14..c0781f8c2c80 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -12,6 +12,7 @@
>   * This work is licensed under the terms of the GNU LGPL, version 2.
>   */
>  #include <libcflat.h>
> +#include <errata.h>
>  #include <asm/setup.h>
>  #include <asm/processor.h>
>  #include <asm/delay.h>
> @@ -812,13 +813,23 @@ static void test_its_migration(void)
>  	check_lpi_stats("dev7/eventid=255 triggers LPI 8196 on PE #2 after migration");
>  }
>  
> +#define ERRATA_UNMAPPED_COLLECTIONS "ERRATA_8c58be34494b"
> +
>  static void test_migrate_unmapped_collection(void)
>  {
> -	struct its_collection *col;
> -	struct its_device *dev2, *dev7;
> +	struct its_collection *col = NULL;
> +	struct its_device *dev2 = NULL, *dev7 = NULL;
> +	bool test_skipped = false;
>  	int pe0 = 0;
>  	u8 config;
>  
> +	if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
> +		report_skip("Skipping test, as this test hangs without the fix. "
> +			    "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
> +		test_skipped = true;
> +		goto do_migrate;
out of curiosity why do you still do the migration and not directly return.

Besides, what caused the migration to fail without 8c58be34494b is
bypassed so:

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>

Thank you for the fixup

Eric

> +	}
> +
>  	if (its_setup1())
>  		return;
>  
> @@ -830,9 +841,12 @@ static void test_migrate_unmapped_collection(void)
>  	its_send_mapti(dev2, 8192, 0, col);
>  	gicv3_lpi_set_config(8192, LPI_PROP_DEFAULT);
>  
> +do_migrate:
>  	puts("Now migrate the VM, then press a key to continue...\n");
>  	(void)getchar();
>  	report_info("Migration complete");
> +	if (test_skipped)
> +		return;
>  
>  	/* on the destination, map the collection */
>  	its_send_mapc(col, true);
> diff --git a/errata.txt b/errata.txt
> index 7d6abc2a7bf6..b66afaa9c079 100644
> --- a/errata.txt
> +++ b/errata.txt
> @@ -5,4 +5,5 @@
>  9e3f7a296940    : 4.9                           : arm64: KVM: pmu: Fix AArch32 cycle counter access
>  7b6b46311a85    : 4.11                          : KVM: arm/arm64: Emulate the EL1 phys timer registers
>  6c7a5dce22b3    : 4.12                          : KVM: arm/arm64: fix races in kvm_psci_vcpu_on
> +8c58be34494b    : 5.6                           : KVM: arm/arm64: vgic-its: Fix restoration of unmapped collections
>  #---------------:-------------------------------:---------------------------------------------------
>
Andrew Jones April 3, 2020, 7:37 a.m. UTC | #2 
On Fri, Apr 03, 2020 at 07:07:10AM +0200, Auger Eric wrote:
> Hi Drew,
> 
> On 4/2/20 8:01 PM, Andrew Jones wrote:
> > [ Without the fix this test would hang, as timeouts don't work with
> >   the migration scripts (yet). Use errata to skip instead of hang. ]
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  arm/gic.c  | 18 ++++++++++++++++--
> >  errata.txt |  1 +
> >  2 files changed, 17 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arm/gic.c b/arm/gic.c
> > index ddf0f9d09b14..c0781f8c2c80 100644
> > --- a/arm/gic.c
> > +++ b/arm/gic.c
> > @@ -12,6 +12,7 @@
> >   * This work is licensed under the terms of the GNU LGPL, version 2.
> >   */
> >  #include <libcflat.h>
> > +#include <errata.h>
> >  #include <asm/setup.h>
> >  #include <asm/processor.h>
> >  #include <asm/delay.h>
> > @@ -812,13 +813,23 @@ static void test_its_migration(void)
> >  	check_lpi_stats("dev7/eventid=255 triggers LPI 8196 on PE #2 after migration");
> >  }
> >  
> > +#define ERRATA_UNMAPPED_COLLECTIONS "ERRATA_8c58be34494b"
> > +
> >  static void test_migrate_unmapped_collection(void)
> >  {
> > -	struct its_collection *col;
> > -	struct its_device *dev2, *dev7;
> > +	struct its_collection *col = NULL;
> > +	struct its_device *dev2 = NULL, *dev7 = NULL;
> > +	bool test_skipped = false;
> >  	int pe0 = 0;
> >  	u8 config;
> >  
> > +	if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
> > +		report_skip("Skipping test, as this test hangs without the fix. "
> > +			    "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
> > +		test_skipped = true;
> > +		goto do_migrate;
> out of curiosity why do you still do the migration and not directly return.

That won't work for the same reason the migration failure doesn't work.
The problem is with the migration scripts not completing when a migration
test doesn't successfully migrate. I plan to fix that when I get a bit of
time, and when I do, I'll post a patch removing this errata as well, as
it will no longer be needed to avoid test hangs. Anybody testing on a
kernel without the kernel fix after the migration scripts are fixed will
just get an appropriate FAIL instead.

Thanks,
drew

> 
> Besides, what caused the migration to fail without 8c58be34494b is
> bypassed so:
> 
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Eric Auger <eric.auger@redhat.com>
> 
> Thank you for the fixup
> 
> Eric
> 
> > +	}
> > +
> >  	if (its_setup1())
> >  		return;
> >  
> > @@ -830,9 +841,12 @@ static void test_migrate_unmapped_collection(void)
> >  	its_send_mapti(dev2, 8192, 0, col);
> >  	gicv3_lpi_set_config(8192, LPI_PROP_DEFAULT);
> >  
> > +do_migrate:
> >  	puts("Now migrate the VM, then press a key to continue...\n");
> >  	(void)getchar();
> >  	report_info("Migration complete");
> > +	if (test_skipped)
> > +		return;
> >  
> >  	/* on the destination, map the collection */
> >  	its_send_mapc(col, true);
> > diff --git a/errata.txt b/errata.txt
> > index 7d6abc2a7bf6..b66afaa9c079 100644
> > --- a/errata.txt
> > +++ b/errata.txt
> > @@ -5,4 +5,5 @@
> >  9e3f7a296940    : 4.9                           : arm64: KVM: pmu: Fix AArch32 cycle counter access
> >  7b6b46311a85    : 4.11                          : KVM: arm/arm64: Emulate the EL1 phys timer registers
> >  6c7a5dce22b3    : 4.12                          : KVM: arm/arm64: fix races in kvm_psci_vcpu_on
> > +8c58be34494b    : 5.6                           : KVM: arm/arm64: vgic-its: Fix restoration of unmapped collections
> >  #---------------:-------------------------------:---------------------------------------------------
> > 
> 
>
Eric Auger April 3, 2020, 8:23 a.m. UTC | #3 
Hi Drew,
On 4/3/20 9:37 AM, Andrew Jones wrote:
> On Fri, Apr 03, 2020 at 07:07:10AM +0200, Auger Eric wrote:
>> Hi Drew,
>>
>> On 4/2/20 8:01 PM, Andrew Jones wrote:
>>> [ Without the fix this test would hang, as timeouts don't work with
>>>   the migration scripts (yet). Use errata to skip instead of hang. ]
>>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>>> ---
>>>  arm/gic.c  | 18 ++++++++++++++++--
>>>  errata.txt |  1 +
>>>  2 files changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arm/gic.c b/arm/gic.c
>>> index ddf0f9d09b14..c0781f8c2c80 100644
>>> --- a/arm/gic.c
>>> +++ b/arm/gic.c
>>> @@ -12,6 +12,7 @@
>>>   * This work is licensed under the terms of the GNU LGPL, version 2.
>>>   */
>>>  #include <libcflat.h>
>>> +#include <errata.h>
>>>  #include <asm/setup.h>
>>>  #include <asm/processor.h>
>>>  #include <asm/delay.h>
>>> @@ -812,13 +813,23 @@ static void test_its_migration(void)
>>>  	check_lpi_stats("dev7/eventid=255 triggers LPI 8196 on PE #2 after migration");
>>>  }
>>>  
>>> +#define ERRATA_UNMAPPED_COLLECTIONS "ERRATA_8c58be34494b"
>>> +
>>>  static void test_migrate_unmapped_collection(void)
>>>  {
>>> -	struct its_collection *col;
>>> -	struct its_device *dev2, *dev7;
>>> +	struct its_collection *col = NULL;
>>> +	struct its_device *dev2 = NULL, *dev7 = NULL;
>>> +	bool test_skipped = false;
>>>  	int pe0 = 0;
>>>  	u8 config;
>>>  
>>> +	if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
>>> +		report_skip("Skipping test, as this test hangs without the fix. "
>>> +			    "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
>>> +		test_skipped = true;
>>> +		goto do_migrate;
>> out of curiosity why do you still do the migration and not directly return.
> 
> That won't work for the same reason the migration failure doesn't work.
> The problem is with the migration scripts not completing when a migration
> test doesn't successfully migrate. I plan to fix that when I get a bit of
> time, and when I do, I'll post a patch removing this errata as well, as
> it will no longer be needed to avoid test hangs. Anybody testing on a
> kernel without the kernel fix after the migration scripts are fixed will
> just get an appropriate FAIL instead.

OK Got it

Thanks

Eric
> 
> Thanks,
> drew
> 
>>
>> Besides, what caused the migration to fail without 8c58be34494b is
>> bypassed so:
>>
>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>> Tested-by: Eric Auger <eric.auger@redhat.com>
>>
>> Thank you for the fixup
>>
>> Eric
>>
>>> +	}
>>> +
>>>  	if (its_setup1())
>>>  		return;
>>>  
>>> @@ -830,9 +841,12 @@ static void test_migrate_unmapped_collection(void)
>>>  	its_send_mapti(dev2, 8192, 0, col);
>>>  	gicv3_lpi_set_config(8192, LPI_PROP_DEFAULT);
>>>  
>>> +do_migrate:
>>>  	puts("Now migrate the VM, then press a key to continue...\n");
>>>  	(void)getchar();
>>>  	report_info("Migration complete");
>>> +	if (test_skipped)
>>> +		return;
>>>  
>>>  	/* on the destination, map the collection */
>>>  	its_send_mapc(col, true);
>>> diff --git a/errata.txt b/errata.txt
>>> index 7d6abc2a7bf6..b66afaa9c079 100644
>>> --- a/errata.txt
>>> +++ b/errata.txt
>>> @@ -5,4 +5,5 @@
>>>  9e3f7a296940    : 4.9                           : arm64: KVM: pmu: Fix AArch32 cycle counter access
>>>  7b6b46311a85    : 4.11                          : KVM: arm/arm64: Emulate the EL1 phys timer registers
>>>  6c7a5dce22b3    : 4.12                          : KVM: arm/arm64: fix races in kvm_psci_vcpu_on
>>> +8c58be34494b    : 5.6                           : KVM: arm/arm64: vgic-its: Fix restoration of unmapped collections
>>>  #---------------:-------------------------------:---------------------------------------------------
>>>
>>
>>
>
diff mbox series

Patch

diff --git a/arm/gic.c b/arm/gic.c
index ddf0f9d09b14..c0781f8c2c80 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -12,6 +12,7 @@ 
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 #include <libcflat.h>
+#include <errata.h>
 #include <asm/setup.h>
 #include <asm/processor.h>
 #include <asm/delay.h>
@@ -812,13 +813,23 @@  static void test_its_migration(void)
 	check_lpi_stats("dev7/eventid=255 triggers LPI 8196 on PE #2 after migration");
 }
 
+#define ERRATA_UNMAPPED_COLLECTIONS "ERRATA_8c58be34494b"
+
 static void test_migrate_unmapped_collection(void)
 {
-	struct its_collection *col;
-	struct its_device *dev2, *dev7;
+	struct its_collection *col = NULL;
+	struct its_device *dev2 = NULL, *dev7 = NULL;
+	bool test_skipped = false;
 	int pe0 = 0;
 	u8 config;
 
+	if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
+		report_skip("Skipping test, as this test hangs without the fix. "
+			    "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
+		test_skipped = true;
+		goto do_migrate;
+	}
+
 	if (its_setup1())
 		return;
 
@@ -830,9 +841,12 @@  static void test_migrate_unmapped_collection(void)
 	its_send_mapti(dev2, 8192, 0, col);
 	gicv3_lpi_set_config(8192, LPI_PROP_DEFAULT);
 
+do_migrate:
 	puts("Now migrate the VM, then press a key to continue...\n");
 	(void)getchar();
 	report_info("Migration complete");
+	if (test_skipped)
+		return;
 
 	/* on the destination, map the collection */
 	its_send_mapc(col, true);
diff --git a/errata.txt b/errata.txt
index 7d6abc2a7bf6..b66afaa9c079 100644
--- a/errata.txt
+++ b/errata.txt
@@ -5,4 +5,5 @@ 
 9e3f7a296940    : 4.9                           : arm64: KVM: pmu: Fix AArch32 cycle counter access
 7b6b46311a85    : 4.11                          : KVM: arm/arm64: Emulate the EL1 phys timer registers
 6c7a5dce22b3    : 4.12                          : KVM: arm/arm64: fix races in kvm_psci_vcpu_on
+8c58be34494b    : 5.6                           : KVM: arm/arm64: vgic-its: Fix restoration of unmapped collections
 #---------------:-------------------------------:---------------------------------------------------