Re: [PATCH v3 6/8] ARM: VGIC: factor out vgic_get_hw_irq_desc()

[Julien Grall <julien.grall@linaro.org>]

Hi,

On 24/01/18 18:10, Andre Przywara wrote:
> At the moment we happily access the VGIC internal struct pending_irq
> (which describes a virtual IRQ) in irq.c.
> Factor out the actually needed functionality to learn the associated
> hardware IRQ and move that into gic-vgic.c to improve abstraction.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>   xen/arch/arm/gic-vgic.c    | 15 +++++++++++++++
>   xen/arch/arm/irq.c         |  7 ++-----
>   xen/include/asm-arm/vgic.h |  2 ++
>   3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
> index d44e4dacd3..3ad98dcd3a 100644
> --- a/xen/arch/arm/gic-vgic.c
> +++ b/xen/arch/arm/gic-vgic.c
> @@ -411,6 +411,21 @@ void gic_dump_vgic_info(struct vcpu *v)
>           printk("Pending irq=%d\n", p->irq);
>   }
>   
> +struct irq_desc *vgic_get_hw_irq_desc(struct domain *d, struct vcpu *v,
> +                                      unsigned int virq)
> +{
> +    struct pending_irq *p;
> +
> +    if ( !v )
> +        v = d->vcpu[0];

I dislike this new function in the current state. Let's imagine someone 
decide to pass a PPI but no vCPU. The vCPU would now be default to vCPU0 
and potentially returning the wrong irq_desc (imagine PPI passthrough 
such as for the vtimer).

So the code needs at least checking the vCPU is passed in the case of a 
PPI. I would be happy with an ASSERT().

Cheers,

-- 
Julien Grall

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