Re: [Xen-ia64-devel] [PATCH][RFC]copy_from/to_guest

[Kouya SHIMURA <kouya@xxxxxxxxxxxxxx>]

Hi Yamahata-san,

Thanks to point out.
I'll take care for SMP-safe from now on.

Actually vcpu_itc_no_srlz() in ia64_map_hypercall_param() is called
only when a guest requests __HYPERVISOR_grant_table_op. I think the
race condition can hardly occur.

So I'd prefer to wait for xencomm rather than fix it now.

Thanks,
Kouya

Isaku Yamahata writes:
 > Hi.
 > 
 > Sorry for delayed point out.
 > At first I thought your hack would be replaced with xencomm.
 > However it seems to take a while to merge xencomm for IA64 so that
 > I decided to point it out now.
 > 
 > 
 > On Fri, Aug 11, 2006 at 07:01:10PM +0900, Kouya SHIMURA wrote:
 > Content-Description: message body text
 > > diff -r fc6c3d866477 xen/arch/ia64/xen/vcpu.c
 > > --- a/xen/arch/ia64/xen/vcpu.c     Wed Aug 09 10:32:23 2006 -0600
 > > +++ b/xen/arch/ia64/xen/vcpu.c     Fri Aug 11 18:35:31 2006 +0900
 > > @@ -2215,3 +2215,27 @@ IA64FAULT vcpu_ptr_i(VCPU *vcpu,UINT64 v
 > >  
 > >    return IA64_NO_FAULT;
 > >  }
 > > +
 > > +int ia64_map_hypercall_param(void)
 > > +{
 > > +  struct vcpu *v = current;
 > > +  struct domain *d = current->domain;
 > > +  u64 vaddr = v->arch.hypercall_param.va & PAGE_MASK;
 > > +  volatile pte_t* pte;
 > > +
 > > +  if (v->arch.hypercall_param.va == 0)
 > > +          return FALSE;
 > > +  pte = lookup_noalloc_domain_pte(d, v->arch.hypercall_param.pa1);
 > > +  if (!pte || !pte_present(*pte))
 > > +          return FALSE;
 > > +  vcpu_itc_no_srlz(v, 2, vaddr, pte_val(*pte), -1UL, PAGE_SHIFT);
 > > +  if (v->arch.hypercall_param.pa2) {
 > > +          vaddr += PAGE_SIZE;
 > > +          pte = lookup_noalloc_domain_pte(d, v->arch.hypercall_param.pa2);
 > > +          if (pte && pte_present(*pte)) {
 > > +                  vcpu_itc_no_srlz(v, 2, vaddr, pte_val(*pte), -1UL, 
 > > PAGE_SHIFT);
 > > +          }
 > > +  }
 > > +  ia64_srlz_d();
 > > +  return TRUE;
 > > +}
 > 
 > 
 > This isn't SMP-safe. Since the p2m table is locklesss,
 > another vcpu might change the p2m table entry after checking pte_present()
 > but before vcpu_itc_no_srlz(). You must be aware of it.
 > 
 > -- 
 > yamahata


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