Le Vendredi 07 Avril 2006 09:18, Tian, Kevin a écrit :
> From: Tristan Gingold [mailto:Tristan.Gingold@xxxxxxxx]
>
> >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...
Ok.
> >> >- 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"
No. pte is volatile.
> >> 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.
Ok.
Tristan.
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home