From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2 1/2] VT-d: Reduce spin timeout to 1ms, which can be boot-time changed. Date: Fri, 11 Dec 2015 08:37:05 +0000 Message-ID: <566A8B31.2040009@citrix.com> References: <1449739990-66155-1-git-send-email-quan.xu@intel.com> <1449739990-66155-2-git-send-email-quan.xu@intel.com> <5669CC64.7050407@citrix.com> <945CA011AD5F084CBEA3E851C0AB28894AE58FB1@SHSMSX103.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <945CA011AD5F084CBEA3E851C0AB28894AE58FB1@SHSMSX103.ccr.corp.intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Xu, Quan" Cc: "Tian, Kevin" , "Wu, Feng" , "jbeulich@suse.com" , "george.dunlap@eu.citrix.com" , "Dong, Eddie" , "tim@xen.org" , "xen-devel@lists.xen.org" , "Nakajima, Jun" , "keir@xen.org" List-Id: xen-devel@lists.xenproject.org On 11/12/2015 02:09, Xu, Quan wrote: > On 11.12.2015 at 3:03pm, wrote: >> On 10/12/15 09:33, Quan Xu wrote: >>> Signed-off-by: Quan Xu >>> --- >>> xen/drivers/passthrough/vtd/qinval.c | 11 +++++++++-- >>> 1 file changed, 9 insertions(+), 2 deletions(-) >>> >>> diff --git a/xen/drivers/passthrough/vtd/qinval.c >>> b/xen/drivers/passthrough/vtd/qinval.c >>> index b81b0bd..990baf2 100644 >>> --- a/xen/drivers/passthrough/vtd/qinval.c >>> +++ b/xen/drivers/passthrough/vtd/qinval.c >>> @@ -28,6 +28,11 @@ >>> #include "vtd.h" >>> #include "extern.h" >>> >>> +static int __read_mostly iommu_qi_timeout_ms = 1; >>> +integer_param("iommu_qi_timeout_ms", iommu_qi_timeout_ms); >>> + >>> +#define IOMMU_QI_TIMEOUT (iommu_qi_timeout_ms * MILLISECS(1)) >>> + >>> static void print_qi_regs(struct iommu *iommu) { >>> u64 val; >>> @@ -167,10 +172,12 @@ static int queue_invalidate_wait(struct iommu >> *iommu, >>> start_time = NOW(); >>> while ( poll_slot != QINVAL_STAT_DONE ) >>> { >>> - if ( NOW() > (start_time + DMAR_OPERATION_TIMEOUT) ) >>> + if ( NOW() > (start_time + IOMMU_QI_TIMEOUT) ) >>> { >>> print_qi_regs(iommu); >>> - panic("queue invalidate wait descriptor was not >> executed"); >>> + dprintk(XENLOG_WARNING VTDPREFIX, >>> + "Queue invalidate wait descriptor was >> timeout.\n"); >>> + return -ETIMEDOUT; >>> } >>> cpu_relax(); >>> } >> This patch misses a second use of DMAR_OPERATION_TIMEOUT, in >> IOMMU_WAIT_OP() which in turn is used in a large number of locations. >> All of these locations equally need to be chopped down to a low number of >> milliseconds. > Andrew, thanks for your comments. > > I know that DMAR_OPERATION_TIMEOUT should be also chopped down to a low number of milliseconds. > As Kevin Tian mentioned in 'Revisit VT-d asynchronous flush issue', We also confirmed with hardware team > that 1ms is large enough for IOMMU internal flush. > So I can change DMAR_OPERATION_TIMEOUT from 1000 ms to 1 ms. Ok - sounds good. > > IOMMU_WAIT_OP() is only for VT-d registers read/write, and there is also a panic. We need a further discussion > whether or how to remove this panic. We certainly should see about removing it. > I can send another patch set to fix it. in this patch set, I want to focus on VT-d > QI flush. Agreed. ~Andrew