Having no_irq_type defined per arch, but using common callbacks is a mess, and
particualrly hard to bootstrap a new architecture with.

Now that the ack()/end() hooks have been exported suitably, move the
definition of no_irq_type into common/irq.c, and into .rodata for good
measure.

No functional change, but a whole lot less tangled.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
CC: Shawn Anastasio <sanastasio@raptorengineering.com>

Oleksii: For RISC-V, you should only need to provide a irq_ack_none() stub now.
---
 xen/arch/arm/irq.c    | 10 ----------
 xen/arch/ppc/stubs.c  |  9 ---------
 xen/arch/x86/irq.c    |  9 ---------
 xen/common/irq.c      | 13 +++++++++++++
 xen/include/xen/irq.h |  2 +-
 5 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 7138f9e7c283..e5fb26a3de2d 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -45,16 +45,6 @@ void irq_end_none(struct irq_desc *irq)
     gic_hw_ops->gic_host_irq_type->end(irq);
 }
 
-hw_irq_controller no_irq_type = {
-    .typename = "none",
-    .startup = irq_startup_none,
-    .shutdown = irq_shutdown_none,
-    .enable = irq_enable_none,
-    .disable = irq_disable_none,
-    .ack = irq_ack_none,
-    .end = irq_end_none
-};
-
 static irq_desc_t irq_desc[NR_IRQS];
 static DEFINE_PER_CPU(irq_desc_t[NR_LOCAL_IRQS], local_irq_desc);
 
diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
index 4e03428e071a..923f0e7b2095 100644
--- a/xen/arch/ppc/stubs.c
+++ b/xen/arch/ppc/stubs.c
@@ -139,15 +139,6 @@ void irq_ack_none(struct irq_desc *desc)
     BUG_ON("unimplemented");
 }
 
-hw_irq_controller no_irq_type = {
-    .typename = "none",
-    .startup = irq_startup_none,
-    .shutdown = irq_shutdown_none,
-    .enable = irq_enable_none,
-    .disable = irq_disable_none,
-    .ack = irq_ack_none,
-};
-
 int arch_init_one_irq_desc(struct irq_desc *desc)
 {
     BUG_ON("unimplemented");
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index cfd7a08479d2..e36e06deaa68 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -472,15 +472,6 @@ void cf_check irq_ack_none(struct irq_desc *desc)
     ack_bad_irq(desc->irq);
 }
 
-hw_irq_controller no_irq_type = {
-    "none",
-    irq_startup_none,
-    irq_shutdown_none,
-    irq_enable_none,
-    irq_disable_none,
-    irq_ack_none,
-};
-
 static vmask_t *irq_get_used_vector_mask(int irq)
 {
     vmask_t *ret = NULL;
diff --git a/xen/common/irq.c b/xen/common/irq.c
index 7225b4637486..29729349a6f2 100644
--- a/xen/common/irq.c
+++ b/xen/common/irq.c
@@ -3,6 +3,19 @@
 
 DEFINE_PER_CPU(const struct cpu_user_regs *, irq_regs);
 
+const hw_irq_controller no_irq_type = {
+    .typename  = "none",
+    .startup   = irq_startup_none,
+    .shutdown  = irq_shutdown_none,
+    .enable    = irq_enable_none,
+    .disable   = irq_disable_none,
+    .ack       = irq_ack_none,
+
+#ifdef irq_end_none /* Hook is optional per arch */
+    .end       = irq_end_none,
+#endif
+};
+
 int init_one_irq_desc(struct irq_desc *desc)
 {
     int err;
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index b71f65db8621..327cd2217c7e 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -122,7 +122,7 @@ extern int request_irq(unsigned int irq, unsigned int irqflags,
                void (*handler)(int irq, void *dev_id),
                const char *devname, void *dev_id);
 
-extern hw_irq_controller no_irq_type;
+extern const hw_irq_controller no_irq_type;
 void cf_check no_action(int cpl, void *dev_id);
 unsigned int cf_check irq_startup_none(struct irq_desc *desc);
 void cf_check irq_actor_none(struct irq_desc *desc);
-- 
2.30.2

Hi Andrew,

On 5/30/24 1:40 PM, Andrew Cooper wrote:
> Having no_irq_type defined per arch, but using common callbacks is a mess, and
> particualrly hard to bootstrap a new architecture with.
> 
> Now that the ack()/end() hooks have been exported suitably, move the
> definition of no_irq_type into common/irq.c, and into .rodata for good
> measure.
> 
> No functional change, but a whole lot less tangled.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>

Thanks,
Shawn

Hi Andrew,

On 30/05/2024 19:40, Andrew Cooper wrote:
> Having no_irq_type defined per arch, but using common callbacks is a mess, and
> particualrly hard to bootstrap a new architecture with.
> 
> Now that the ack()/end() hooks have been exported suitably, move the
> definition of no_irq_type into common/irq.c, and into .rodata for good
> measure.
> 
> No functional change, but a whole lot less tangled.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

On Thu, 2024-05-30 at 19:40 +0100, Andrew Cooper wrote:
> Having no_irq_type defined per arch, but using common callbacks is a
> mess, and
> particualrly hard to bootstrap a new architecture with.
> 
> Now that the ack()/end() hooks have been exported suitably, move the
> definition of no_irq_type into common/irq.c, and into .rodata for
> good
> measure.
> 
> No functional change, but a whole lot less tangled.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
LGTM: Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>


> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis <bertrand.marquis@arm.com>
> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> CC: Shawn Anastasio <sanastasio@raptorengineering.com>
> 
> Oleksii: For RISC-V, you should only need to provide a irq_ack_none()
> stub now.
Sure, I will update my patch series during rebase.

~ Oleksii
> ---
>  xen/arch/arm/irq.c    | 10 ----------
>  xen/arch/ppc/stubs.c  |  9 ---------
>  xen/arch/x86/irq.c    |  9 ---------
>  xen/common/irq.c      | 13 +++++++++++++
>  xen/include/xen/irq.h |  2 +-
>  5 files changed, 14 insertions(+), 29 deletions(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 7138f9e7c283..e5fb26a3de2d 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -45,16 +45,6 @@ void irq_end_none(struct irq_desc *irq)
>      gic_hw_ops->gic_host_irq_type->end(irq);
>  }
>  
> -hw_irq_controller no_irq_type = {
> -    .typename = "none",
> -    .startup = irq_startup_none,
> -    .shutdown = irq_shutdown_none,
> -    .enable = irq_enable_none,
> -    .disable = irq_disable_none,
> -    .ack = irq_ack_none,
> -    .end = irq_end_none
> -};
> -
>  static irq_desc_t irq_desc[NR_IRQS];
>  static DEFINE_PER_CPU(irq_desc_t[NR_LOCAL_IRQS], local_irq_desc);
>  
> diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
> index 4e03428e071a..923f0e7b2095 100644
> --- a/xen/arch/ppc/stubs.c
> +++ b/xen/arch/ppc/stubs.c
> @@ -139,15 +139,6 @@ void irq_ack_none(struct irq_desc *desc)
>      BUG_ON("unimplemented");
>  }
>  
> -hw_irq_controller no_irq_type = {
> -    .typename = "none",
> -    .startup = irq_startup_none,
> -    .shutdown = irq_shutdown_none,
> -    .enable = irq_enable_none,
> -    .disable = irq_disable_none,
> -    .ack = irq_ack_none,
> -};
> -
>  int arch_init_one_irq_desc(struct irq_desc *desc)
>  {
>      BUG_ON("unimplemented");
> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> index cfd7a08479d2..e36e06deaa68 100644
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -472,15 +472,6 @@ void cf_check irq_ack_none(struct irq_desc
> *desc)
>      ack_bad_irq(desc->irq);
>  }
>  
> -hw_irq_controller no_irq_type = {
> -    "none",
> -    irq_startup_none,
> -    irq_shutdown_none,
> -    irq_enable_none,
> -    irq_disable_none,
> -    irq_ack_none,
> -};
> -
>  static vmask_t *irq_get_used_vector_mask(int irq)
>  {
>      vmask_t *ret = NULL;
> diff --git a/xen/common/irq.c b/xen/common/irq.c
> index 7225b4637486..29729349a6f2 100644
> --- a/xen/common/irq.c
> +++ b/xen/common/irq.c
> @@ -3,6 +3,19 @@
>  
>  DEFINE_PER_CPU(const struct cpu_user_regs *, irq_regs);
>  
> +const hw_irq_controller no_irq_type = {
> +    .typename  = "none",
> +    .startup   = irq_startup_none,
> +    .shutdown  = irq_shutdown_none,
> +    .enable    = irq_enable_none,
> +    .disable   = irq_disable_none,
> +    .ack       = irq_ack_none,
> +
> +#ifdef irq_end_none /* Hook is optional per arch */
> +    .end       = irq_end_none,
> +#endif
> +};
> +
>  int init_one_irq_desc(struct irq_desc *desc)
>  {
>      int err;
> diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
> index b71f65db8621..327cd2217c7e 100644
> --- a/xen/include/xen/irq.h
> +++ b/xen/include/xen/irq.h
> @@ -122,7 +122,7 @@ extern int request_irq(unsigned int irq, unsigned
> int irqflags,
>                 void (*handler)(int irq, void *dev_id),
>                 const char *devname, void *dev_id);
>  
> -extern hw_irq_controller no_irq_type;
> +extern const hw_irq_controller no_irq_type;
>  void cf_check no_action(int cpl, void *dev_id);
>  unsigned int cf_check irq_startup_none(struct irq_desc *desc);
>  void cf_check irq_actor_none(struct irq_desc *desc);