Re: [PATCH v8 1/3] VT-d: Reduce spin timeout to 1ms, which can be boot-time changed

From: Dario Faggioli <dario.faggioli@citrix.com>
To: Quan Xu <quan.xu@intel.com>, xen-devel@lists.xen.org
Cc: kevin.tian@intel.com, feng.wu@intel.com, jbeulich@suse.com
Subject: Re: [PATCH v8 1/3] VT-d: Reduce spin timeout to 1ms, which can be boot-time changed
Date: Thu, 24 Mar 2016 12:04:29 +0100	[thread overview]
Message-ID: <1458817469.5082.14.camel@citrix.com> (raw)
In-Reply-To: <1458799079-79825-3-git-send-email-quan.xu@intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 1919 bytes --]

On Thu, 2016-03-24 at 13:57 +0800, Quan Xu wrote:
> 
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1532,6 +1532,13 @@ Note that if **watchdog** option is also
> specified vpmu will be turned off.
>  As the virtualisation is not 100% safe, don't use the vpmu flag on
>  production systems (see http://xenbits.xen.org/xsa/advisory-163.html
> )!
>  
> +### vtd\_qi\_timeout (VT-d)
> +> `= <integer>`
> +
> +> Default: `1`
> +
> +Specify the timeout of the VT-d Queued Invalidation in milliseconds.
> +
Perhaps we should put down a quick paragraph to explain what this
parameter is about, and what happens if the timeout expires?

Giving hints on under what conditions one should change this value
would be even more welcome (e.g. "if you see error XXX, try increasing
the timeout to nnn").

> --- a/xen/drivers/passthrough/vtd/qinval.c
> +++ b/xen/drivers/passthrough/vtd/qinval.c
> 
> @@ -130,10 +135,10 @@ static void queue_invalidate_iotlb(struct iommu
> 
> *iommu,
>      spin_unlock_irqrestore(&iommu->register_lock, flags);
>  }
>  
> -static int queue_invalidate_wait(struct iommu *iommu,
> +static int __must_check queue_invalidate_wait(struct iommu *iommu,
>      u8 iflag, u8 sw, u8 fn)
>  {
>
I'm not sure it is ok to add the __must_check annotation here in this
patch, considering that the subject says "VT-d: Reduce spin timeout to
1ms, which can be boot-time changed", and there is no changelog.

Personally, I think it could be ok, but at least add a few words of
changelog, mentioning that this is also being done.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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