Re: [Xen-devel] [PATCH RFC 3/6] xen/arm: Allow platforms to hook IRQ routing.

[Julien Grall <julien.grall@xxxxxxx>]

Hello Kyle,

On 05/09/2016 11:13, Kyle Temkin wrote:
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 52c9a01..402c766 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
[...]

> +
> +bool_t platform_irq_is_routable(struct dt_raw_irq * rirq)
We are trying to move the code base to bool rather than bool_t.

> +{
> +    /*
> +     * 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);
> +}
> +
> +int platform_irq_for_device(const struct dt_device_node *dev, int index)
> +{
> +    if ( platform && platform->irq_for_device )
> +        return platform->irq_for_device(dev, index);
> +    else
> +        return platform_get_irq(dev, index);
IHMO the naming of this new function will confuse the user. Without any 
documentation it is not clear whether the user should call 
platform_irq_for_device or platform_get_irq.
However, I am not sure to understand why you need to override 
platform_get_irq as the code is exactly the same and dt_irq_xlate could 
be overidden to return the IRQ.
> +}
> +
> +
>  /*
>   * Local variables:
>   * mode: C
[...]

> diff --git a/xen/drivers/passthrough/arm/smmu.c 
> b/xen/drivers/passthrough/arm/smmu.c
> index cf8b8b8..94d035a 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -103,7 +103,7 @@ static struct resource *platform_get_resource(struct 
> platform_device *pdev,
>                 return ((ret) ? NULL : &res);
>
>         case IORESOURCE_IRQ:
> -               ret = platform_get_irq(pdev, num);
> +               ret = platform_irq_for_device(pdev, num);
>                 if (ret < 0)
>                         return NULL;
>
> @@ -2349,7 +2349,7 @@ static int arm_smmu_device_dt_probe(struct 
> platform_device *pdev)
>         }
>
>         for (i = 0; i < num_irqs; ++i) {
> -               int irq = platform_get_irq(pdev, i);
> +               int irq = platform_irq_for_device(pdev, i);
>
>                 if (irq < 0) {
>                         dev_err(dev, "failed to get irq index %d\n", i);
> diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
> index f97315d..4ea278b 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -26,6 +26,13 @@ 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_t (*irq_is_routable)(struct dt_raw_irq * rirq);
Please use bool here.

> +    int (*irq_for_device)(const struct dt_device_node *dev, int index);
> +
>      /*
>       * Platform blacklist devices
>       * List of devices which must not pass-through to a guest
> @@ -42,6 +49,13 @@ int platform_cpu_up(int cpu);
>  #endif
>  void platform_reset(void);
>  void platform_poweroff(void);
> +
> +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_t platform_irq_is_routable(struct dt_raw_irq *rirq);
Ditto.

> +int platform_irq_for_device(const struct dt_device_node *dev, int index);
> +
>  bool_t platform_device_is_blacklisted(const struct dt_device_node *node);
>
>  #define PLATFORM_START(_name, _namestr)                         \
Regards,

--
Julien Grall

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