[Xen-ia64-devel] Re: [PATCH] fix vmx_vcpu_flush_tlb_all()

[Kouya Shimura <kouya@xxxxxxxxxxxxxx>]

Hi Yamahata-san,

I think that calling ptc_ga_remote_func() during __thash_purge_all()
is harmless.
Because all that ptc_ga_remote_func() does is to set the 
invalid value(1UL << 63) to a tag in vcpu->arch.vtlb. 
It doesn't affect __thash_purge_all() at all.

__thash_purge_all() takes over 10 milliseconds. 
IMHO, it's not so good to disable interrupts for such a long period.

Thanks,
Kouya

Isaku Yamahata writes:
> [IA64] fix vmx_vcpu_flush_tlb_all()
> 
> There is a race between vmx_vcpu_flush_tlb_all() and vmx_vcpu_ptc_ga().
> __thash_purge_all() can be called without interrupt disabled so that
> vmx_vcpu_ptc_ga() may issue ipi to call ptc_ga_remote_func() on that
> pCPU during __thash_purge_all() is executing. And both of the functions
> manipulate vcpu->arch.vtlb.
> To avoid the race, mask interrupts in __thash_purge_all().
> 
> Signed-off-by: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
> 
> diff -r b5c925071c07 xen/arch/ia64/vmx/vtlb.c
> --- a/xen/arch/ia64/vmx/vtlb.c        Thu Jul 31 19:01:03 2008 +0900
> +++ b/xen/arch/ia64/vmx/vtlb.c        Thu Jul 31 20:14:47 2008 +0900
> @@ -626,9 +626,14 @@
>  static void __thash_purge_all(void *arg)
>  {
>      struct vcpu *v = arg;
> +    unsigned long flags;
>  
>      BUG_ON(vcpu_runnable(v) || v->is_running);
> +
> +    /* we are racing with vmx_vcpu_ptc_ga() */
> +    local_irq_save(flags);
>      thash_purge_all(v);
> +    local_irq_restore(flags);
>  }
>  
>  void vmx_vcpu_flush_vtlb_all(VCPU *v)
> 
> -- 
> yamahata

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel