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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 28 Nov 2018, 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 need if Xen is going to use the translation regime
                   ^ needed


> before returning to the guest (exception returns will synchronized the
                                                        ^ synchronize

> context).
> 
> Remove unecessary isb() and document the ones left.
         ^ unnecessary



> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/arm/p2m.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index e8bacab9d2..844833c4c3 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -112,22 +112,28 @@ 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();

This explanation makes sense to me


> +    /*
>       * Flush local TLB for the domain to prevent wrong TLB translation
>       * when running multiple vCPU of the same domain on a single pCPU.
>       */
>      if ( *last_vcpu_ran != INVALID_VCPU_ID && *last_vcpu_ran != n->vcpu_id )
> +    {
>          flush_tlb_local();
> +    }

Spurious change?


>      *last_vcpu_ran = n->vcpu_id;
>  }
> @@ -153,6 +159,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();
>      }
>  
> @@ -161,6 +168,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);
>      }
> @@ -1496,7 +1504,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