Re: [kvm-unit-tests PATCH v7 10/13] arm/arm64: ITS: INT functional tests

[Zenghui Yu]

Hi Eric,

On 2020/3/20 17:24, Eric Auger wrote:
> Triggers LPIs through the INT command.
>
> the test checks the LPI hits the right CPU and triggers
> the right LPI intid, ie. the translation is correct.
>
> Updates to the config table also are tested, along with inv
> and invall commands.
>
> Signed-off-by: Eric Auger <address@hidden>

[...]

So I've tested this series and found that the "INT" test will sometimes
fail.

"not ok 12 - gicv3: its-migration: dev2/eventid=20 triggers LPI 8195 en 
PE #3 after migration
not ok 13 - gicv3: its-migration: dev7/eventid=255 triggers LPI 8196 on 
PE #2 after migration"

From logs:
"INFO: gicv3: its-migration: Migration complete
INT dev_id=2 event_id=20
INFO: gicv3: its-migration: No LPI received whereas (cpuid=3, 
intid=8195) was expected
FAIL: gicv3: its-migration: dev2/eventid=20 triggers LPI 8195 en PE #3 
after migration
INT dev_id=7 event_id=255
INFO: gicv3: its-migration: No LPI received whereas (cpuid=2, 
intid=8196) was expected
FAIL: gicv3: its-migration: dev7/eventid=255 triggers LPI 8196 on PE #2 
after migration"

> +static void check_lpi_stats(const char *msg)
> +{
> +       bool pass = false;
> +
> +       mdelay(100);

After changing this to 'mdelay(1000)', the above error doesn't show up
anymore. But it sounds strange that 100ms is not enough to deliver a
single LPI. I haven't dig it further but will get back here later.

> +       smp_rmb(); /* pairs with wmb in lpi_handler */
> +       if (lpi_stats.observed.cpu_id != lpi_stats.expected.cpu_id ||
> +           lpi_stats.observed.lpi_id != lpi_stats.expected.lpi_id) {
> +               if (lpi_stats.observed.cpu_id == -1 &&
> +                   lpi_stats.observed.lpi_id == -1) {
> +                       report_info("No LPI received whereas (cpuid=%d, intid=%d) 
> "
> +                                   "was expected", lpi_stats.expected.cpu_id,
> +                                   lpi_stats.expected.lpi_id);
> +               } else {
> +                       report_info("Unexpected LPI (cpuid=%d, intid=%d)",
> +                                   lpi_stats.observed.cpu_id,
> +                                   lpi_stats.observed.lpi_id);
> +               }
> +       } else {
> +               pass = true;
> +       }
> +       report(pass, "%s", msg);
> +}

This patch itself looks good to me,
Reviewed-by: Zenghui Yu <address@hidden>


Thanks