Re: [Xen-devel] [RFC 8/8] xen/arm: acpi: route all unused IRQs to DOM0

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 7 Jun 2016, Julien Grall wrote:
> It is not possible to know which IRQs will be used by DOM0 when ACPI is
> inuse. The approach implemented by this patch, will route all unused
> IRQs to DOM0 before it has booted.
> 
> The number of IRQs routed is based on the maximum SPIs supported by the
> hardware (up to ~1000). However, some of them might not be wired. So we
> would allocate resource for nothing.
> 
> For each IRQ routed, Xen is allocating memory for irqaction (40 bytes)
> and irq_guest (16 bytes). So in the worst case scenario ~54KB of memory
> will be allocated. Given that ACPI will mostly be used by server, I
> think it is a small drawback.

Yeah, it's a pity. The patch is certainly an improvement and it would
fix ACPI, which is currently broken, so I think it should go in pretty
much as is. (See only one small comment below.)

But I wonder if we could do something better by the next release. Have
you considered using something like a tasklet to call
route_irq_to_guest? The tasklet would be scheduled only after
vgic_enable_irqs is called.

Something like:

- guest writes to GICD_ISENABLER
- run vgic_enable_irqs
  - enable tasklet
- tasklet run
  - call route_irq_to_guest

?


> map_irq_to_domain is slightly reworked to remove the dependency on
> device-tree. So the function can be also be used for ACPI and will
> avoid code duplication.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/arm/domain_build.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 00dc07a..76d503d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -953,11 +953,10 @@ static int make_timer_node(const struct domain *d, void 
> *fdt,
>      return res;
>  }
>  
> -static int map_irq_to_domain(const struct dt_device_node *dev,
> -                             struct domain *d, unsigned int irq)
> +static int map_irq_to_domain(struct domain *d, unsigned int irq,
> +                             bool_t need_mapping, const char *devname)
>  
>  {
> -    bool_t need_mapping = !dt_device_for_passthrough(dev);
>      int res;
>  
>      res = irq_permit_access(d, irq);
> @@ -977,7 +976,7 @@ static int map_irq_to_domain(const struct dt_device_node 
> *dev,
>           */
>          vgic_reserve_virq(d, irq);
>  
> -        res = route_irq_to_guest(d, irq, irq, dt_node_name(dev));
> +        res = route_irq_to_guest(d, irq, irq, devname);
>          if ( res < 0 )
>          {
>              printk(XENLOG_ERR "Unable to map IRQ%"PRId32" to dom%d\n",
> @@ -997,6 +996,7 @@ static int map_dt_irq_to_domain(const struct 
> dt_device_node *dev,
>      struct domain *d = data;
>      unsigned int irq = dt_irq->irq;
>      int res;
> +    bool_t need_mapping = !dt_device_for_passthrough(dev);
>  
>      if ( irq < NR_LOCAL_IRQS )
>      {
> @@ -1015,7 +1015,7 @@ static int map_dt_irq_to_domain(const struct 
> dt_device_node *dev,
>          return res;
>      }
>  
> -    res = map_irq_to_domain(dev, d, irq);
> +    res = map_irq_to_domain(d, irq, need_mapping, dt_node_name(dev));
>  
>      return 0;
>  }
> @@ -1152,7 +1152,7 @@ static int handle_device(struct domain *d, struct 
> dt_device_node *dev)
>              return res;
>          }
>  
> -        res = map_irq_to_domain(dev, d, res);
> +        res = map_irq_to_domain(d, res, need_mapping, dt_node_name(dev));
>          if ( res )
>              return res;
>      }
> @@ -1411,13 +1411,10 @@ static int acpi_permit_spi_access(struct domain *d)

I would rename acpi_permit_spi_access to acpi_route_spis_to_dom0 or
something like that, to reflect the change in behavior of the function.



>          if ( desc->action != NULL)
>              continue;
>  
> -        res = irq_permit_access(d, i);
> +        /* XXX: Shall we use a proper devname? */
> +        res = map_irq_to_domain(d, i, true, "ACPI");
>          if ( res )
> -        {
> -            printk(XENLOG_ERR "Unable to permit to dom%u access to IRQ %u\n",
> -                   d->domain_id, i);
>              return res;
> -        }
>      }


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