From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Quan Xu <quan.xu@intel.com>
Cc: kevin.tian@intel.com, feng.wu@intel.com,
dario.faggioli@citrix.com, jbeulich@suse.com,
xen-devel@lists.xen.org
Subject: Re: [PATCH v8 1/3] VT-d: Reduce spin timeout to 1ms, which can be boot-time changed
Date: Fri, 25 Mar 2016 16:06:39 -0400 [thread overview]
Message-ID: <20160325200639.GF14689@char.us.oracle.com> (raw)
In-Reply-To: <1458799079-79825-3-git-send-email-quan.xu@intel.com>
On Thu, Mar 24, 2016 at 01:57:58PM +0800, Quan Xu wrote:
Hey!
Thanks for the patch!
I see that you have __must_check..
But if you check the callchain:
iommu_flush_iec_[index|global] ->
__iommu_flush_iec->invalidate_sync-> queue_invalidate_wait
you will see that the callers of iommu_flush_iec_[index|global]
ignore the return value.
So ... perhaps you could explain in this commit description on how to address
that? Is there an followup patch (if so just put in the name in here) to
address that?
Or should the top callers: enable_intremap, ioapic_rte_to_remap_entry,
free_remap_entry, msi_msg_to_remap_entry, and pi_update_irte do something?
I guess the 'free_remap_entry' reall can't. As you are suppose to always
be able to free something.
msi_msg_to_remap_entry, _msg_to_remap_entry, and ioapic_rte_to_remap_entry
could return an value... Or considering this is v8 - was there some epic
conversation that went over this quite a lot? In which case I would recommend
you say why it was not attempted this way so that folks six months from
now when reading this patch won't ask again.
> Signed-off-by: Quan Xu <quan.xu@intel.com>
> ---
> docs/misc/xen-command-line.markdown | 7 +++++++
> xen/drivers/passthrough/vtd/qinval.c | 17 ++++++++++++-----
> 2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index ca77e3b..384dde7 100644
> --- 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.
> +
> ### watchdog
> > `= force | <boolean>`
>
> diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
> index b81b0bd..52ba2c2 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 unsigned int __read_mostly vtd_qi_timeout = 1;
> +integer_param("vtd_qi_timeout", vtd_qi_timeout);
> +
> +#define IOMMU_QI_TIMEOUT (vtd_qi_timeout * MILLISECS(1))
> +
> static void print_qi_regs(struct iommu *iommu)
> {
> u64 val;
> @@ -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)
> {
> - s_time_t start_time;
> + s_time_t timeout;
> volatile u32 poll_slot = QINVAL_STAT_INIT;
> unsigned int index;
> unsigned long flags;
> @@ -164,13 +169,15 @@ static int queue_invalidate_wait(struct iommu *iommu,
> if ( sw )
> {
> /* In case all wait descriptor writes to same addr with same data */
> - start_time = NOW();
> + timeout = NOW() + IOMMU_QI_TIMEOUT;
> while ( poll_slot != QINVAL_STAT_DONE )
> {
> - if ( NOW() > (start_time + DMAR_OPERATION_TIMEOUT) )
> + if ( NOW() > timeout )
> {
> print_qi_regs(iommu);
> - panic("queue invalidate wait descriptor was not executed");
> + printk(XENLOG_WARNING VTDPREFIX
> + "Queue invalidate wait descriptor timed out.\n");
> + return -ETIMEDOUT;
> }
> cpu_relax();
> }
> --
> 1.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-03-25 20:06 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-24 5:57 [PATCH v8 0/3] VT-d Device-TLB flush issue Quan Xu
2016-03-24 5:57 ` [PATCH v8 2/3] VT-d: Wrap a _sync version for all VT-d flush interfaces Quan Xu
2016-03-24 13:56 ` Dario Faggioli
2016-03-24 15:06 ` Dario Faggioli
2016-03-25 3:11 ` Xu, Quan
2016-03-24 5:57 ` [PATCH v8 1/3] VT-d: Reduce spin timeout to 1ms, which can be boot-time changed Quan Xu
2016-03-24 11:04 ` Dario Faggioli
2016-03-24 11:28 ` Xu, Quan
2016-03-25 20:06 ` Konrad Rzeszutek Wilk [this message]
2016-03-28 6:27 ` Xu, Quan
2016-03-28 13:31 ` Konrad Rzeszutek Wilk
2016-04-01 15:03 ` Xu, Quan
2016-03-24 5:57 ` [PATCH v8 3/3] VT-d: Fix vt-d Device-TLB flush timeout issue Quan Xu
2016-03-24 15:38 ` Dario Faggioli
2016-03-25 3:43 ` Xu, Quan
2016-03-25 20:40 ` Konrad Rzeszutek Wilk
2016-03-28 3:44 ` Xu, Quan
2016-03-28 7:45 ` Xu, Quan
2016-03-25 20:31 ` Konrad Rzeszutek Wilk
2016-03-28 3:56 ` Xu, Quan
2016-03-28 14:11 ` Konrad Rzeszutek Wilk
2016-03-29 1:32 ` Xu, Quan
2016-03-29 14:20 ` Konrad Rzeszutek Wilk
2016-03-29 14:32 ` Xu, Quan
2016-03-24 10:33 ` [PATCH v8 0/3] VT-d Device-TLB flush issue Jan Beulich
2016-03-24 11:11 ` Xu, Quan
2016-04-01 14:47 ` Xu, Quan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160325200639.GF14689@char.us.oracle.com \
--to=konrad.wilk@oracle.com \
--cc=dario.faggioli@citrix.com \
--cc=feng.wu@intel.com \
--cc=jbeulich@suse.com \
--cc=kevin.tian@intel.com \
--cc=quan.xu@intel.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).