diff mbox series

[kvm-unit-tests,v5,10/13] arm/arm64: ITS: INT functional tests

Message ID 20200310145410.26308-11-eric.auger@redhat.com (mailing list archive)
State New, archived
Headers show
Series arm/arm64: Add ITS tests | expand

Commit Message

Eric Auger March 10, 2020, 2:54 p.m. UTC
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 <eric.auger@redhat.com>

---
v4 -> v5:
- move the test stub from the header to arm/gic.c

v3 -> v4:
- assert in lpi_handler if the interrupt is not an LPI
- remove check_lpi_stats from its_prerequisites()

v2 -> v3:
- add comments
- keep the report_skip in case there aren't 4 vcpus to be able to
  run other tests in the its category.
- fix the prefix pop
- move its_event and its_stats to arm/gic.c
---
 arm/gic.c         | 223 +++++++++++++++++++++++++++++++++++++++++++---
 arm/unittests.cfg |   7 ++
 2 files changed, 219 insertions(+), 11 deletions(-)

Comments

Zenghui Yu March 11, 2020, 11:59 a.m. UTC | #1
Hi Eric,

On 2020/3/10 22:54, 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 <eric.auger@redhat.com>
> 
> ---

[...]

> +static void test_its_trigger(void)
> +{
> +	struct its_collection *col3, *col2;
> +	struct its_device *dev2, *dev7;
> +
> +	if (its_prerequisites(4))
> +		return;
> +
> +	dev2 = its_create_device(2 /* dev id */, 8 /* nb_ites */);
> +	dev7 = its_create_device(7 /* dev id */, 8 /* nb_ites */);
> +
> +	col3 = its_create_collection(3 /* col id */, 3/* target PE */);
> +	col2 = its_create_collection(2 /* col id */, 2/* target PE */);
> +
> +	gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT);
> +	gicv3_lpi_set_config(8196, LPI_PROP_DEFAULT);
> +
> +	its_send_invall(col2);
> +	its_send_invall(col3);

These two INVALLs should be issued after col2 and col3 are mapped,
otherwise this will cause the INVALL command error as per the spec
(though KVM doesn't complain it at all).

> +
> +	report_prefix_push("int");
> +	/*
> +	 * dev=2, eventid=20  -> lpi= 8195, col=3
> +	 * dev=7, eventid=255 -> lpi= 8196, col=2
> +	 * Trigger dev2, eventid=20 and dev7, eventid=255
> +	 * Check both LPIs hit
> +	 */
> +
> +	its_send_mapd(dev2, true);
> +	its_send_mapd(dev7, true);
> +
> +	its_send_mapc(col3, true);
> +	its_send_mapc(col2, true);
> +
> +	its_send_mapti(dev2, 8195 /* lpi id */, 20 /* event id */, col3);
> +	its_send_mapti(dev7, 8196 /* lpi id */, 255 /* event id */, col2);
> +
> +	lpi_stats_expect(3, 8195);
> +	its_send_int(dev2, 20);
> +	check_lpi_stats("dev=2, eventid=20  -> lpi= 8195, col=3");
> +
> +	lpi_stats_expect(2, 8196);
> +	its_send_int(dev7, 255);
> +	check_lpi_stats("dev=7, eventid=255 -> lpi= 8196, col=2");
> +
> +	report_prefix_pop();
> +
> +	report_prefix_push("inv/invall");
> +
> +	/*
> +	 * disable 8195, check dev2/eventid=20 does not trigger the
> +	 * corresponding LPI
> +	 */
> +	gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT & ~LPI_PROP_ENABLED);
> +	its_send_inv(dev2, 20);
> +
> +	lpi_stats_expect(-1, -1);
> +	its_send_int(dev2, 20);
> +	check_lpi_stats("dev2/eventid=20 does not trigger any LPI");
> +
> +	/*
> +	 * 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
> +	 */
> +	gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT);
> +	lpi_stats_expect(-1, -1);
> +	its_send_int(dev2, 20);
> +	check_lpi_stats("dev2/eventid=20 still does not trigger any LPI");
> +
> +	/* Now call the invall and check the LPI hits */
> +	its_send_invall(col3);
> +	lpi_stats_expect(3, 8195);
> +	its_send_int(dev2, 20);
> +	check_lpi_stats("dev2/eventid=20 now triggers an LPI");
> +
> +	report_prefix_pop();
> +
> +	report_prefix_push("mapd valid=false");
> +	/*
> +	 * Unmap device 2 and check the eventid 20 formerly
> +	 * attached to it does not hit anymore
> +	 */
> +
> +	its_send_mapd(dev2, false);
> +	lpi_stats_expect(-1, -1);
> +	its_send_int(dev2, 20);
> +	check_lpi_stats("no LPI after device unmap");
> +	report_prefix_pop();
> +
> +	/* Unmap the collection this time and check no LPI does hit */
> +	report_prefix_push("mapc valid=false");
> +	its_send_mapc(col2, false);

And as for the MAPC, the spec says:

" When V is 0:
Behavior is unpredictable if there are interrupts that are mapped to the
specified collection, with the restriction that further translation
requests from that device are ignored. "

So this collection-unmap test may not make sense?

> +	lpi_stats_expect(-1, -1);
> +	its_send_int(dev7, 255);
> +	check_lpi_stats("no LPI after collection unmap");
> +	report_prefix_pop();
> +}

[...]

Otherwise looks good.


Thanks,
Zenghui
Eric Auger March 11, 2020, 1:48 p.m. UTC | #2
Hi Zenghui,

On 3/11/20 12:59 PM, Zenghui Yu wrote:
> Hi Eric,
> 
> On 2020/3/10 22:54, 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 <eric.auger@redhat.com>
>>
>> ---
> 
> [...]
> 
>> +static void test_its_trigger(void)
>> +{
>> +    struct its_collection *col3, *col2;
>> +    struct its_device *dev2, *dev7;
>> +
>> +    if (its_prerequisites(4))
>> +        return;
>> +
>> +    dev2 = its_create_device(2 /* dev id */, 8 /* nb_ites */);
>> +    dev7 = its_create_device(7 /* dev id */, 8 /* nb_ites */);
>> +
>> +    col3 = its_create_collection(3 /* col id */, 3/* target PE */);
>> +    col2 = its_create_collection(2 /* col id */, 2/* target PE */);
>> +
>> +    gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT);
>> +    gicv3_lpi_set_config(8196, LPI_PROP_DEFAULT);
>> +
>> +    its_send_invall(col2);
>> +    its_send_invall(col3);
> 
> These two INVALLs should be issued after col2 and col3 are mapped,
> otherwise this will cause the INVALL command error as per the spec
> (though KVM doesn't complain it at all).
Yes you're right. reading the spec again:

A command error occurs if any of the following apply:
../..
The collection specified by ICID has not been mapped to an RDbase using
MAPC.

But as mentionned in the cover letter, no real means to retrieve the
error at the moment.

> 
>> +
>> +    report_prefix_push("int");
>> +    /*
>> +     * dev=2, eventid=20  -> lpi= 8195, col=3
>> +     * dev=7, eventid=255 -> lpi= 8196, col=2
>> +     * Trigger dev2, eventid=20 and dev7, eventid=255
>> +     * Check both LPIs hit
>> +     */
>> +
>> +    its_send_mapd(dev2, true);
>> +    its_send_mapd(dev7, true);
>> +
>> +    its_send_mapc(col3, true);
>> +    its_send_mapc(col2, true);
>> +
>> +    its_send_mapti(dev2, 8195 /* lpi id */, 20 /* event id */, col3);
>> +    its_send_mapti(dev7, 8196 /* lpi id */, 255 /* event id */, col2);
>> +
>> +    lpi_stats_expect(3, 8195);
>> +    its_send_int(dev2, 20);
>> +    check_lpi_stats("dev=2, eventid=20  -> lpi= 8195, col=3");
>> +
>> +    lpi_stats_expect(2, 8196);
>> +    its_send_int(dev7, 255);
>> +    check_lpi_stats("dev=7, eventid=255 -> lpi= 8196, col=2");
>> +
>> +    report_prefix_pop();
>> +
>> +    report_prefix_push("inv/invall");
>> +
>> +    /*
>> +     * disable 8195, check dev2/eventid=20 does not trigger the
>> +     * corresponding LPI
>> +     */
>> +    gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT & ~LPI_PROP_ENABLED);
>> +    its_send_inv(dev2, 20);
>> +
>> +    lpi_stats_expect(-1, -1);
>> +    its_send_int(dev2, 20);
>> +    check_lpi_stats("dev2/eventid=20 does not trigger any LPI");
>> +
>> +    /*
>> +     * 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
>> +     */
>> +    gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT);
>> +    lpi_stats_expect(-1, -1);
>> +    its_send_int(dev2, 20);
>> +    check_lpi_stats("dev2/eventid=20 still does not trigger any LPI");
>> +
>> +    /* Now call the invall and check the LPI hits */
>> +    its_send_invall(col3);
>> +    lpi_stats_expect(3, 8195);
>> +    its_send_int(dev2, 20);
>> +    check_lpi_stats("dev2/eventid=20 now triggers an LPI");
>> +
>> +    report_prefix_pop();
>> +
>> +    report_prefix_push("mapd valid=false");
>> +    /*
>> +     * Unmap device 2 and check the eventid 20 formerly
>> +     * attached to it does not hit anymore
>> +     */
>> +
>> +    its_send_mapd(dev2, false);
>> +    lpi_stats_expect(-1, -1);
>> +    its_send_int(dev2, 20);
>> +    check_lpi_stats("no LPI after device unmap");
>> +    report_prefix_pop();
>> +
>> +    /* Unmap the collection this time and check no LPI does hit */
>> +    report_prefix_push("mapc valid=false");
>> +    its_send_mapc(col2, false);
> 
> And as for the MAPC, the spec says:
> 
> " When V is 0:
> Behavior is unpredictable if there are interrupts that are mapped to the
> specified collection, with the restriction that further translation
> requests from that device are ignored. "
> 
> So this collection-unmap test may not make sense?
makes sense. Removing it.
> 
>> +    lpi_stats_expect(-1, -1);
>> +    its_send_int(dev7, 255);
>> +    check_lpi_stats("no LPI after collection unmap");
>> +    report_prefix_pop();
>> +}
> 
> [...]
> 
> Otherwise looks good.
Thanks!

Eric
> 
> 
> Thanks,
> Zenghui
>
Marc Zyngier March 11, 2020, 2 p.m. UTC | #3
On 2020-03-11 13:48, Auger Eric wrote:
> Hi Zenghui,
> 
> On 3/11/20 12:59 PM, Zenghui Yu wrote:
>> Hi Eric,
>> 
>> On 2020/3/10 22:54, 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 <eric.auger@redhat.com>
>>> 
>>> ---
>> 
>> [...]
>> 
>>> +static void test_its_trigger(void)
>>> +{
>>> +    struct its_collection *col3, *col2;
>>> +    struct its_device *dev2, *dev7;
>>> +
>>> +    if (its_prerequisites(4))
>>> +        return;
>>> +
>>> +    dev2 = its_create_device(2 /* dev id */, 8 /* nb_ites */);
>>> +    dev7 = its_create_device(7 /* dev id */, 8 /* nb_ites */);
>>> +
>>> +    col3 = its_create_collection(3 /* col id */, 3/* target PE */);
>>> +    col2 = its_create_collection(2 /* col id */, 2/* target PE */);
>>> +
>>> +    gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT);
>>> +    gicv3_lpi_set_config(8196, LPI_PROP_DEFAULT);
>>> +
>>> +    its_send_invall(col2);
>>> +    its_send_invall(col3);
>> 
>> These two INVALLs should be issued after col2 and col3 are mapped,
>> otherwise this will cause the INVALL command error as per the spec
>> (though KVM doesn't complain it at all).
> Yes you're right. reading the spec again:
> 
> A command error occurs if any of the following apply:
> ../..
> The collection specified by ICID has not been mapped to an RDbase using
> MAPC.
> 
> But as mentionned in the cover letter, no real means to retrieve the
> error at the moment.

That is still a problem with the ITS. There is no architectural way
to report an error, even if the error numbers are architected...

One thing we could do though is to implement the stall model (as 
described
in 5.3.2). It still doesn't give us the error, but at least the command
queue would stop on detecting an error.

         M.
Zenghui Yu March 12, 2020, 9:19 a.m. UTC | #4
On 2020/3/11 22:00, Marc Zyngier wrote:
> That is still a problem with the ITS. There is no architectural way
> to report an error, even if the error numbers are architected...
> 
> One thing we could do though is to implement the stall model (as described
> in 5.3.2). It still doesn't give us the error, but at least the command
> queue would stop on detecting an error.

It would be interesting to see the buggy guest's behavior under the
stall mode. I've used the following diff (absolutely *not* a formal
patch, don't handle CREADR.Stalled and CWRITER.Retry at all) to have
a try, and caught another command error in the 'its-trigger' test.

logs/its-trigger.log:
" INT dev_id=2 event_id=20
lib/arm64/gic-v3-its-cmd.c:194: assert failed: false: INT timeout! "

dmesg:
[13297.711958] ------------[ cut here ]------------
[13297.711964] ITS command error encoding 0x10307

It's the last INT test in test_its_trigger() who has triggered this
error, Eric?


Thanks.

---8<---
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 9d53f545a3d5..5717f5da0f22 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -179,6 +179,7 @@ struct vgic_its {
  	u64			cbaser;
  	u32			creadr;
  	u32			cwriter;
+	bool			stalled;

  	/* migration ABI revision in use */
  	u32			abi_rev;
diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index d53d34a33e35..72422b75e627 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -1519,6 +1519,9 @@ static void vgic_its_process_commands(struct kvm 
*kvm, struct vgic_its *its)
  	if (!its->enabled)
  		return;

+	if (unlikely(its->stalled))
+		return;
+
  	cbaser = GITS_CBASER_ADDRESS(its->cbaser);

  	while (its->cwriter != its->creadr) {
@@ -1531,9 +1534,34 @@ static void vgic_its_process_commands(struct kvm 
*kvm, struct vgic_its *its)
  		 * According to section 6.3.2 in the GICv3 spec we can just
  		 * ignore that command then.
  		 */
-		if (!ret)
-			vgic_its_handle_command(kvm, its, cmd_buf);
+		if (ret)
+			goto done;
+
+		ret = vgic_its_handle_command(kvm, its, cmd_buf);
+
+		/*
+		 * Choose the stall mode on detection of command errors.
+		 * Guest still can't get the architected error numbers though...
+		 */
+		if (ret) {
+			/* GITS_CREADR.Stalled is set to 1. */
+			its->stalled = true;
+
+			/*
+			 * GITS_TYPER.SEIS is 0 atm, no System error will be
+			 * generated.  Instead report error encodings at ITS
+			 * level.
+			 */
+			WARN_RATELIMIT(ret, "ITS command error encoding 0x%x", ret);
+
+			/*
+			 * GITS_CREADR is not incremented and continues to
+			 * point to the entry that triggered the error.
+			 */
+			break;
+		}

+done:
  		its->creadr += ITS_CMD_SIZE;
  		if (its->creadr == ITS_CMD_BUFFER_SIZE(its->cbaser))
  			its->creadr = 0;
Eric Auger March 12, 2020, 9:59 a.m. UTC | #5
Hi Zenghui,

On 3/12/20 10:19 AM, Zenghui Yu wrote:
> On 2020/3/11 22:00, Marc Zyngier wrote:
>> That is still a problem with the ITS. There is no architectural way
>> to report an error, even if the error numbers are architected...
>>
>> One thing we could do though is to implement the stall model (as
>> described
>> in 5.3.2). It still doesn't give us the error, but at least the command
>> queue would stop on detecting an error.
> 
> It would be interesting to see the buggy guest's behavior under the
> stall mode. I've used the following diff (absolutely *not* a formal
> patch, don't handle CREADR.Stalled and CWRITER.Retry at all) to have
> a try, and caught another command error in the 'its-trigger' test.
> 
> logs/its-trigger.log:
> " INT dev_id=2 event_id=20
> lib/arm64/gic-v3-its-cmd.c:194: assert failed: false: INT timeout! "
> 
> dmesg:
> [13297.711958] ------------[ cut here ]------------
> [13297.711964] ITS command error encoding 0x10307
> 
> It's the last INT test in test_its_trigger() who has triggered this
> error, Eric?

Yes it may be the culprit. Anyway I removed the collection unmap in v6.

By the way are you OK now with v6? I think Drew plans to send a pull
request by the end of this week.

Thanks

Eric
> 
> 
> Thanks.
> 
> ---8<---
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 9d53f545a3d5..5717f5da0f22 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -179,6 +179,7 @@ struct vgic_its {
>      u64            cbaser;
>      u32            creadr;
>      u32            cwriter;
> +    bool            stalled;
> 
>      /* migration ABI revision in use */
>      u32            abi_rev;
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index d53d34a33e35..72422b75e627 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -1519,6 +1519,9 @@ static void vgic_its_process_commands(struct kvm
> *kvm, struct vgic_its *its)
>      if (!its->enabled)
>          return;
> 
> +    if (unlikely(its->stalled))
> +        return;
> +
>      cbaser = GITS_CBASER_ADDRESS(its->cbaser);
> 
>      while (its->cwriter != its->creadr) {
> @@ -1531,9 +1534,34 @@ static void vgic_its_process_commands(struct kvm
> *kvm, struct vgic_its *its)
>           * According to section 6.3.2 in the GICv3 spec we can just
>           * ignore that command then.
>           */
> -        if (!ret)
> -            vgic_its_handle_command(kvm, its, cmd_buf);
> +        if (ret)
> +            goto done;
> +
> +        ret = vgic_its_handle_command(kvm, its, cmd_buf);
> +
> +        /*
> +         * Choose the stall mode on detection of command errors.
> +         * Guest still can't get the architected error numbers though...
> +         */
> +        if (ret) {
> +            /* GITS_CREADR.Stalled is set to 1. */
> +            its->stalled = true;
> +
> +            /*
> +             * GITS_TYPER.SEIS is 0 atm, no System error will be
> +             * generated.  Instead report error encodings at ITS
> +             * level.
> +             */
> +            WARN_RATELIMIT(ret, "ITS command error encoding 0x%x", ret);
> +
> +            /*
> +             * GITS_CREADR is not incremented and continues to
> +             * point to the entry that triggered the error.
> +             */
> +            break;
> +        }
> 
> +done:
>          its->creadr += ITS_CMD_SIZE;
>          if (its->creadr == ITS_CMD_BUFFER_SIZE(its->cbaser))
>              its->creadr = 0;
>
Zenghui Yu March 13, 2020, 1:55 a.m. UTC | #6
Hi Eric,

On 2020/3/12 17:59, Auger Eric wrote:
> Hi Zenghui,
> 
> On 3/12/20 10:19 AM, Zenghui Yu wrote:
>> On 2020/3/11 22:00, Marc Zyngier wrote:
>>> That is still a problem with the ITS. There is no architectural way
>>> to report an error, even if the error numbers are architected...
>>>
>>> One thing we could do though is to implement the stall model (as
>>> described
>>> in 5.3.2). It still doesn't give us the error, but at least the command
>>> queue would stop on detecting an error.
>>
>> It would be interesting to see the buggy guest's behavior under the
>> stall mode. I've used the following diff (absolutely *not* a formal
>> patch, don't handle CREADR.Stalled and CWRITER.Retry at all) to have
>> a try, and caught another command error in the 'its-trigger' test.
>>
>> logs/its-trigger.log:
>> " INT dev_id=2 event_id=20
>> lib/arm64/gic-v3-its-cmd.c:194: assert failed: false: INT timeout! "
>>
>> dmesg:
>> [13297.711958] ------------[ cut here ]------------
>> [13297.711964] ITS command error encoding 0x10307
>>
>> It's the last INT test in test_its_trigger() who has triggered this
>> error, Eric?
> 
> Yes it may be the culprit. Anyway I removed the collection unmap in v6.

I forgot to mention that this is based on your v6. I'll reply to it.

> 
> By the way are you OK now with v6? I think Drew plans to send a pull
> request by the end of this week.

Sorry I haven't looked at it yet (v5 already looks good except for
some minor issues).


Thanks,
Zenghui
diff mbox series

Patch

diff --git a/arm/gic.c b/arm/gic.c
index 649ed81..32b709e 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -159,6 +159,85 @@  static void ipi_handler(struct pt_regs *regs __unused)
 	}
 }
 
+static void setup_irq(irq_handler_fn handler)
+{
+	gic_enable_defaults();
+#ifdef __arm__
+	install_exception_handler(EXCPTN_IRQ, handler);
+#else
+	install_irq_handler(EL1H_IRQ, handler);
+#endif
+	local_irq_enable();
+}
+
+#if defined(__aarch64__)
+struct its_event {
+	int cpu_id;
+	int lpi_id;
+};
+
+struct its_stats {
+	struct its_event expected;
+	struct its_event observed;
+};
+
+static struct its_stats lpi_stats;
+
+static void lpi_handler(struct pt_regs *regs __unused)
+{
+	u32 irqstat = gic_read_iar();
+	int irqnr = gic_iar_irqnr(irqstat);
+
+	gic_write_eoir(irqstat);
+	assert(irqnr >= 8192);
+	smp_rmb(); /* pairs with wmb in lpi_stats_expect */
+	lpi_stats.observed.cpu_id = smp_processor_id();
+	lpi_stats.observed.lpi_id = irqnr;
+	smp_wmb(); /* pairs with rmb in check_lpi_stats */
+}
+
+static void lpi_stats_expect(int exp_cpu_id, int exp_lpi_id)
+{
+	lpi_stats.expected.cpu_id = exp_cpu_id;
+	lpi_stats.expected.lpi_id = exp_lpi_id;
+	lpi_stats.observed.cpu_id = -1;
+	lpi_stats.observed.lpi_id = -1;
+	smp_wmb(); /* pairs with rmb in handler */
+}
+
+static void check_lpi_stats(const char *msg)
+{
+	bool pass = false;
+
+	mdelay(100);
+	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);
+}
+
+static void secondary_lpi_test(void)
+{
+	setup_irq(lpi_handler);
+	cpumask_set_cpu(smp_processor_id(), &ready);
+	while (1)
+		wfi();
+}
+#endif
+
 static void gicv2_ipi_send_self(void)
 {
 	writel(2 << 24 | IPI_IRQ, gicv2_dist_base() + GICD_SGIR);
@@ -216,17 +295,6 @@  static void ipi_test_smp(void)
 	report_prefix_pop();
 }
 
-static void setup_irq(irq_handler_fn handler)
-{
-	gic_enable_defaults();
-#ifdef __arm__
-	install_exception_handler(EXCPTN_IRQ, handler);
-#else
-	install_irq_handler(EL1H_IRQ, handler);
-#endif
-	local_irq_enable();
-}
-
 static void ipi_send(void)
 {
 	setup_irq(ipi_handler);
@@ -521,6 +589,7 @@  static void gic_test_mmio(void)
 #if defined(__arm__)
 
 static void test_its_introspection(void) {}
+static void test_its_trigger(void) {}
 
 #else /* __aarch64__ */
 
@@ -559,6 +628,134 @@  static void test_its_introspection(void)
 	report_info("collection table entry_size = 0x%x", coll_baser->esz);
 }
 
+static int its_prerequisites(int nb_cpus)
+{
+	int cpu;
+
+	if (!gicv3_its_base()) {
+		report_skip("No ITS, skip ...");
+		return -1;
+	}
+
+	if (nr_cpus < nb_cpus) {
+		report_skip("Test requires at least %d vcpus", nb_cpus);
+		return -1;
+	}
+
+	stats_reset();
+
+	setup_irq(lpi_handler);
+
+	for_each_present_cpu(cpu) {
+		if (cpu == 0)
+			continue;
+		smp_boot_secondary(cpu, secondary_lpi_test);
+	}
+	wait_on_ready();
+
+	its_enable_defaults();
+
+	return 0;
+}
+
+static void test_its_trigger(void)
+{
+	struct its_collection *col3, *col2;
+	struct its_device *dev2, *dev7;
+
+	if (its_prerequisites(4))
+		return;
+
+	dev2 = its_create_device(2 /* dev id */, 8 /* nb_ites */);
+	dev7 = its_create_device(7 /* dev id */, 8 /* nb_ites */);
+
+	col3 = its_create_collection(3 /* col id */, 3/* target PE */);
+	col2 = its_create_collection(2 /* col id */, 2/* target PE */);
+
+	gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT);
+	gicv3_lpi_set_config(8196, LPI_PROP_DEFAULT);
+
+	its_send_invall(col2);
+	its_send_invall(col3);
+
+	report_prefix_push("int");
+	/*
+	 * dev=2, eventid=20  -> lpi= 8195, col=3
+	 * dev=7, eventid=255 -> lpi= 8196, col=2
+	 * Trigger dev2, eventid=20 and dev7, eventid=255
+	 * Check both LPIs hit
+	 */
+
+	its_send_mapd(dev2, true);
+	its_send_mapd(dev7, true);
+
+	its_send_mapc(col3, true);
+	its_send_mapc(col2, true);
+
+	its_send_mapti(dev2, 8195 /* lpi id */, 20 /* event id */, col3);
+	its_send_mapti(dev7, 8196 /* lpi id */, 255 /* event id */, col2);
+
+	lpi_stats_expect(3, 8195);
+	its_send_int(dev2, 20);
+	check_lpi_stats("dev=2, eventid=20  -> lpi= 8195, col=3");
+
+	lpi_stats_expect(2, 8196);
+	its_send_int(dev7, 255);
+	check_lpi_stats("dev=7, eventid=255 -> lpi= 8196, col=2");
+
+	report_prefix_pop();
+
+	report_prefix_push("inv/invall");
+
+	/*
+	 * disable 8195, check dev2/eventid=20 does not trigger the
+	 * corresponding LPI
+	 */
+	gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT & ~LPI_PROP_ENABLED);
+	its_send_inv(dev2, 20);
+
+	lpi_stats_expect(-1, -1);
+	its_send_int(dev2, 20);
+	check_lpi_stats("dev2/eventid=20 does not trigger any LPI");
+
+	/*
+	 * 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
+	 */
+	gicv3_lpi_set_config(8195, LPI_PROP_DEFAULT);
+	lpi_stats_expect(-1, -1);
+	its_send_int(dev2, 20);
+	check_lpi_stats("dev2/eventid=20 still does not trigger any LPI");
+
+	/* Now call the invall and check the LPI hits */
+	its_send_invall(col3);
+	lpi_stats_expect(3, 8195);
+	its_send_int(dev2, 20);
+	check_lpi_stats("dev2/eventid=20 now triggers an LPI");
+
+	report_prefix_pop();
+
+	report_prefix_push("mapd valid=false");
+	/*
+	 * Unmap device 2 and check the eventid 20 formerly
+	 * attached to it does not hit anymore
+	 */
+
+	its_send_mapd(dev2, false);
+	lpi_stats_expect(-1, -1);
+	its_send_int(dev2, 20);
+	check_lpi_stats("no LPI after device unmap");
+	report_prefix_pop();
+
+	/* Unmap the collection this time and check no LPI does hit */
+	report_prefix_push("mapc valid=false");
+	its_send_mapc(col2, false);
+	lpi_stats_expect(-1, -1);
+	its_send_int(dev7, 255);
+	check_lpi_stats("no LPI after collection unmap");
+	report_prefix_pop();
+}
 #endif
 
 int main(int argc, char **argv)
@@ -592,6 +789,10 @@  int main(int argc, char **argv)
 		report_prefix_push(argv[1]);
 		gic_test_mmio();
 		report_prefix_pop();
+	} else if (!strcmp(argv[1], "its-trigger")) {
+		report_prefix_push(argv[1]);
+		test_its_trigger();
+		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 23d378e..b9a7a2c 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -129,6 +129,13 @@  extra_params = -machine gic-version=3 -append 'its-introspection'
 groups = its
 arch = arm64
 
+[its-trigger]
+file = gic.flat
+smp = $MAX_SMP
+extra_params = -machine gic-version=3 -append 'its-trigger'
+groups = its
+arch = arm64
+
 # Test PSCI emulation
 [psci]
 file = psci.flat