[v4] arm/arm64: Add ITS tests

[kvm-unit-tests PATCH v4 03/13] arm/arm64: gic: Introduce setup_irq() helper

Posted by Eric Auger 5 years, 8 months ago

ipi_enable() code would be reusable for other interrupts
than IPI. Let's rename it setup_irq() and pass an interrupt
handler pointer.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v2 -> v3:
- do not export setup_irq anymore
---
 arm/gic.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/arm/gic.c b/arm/gic.c
index fcf4c1f..abf08c7 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -34,6 +34,7 @@ static struct gic *gic;
 static int acked[NR_CPUS], spurious[NR_CPUS];
 static int bad_sender[NR_CPUS], bad_irq[NR_CPUS];
 static cpumask_t ready;
+typedef void (*handler_t)(struct pt_regs *regs __unused);
 
 static void nr_cpu_check(int nr)
 {
@@ -215,20 +216,20 @@ static void ipi_test_smp(void)
 	report_prefix_pop();
 }
 
-static void ipi_enable(void)
+static void setup_irq(handler_t handler)
 {
 	gic_enable_defaults();
 #ifdef __arm__
-	install_exception_handler(EXCPTN_IRQ, ipi_handler);
+	install_exception_handler(EXCPTN_IRQ, handler);
 #else
-	install_irq_handler(EL1H_IRQ, ipi_handler);
+	install_irq_handler(EL1H_IRQ, handler);
 #endif
 	local_irq_enable();
 }
 
 static void ipi_send(void)
 {
-	ipi_enable();
+	setup_irq(ipi_handler);
 	wait_on_ready();
 	ipi_test_self();
 	ipi_test_smp();
@@ -238,7 +239,7 @@ static void ipi_send(void)
 
 static void ipi_recv(void)
 {
-	ipi_enable();
+	setup_irq(ipi_handler);
 	cpumask_set_cpu(smp_processor_id(), &ready);
 	while (1)
 		wfi();
@@ -295,14 +296,7 @@ static void ipi_clear_active_handler(struct pt_regs *regs __unused)
 static void run_active_clear_test(void)
 {
 	report_prefix_push("active");
-	gic_enable_defaults();
-#ifdef __arm__
-	install_exception_handler(EXCPTN_IRQ, ipi_clear_active_handler);
-#else
-	install_irq_handler(EL1H_IRQ, ipi_clear_active_handler);
-#endif
-	local_irq_enable();
-
+	setup_irq(ipi_clear_active_handler);
 	ipi_test_self();
 	report_prefix_pop();
 }
-- 
2.20.1

Re: [kvm-unit-tests PATCH v4 03/13] arm/arm64: gic: Introduce setup_irq() helper

Posted by Andrew Jones 5 years, 8 months ago

On Mon, Mar 09, 2020 at 11:24:10AM +0100, Eric Auger wrote:
> ipi_enable() code would be reusable for other interrupts
> than IPI. Let's rename it setup_irq() and pass an interrupt
> handler pointer.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> 
> v2 -> v3:
> - do not export setup_irq anymore
> ---
>  arm/gic.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/arm/gic.c b/arm/gic.c
> index fcf4c1f..abf08c7 100644
> --- a/arm/gic.c
> +++ b/arm/gic.c
> @@ -34,6 +34,7 @@ static struct gic *gic;
>  static int acked[NR_CPUS], spurious[NR_CPUS];
>  static int bad_sender[NR_CPUS], bad_irq[NR_CPUS];
>  static cpumask_t ready;
> +typedef void (*handler_t)(struct pt_regs *regs __unused);

This is just irq_handler_fn, which is already defined in processor.h.
We don't need the __unused, not since 6b07148d06b1 ("Replace -Wextra
with a saner list of warning flags").

>  
>  static void nr_cpu_check(int nr)
>  {
> @@ -215,20 +216,20 @@ static void ipi_test_smp(void)
>  	report_prefix_pop();
>  }
>  
> -static void ipi_enable(void)
> +static void setup_irq(handler_t handler)
>  {
>  	gic_enable_defaults();
>  #ifdef __arm__
> -	install_exception_handler(EXCPTN_IRQ, ipi_handler);
> +	install_exception_handler(EXCPTN_IRQ, handler);
>  #else
> -	install_irq_handler(EL1H_IRQ, ipi_handler);
> +	install_irq_handler(EL1H_IRQ, handler);
>  #endif
>  	local_irq_enable();
>  }
>  
>  static void ipi_send(void)
>  {
> -	ipi_enable();
> +	setup_irq(ipi_handler);
>  	wait_on_ready();
>  	ipi_test_self();
>  	ipi_test_smp();
> @@ -238,7 +239,7 @@ static void ipi_send(void)
>  
>  static void ipi_recv(void)
>  {
> -	ipi_enable();
> +	setup_irq(ipi_handler);
>  	cpumask_set_cpu(smp_processor_id(), &ready);
>  	while (1)
>  		wfi();
> @@ -295,14 +296,7 @@ static void ipi_clear_active_handler(struct pt_regs *regs __unused)
>  static void run_active_clear_test(void)
>  {
>  	report_prefix_push("active");
> -	gic_enable_defaults();
> -#ifdef __arm__
> -	install_exception_handler(EXCPTN_IRQ, ipi_clear_active_handler);
> -#else
> -	install_irq_handler(EL1H_IRQ, ipi_clear_active_handler);
> -#endif
> -	local_irq_enable();
> -
> +	setup_irq(ipi_clear_active_handler);
>  	ipi_test_self();
>  	report_prefix_pop();
>  }
> -- 
> 2.20.1
>

Re: [kvm-unit-tests PATCH v4 03/13] arm/arm64: gic: Introduce setup_irq() helper

Posted by Auger Eric 5 years, 8 months ago

Hi Drew,
On 3/9/20 11:56 AM, Andrew Jones wrote:
> On Mon, Mar 09, 2020 at 11:24:10AM +0100, Eric Auger wrote:
>> ipi_enable() code would be reusable for other interrupts
>> than IPI. Let's rename it setup_irq() and pass an interrupt
>> handler pointer.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v2 -> v3:
>> - do not export setup_irq anymore
>> ---
>>  arm/gic.c | 20 +++++++-------------
>>  1 file changed, 7 insertions(+), 13 deletions(-)
>>
>> diff --git a/arm/gic.c b/arm/gic.c
>> index fcf4c1f..abf08c7 100644
>> --- a/arm/gic.c
>> +++ b/arm/gic.c
>> @@ -34,6 +34,7 @@ static struct gic *gic;
>>  static int acked[NR_CPUS], spurious[NR_CPUS];
>>  static int bad_sender[NR_CPUS], bad_irq[NR_CPUS];
>>  static cpumask_t ready;
>> +typedef void (*handler_t)(struct pt_regs *regs __unused);
> 
> This is just irq_handler_fn, which is already defined in processor.h.
> We don't need the __unused, not since 6b07148d06b1 ("Replace -Wextra
> with a saner list of warning flags").
Shall I duplicate it into ./lib/arm/asm/processor.h as well?

Thanks

Eric
> 
>>  
>>  static void nr_cpu_check(int nr)
>>  {
>> @@ -215,20 +216,20 @@ static void ipi_test_smp(void)
>>  	report_prefix_pop();
>>  }
>>  
>> -static void ipi_enable(void)
>> +static void setup_irq(handler_t handler)
>>  {
>>  	gic_enable_defaults();
>>  #ifdef __arm__
>> -	install_exception_handler(EXCPTN_IRQ, ipi_handler);
>> +	install_exception_handler(EXCPTN_IRQ, handler);
>>  #else
>> -	install_irq_handler(EL1H_IRQ, ipi_handler);
>> +	install_irq_handler(EL1H_IRQ, handler);
>>  #endif
>>  	local_irq_enable();
>>  }
>>  
>>  static void ipi_send(void)
>>  {
>> -	ipi_enable();
>> +	setup_irq(ipi_handler);
>>  	wait_on_ready();
>>  	ipi_test_self();
>>  	ipi_test_smp();
>> @@ -238,7 +239,7 @@ static void ipi_send(void)
>>  
>>  static void ipi_recv(void)
>>  {
>> -	ipi_enable();
>> +	setup_irq(ipi_handler);
>>  	cpumask_set_cpu(smp_processor_id(), &ready);
>>  	while (1)
>>  		wfi();
>> @@ -295,14 +296,7 @@ static void ipi_clear_active_handler(struct pt_regs *regs __unused)
>>  static void run_active_clear_test(void)
>>  {
>>  	report_prefix_push("active");
>> -	gic_enable_defaults();
>> -#ifdef __arm__
>> -	install_exception_handler(EXCPTN_IRQ, ipi_clear_active_handler);
>> -#else
>> -	install_irq_handler(EL1H_IRQ, ipi_clear_active_handler);
>> -#endif
>> -	local_irq_enable();
>> -
>> +	setup_irq(ipi_clear_active_handler);
>>  	ipi_test_self();
>>  	report_prefix_pop();
>>  }
>> -- 
>> 2.20.1
>>

Re: [kvm-unit-tests PATCH v4 03/13] arm/arm64: gic: Introduce setup_irq() helper

Posted by Andrew Jones 5 years, 8 months ago

On Tue, Mar 10, 2020 at 12:00:19PM +0100, Auger Eric wrote:
> Hi Drew,
> On 3/9/20 11:56 AM, Andrew Jones wrote:
> > On Mon, Mar 09, 2020 at 11:24:10AM +0100, Eric Auger wrote:
> >> ipi_enable() code would be reusable for other interrupts
> >> than IPI. Let's rename it setup_irq() and pass an interrupt
> >> handler pointer.
> >>
> >> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >>
> >> ---
> >>
> >> v2 -> v3:
> >> - do not export setup_irq anymore
> >> ---
> >>  arm/gic.c | 20 +++++++-------------
> >>  1 file changed, 7 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/arm/gic.c b/arm/gic.c
> >> index fcf4c1f..abf08c7 100644
> >> --- a/arm/gic.c
> >> +++ b/arm/gic.c
> >> @@ -34,6 +34,7 @@ static struct gic *gic;
> >>  static int acked[NR_CPUS], spurious[NR_CPUS];
> >>  static int bad_sender[NR_CPUS], bad_irq[NR_CPUS];
> >>  static cpumask_t ready;
> >> +typedef void (*handler_t)(struct pt_regs *regs __unused);
> > 
> > This is just irq_handler_fn, which is already defined in processor.h.
> > We don't need the __unused, not since 6b07148d06b1 ("Replace -Wextra
> > with a saner list of warning flags").
> Shall I duplicate it into ./lib/arm/asm/processor.h as well?

Yes, please do

Thanks,
drew
> 
> Thanks
> 
> Eric
> > 
> >>  
> >>  static void nr_cpu_check(int nr)
> >>  {
> >> @@ -215,20 +216,20 @@ static void ipi_test_smp(void)
> >>  	report_prefix_pop();
> >>  }
> >>  
> >> -static void ipi_enable(void)
> >> +static void setup_irq(handler_t handler)
> >>  {
> >>  	gic_enable_defaults();
> >>  #ifdef __arm__
> >> -	install_exception_handler(EXCPTN_IRQ, ipi_handler);
> >> +	install_exception_handler(EXCPTN_IRQ, handler);
> >>  #else
> >> -	install_irq_handler(EL1H_IRQ, ipi_handler);
> >> +	install_irq_handler(EL1H_IRQ, handler);
> >>  #endif
> >>  	local_irq_enable();
> >>  }
> >>  
> >>  static void ipi_send(void)
> >>  {
> >> -	ipi_enable();
> >> +	setup_irq(ipi_handler);
> >>  	wait_on_ready();
> >>  	ipi_test_self();
> >>  	ipi_test_smp();
> >> @@ -238,7 +239,7 @@ static void ipi_send(void)
> >>  
> >>  static void ipi_recv(void)
> >>  {
> >> -	ipi_enable();
> >> +	setup_irq(ipi_handler);
> >>  	cpumask_set_cpu(smp_processor_id(), &ready);
> >>  	while (1)
> >>  		wfi();
> >> @@ -295,14 +296,7 @@ static void ipi_clear_active_handler(struct pt_regs *regs __unused)
> >>  static void run_active_clear_test(void)
> >>  {
> >>  	report_prefix_push("active");
> >> -	gic_enable_defaults();
> >> -#ifdef __arm__
> >> -	install_exception_handler(EXCPTN_IRQ, ipi_clear_active_handler);
> >> -#else
> >> -	install_irq_handler(EL1H_IRQ, ipi_clear_active_handler);
> >> -#endif
> >> -	local_irq_enable();
> >> -
> >> +	setup_irq(ipi_clear_active_handler);
> >>  	ipi_test_self();
> >>  	report_prefix_pop();
> >>  }
> >> -- 
> >> 2.20.1
> >>
> 
>