Re: [Xen-ia64-devel] [PATCH] Fix SMP-unsafe with XENMEM_add_to_physmap on HVM

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

Applied, thanks.

On Mon, Jul 07, 2008 at 01:04:06PM +0900, Kouya Shimura wrote:
Content-Description: message body text
> Isaku Yamahata writes:
> > Doesn't flush_tlb_for_log_dirty() have same issue?
> 
> It's OK since flush_tlb_for_log_dirty() is called only while a domain
> is paused.
> 
> > As a minor code nit.
> > Could it be moved the new code fragment under the directory,
> > arch/ia64/vmx?
> 
> Sure, new one attached.
> 
> Thanks,
> Kouya
> 
> Signed-off-by: Kouya Shimura <kouya@xxxxxxxxxxxxxx>
> 

> diff -r 08f77df14cba xen/arch/ia64/vmx/vtlb.c
> --- a/xen/arch/ia64/vmx/vtlb.c        Wed Jul 02 11:30:37 2008 +0900
> +++ b/xen/arch/ia64/vmx/vtlb.c        Mon Jul 07 12:42:03 2008 +0900
> @@ -623,6 +623,30 @@
>      local_flush_tlb_all();
>  }
>  
> +static void __thash_purge_all(void *arg)
> +{
> +    struct vcpu *v = arg;
> +
> +    BUG_ON(vcpu_runnable(v) || v->is_running);
> +    thash_purge_all(v);
> +}
> +
> +void vmx_vcpu_flush_vtlb_all(VCPU *v)
> +{
> +    if (v == current) {
> +        thash_purge_all(v);
> +        return;
> +    }
> +
> +    /* SMP safe */
> +    vcpu_pause(v);
> +    if (v->processor == smp_processor_id())
> +        __thash_purge_all(v);
> +    else
> +        smp_call_function_single(v->processor, __thash_purge_all, v, 1, 1);
> +    vcpu_unpause(v);
> +}
> +
>  
>  /*
>   * Lookup the hash table and its collision chain to find an entry
> diff -r 08f77df14cba xen/arch/ia64/xen/vhpt.c
> --- a/xen/arch/ia64/xen/vhpt.c        Wed Jul 02 11:30:37 2008 +0900
> +++ b/xen/arch/ia64/xen/vhpt.c        Mon Jul 07 12:42:03 2008 +0900
> @@ -249,31 +249,20 @@
>  // (e.g. vcpu == current), smp_mb() is unnecessary.
>  void vcpu_flush_vtlb_all(struct vcpu *v)
>  {
> -     if (VMX_DOMAIN(v)) {
> -             /* This code may be call for remapping shared_info and
> -                grant_table share page from guest_physmap_remove_page()
> -                in arch_memory_op() XENMEM_add_to_physmap to realize
> -                PV-on-HVM feature. */
> -             /* FIXME: This is not SMP-safe yet about p2m table */
> -             /* Purge vTLB for VT-i domain */
> -             thash_purge_all(v);
> -     }
> -     else {
> -             /* First VCPU tlb.  */
> -             vcpu_purge_tr_entry(&PSCBX(v,dtlb));
> -             vcpu_purge_tr_entry(&PSCBX(v,itlb));
> -             smp_mb();
> +     /* First VCPU tlb.  */
> +     vcpu_purge_tr_entry(&PSCBX(v,dtlb));
> +     vcpu_purge_tr_entry(&PSCBX(v,itlb));
> +     smp_mb();
>  
> -             /* Then VHPT.  */
> -             if (HAS_PERVCPU_VHPT(v->domain))
> -                     vcpu_vhpt_flush(v);
> -             else
> -                     local_vhpt_flush();
> -             smp_mb();
> +     /* Then VHPT.  */
> +     if (HAS_PERVCPU_VHPT(v->domain))
> +             vcpu_vhpt_flush(v);
> +     else
> +             local_vhpt_flush();
> +     smp_mb();
>  
> -             /* Then mTLB.  */
> -             local_flush_tlb_all();
> -     }
> +     /* Then mTLB.  */
> +     local_flush_tlb_all();
>  
>       /* We could clear bit in d->domain_dirty_cpumask only if domain d in
>          not running on this processor.  There is currently no easy way to
> @@ -296,6 +285,15 @@
>       for_each_vcpu(d, v) {
>               if (!v->is_initialised)
>                       continue;
> +
> +             if (VMX_DOMAIN(v)) {
> +                     // This code may be called for remapping shared_info
> +                     // and grant_table from guest_physmap_remove_page()
> +                     // in arch_memory_op() XENMEM_add_to_physmap to realize
> +                     // PV-on-HVM feature.
> +                     vmx_vcpu_flush_vtlb_all(v);
> +                     continue;
> +             }
>  
>               if (v->processor == cpu)
>                       vcpu_flush_vtlb_all(v);
> diff -r 08f77df14cba xen/include/asm-ia64/vmmu.h
> --- a/xen/include/asm-ia64/vmmu.h     Wed Jul 02 11:30:37 2008 +0900
> +++ b/xen/include/asm-ia64/vmmu.h     Mon Jul 07 12:42:03 2008 +0900
> @@ -175,6 +175,7 @@
>   *
>   */
>  extern void thash_purge_all(struct vcpu *v);
> +extern void vmx_vcpu_flush_vtlb_all(struct vcpu *v);
>  
>  /*
>   * Lookup the hash table and its collision chain to find an entry

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

-- 
yamahata

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