Re: [Xen-devel] [PATCH 2/4] xen/arm: gic: Ensure ordering between read of INTACK and shared data

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 23 Oct 2018, Julien Grall wrote:
> When an IPI is generated by a CPU, the pattern looks roughly like:
> 
>   <write shared data>
>   dsb(sy);
>   <write to GIC to signal SGI>
> 
> On the receiving CPU we rely on the fact that, once we've taken the
> interrupt, then the freshly written shared data must be visible to us.
> Put another way, the CPU isn't going to speculate taking an interrupt.
> 
> Unfortunately, this assumption turns out to be broken.
> 
> Consider that CPUx wants to send an IPI to CPUy, which will cause CPUy
> to read some shared_data. Before CPUx has done anything, a random
> peripheral raises an IRQ to the GIC and the IRQ line on CPUy is raised.
> CPUy then takes the IRQ and starts executing the entry code, heading
> towards gic_handle_irq. Furthermore, let's assume that a bunch of the
> previous interrupts handled by CPUy were SGIs, so the branch predictor
> kicks in and speculates that irqnr will be <16 and we're likely to
> head into handle_IPI. The prefetcher then grabs a speculative copy of
> shared_data which contains a stale value.
> 
> Meanwhile, CPUx gets round to updating shared_data and asking the GIC
> to send an SGI to CPUy. Internally, the GIC decides that the SGI is
> more important than the peripheral interrupt (which hasn't yet been
> ACKed) but doesn't need to do anything to CPUy, because the IRQ line
> is already raised.
> 
> CPUy then reads the ACK register on the GIC, sees the SGI value which
> confirms the branch prediction and we end up with a stale shared_data
> value.
> 
> This patch fixes the problem by adding an smp_rmb() to the IPI entry
> code in do_SGI.
> 
> At the same time document the write barrier.
> 
> Based on Linux commit f86c4fbd930ff6fecf3d8a1c313182bd0f49f496
> "irqchip/gic: Ensure ordering between read of INTACK and shared data".
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

> ---
>     This patch is candidate for backporting up to Xen 4.9.
> ---
>  xen/arch/arm/gic.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 305fbd66dd..30c0fba0d7 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -300,6 +300,11 @@ void send_SGI_mask(const cpumask_t *cpumask, enum 
> gic_sgi sgi)
>  {
>      ASSERT(sgi < 16); /* There are only 16 SGIs */
>  
> +   /*
> +    * Ensure that stores to Normal memory are visible to the other CPUs
> +    * before issuing the IPI.
> +    * Matches the read barrier in do_sgi.
> +    */
>      dsb(sy);
>      gic_hw_ops->send_SGI(sgi, SGI_TARGET_LIST, cpumask);
>  }
> @@ -313,6 +318,11 @@ void send_SGI_self(enum gic_sgi sgi)
>  {
>      ASSERT(sgi < 16); /* There are only 16 SGIs */
>  
> +   /*
> +    * Ensure that stores to Normal memory are visible to the other CPUs
> +    * before issuing the IPI.
> +    * Matches the read barrier in do_sgi.
> +    */
>      dsb(sy);
>      gic_hw_ops->send_SGI(sgi, SGI_TARGET_SELF, NULL);
>  }
> @@ -321,6 +331,11 @@ void send_SGI_allbutself(enum gic_sgi sgi)
>  {
>     ASSERT(sgi < 16); /* There are only 16 SGIs */
>  
> +   /*
> +    * Ensure that stores to Normal memory are visible to the other CPUs
> +    * before issuing the IPI.
> +    * Matches the read barrier in do_sgi.
> +    */
>     dsb(sy);
>     gic_hw_ops->send_SGI(sgi, SGI_TARGET_OTHERS, NULL);
>  }
> @@ -356,6 +371,13 @@ static void do_sgi(struct cpu_user_regs *regs, enum 
> gic_sgi sgi)
>      /* Lower the priority */
>      gic_hw_ops->eoi_irq(desc);
>  
> +    /*
> +     * Ensure any shared data written by the CPU sending
> +     * the IPI is read after we've read the ACK register on the GIC.
> +     * Matches the write barrier in send_SGI_* helpers.
> +     */
> +    smp_rmb();
> +
>      switch (sgi)
>      {
>      case GIC_SGI_EVENT_CHECK:
> -- 
> 2.11.0
> 

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