Re: [PATCH kvm-unit-tests] arm64: gic-v3: Avoid NULL dereferences

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

Hi Drew,
On 10/11/21 6:04 PM, Andrew Jones wrote:
> LPI allocation requires that the redistributors are configured first.
> It's unlikely that offline cpus have had their redistributors
> configured, so filter them out right away. Also, assert on any cpu,
> not just the calling cpu, in gicv3_lpi_alloc_tables() when we detect
> a unit test failed to follow instructions. Improve the assert with a
> hint message while we're at it.
>
> Cc: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
> ---
>  lib/arm/gic-v3.c       | 6 +++---
>  lib/arm64/gic-v3-its.c | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/arm/gic-v3.c b/lib/arm/gic-v3.c
> index 2c067e4e9ba2..2f7870ab28bf 100644
> --- a/lib/arm/gic-v3.c
> +++ b/lib/arm/gic-v3.c
> @@ -171,17 +171,17 @@ void gicv3_lpi_alloc_tables(void)
>  	u64 prop_val;
>  	int cpu;
>  
> -	assert(gicv3_redist_base());
> -
>  	gicv3_data.lpi_prop = alloc_pages(order);
>  
>  	/* ID bits = 13, ie. up to 14b LPI INTID */
>  	prop_val = (u64)(virt_to_phys(gicv3_data.lpi_prop)) | 13;
>  
> -	for_each_present_cpu(cpu) {
> +	for_each_online_cpu(cpu) {
>  		u64 pend_val;
>  		void *ptr;
>  
> +		assert_msg(gicv3_data.redist_base[cpu], "Redistributor for cpu%d not initialized. "
> +							"Did cpu%d enable the GIC?", cpu, cpu);
>  		ptr = gicv3_data.redist_base[cpu];
>  
>  		writeq(prop_val, ptr + GICR_PROPBASER);
> diff --git a/lib/arm64/gic-v3-its.c b/lib/arm64/gic-v3-its.c
> index c22bda3a8ba2..2c69cfda0963 100644
> --- a/lib/arm64/gic-v3-its.c
> +++ b/lib/arm64/gic-v3-its.c
> @@ -104,7 +104,7 @@ void its_enable_defaults(void)
>  	/* Allocate LPI config and pending tables */
>  	gicv3_lpi_alloc_tables();
>  
> -	for_each_present_cpu(cpu)
> +	for_each_online_cpu(cpu)
>  		gicv3_lpi_rdist_enable(cpu);
>  
>  	writel(GITS_CTLR_ENABLE, its_data.base + GITS_CTLR);