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

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

[Andrew Jones]

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>
---
 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);
-- 
2.31.1

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

[Eric Auger]

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);