From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Tian, Kevin" Subject: Re: VT-d spin loops Date: Wed, 9 Jul 2014 23:22:07 +0000 Message-ID: References: <53A96B80020000780001CB8F@mail.emea.novell.com> <53AC124E020000780001D8FF@mail.emea.novell.com> <53AC124E020000780001D8FF@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1X51Bm-00029I-RX for xen-devel@lists.xenproject.org; Wed, 09 Jul 2014 23:22:15 +0000 In-Reply-To: <53AC124E020000780001D8FF@mail.emea.novell.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , "Zhang, Yang Z" , xen-devel Cc: Andrew Cooper , Malcolm Crossley , "Dugger, Donald D" List-Id: xen-devel@lists.xenproject.org > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: Thursday, June 26, 2014 3:30 AM > > 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). yes, that's not a good design. Most waits happen in IOMMU initialization, where 1s timeout is less a big issue. At runtime cache/tlb flush and root entry manipulation are definitely not good with long spinning . > > 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. ATS should be fine. Device TLB can ONLY be validated through qinval interface, which is asynchronous so no need to consider 1 minute timeout even in current spinning model. > > 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). > In general yes a non-spinning model is better, but it requires non-trivial change to make all IOMMU operations asynchronous. If ATS is not a concern, is it still worthy of change besides auditing existing usages? :-) Thanks Kevin Thanks Kevin