[Xen-devel,v2,02/10] ARM: vGIC: fix nr_irq definition

Message ID 20171207161415.20380-3-andre.przywara@linaro.org
State New
Headers show
Series
  • ARM: VGIC/GIC separation cleanups
Related show

Commit Message

Andre Przywara Dec. 7, 2017, 4:14 p.m.
The global variable "nr_irqs" is used for x86 and some common Xen code.
To make the latter work easily for ARM, it was #defined to NR_IRQS.
This not only violated the common habit of capitalizing macros, but
also caused issues if one wanted to use a rather innocent "nr_irqs" as
a local variable name or as a function parameter.
Drop the optimization and make nr_irqs a normal variable for ARM also.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/irq.c        | 2 ++
 xen/include/asm-arm/irq.h | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Julien Grall Dec. 8, 2017, 9:42 p.m. | #1
Hi,

On 12/07/2017 04:14 PM, Andre Przywara wrote:
> The global variable "nr_irqs" is used for x86 and some common Xen code.
> To make the latter work easily for ARM, it was #defined to NR_IRQS.
> This not only violated the common habit of capitalizing macros, but
> also caused issues if one wanted to use a rather innocent "nr_irqs" as
> a local variable name or as a function parameter.
> Drop the optimization and make nr_irqs a normal variable for ARM also.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Surprisingly the reviewed-by was not carried in the merge. Anyway...

> ---
>   xen/arch/arm/irq.c        | 2 ++
>   xen/include/asm-arm/irq.h | 2 +-
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index cbc7e6ebb8..7f133de549 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -27,6 +27,8 @@
>   #include <asm/gic.h>
>   #include <asm/vgic.h>
>   
> +unsigned int __read_mostly nr_irqs = NR_IRQS;

I am ok with turning this to a variable. But this should really be const 
and not let a chance of someone to override it by mistake.

Can you please send a follow-up to fix that.

> +
>   static unsigned int local_irqs_type[NR_LOCAL_IRQS];
>   static DEFINE_SPINLOCK(local_irqs_type_lock);
>   
> diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
> index 2de76d0f56..abc8f06a13 100644
> --- a/xen/include/asm-arm/irq.h
> +++ b/xen/include/asm-arm/irq.h
> @@ -31,7 +31,7 @@ struct arch_irq_desc {
>   /* LPIs are always numbered starting at 8192, so 0 is a good invalid case. */
>   #define INVALID_LPI     0
>   
> -#define nr_irqs NR_IRQS
> +extern unsigned int nr_irqs;
>   #define nr_static_irqs NR_IRQS
>   #define arch_hwdom_irqs(domid) NR_IRQS
>   
> 

Cheers

Patch

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index cbc7e6ebb8..7f133de549 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -27,6 +27,8 @@ 
 #include <asm/gic.h>
 #include <asm/vgic.h>
 
+unsigned int __read_mostly nr_irqs = NR_IRQS;
+
 static unsigned int local_irqs_type[NR_LOCAL_IRQS];
 static DEFINE_SPINLOCK(local_irqs_type_lock);
 
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 2de76d0f56..abc8f06a13 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -31,7 +31,7 @@  struct arch_irq_desc {
 /* LPIs are always numbered starting at 8192, so 0 is a good invalid case. */
 #define INVALID_LPI     0
 
-#define nr_irqs NR_IRQS
+extern unsigned int nr_irqs;
 #define nr_static_irqs NR_IRQS
 #define arch_hwdom_irqs(domid) NR_IRQS