From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@suse.com>
Subject: Re: [PATCH v2 1/2] VT-d: Reduce spin timeout to 1ms,
 which can be boot-time changed.
Date: Mon, 14 Dec 2015 01:18:47 -0700
Message-ID: <566E897702000078000BEF62@prv-mh.provo.novell.com>
References: <1449739990-66155-1-git-send-email-quan.xu@intel.com>
	<1449739990-66155-2-git-send-email-quan.xu@intel.com>
	<566AAD0902000078000BE8C5@prv-mh.provo.novell.com>
	<945CA011AD5F084CBEA3E851C0AB28894AE59E11@SHSMSX103.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <945CA011AD5F084CBEA3E851C0AB28894AE59E11@SHSMSX103.ccr.corp.intel.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Quan Xu <quan.xu@intel.com>
Cc: "tim@xen.org" <tim@xen.org>, Kevin Tian <kevin.tian@intel.com>, Feng Wu <feng.wu@intel.com>, "george.dunlap@eu.citrix.com" <george.dunlap@eu.citrix.com>, "andrew.cooper3@citrix.com" <andrew.cooper3@citrix.com>, Eddie Dong <eddie.dong@intel.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Jun Nakajima <jun.nakajima@intel.com>, "keir@xen.org" <keir@xen.org>
List-Id: xen-devel@lists.xenproject.org

>>> On 12.12.15 at 10:03, <quan.xu@intel.com> wrote:
>> On 11.12.2015 at 6:01pm, <JBeulich@suse.com> wrpte:
>> >>> On 10.12.15 at 10:33, <quan.xu@intel.com> wrote:
>> > @@ -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;
>> >              }
>> 
>> I don't see such a change be valid without making sure callers actually honor
>> errors. For example, no caller of iommu_flush_iec_{global,index}() cares to
>> check. And not even your second patch addresses this (i.e.
>> it's also not just bad patch ordering).
>> 
> 
> I check it again. 
> For iommu_flush_iec_{global,index}()  are both call __iommu_flush_iec().
> In my patch, I have check it in __iommu_flush_iec().
> I think it does not need to check it in iommu_flush_iec_{global,index}() 
> again. Right?

No, not in the v2 version of it. While you call invalidate_timeout() in
the -ETIMEDOUT case, you still pass on the error code, and hence
the callers need to also either pass it on or deal with it.

Jan