Re: [PATCH 2/2] arch/irq: Centralise no_irq_type

["Oleksii K." <oleksii.kurochko@xxxxxxxxx>]

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@xxxxxxxxxx>
LGTM: Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>


> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> CC: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> CC: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
> 
> 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);