Re: [PATCH 3/6] xen/arm: Allow platforms to hook IRQ routing

[Stefano Stabellini <sstabellini@kernel.org>]

On Thu, 6 Apr 2017, Chris Patterson wrote:
> From: "Chris Patterson" <pattersonc@ainfosec.com>
> 
> Some common platforms (e.g. Tegra) have non-traditional IRQ controllers
> that must be programmed in addition to their primary GICs-- and which
> can come in unusual topologies. Device trees for targets that feature
> these controllers often deviate from the conventions that Xen expects.
> 
> This commit provides a foundation for support of these platforms, by
> allowing the platform to decide which IRQs can be routed by Xen, rather
> than assuming that only GIC-connected IRQs can be routed.  This enables
> platform specific logic to routing the IRQ to Xen and Guest.
> 
> As dt_irq_translate() is presently hard-coded to just support the
> primary interrupt controller, instead rely on the newly added
> platform_irq_is_routable() check instead.  The default behaviour of this
> new function should be consistent with the previous checks for platforms
> that do not implement it.
> 
> Authored-by: Kyle Temkin <temkink@ainfosec.com>
> Signed-off-by: Kyle Temkin <temkink@ainfosec.com>
> Signed-off-by: Chris Patterson <pattersonc@ainfosec.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> 
> changes since rfc:
> - use bool instead of bool_t
> - formatting & code style cleanup
> - reuse dt_irq_translate() path and drop platform_irq_for_device() approach
> - use const qualifier for platform_irq_is_routable rirq argument
> 
> ---
> 
>  xen/arch/arm/domain_build.c    | 12 ++++++++----
>  xen/arch/arm/irq.c             |  5 +++--
>  xen/arch/arm/platform.c        | 30 ++++++++++++++++++++++++++++++
>  xen/common/device_tree.c       |  8 +++-----
>  xen/include/asm-arm/platform.h | 12 ++++++++++++
>  5 files changed, 56 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index cb66304..92536dd 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1120,12 +1120,16 @@ static int handle_device(struct domain *d, struct dt_device_node *dev,
>  
>          /*
>           * Don't map IRQ that have no physical meaning
> -         * ie: IRQ whose controller is not the GIC
> +         * ie: IRQ that does not wind up being controlled by the GIC
> +         * (Note that we can't just check to see if an IRQ is owned by the GIC,
> +         *  as some platforms have a controller between the device irq and the GIC,
> +         *  such as the Tegra legacy interrupt controller.)
>           */
> -        if ( rirq.controller != dt_interrupt_controller )
> +        if ( !platform_irq_is_routable(&rirq) )
>          {
> -            dt_dprintk("irq %u not connected to primary controller. Connected to %s\n",
> -                      i, dt_node_full_name(rirq.controller));
> +            dt_dprintk("irq %u not (directly or indirectly) connected to primary"
> +                        "controller. Connected to %s\n", i,
> +                        dt_node_full_name(rirq.controller));
>              continue;
>          }
>  
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index f3f20a6..0b4eaa9 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -26,6 +26,7 @@
>  
>  #include <asm/gic.h>
>  #include <asm/vgic.h>
> +#include <asm/platform.h>
>  
>  static unsigned int local_irqs_type[NR_LOCAL_IRQS];
>  static DEFINE_SPINLOCK(local_irqs_type_lock);
> @@ -369,7 +370,7 @@ int setup_irq(unsigned int irq, unsigned int irqflags, struct irqaction *new)
>      /* First time the IRQ is setup */
>      if ( disabled )
>      {
> -        gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
> +        platform_route_irq_to_xen(desc, GIC_PRI_IRQ);
>          /* It's fine to use smp_processor_id() because:
>           * For PPI: irq_desc is banked
>           * For SPI: we don't care for now which CPU will receive the
> @@ -506,7 +507,7 @@ int route_irq_to_guest(struct domain *d, unsigned int virq,
>      if ( retval )
>          goto out;
>  
> -    retval = gic_route_irq_to_guest(d, virq, desc, GIC_PRI_IRQ);
> +    retval = platform_route_irq_to_guest(d, virq, desc, GIC_PRI_IRQ);
>  
>      spin_unlock_irqrestore(&desc->lock, flags);
>  
> diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
> index 0af6d57..539ee3b 100644
> --- a/xen/arch/arm/platform.c
> +++ b/xen/arch/arm/platform.c
> @@ -147,6 +147,36 @@ bool_t platform_device_is_blacklisted(const struct dt_device_node *node)
>      return (dt_match_node(blacklist, node) != NULL);
>  }
>  
> +int platform_route_irq_to_guest(struct domain *d, unsigned int virq,
> +                                struct irq_desc *desc, unsigned int priority)
> +{
> +    if ( platform && platform->route_irq_to_guest )
> +        return platform->route_irq_to_guest(d, virq, desc, priority);
> +    else
> +        return gic_route_irq_to_guest(d, virq, desc, priority);
> +}
> +
> +void platform_route_irq_to_xen(struct irq_desc *desc, unsigned int priority)
> +{
> +    if ( platform && platform->route_irq_to_xen )
> +        platform->route_irq_to_xen(desc, priority);
> +    else
> +        gic_route_irq_to_xen(desc, priority);
> +}
> +
> +bool platform_irq_is_routable(const struct dt_raw_irq * rirq)
> +{
> +    /*
> +     * If we have a platform-specific method to determine if an IRQ is routable,
> +     * check that; otherwise fall back to checking to see if an IRQ belongs to
> +     * the GIC.
> +     */
> +    if ( platform && platform->irq_is_routable )
> +        return platform->irq_is_routable(rirq);
> +    else
> +        return (rirq->controller == dt_interrupt_controller);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 7b009ea..061693b 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -22,6 +22,7 @@
>  #include <xen/string.h>
>  #include <xen/cpumask.h>
>  #include <xen/ctype.h>
> +#include <asm/platform.h>
>  #include <asm/setup.h>
>  #include <xen/err.h>
>  
> @@ -1467,11 +1468,8 @@ int dt_irq_translate(const struct dt_raw_irq *raw,
>      ASSERT(dt_irq_xlate != NULL);
>      ASSERT(dt_interrupt_controller != NULL);
>  
> -    /*
> -     * TODO: Retrieve the right irq_xlate. This is only works for the primary
> -     * interrupt controller.
> -     */
> -    if ( raw->controller != dt_interrupt_controller )
> +    /* Only proceed with translation if the irq is routable on the platform. */
> +    if ( !platform_irq_is_routable(raw) )
>          return -EINVAL;
>  
>      return dt_irq_xlate(raw->specifier, raw->size,
> diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
> index 08010ba..119ad2e 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -26,6 +26,12 @@ struct platform_desc {
>      void (*reset)(void);
>      /* Platform power-off */
>      void (*poweroff)(void);
> +    /* Platform-specific IRQ routing */
> +    int (*route_irq_to_guest)(struct domain *d, unsigned int virq,
> +                               struct irq_desc *desc, unsigned int priority);
> +    void (*route_irq_to_xen)(struct irq_desc *desc, unsigned int priority);
> +    bool (*irq_is_routable)(const struct dt_raw_irq * rirq);
> +
>      /*
>       * Platform quirks
>       * Defined has a function because a platform can support multiple
> @@ -56,6 +62,12 @@ int platform_cpu_up(int cpu);
>  void platform_reset(void);
>  void platform_poweroff(void);
>  bool_t platform_has_quirk(uint32_t quirk);
> +
> +int platform_route_irq_to_guest(struct domain *d, unsigned int virq,
> +                                 struct irq_desc *desc, unsigned int priority);
> +void platform_route_irq_to_xen(struct irq_desc *desc, unsigned int priority);
> +bool platform_irq_is_routable(const struct dt_raw_irq *rirq);
> +
>  bool_t platform_device_is_blacklisted(const struct dt_device_node *node);
>  
>  #define PLATFORM_START(_name, _namestr)                         \
> -- 
> 2.1.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel