Re: [kvm-unit-tests PATCH v1 1/4] arm64: split its-trigger test into KVM and TCG variants

[Alex Bennée]

Alex Bennée <alex.bennee@linaro.org> writes:

> Marc Zyngier <maz@kernel.org> writes:
>
>> On Wed, 28 Apr 2021 15:00:15 +0100,
>> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>>> 
>>> I interpret that as that an INVALL guarantees that a change is
>>> visible, but it the change can become visible even without the
>>> INVALL.
>>
>> Yes. Expecting the LPI to be delivered or not in the absence of an
>> invalidate when its configuration has been altered is wrong. The
>> architecture doesn't guarantee anything of the sort.
>
> Is the underlying hypervisor allowed to invalidate and reload the
> configuration whenever it wants or should it only be driven by the
> guests requests?
>
> I did consider a more nuanced variant of the test that allowed for a
> delivery pre-inval and a pass for post-inval as long as it had been
> delivered one way or another:
>
> --8<---------------cut here---------------start------------->8---
> modified   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)
>  {
> @@ -687,6 +688,7 @@ static void test_its_trigger(void)
>       struct its_collection *col3;
>       struct its_device *dev2, *dev7;
>       cpumask_t mask;
> +     bool before, after;
>  
>       if (its_setup1())
>               return;
> @@ -734,15 +736,17 @@ static void test_its_trigger(void)
>       /*
>        * re-enable the LPI but willingly do not call invall
>        * so the change in config is not taken into account.
> -      * The LPI should not hit
> +      * The LPI should not hit. This does however depend on
> +      * implementation defined behaviour - under QEMU TCG emulation
> +      * it can quite correctly process the event directly.
>        */
>       gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT);
>       stats_reset();
>       cpumask_clear(&mask);
>       its_send_int(dev2, 20);
>       wait_for_interrupts(&mask);
> -     report(check_acked(&mask, -1, -1),
> -                     "dev2/eventid=20 still does not trigger any LPI");
> +     before = check_acked(&mask, -1, -1);
> +     report_xfail(under_tcg, before, "dev2/eventid=20 still may not trigger 
> any LPI");
>  
>       /* Now call the invall and check the LPI hits */
>       stats_reset();
> @@ -750,8 +754,8 @@ static void test_its_trigger(void)
>       cpumask_set_cpu(3, &mask);
>       its_send_invall(col3);
>       wait_for_interrupts(&mask);
> -     report(check_acked(&mask, 0, 8195),
> -                     "dev2/eventid=20 pending LPI is received");
> +     after = check_acked(&mask, 0, 8195);
> +     report(before != after, "dev2/eventid=20 pending LPI is
>       received");

Actually that should be:

         report(after || !before, "dev2/eventid=20 pending LPI is received");

so either the IRQ arrives after the flush or it had previously.

-- 
Alex Bennée