| Message ID | 20200320092428.20880-14-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm/arm64: Add ITS tests | expand |
Hi Eric, On 2020/3/20 17:24, Eric Auger wrote: > Add two new migration tests. One testing the migration of > a topology where collection were unmapped. The second test > checks the migration of the pending table. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> [...] > @@ -659,6 +678,15 @@ static int its_prerequisites(int nb_cpus) > return 0; > } > > +static void set_lpi(struct its_device *dev, u32 eventid, u32 physid, > + struct its_collection *col) > +{ > + assert(dev && col); > + > + its_send_mapti(dev, physid, eventid, col); > + gicv3_lpi_set_config(physid, LPI_PROP_DEFAULT); > +} I'd say we can just drop this helper and open-code it anywhere necessarily. The name 'set_lpi' doesn't tell me too much about what has been implemented inside the helper. > + > /* > * Setup the configuration for those mappings: > * dev_id=2 event=20 -> vcpu 3, intid=8195 > @@ -790,6 +818,108 @@ static void test_its_migration(void) > its_send_int(dev7, 255); > check_lpi_stats("dev7/eventid=255 triggers LPI 8196 on PE #2 after migration"); > } > + > +static void test_migrate_unmapped_collection(void) > +{ > + struct its_collection *col; > + struct its_device *dev2, *dev7; > + int pe0 = 0; > + u8 config; > + > + if (its_setup1()) > + return; > + > + col = its_create_collection(pe0, pe0); > + dev2 = its_get_device(2); > + dev7 = its_get_device(7); > + > + /* MAPTI with the collection unmapped */ > + set_lpi(dev2, 0, 8192, col); ... and it's only invoked here. > + > + puts("Now migrate the VM, then press a key to continue...\n"); > + (void)getchar(); > + report_info("Migration complete"); > + > + /* on the destination, map the collection */ > + its_send_mapc(col, true); > + its_send_invall(col); > + > + lpi_stats_expect(2, 8196); > + its_send_int(dev7, 255); > + check_lpi_stats("dev7/eventid= 255 triggered LPI 8196 on PE #2"); > + > + config = gicv3_lpi_get_config(8192); > + report(config == LPI_PROP_DEFAULT, > + "Config of LPI 8192 was properly migrated"); > + > + lpi_stats_expect(pe0, 8192); > + its_send_int(dev2, 0); > + check_lpi_stats("dev2/eventid = 0 triggered LPI 8192 on PE0"); > +} > + > +static void test_its_pending_migration(void) > +{ > + struct its_device *dev; > + struct its_collection *collection[2]; > + int *expected = malloc(nr_cpus * sizeof(int)); > + int pe0 = nr_cpus - 1, pe1 = nr_cpus - 2; > + u64 pendbaser; > + void *ptr; > + int i; > + > + if (its_prerequisites(4)) > + return; > + > + dev = its_create_device(2 /* dev id */, 8 /* nb_ites */); > + its_send_mapd(dev, true); > + > + collection[0] = its_create_collection(pe0, pe0); > + collection[1] = its_create_collection(pe1, pe1); > + its_send_mapc(collection[0], true); > + its_send_mapc(collection[1], true); > + > + /* disable lpi at redist level */ > + gicv3_lpi_rdist_disable(pe0); > + gicv3_lpi_rdist_disable(pe1); > + > + /* lpis are interleaved inbetween the 2 PEs */ > + for (i = 0; i < 256; i++) { > + struct its_collection *col = i % 2 ? collection[0] : > + collection[1]; > + int vcpu = col->target_address >> 16; > + > + its_send_mapti(dev, LPI(i), i, col); > + gicv3_lpi_set_config(LPI(i), LPI_PROP_DEFAULT); > + gicv3_lpi_set_clr_pending(vcpu, LPI(i), true); > + } > + its_send_invall(collection[0]); > + its_send_invall(collection[1]); > + > + /* Clear the PTZ bit on each pendbaser */ > + > + expected[pe0] = 128; > + expected[pe1] = 128; Do we need to initialize expected[] for other PEs? Or it has always been zeroed by the kvm-unit-tests implementation of malloc()? > + > + ptr = gicv3_data.redist_base[pe0] + GICR_PENDBASER; > + pendbaser = readq(ptr); > + writeq(pendbaser & ~GICR_PENDBASER_PTZ, ptr); > + > + ptr = gicv3_data.redist_base[pe1] + GICR_PENDBASER; > + pendbaser = readq(ptr); > + writeq(pendbaser & ~GICR_PENDBASER_PTZ, ptr); > + > + gicv3_lpi_rdist_enable(pe0); > + gicv3_lpi_rdist_enable(pe1); I don't know how the migration gets implemented in kvm-unit-tests. But is there any guarantee that the LPIs will only be triggered on the destination side? As once the EnableLPIs bit becomes 1, VGIC will start reading the pending bit in guest memory and potentially injecting LPIs into the target vcpu (in the source side). > + > + puts("Now migrate the VM, then press a key to continue...\n"); > + (void)getchar(); > + report_info("Migration complete"); > + > + /* let's wait for the 256 LPIs to be handled */ > + mdelay(1000); > + > + check_lpi_hits(expected, "128 LPIs on both PE0 and PE1 after migration"); > +} Thanks, Zenghui
Hi Zenghui, On 3/30/20 2:06 PM, Zenghui Yu wrote: > Hi Eric, > > On 2020/3/20 17:24, Eric Auger wrote: >> Add two new migration tests. One testing the migration of >> a topology where collection were unmapped. The second test >> checks the migration of the pending table. >> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> > > [...] > >> @@ -659,6 +678,15 @@ static int its_prerequisites(int nb_cpus) >> return 0; >> } >> +static void set_lpi(struct its_device *dev, u32 eventid, u32 physid, >> + struct its_collection *col) >> +{ >> + assert(dev && col); >> + >> + its_send_mapti(dev, physid, eventid, col); >> + gicv3_lpi_set_config(physid, LPI_PROP_DEFAULT); >> +} > > I'd say we can just drop this helper and open-code it anywhere > necessarily. The name 'set_lpi' doesn't tell me too much about > what has been implemented inside the helper. > >> + >> /* >> * Setup the configuration for those mappings: >> * dev_id=2 event=20 -> vcpu 3, intid=8195 >> @@ -790,6 +818,108 @@ static void test_its_migration(void) >> its_send_int(dev7, 255); >> check_lpi_stats("dev7/eventid=255 triggers LPI 8196 on PE #2 >> after migration"); >> } >> + >> +static void test_migrate_unmapped_collection(void) >> +{ >> + struct its_collection *col; >> + struct its_device *dev2, *dev7; >> + int pe0 = 0; >> + u8 config; >> + >> + if (its_setup1()) >> + return; >> + >> + col = its_create_collection(pe0, pe0); >> + dev2 = its_get_device(2); >> + dev7 = its_get_device(7); >> + >> + /* MAPTI with the collection unmapped */ >> + set_lpi(dev2, 0, 8192, col); > > ... and it's only invoked here. > >> + >> + puts("Now migrate the VM, then press a key to continue...\n"); >> + (void)getchar(); >> + report_info("Migration complete"); >> + >> + /* on the destination, map the collection */ >> + its_send_mapc(col, true); >> + its_send_invall(col); >> + >> + lpi_stats_expect(2, 8196); >> + its_send_int(dev7, 255); >> + check_lpi_stats("dev7/eventid= 255 triggered LPI 8196 on PE #2"); >> + >> + config = gicv3_lpi_get_config(8192); >> + report(config == LPI_PROP_DEFAULT, >> + "Config of LPI 8192 was properly migrated"); >> + >> + lpi_stats_expect(pe0, 8192); >> + its_send_int(dev2, 0); >> + check_lpi_stats("dev2/eventid = 0 triggered LPI 8192 on PE0"); >> +} >> + >> +static void test_its_pending_migration(void) >> +{ >> + struct its_device *dev; >> + struct its_collection *collection[2]; >> + int *expected = malloc(nr_cpus * sizeof(int)); >> + int pe0 = nr_cpus - 1, pe1 = nr_cpus - 2; >> + u64 pendbaser; >> + void *ptr; >> + int i; >> + >> + if (its_prerequisites(4)) >> + return; >> + >> + dev = its_create_device(2 /* dev id */, 8 /* nb_ites */); >> + its_send_mapd(dev, true); >> + >> + collection[0] = its_create_collection(pe0, pe0); >> + collection[1] = its_create_collection(pe1, pe1); >> + its_send_mapc(collection[0], true); >> + its_send_mapc(collection[1], true); >> + >> + /* disable lpi at redist level */ >> + gicv3_lpi_rdist_disable(pe0); >> + gicv3_lpi_rdist_disable(pe1); >> + >> + /* lpis are interleaved inbetween the 2 PEs */ >> + for (i = 0; i < 256; i++) { >> + struct its_collection *col = i % 2 ? collection[0] : >> + collection[1]; >> + int vcpu = col->target_address >> 16; >> + >> + its_send_mapti(dev, LPI(i), i, col); >> + gicv3_lpi_set_config(LPI(i), LPI_PROP_DEFAULT); >> + gicv3_lpi_set_clr_pending(vcpu, LPI(i), true); >> + } >> + its_send_invall(collection[0]); >> + its_send_invall(collection[1]); >> + >> + /* Clear the PTZ bit on each pendbaser */ >> + >> + expected[pe0] = 128; >> + expected[pe1] = 128; > > Do we need to initialize expected[] for other PEs? Or it has always > been zeroed by the kvm-unit-tests implementation of malloc()? > >> + >> + ptr = gicv3_data.redist_base[pe0] + GICR_PENDBASER; >> + pendbaser = readq(ptr); >> + writeq(pendbaser & ~GICR_PENDBASER_PTZ, ptr); >> + >> + ptr = gicv3_data.redist_base[pe1] + GICR_PENDBASER; >> + pendbaser = readq(ptr); >> + writeq(pendbaser & ~GICR_PENDBASER_PTZ, ptr); >> + >> + gicv3_lpi_rdist_enable(pe0); >> + gicv3_lpi_rdist_enable(pe1); > > I don't know how the migration gets implemented in kvm-unit-tests. > But is there any guarantee that the LPIs will only be triggered on the > destination side? As once the EnableLPIs bit becomes 1, VGIC will start > reading the pending bit in guest memory and potentially injecting LPIs > into the target vcpu (in the source side). I expect some LPIs to hit on source and some others to hit on the destination. To me, this does not really matter as long as the handlers gets called and accumulate the stats. Given the number of LPIs, we will at least test the migration of some of the pending bits and especially adjacent ones. It does work as it allows to test your fix: ca185b260951 KVM: arm/arm64: vgic: Don't rely on the wrong pending table Thanks Eric > >> + >> + puts("Now migrate the VM, then press a key to continue...\n"); >> + (void)getchar(); >> + report_info("Migration complete"); >> + >> + /* let's wait for the 256 LPIs to be handled */ >> + mdelay(1000); >> + >> + check_lpi_hits(expected, "128 LPIs on both PE0 and PE1 after >> migration"); >> +} > > > Thanks, > Zenghui > >
On 2020/3/30 20:38, Auger Eric wrote: > Hi Zenghui, [...] >>> + >>> + ptr = gicv3_data.redist_base[pe0] + GICR_PENDBASER; >>> + pendbaser = readq(ptr); >>> + writeq(pendbaser & ~GICR_PENDBASER_PTZ, ptr); >>> + >>> + ptr = gicv3_data.redist_base[pe1] + GICR_PENDBASER; >>> + pendbaser = readq(ptr); >>> + writeq(pendbaser & ~GICR_PENDBASER_PTZ, ptr); >>> + >>> + gicv3_lpi_rdist_enable(pe0); >>> + gicv3_lpi_rdist_enable(pe1); >> >> I don't know how the migration gets implemented in kvm-unit-tests. >> But is there any guarantee that the LPIs will only be triggered on the >> destination side? As once the EnableLPIs bit becomes 1, VGIC will start >> reading the pending bit in guest memory and potentially injecting LPIs >> into the target vcpu (in the source side). > > I expect some LPIs to hit on source and some others to hit on the > destination. To me, this does not really matter as long as the handlers > gets called and accumulate the stats. Given the number of LPIs, we will > at least test the migration of some of the pending bits and especially > adjacent ones. It does work as it allows to test your fix: > > ca185b260951 KVM: arm/arm64: vgic: Don't rely on the wrong pending table Fair enough. Thanks for your explanation! Zenghui
diff --git a/arm/gic.c b/arm/gic.c index 6ecfdbc..2c661dc 100644 --- a/arm/gic.c +++ b/arm/gic.c @@ -193,6 +193,7 @@ static void lpi_handler(struct pt_regs *regs __unused) smp_rmb(); /* pairs with wmb in lpi_stats_expect */ lpi_stats.observed.cpu_id = smp_processor_id(); lpi_stats.observed.lpi_id = irqnr; + acked[lpi_stats.observed.cpu_id]++; smp_wmb(); /* pairs with rmb in check_lpi_stats */ } @@ -236,6 +237,22 @@ static void secondary_lpi_test(void) while (1) wfi(); } + +static void check_lpi_hits(int *expected, const char *msg) +{ + bool pass = true; + int i; + + for_each_present_cpu(i) { + if (acked[i] != expected[i]) { + report_info("expected %d LPIs on PE #%d, %d observed", + expected[i], i, acked[i]); + pass = false; + break; + } + } + report(pass, "%s", msg); +} #endif static void gicv2_ipi_send_self(void) @@ -591,6 +608,8 @@ static void gic_test_mmio(void) static void test_its_introspection(void) {} static void test_its_trigger(void) {} static void test_its_migration(void) {} +static void test_its_pending_migration(void) {} +static void test_migrate_unmapped_collection(void) {} #else /* __aarch64__ */ @@ -659,6 +678,15 @@ static int its_prerequisites(int nb_cpus) return 0; } +static void set_lpi(struct its_device *dev, u32 eventid, u32 physid, + struct its_collection *col) +{ + assert(dev && col); + + its_send_mapti(dev, physid, eventid, col); + gicv3_lpi_set_config(physid, LPI_PROP_DEFAULT); +} + /* * Setup the configuration for those mappings: * dev_id=2 event=20 -> vcpu 3, intid=8195 @@ -790,6 +818,108 @@ static void test_its_migration(void) its_send_int(dev7, 255); check_lpi_stats("dev7/eventid=255 triggers LPI 8196 on PE #2 after migration"); } + +static void test_migrate_unmapped_collection(void) +{ + struct its_collection *col; + struct its_device *dev2, *dev7; + int pe0 = 0; + u8 config; + + if (its_setup1()) + return; + + col = its_create_collection(pe0, pe0); + dev2 = its_get_device(2); + dev7 = its_get_device(7); + + /* MAPTI with the collection unmapped */ + set_lpi(dev2, 0, 8192, col); + + puts("Now migrate the VM, then press a key to continue...\n"); + (void)getchar(); + report_info("Migration complete"); + + /* on the destination, map the collection */ + its_send_mapc(col, true); + its_send_invall(col); + + lpi_stats_expect(2, 8196); + its_send_int(dev7, 255); + check_lpi_stats("dev7/eventid= 255 triggered LPI 8196 on PE #2"); + + config = gicv3_lpi_get_config(8192); + report(config == LPI_PROP_DEFAULT, + "Config of LPI 8192 was properly migrated"); + + lpi_stats_expect(pe0, 8192); + its_send_int(dev2, 0); + check_lpi_stats("dev2/eventid = 0 triggered LPI 8192 on PE0"); +} + +static void test_its_pending_migration(void) +{ + struct its_device *dev; + struct its_collection *collection[2]; + int *expected = malloc(nr_cpus * sizeof(int)); + int pe0 = nr_cpus - 1, pe1 = nr_cpus - 2; + u64 pendbaser; + void *ptr; + int i; + + if (its_prerequisites(4)) + return; + + dev = its_create_device(2 /* dev id */, 8 /* nb_ites */); + its_send_mapd(dev, true); + + collection[0] = its_create_collection(pe0, pe0); + collection[1] = its_create_collection(pe1, pe1); + its_send_mapc(collection[0], true); + its_send_mapc(collection[1], true); + + /* disable lpi at redist level */ + gicv3_lpi_rdist_disable(pe0); + gicv3_lpi_rdist_disable(pe1); + + /* lpis are interleaved inbetween the 2 PEs */ + for (i = 0; i < 256; i++) { + struct its_collection *col = i % 2 ? collection[0] : + collection[1]; + int vcpu = col->target_address >> 16; + + its_send_mapti(dev, LPI(i), i, col); + gicv3_lpi_set_config(LPI(i), LPI_PROP_DEFAULT); + gicv3_lpi_set_clr_pending(vcpu, LPI(i), true); + } + its_send_invall(collection[0]); + its_send_invall(collection[1]); + + /* Clear the PTZ bit on each pendbaser */ + + expected[pe0] = 128; + expected[pe1] = 128; + + ptr = gicv3_data.redist_base[pe0] + GICR_PENDBASER; + pendbaser = readq(ptr); + writeq(pendbaser & ~GICR_PENDBASER_PTZ, ptr); + + ptr = gicv3_data.redist_base[pe1] + GICR_PENDBASER; + pendbaser = readq(ptr); + writeq(pendbaser & ~GICR_PENDBASER_PTZ, ptr); + + gicv3_lpi_rdist_enable(pe0); + gicv3_lpi_rdist_enable(pe1); + + puts("Now migrate the VM, then press a key to continue...\n"); + (void)getchar(); + report_info("Migration complete"); + + /* let's wait for the 256 LPIs to be handled */ + mdelay(1000); + + check_lpi_hits(expected, "128 LPIs on both PE0 and PE1 after migration"); +} #endif int main(int argc, char **argv) @@ -831,6 +961,14 @@ int main(int argc, char **argv) report_prefix_push(argv[1]); test_its_migration(); report_prefix_pop(); + } else if (!strcmp(argv[1], "its-pending-migration")) { + report_prefix_push(argv[1]); + test_its_pending_migration(); + report_prefix_pop(); + } else if (!strcmp(argv[1], "its-migrate-unmapped-collection")) { + 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 480adec..b96f0a1 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -144,6 +144,22 @@ extra_params = -machine gic-version=3 -append 'its-migration' groups = its migration 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 + +[its-migrate-unmapped-collection] +file = gic.flat +smp = $MAX_SMP +accel = kvm +extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection' +groups = its migration +arch = arm64 + # Test PSCI emulation [psci] file = psci.flat
Add two new migration tests. One testing the migration of a topology where collection were unmapped. The second test checks the migration of the pending table. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- v6 -> v7: - test_migrate_unmapped_collection now uses pe0=0. Otherwise, depending on SMP value it collides with collections created by setup1() - use of for_each_present_cpu v5 -> v6: - s/Set the PTZ/Clear the PTZ - Move the collection inval after MAPC - remove the unmap collection test v4 -> v5: - move stub from header to arm/gic.c v3 -> v4: - do not talk about odd/even CPUs, use pe0 and pe1 - comment the delay v2 -> v3: - tests belong to both its and migration groups - use LPI(i) - gicv3_lpi_set_pending_table_bit renamed into gicv3_lpi_set_clr_pending --- arm/gic.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++ arm/unittests.cfg | 16 ++++++ 2 files changed, 154 insertions(+)