Re: [Xen-devel] [PATCH for-4.12 v2 5/7] xen/arm: p2m: Only use isb() when it is necessary

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 28 Jan 2019, Julien Grall wrote:
> The EL1 translation regime is out-of-context when running at EL2. This
> means the processor cannot speculate memory accesses using the registers
> associated to that regime.
> 
> An isb() is only needed if Xen is going to use the translation regime
> before returning to the guest (exception returns will synchronize the
> context).
> 
> Remove unnecessary isb() and document the ones left.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> Reviewed-by: Andrii Anisov <andrii_anisov@xxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>     Changes in v2:
>         - Remove pointless {}
>         - Fix typoes
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/p2m.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 9844bfb936..44391a5f8c 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -106,17 +106,21 @@ void p2m_restore_state(struct vcpu *n)
>          return;
>  
>      WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
> -    isb();
> -
>      WRITE_SYSREG(n->arch.sctlr, SCTLR_EL1);
> -    isb();
> -
>      WRITE_SYSREG(n->arch.hcr_el2, HCR_EL2);
> -    isb();
>  
>      last_vcpu_ran = &p2m->last_vcpu_ran[smp_processor_id()];
>  
>      /*
> +     * While we are restoring an out-of-context translation regime
> +     * we still need to ensure:
> +     *  - VTTBR_EL2 is synchronized before flushing the TLBs
> +     *  - All registers for EL1 are synchronized before executing an AT
> +     *  instructions targeting S1/S2.
> +     */
> +    isb();
> +
> +    /*
>       * Flush local TLB for the domain to prevent wrong TLB translation
>       * when running multiple vCPU of the same domain on a single pCPU.
>       */
> @@ -147,6 +151,7 @@ static void p2m_force_tlb_flush_sync(struct p2m_domain 
> *p2m)
>      {
>          local_irq_save(flags);
>          WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
> +        /* Ensure VTTBR_EL2 is synchronized before flushing the TLBs */
>          isb();
>      }
>  
> @@ -155,6 +160,7 @@ static void p2m_force_tlb_flush_sync(struct p2m_domain 
> *p2m)
>      if ( ovttbr != READ_SYSREG64(VTTBR_EL2) )
>      {
>          WRITE_SYSREG64(ovttbr, VTTBR_EL2);
> +        /* Ensure VTTBR_EL2 is back in place before continuing. */
>          isb();
>          local_irq_restore(flags);
>      }
> @@ -1907,7 +1913,6 @@ static uint32_t __read_mostly vtcr;
>  static void setup_virt_paging_one(void *data)
>  {
>      WRITE_SYSREG32(vtcr, VTCR_EL2);
> -    isb();
>  }
>  
>  void __init setup_virt_paging(void)
> -- 
> 2.11.0
> 

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