Re: [PATCH v2 31/39] xen/riscv: implement APLIC-hart sync barrier for vCPU migration

[Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>]

> During migration of a virtual hart to a different guest interrupt file,
> straggler MSIs from the APLIC could arrive at the old interrupt file
> after the switch.
> 
> genmsi is used despite not supporting guest interrupt files because the
> AIA spec guarantees that all MSIs previously sent from the APLIC to the
> same hart are visible at the hart's IMSIC before the extempore MSI from
> genmsi becomes visible.


> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
>
> diff --git a/xen/arch/riscv/aplic.c b/xen/arch/riscv/aplic.c
> index 0af13f28e4..cb11d6aeaa 100644
> --- a/xen/arch/riscv/aplic.c
> +++ b/xen/arch/riscv/aplic.c
> @@ -27,7 +27,9 @@
>  #include <asm/imsic.h>
>  #include <asm/intc.h>
>  #include <asm/io.h>
> +#include <asm/processor.h>
>  #include <asm/riscv_encoding.h>
> +#include <asm/smp.h>
>  
>  static struct aplic_priv aplic = {
>      .lock = SPIN_LOCK_UNLOCKED,
> @@ -205,6 +207,30 @@ void aplic_hw_write_reg(unsigned int offset, uint32_t 
> value)
>      spin_unlock_irqrestore(&aplic.lock, flags);
>  }
>  
> +/*
> + * As needed, synchronize with all IOMMUs and APLICs to ensure that no
> + * straggler MSIs will arrive at the old interrupt file after this step.
> + */
> +void aplic_genmsi_barrier(void)
> +{
> +    const struct imsic_config *imsic = imsic_get_config();
> +    unsigned int cpu = smp_processor_id();
> +    unsigned long flags;
> +    uint32_t val;
> +
> +    val = MASK_INSR(aplic_hart_field(cpu), APLIC_TARGET_HART_IDX) |
> +          (imsic->sync_id & APLIC_TARGET_EIID);


> +
> +    spin_lock_irqsave(&aplic.lock, flags);
> +
> +    writel(val, &aplic.regs->genmsi);
> +
> +    while ( readl(&aplic.regs->genmsi) & APLIC_GENMSI_BUSY )
> +        cpu_relax();
> +
> +    spin_unlock_irqrestore(&aplic.lock, flags);
> +}
> +
According to AIA spec §4.9.3 (Synchronizing interactions between a hart and the 
APLIC), the sequence needs 6 steps; this implements only steps 2-5:

- Step 1: clear the pending bit for sync_id at the hart's IMSIC before writing 
genmsi.
- Step 6: after releasing the lock, poll the pending bit for sync_id at the 
hart's IMSIC until it's set.

Step 4 (Busy clear) only means the APLIC has accepted/sent the MSI, not that it 
has arrived at the hart (the spec notes an unspecified travel delay).
Without step 6, aplic_genmsi_barrier() returns before the MSI (and thus prior 
MSIs) actually reach the hart, so it doesn't achieve the barrier it's meant to
provide.

-- 
Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>