Re: [Xen-devel] [PATCH 04/12] ARM: VGIC: move gic_remove_irq_from_queues()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 19 Oct 2017, Andre Przywara wrote:
> gic_remove_irq_from_queues() was not only misnamed, it also has the wrong
> abstraction, as it should not live in gic.c.
> Move it into vgic.c and vgic.h, where it belongs to, and rename it on
> the way.
> 
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>

Yes, gic_remove_irq_from_queues could be in the vgic.

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

One comment about cosmetics below.


> ---
>  xen/arch/arm/gic.c         |  9 ---------
>  xen/arch/arm/vgic-v3-its.c |  4 ++--
>  xen/arch/arm/vgic.c        | 11 ++++++++++-
>  xen/include/asm-arm/gic.h  |  1 -
>  xen/include/asm-arm/vgic.h |  1 +
>  5 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 75b2e0e0ca..ef041354ea 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -411,15 +411,6 @@ void gic_remove_from_lr_pending(struct vcpu *v, struct 
> pending_irq *p)
>      list_del_init(&p->lr_queue);
>  }
>  
> -void gic_remove_irq_from_queues(struct vcpu *v, struct pending_irq *p)
> -{
> -    ASSERT(spin_is_locked(&v->arch.vgic.lock));
> -
> -    clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status);
> -    list_del_init(&p->inflight);
> -    gic_remove_from_lr_pending(v, p);
> -}
> -
>  void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq)
>  {
>      struct pending_irq *n = irq_to_pending(v, virtual_irq);
> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> index 72a5c70656..d8fa44258d 100644
> --- a/xen/arch/arm/vgic-v3-its.c
> +++ b/xen/arch/arm/vgic-v3-its.c
> @@ -381,7 +381,7 @@ static int its_handle_clear(struct virt_its *its, 
> uint64_t *cmdptr)
>       * have no active state, we don't need to care about this here.
>       */
>      if ( !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
> -        gic_remove_irq_from_queues(vcpu, p);
> +        vgic_remove_irq_from_queues(vcpu, p);
>  
>      spin_unlock_irqrestore(&vcpu->arch.vgic.lock, flags);
>      ret = 0;
> @@ -619,7 +619,7 @@ static int its_discard_event(struct virt_its *its,
>      }
>  
>      /* Cleanup the pending_irq and disconnect it from the LPI. */
> -    gic_remove_irq_from_queues(vcpu, p);
> +    vgic_remove_irq_from_queues(vcpu, p);
>      vgic_init_pending_irq(p, INVALID_LPI);
>  
>      spin_unlock_irqrestore(&vcpu->arch.vgic.lock, flags);
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 451a306a98..cd50b90d67 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -281,7 +281,7 @@ bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, 
> unsigned int irq)
>      /* If the IRQ is still lr_pending, re-inject it to the new vcpu */
>      if ( !list_empty(&p->lr_queue) )
>      {
> -        gic_remove_irq_from_queues(old, p);
> +        vgic_remove_irq_from_queues(old, p);
>          irq_set_affinity(p->desc, cpumask_of(new->processor));
>          spin_unlock_irqrestore(&old->arch.vgic.lock, flags);
>          vgic_vcpu_inject_irq(new, irq);
> @@ -510,6 +510,15 @@ void vgic_clear_pending_irqs(struct vcpu *v)
>      spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
>  }
>  
> +void vgic_remove_irq_from_queues(struct vcpu *v, struct pending_irq *p)
> +{
> +    ASSERT(spin_is_locked(&v->arch.vgic.lock));
> +
> +    clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status);
> +    list_del_init(&p->inflight);
> +    gic_remove_from_lr_pending(v, p);
> +}
> +
>  void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int virq)
>  {
>      uint8_t priority;
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 2f248301ce..030c1d86a7 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -243,7 +243,6 @@ extern void gic_raise_guest_irq(struct vcpu *v, unsigned 
> int irq,
>          unsigned int priority);
>  extern void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq);
>  extern void gic_remove_from_lr_pending(struct vcpu *v, struct pending_irq 
> *p);
> -extern void gic_remove_irq_from_queues(struct vcpu *v, struct pending_irq 
> *p);
>  
>  /* Accept an interrupt from the GIC and dispatch its handler */
>  extern void gic_interrupt(struct cpu_user_regs *regs, int is_fiq);
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index e489d0bf21..8d0ff65708 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -204,6 +204,7 @@ extern int vcpu_vgic_init(struct vcpu *v);
>  extern struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int virq);
>  extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int virq);
>  extern void vgic_vcpu_inject_spi(struct domain *d, unsigned int virq);
> +void vgic_remove_irq_from_queues(struct vcpu *v, struct pending_irq *p);

cosmetic: you might as well add an extern


>  extern void vgic_clear_pending_irqs(struct vcpu *v);
>  extern void vgic_init_pending_irq(struct pending_irq *p, unsigned int virq);
>  extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq);
> -- 
> 2.14.1
> 

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