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

[Andrew Jones]

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 <address@hidden>
> > ---
> >  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 <address@hidden>
> Tested-by: Eric Auger <address@hidden>
> 
> 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
> >  
> > #---------------:-------------------------------:---------------------------------------------------
> > 
> 
>