Re: [Xen-devel] [PATCH 08/12] ARM: VGIC: split up gic_dump_info() to cover virtual part separately

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 19 Oct 2017, Andre Przywara wrote:
> Currently gic_dump_info() not only dumps the hardware state of the GIC,
> but also the VGIC internal virtual IRQ lists.
> Split the latter off and move it into vgic.c to observe the abstraction.
> 
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>

Same comment on lr_pending belonging to gic.c (or now gic-vgic.c).

vgic.c is not allowed to access lr_pending, but gic.c is allowed to
access inflight, only for reading or removing interrupts at the end of
the cycle (after EOI). vgic.c is expected to manage adding irqs to
inflight and removing them, for any reasons other than "the EOI is
done". I admit it is not clear from the code.


> ---
>  xen/arch/arm/domain.c      |  1 +
>  xen/arch/arm/gic.c         | 12 ------------
>  xen/arch/arm/vgic.c        | 11 +++++++++++
>  xen/include/asm-arm/vgic.h |  2 ++
>  4 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 73f4d4b2b2..5250bc2f88 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -942,6 +942,7 @@ long arch_do_vcpu_op(int cmd, struct vcpu *v, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  void arch_dump_vcpu_info(struct vcpu *v)
>  {
>      gic_dump_info(v);
> +    vgic_dump_info(v);
>  }
>  
>  void vcpu_mark_events_pending(struct vcpu *v)
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 04e6d66b69..4cb74d449e 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -443,20 +443,8 @@ static void maintenance_interrupt(int irq, void *dev_id, 
> struct cpu_user_regs *r
>  
>  void gic_dump_info(struct vcpu *v)
>  {
> -    struct pending_irq *p;
> -
>      printk("GICH_LRs (vcpu %d) mask=%"PRIx64"\n", v->vcpu_id, 
> v->arch.lr_mask);
>      gic_hw_ops->dump_state(v);
> -
> -    list_for_each_entry ( p, &v->arch.vgic.inflight_irqs, inflight )
> -    {
> -        printk("Inflight irq=%u lr=%u\n", p->irq, p->lr);
> -    }
> -
> -    list_for_each_entry( p, &v->arch.vgic.lr_pending, lr_queue )
> -    {
> -        printk("Pending irq=%d\n", p->irq);
> -    }
>  }
>  
>  void init_maintenance_interrupt(void)
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 2cdaca7480..37a083e804 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -656,6 +656,17 @@ void vgic_free_virq(struct domain *d, unsigned int virq)
>      clear_bit(virq, d->arch.vgic.allocated_irqs);
>  }
>  
> +void vgic_dump_info(struct vcpu *v)
> +{
> +    struct pending_irq *p;
> +
> +    list_for_each_entry ( p, &v->arch.vgic.inflight_irqs, inflight )
> +        printk("Inflight irq=%u lr=%u\n", p->irq, p->lr);
> +
> +    list_for_each_entry( p, &v->arch.vgic.lr_pending, lr_queue )
> +        printk("Pending irq=%d\n", p->irq);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index 0d3810e6af..49b8a4bec0 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -226,6 +226,8 @@ extern bool vgic_to_sgi(struct vcpu *v, register_t sgir,
>                          const struct sgi_target *target);
>  extern bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned 
> int irq);
>  
> +void vgic_dump_info(struct vcpu *v);
> +
>  /* Reserve a specific guest vIRQ */
>  extern bool vgic_reserve_virq(struct domain *d, unsigned int virq);
>  
> -- 
> 2.14.1
> 

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