Re: [Xen-devel] [PATCH v8 20/27] ARM: GICv3: handle unmapped LPIs

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 12/04/17 01:44, Andre Przywara wrote:
> When LPIs get unmapped by a guest, they might still be in some LR of
> some VCPU. Nevertheless we remove the corresponding pending_irq
> (possibly freeing it), and detect this case (irq_to_pending() returns
> NULL) when the LR gets cleaned up later.
> However a *new* LPI may get mapped with the same number while the old
> LPI is *still* in some LR. To avoid getting the wrong state, we mark
> every newly mapped LPI as PRISTINE, which means: has never been in an
> LR before. If we detect the LPI in an LR anyway, it must have been an
> older one, which we can simply retire.
> 
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> ---
>  xen/arch/arm/gic.c         | 17 ++++++++++++++++-
>  xen/arch/arm/vgic-v3-its.c |  5 +++++
>  xen/include/asm-arm/vgic.h |  1 +
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index d752352..e8c3202 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -373,6 +373,8 @@ static inline void gic_set_lr(int lr, struct pending_irq 
> *p,
>  {
>      ASSERT(!local_irq_is_enabled());
>  
> +    clear_bit(GIC_IRQ_GUEST_PRISTINE_LPI, &p->status);
> +
>      gic_hw_ops->update_lr(lr, p, state);
>  
>      set_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
> @@ -510,7 +512,17 @@ static void gic_update_one_lr(struct vcpu *v, int i)
>      }
>      else if ( lr_val.state & GICH_LR_PENDING )
>      {
> -        int q __attribute__ ((unused)) = 
> test_and_clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status);
> +        int q __attribute__ ((unused));
> +
> +        if ( test_and_clear_bit(GIC_IRQ_GUEST_PRISTINE_LPI, &p->status) )
> +        {
> +            gic_hw_ops->clear_lr(i);
> +            clear_bit(i, &this_cpu(lr_mask));
> +
> +            return;
> +        }
> +
> +        q = test_and_clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status);
>  #ifdef GIC_DEBUG
>          if ( q )
>              gdprintk(XENLOG_DEBUG, "trying to inject irq=%d into d%dv%d, 
> when it is already pending in LR%d\n",
> @@ -522,6 +534,9 @@ static void gic_update_one_lr(struct vcpu *v, int i)
>          gic_hw_ops->clear_lr(i);
>          clear_bit(i, &this_cpu(lr_mask));
>  
> +        if ( test_and_clear_bit(GIC_IRQ_GUEST_PRISTINE_LPI, &p->status) )
> +            return;
> +
>          if ( p->desc != NULL )
>              clear_bit(_IRQ_INPROGRESS, &p->desc->status);
>          clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> index b7e61b2..0765810 100644
> --- a/xen/arch/arm/vgic-v3-its.c
> +++ b/xen/arch/arm/vgic-v3-its.c
> @@ -618,6 +618,11 @@ static int its_handle_mapti(struct virt_its *its, 
> uint64_t *cmdptr)
>          goto out_remove_host_entry;
>  
>      pirq->lpi_vcpu_id = vcpu->vcpu_id;
> +    /*
> +     * Mark this LPI as new, so any older (now unmapped) LPI in any LR
> +     * can be easily recognised as such.
> +     */
> +    set_bit(GIC_IRQ_GUEST_PRISTINE_LPI, pirq->status);

Arrgh, so I somehow managed to introduce that typo after testing, but
before sending (note to self: don't send stuff at 1:44 am), sorry for
that! Of course it must be "..., &pirq->status);"

Just compile-tested this series (with that typo fixed) again: every
commit compiles without warnings for arm64 (w/ and w/o ITS configured)
and arm32.
Also successfully boot-tested on the model with and without ITS support.

Cheers,
Andre.

>      /*
>       * Now insert the pending_irq into the domain's LPI tree, so that
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index 02732db..b1a7525 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -66,6 +66,7 @@ struct pending_irq
>  #define GIC_IRQ_GUEST_VISIBLE  2
>  #define GIC_IRQ_GUEST_ENABLED  3
>  #define GIC_IRQ_GUEST_MIGRATING   4
> +#define GIC_IRQ_GUEST_PRISTINE_LPI  5
>      unsigned long status;
>      struct irq_desc *desc; /* only set it the irq corresponds to a physical 
> irq */
>      unsigned int irq;
> 

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