>From: Tristan Gingold [mailto:[EMAIL PROTECTED] >Sent: 2006年4月7日 15:04 >> Sorry for my question, but I really can't convince myself such >> code immune from race. :-) It's possible another side to clear >> present bit of current dtlb right after pass of above check. In >> that case, the TC entry is still inserted into TLB. Since the logic >> can't prevent all the cases, that check seems excessive here. >I don't agree. If a vcpu_ptc_ga clears dtlb after this, it will also execute >the ptc.ga.
Yes, this answers my concern... >> >- if (/* is_data && */ vcpu_match_tr_entry(trp,address,rid)) { >> >- if (vcpu->domain==dom0 && !in_tpa) *pteval = >> >trp->page_flags; >> >+ pte = trp->pte; >> >+ if (/* is_data && */ pte.p >> >+ && vcpu_match_tr_entry_no_p(trp,address,rid)) { >> >> Above change seems no difference as original code. >It uses _no_p. >The aim is to read pags_flags once! If you use vcpu_match_tr_entry here, you still only read page_flags once, see below: +static inline int vcpu_match_tr_entry(TR_ENTRY *trp, UINT64 ifa, UINT64 rid) +{ + return trp->pte.p && vcpu_match_tr_entry_no_p(trp, ifa, rid); +} I think at assembly level, trp->pte.p is expanded same as "pte=trp->pte; pte.p" >> Concerns here: >> - The per-LP vhpt table is shared by all domains, and the entry >> sitting at the hash position may not belong to target domain. By above >> way, you may finally purge unrelated entry of other domain. So check >> upon tag value is required >Yes. Call this an optimization. Just in case there's corner case though I didn't came up one. :-( >> - When hash tlb is added with collision chain support, this remote >> purge is more dangerous since there'll be link list operation which >> however has no lock protection. >Hash tlb requires more analysis. Hope the interface to be unique irregardless of whether hash or not. > >> Last point, so far your patch only aims for vtlb and vhpt. Machine >> TLB purge is not covered yet which however is the most direct place >> required to be handled. :-) >I don't understand. >vcpu_ptc_ga still calls ia64_global_tlb_purge, which does ptc.ga My fault, since I only looked at your patch where no existence of global_tlb_purge (it's in original code). So, no more questions from me by far. Thanks, Kevin _______________________________________________ Xen-ia64-devel mailing list Xen-ia64-devel@lists.xensource.com http://lists.xensource.com/xen-ia64-devel