Re: [Xen-devel] VT-d spin loops

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/06/14 11:38, Andrew Cooper wrote:
> On 26/06/14 11:30, Jan Beulich wrote:
>> All,
>>
>> VT-d code currently has a number of cases where completion of certain
>> operations is being waited for by way of spinning. The various instances
>> can be identified relatively easily by grep-ing for all uses of
>> DMAR_OPERATION_TIMEOUT (the majority of instances use that
>> variable indirectly through IOMMU_WAIT_OP()), allowing for loops of
>> up to 1 second. While in many of the cases this _may_ be acceptable
>> (which would need to be proven for each individual case, also taking into
>> consideration how many of these spinning loops may be executed in a
>> row with no preemption/scheduling in between), the invalidation case
>> seems particularly problematic: Using DMAR_OPERATION_TIMEOUT is
>> a mistake here in the first place, as the timeout here isn't related to
>> response times by the IOMMU engine. Instead - with ATS in use - the
>> specification mandates a timeout of 1 _minute_ (with a 50% slack, the
>> meaning of which none of us [Andrew and Malcolm brought this issue
>> to my attention in private talks on the hackathon] was able to really
>> interpret in a sensible way).
> From a worst case point of view, this 50% slack means 90 seconds total.
>
>> So there are two things that need doing rather sooner than later:
>>
>> First and foremost the ATS case needs to be taken into consideration
>> when doing invalidations. Obviously we can't spin for a minute, so
>> invalidation absolutely needs to be converted to a non-spinning model.
>> We realize this isn't going to be trivial, which is why we bring this up
>> here rather than coming forward with a patch right away.
>>
>> Second, looking at Linux (which interestingly enough also spins, and
>> that even without any timeout) there are flags in the fault status
>> register that can be used to detect at least some loop abort conditions.
>> We should definitely make use of anything that can shorten these
>> spinning loops (as was already done in commit dd6d87a4 ["VT-d: drop
>> redundant calls to invalidate_sync()"] as a very tiny first step). The
>> main problem with trying to clone at least some of the functionality
>> from Linux is that I'm not convinced the replaying they do can
>> actually do anything good. Plus it's clear that - spinning or not - the
>> consequences of an invalidation request not completing successfully
>> need to be taken care of (and it's of no help that in all cases I looked
>> at so far errors passed up from the leaf functions sooner or later
>> get dropped on the floor or mis-interpreted).
>>
>> And finally, all other spinning instances need to be audited to make
>> sure they can't add up to multiple-second spins (iirc we can't
>> tolerate more than about 4s without running into time problems on
>> certain hardware).
>>
>> Jan
>>
> With a default watchdog, the maximum possible spin time is 5 seconds.
>
> However, given the 1 second time calibration rendezvous, any spin longer
> than 1 second will hold all other Xen cpus up waiting for the spinning
> cpu, which is a stall of the entire results in a stall of the entire
> system until the spinner has exited.
>
> ~Andrew

Hmm sorry - that sentence didn't come out quite correctly.

Any spin for the IOMMU longer than 1 second will stall the entire rest
of the system until it is complete, due to the 1 second time calibration
rendezvous, as some of the IOMMU spins are with interrupts disabled.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel