Re: [kvm-unit-tests PATCH 10/17] arm: gic: Check for writable IGROUPR registers

[Alexandru Elisei <alexandru.elisei@arm.com>]

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@arm.com>
> ---
>  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)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel