On 22/07/2019 09:34, Jan Beulich wrote:
> On 19.07.2019 19:27, Andrew Cooper wrote:
>> On 16/07/2019 17:38, Jan Beulich wrote:
>>> @@ -142,7 +178,15 @@ static void free_intremap_entry(const st
>>> {
>>> union irte_ptr entry = get_intremap_entry(iommu, bdf, index);
>>>
>>> - ACCESS_ONCE(entry.ptr32->raw[0]) = 0;
>>> + if ( iommu->ctrl.ga_en )
>>> + {
>>> + ACCESS_ONCE(entry.ptr128->raw[0]) = 0;
>>> + /* Low half (containing RemapEn) needs to be cleared first. */
>>> + barrier();
>> While this will function on x86, I still consider this buggy. From a
>> conceptual point of view, barrier() is not the correct construction to
>> use, whereas smp_wmb() is.
> I think it's the 3rd time now that I respond saying that barrier() is
> as good or as bad as smp_wmb(), just for different reasons.
barrier() and smp_wmb() are different constructs, with specific,
*different* meanings. From a programmers point of view, they should be
considered black boxes of functionality.
barrier() is for forcing the compiler to not reorder things.
smp_wmb() is about the external visibility of writes, as observed by a
different entity on a coherent fabric.
The fact they alias on x86 in an implementation detail of x86 cache
coherency - it does not mean they can legitimately be alternated in code.
This piece of code is a 2-way communication between the CPU core and the
IOMMU, over a coherent cache. The IOMMU logically has an smp_rmb() in
its mirror functionality (although that is likely not how the property
is expressed).
> While I
> agree with you that barrier() is correct on x86 only, I'm yet to hear
> back from you on my argument that smp_wmb() is incorrect when
> considering its UP semantics (which we don't currently implement, but
> which Linux as the origin of the construct can well be used for
> reference).
UP vs SMP doesn't affect which is the correct construct to use.
> And I think we both don't really want wmb() here.
No, because wmb() is definitely not the right thing to use.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
