Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.

[Auger Eric <eric.auger@redhat.com>]

Hi Jingyi,

On 7/2/20 5:01 AM, Jingyi Wang wrote:
> If gicv4.1(sgi hardware injection) supported, we test ipi injection
> via hw/sw way separately.
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@huawei.com>
> ---
>  arm/micro-bench.c    | 45 +++++++++++++++++++++++++++++++++++++++-----
>  lib/arm/asm/gic-v3.h |  3 +++
>  lib/arm/asm/gic.h    |  1 +
>  3 files changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fc4d356..80d8db3 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -91,9 +91,40 @@ static void gic_prep_common(void)
>  	assert(irq_ready);
>  }
>  
> -static void ipi_prep(void)
> +static bool ipi_prep(void)
Any reason why the bool returned value is preferred over the standard int?
>  {
> +	u32 val;
> +
> +	val = readl(vgic_dist_base + GICD_CTLR);
> +	if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
> +		val &= ~GICD_CTLR_ENABLE_G1A;
> +		val &= ~GICD_CTLR_nASSGIreq;
> +		writel(val, vgic_dist_base + GICD_CTLR);
> +		val |= GICD_CTLR_ENABLE_G1A;
> +		writel(val, vgic_dist_base + GICD_CTLR);
Why do we need this G1A dance?
> +	}
> +
>  	gic_prep_common();
> +	return true;
> +}
> +
> +static bool ipi_hw_prep(void)
> +{
> +	u32 val;
> +
> +	val = readl(vgic_dist_base + GICD_CTLR);
> +	if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
> +		val &= ~GICD_CTLR_ENABLE_G1A;
> +		val |= GICD_CTLR_nASSGIreq;
> +		writel(val, vgic_dist_base + GICD_CTLR);
> +		val |= GICD_CTLR_ENABLE_G1A;
> +		writel(val, vgic_dist_base + GICD_CTLR);
> +	} else {
> +		return false;
> +	}
> +
> +	gic_prep_common();
> +	return true;
>  }
>  
>  static void ipi_exec(void)
> @@ -147,7 +178,7 @@ static void eoi_exec(void)
>  
>  struct exit_test {
>  	const char *name;
> -	void (*prep)(void);
> +	bool (*prep)(void);
>  	void (*exec)(void);
>  	bool run;
>  };
> @@ -158,6 +189,7 @@ static struct exit_test tests[] = {
>  	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	true},
>  	{"eoi",			NULL,		eoi_exec,		true},
>  	{"ipi",			ipi_prep,	ipi_exec,		true},
> +	{"ipi_hw",		ipi_hw_prep,	ipi_exec,		true},
>  };
>  
>  struct ns_time {
> @@ -181,9 +213,12 @@ static void loop_test(struct exit_test *test)
>  	uint64_t start, end, total_ticks, ntimes = NTIMES;
>  	struct ns_time total_ns, avg_ns;
>  
> -	if (test->prep)
> -		test->prep();
> -
> +	if (test->prep) {
> +		if(!test->prep()) {
> +			printf("%s test skipped\n", test->name);
> +			return;
> +		}
> +	}
>  	isb();
>  	start = read_sysreg(cntpct_el0);
>  	while (ntimes--)
> diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h
> index cb72922..b4ce130 100644
> --- a/lib/arm/asm/gic-v3.h
> +++ b/lib/arm/asm/gic-v3.h
> @@ -20,10 +20,13 @@
>   */
>  #define GICD_CTLR			0x0000
>  #define GICD_CTLR_RWP			(1U << 31)
> +#define GICD_CTLR_nASSGIreq		(1U << 8)
>  #define GICD_CTLR_ARE_NS		(1U << 4)
>  #define GICD_CTLR_ENABLE_G1A		(1U << 1)
>  #define GICD_CTLR_ENABLE_G1		(1U << 0)
>  
> +#define GICD_TYPER2_nASSGIcap		(1U << 8)
> +
>  /* Re-Distributor registers, offsets from RD_base */
>  #define GICR_TYPER			0x0008
>  
> diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
> index 38e79b2..1898400 100644
> --- a/lib/arm/asm/gic.h
> +++ b/lib/arm/asm/gic.h
> @@ -13,6 +13,7 @@
>  #define GICD_CTLR			0x0000
>  #define GICD_TYPER			0x0004
>  #define GICD_IIDR			0x0008
> +#define GICD_TYPER2			0x000C
>  #define GICD_IGROUPR			0x0080
>  #define GICD_ISENABLER			0x0100
>  #define GICD_ICENABLER			0x0180
> 

Thanks

Eric