WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ia64-devel

Re: [Xen-ia64-devel][PATCH][VTD] small patches for VTD

On Wed, Oct 22, 2008 at 01:56:05PM +0800, Xu, Anthony wrote:
> >> diff -r e1ed3e5cd001 xen/arch/ia64/xen/mm.c
> >> --- a/xen/arch/ia64/xen/mm.c  Tue Oct 21 18:55:22 2008 +0800
> >> +++ b/xen/arch/ia64/xen/mm.c  Tue Oct 21 19:13:45 2008 +0800 @@
> >> -189,6 +189,10 @@
> >>
> >>  static void __xencomm_mark_dirty(struct domain *d,
> >>                                   unsigned long addr, unsigned int
> >> len); + +static void
> >> +zap_domain_page_one(struct domain *d, unsigned long mpaddr,
> >> +                    int clear_PGC_allocate, unsigned long mfn);
> >>
> >>  extern unsigned long ia64_iobase;
> >>
> >> @@ -908,20 +912,21 @@
> >>                       unsigned long flags)
> >>  {
> >>      volatile pte_t *pte;
> >> -    pte_t old_pte;
> >>      pte_t new_pte;
> >>      pte_t ret_pte;
> >>      unsigned long prot = flags_to_prot(flags);
> >>
> >>      pte = lookup_alloc_domain_pte(d, mpaddr);
> >> +    new_pte = pfn_pte(physaddr >> PAGE_SHIFT, __pgprot(prot));
> >> +    if(pte_val(new_pte) == pte_val(*pte))
> >> +        return 0;
> >>
> >> -    old_pte = __pte(0);
> >> -    new_pte = pfn_pte(physaddr >> PAGE_SHIFT, __pgprot(prot));
> >> -    ret_pte = ptep_cmpxchg_rel(&d->arch.mm, mpaddr, pte, old_pte,
> >> new_pte);
> >> -    if (pte_val(ret_pte) == pte_val(old_pte)) {
> >> -        smp_mb();
> >> -        return 0;
> >> -    }
> >> +    /* for assigned MMIO, the old pte may be set to _PAGE_IO
> >> attribute, +     * so zap it first, then set up it.
> >> +     */
> >> +
> >> +    zap_domain_page_one(d, mpaddr, 1, INVALID_MFN);
> >> +    ret_pte = ptep_xchg(&d->arch.mm, mpaddr, pte, new_pte);
> >>
> >>      // dom0 tries to map real machine's I/O region, but failed.
> >>      // It is very likely that dom0 doesn't boot correctly because
> >
> > Hmmm, are you really sure that the above is SMP-safe?
> > We are touching p2m table locklessly so we must be extremely
> > careful. The above hunk split the atomic operation into two phase
> > and makes the following logic not make sense.
> >
> >
> 
> Yes, it is not SMP-safe there is lock for p2m.
> Modifying p2m is not a frequent operation, why not add a lock for it?

It is frequent to read p2m table. So lockless approach was adopted
for scalability.
It doesn't make sense to lock around only writer side.

thanks,
-- 
yamahata

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