RE: [Xen-ia64-devel] Some things to be considered for ptc.ga emulation

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

>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...

>> >-   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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel