Hi,
On 11/8/19 2:42 PM, Andre Przywara wrote:
> When both groups are avaiable to the non-secure side, the MMIO group
> registers need to be writable, so that the group that an IRQ belongs to
> can be programmed.
>
> Check that the group can be flipped, after having established that both
> groups are usable.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> ---
> arm/gic.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arm/gic.c b/arm/gic.c
> index c882a24..485ca4f 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -683,6 +683,7 @@ static bool gicv3_check_security(void *gicd_base)
> static void test_irq_group(void *gicd_base)
> {
> bool is_gicv3 = (gic_version() == 3);
> + u32 reg;
The return value for gic_get_irq_group is an int, not a u32. Also, maybe it should
be named group instead, so it's clear what it represents.
>
> report_prefix_push("GROUP");
> gic_enable_defaults();
> @@ -692,6 +693,16 @@ static void test_irq_group(void *gicd_base)
> if (!gicv3_check_security(gicd_base))
> return;
> }
> +
> + /*
> + * On a security aware GIC in non-secure world the IGROUPR registers
> + * are RAZ/WI. KVM emulates a single-security-state GIC, so both
> + * groups are available and the IGROUPR registers are writable.
> + */
> + reg = gic_get_irq_group(SPI_IRQ);
> + gic_set_irq_group(SPI_IRQ, !reg);
> + report("IGROUPR is writable", gic_get_irq_group(SPI_IRQ) != reg);
This is nitpicking, but from a consistency point of view, shouldn't you check that
the new group is the value that you wrote, meaning the check should be
gic_get_irq_group(SPI_IRQ) == !reg?
Thanks,
Alex
> + gic_set_irq_group(SPI_IRQ, reg);
> }
>
> static void spi_send(void)
