All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Heyi Guo <guoheyi@huawei.com>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Sasha Levin <sashal@kernel.org>
Subject: [PATCH 4.19 07/47] irqchip/gic-v3-its: Fix command queue pointer comparison bug
Date: Thu, 18 Jul 2019 12:01:21 +0900	[thread overview]
Message-ID: <20190718030049.020637230@linuxfoundation.org> (raw)
In-Reply-To: <20190718030045.780672747@linuxfoundation.org>

[ Upstream commit a050fa5476d418fc16b25abe168b3d38ba11e13c ]

When we run several VMs with PCI passthrough and GICv4 enabled, not
pinning vCPUs, we will occasionally see below warnings in dmesg:

ITS queue timeout (65440 65504 480)
ITS cmd its_build_vmovp_cmd failed

The reason for the above issue is that in BUILD_SINGLE_CMD_FUNC:
1. Post the write command.
2. Release the lock.
3. Start to read GITS_CREADR to get the reader pointer.
4. Compare the reader pointer to the target pointer.
5. If reader pointer does not reach the target, sleep 1us and continue
to try.

If we have several processors running the above concurrently, other
CPUs will post write commands while the 1st CPU is waiting the
completion. So we may have below issue:

phase 1:
---rd_idx-----from_idx-----to_idx--0---------

wait 1us:

phase 2:
--------------from_idx-----to_idx--0-rd_idx--

That is the rd_idx may fly ahead of to_idx, and if in case to_idx is
near the wrap point, rd_idx will wrap around. So the below condition
will not be met even after 1s:

if (from_idx < to_idx && rd_idx >= to_idx)

There is another theoretical issue. For a slow and busy ITS, the
initial rd_idx may fall behind from_idx a lot, just as below:

---rd_idx---0--from_idx-----to_idx-----------

This will cause the wait function exit too early.

Actually, it does not make much sense to use from_idx to judge if
to_idx is wrapped, but we need a initial rd_idx when lock is still
acquired, and it can be used to judge whether to_idx is wrapped and
the current rd_idx is wrapped.

We switch to a method of calculating the delta of two adjacent reads
and accumulating it to get the sum, so that we can get the real rd_idx
from the wrapped value even when the queue is almost full.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jason Cooper <jason@lakedaemon.net>
Signed-off-by: Heyi Guo <guoheyi@huawei.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 35 ++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 65ab2c80529c..ee30e8965d1b 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -740,32 +740,43 @@ static void its_flush_cmd(struct its_node *its, struct its_cmd_block *cmd)
 }
 
 static int its_wait_for_range_completion(struct its_node *its,
-					 struct its_cmd_block *from,
+					 u64	prev_idx,
 					 struct its_cmd_block *to)
 {
-	u64 rd_idx, from_idx, to_idx;
+	u64 rd_idx, to_idx, linear_idx;
 	u32 count = 1000000;	/* 1s! */
 
-	from_idx = its_cmd_ptr_to_offset(its, from);
+	/* Linearize to_idx if the command set has wrapped around */
 	to_idx = its_cmd_ptr_to_offset(its, to);
+	if (to_idx < prev_idx)
+		to_idx += ITS_CMD_QUEUE_SZ;
+
+	linear_idx = prev_idx;
 
 	while (1) {
+		s64 delta;
+
 		rd_idx = readl_relaxed(its->base + GITS_CREADR);
 
-		/* Direct case */
-		if (from_idx < to_idx && rd_idx >= to_idx)
-			break;
+		/*
+		 * Compute the read pointer progress, taking the
+		 * potential wrap-around into account.
+		 */
+		delta = rd_idx - prev_idx;
+		if (rd_idx < prev_idx)
+			delta += ITS_CMD_QUEUE_SZ;
 
-		/* Wrapped case */
-		if (from_idx >= to_idx && rd_idx >= to_idx && rd_idx < from_idx)
+		linear_idx += delta;
+		if (linear_idx >= to_idx)
 			break;
 
 		count--;
 		if (!count) {
-			pr_err_ratelimited("ITS queue timeout (%llu %llu %llu)\n",
-					   from_idx, to_idx, rd_idx);
+			pr_err_ratelimited("ITS queue timeout (%llu %llu)\n",
+					   to_idx, linear_idx);
 			return -1;
 		}
+		prev_idx = rd_idx;
 		cpu_relax();
 		udelay(1);
 	}
@@ -782,6 +793,7 @@ void name(struct its_node *its,						\
 	struct its_cmd_block *cmd, *sync_cmd, *next_cmd;		\
 	synctype *sync_obj;						\
 	unsigned long flags;						\
+	u64 rd_idx;							\
 									\
 	raw_spin_lock_irqsave(&its->lock, flags);			\
 									\
@@ -803,10 +815,11 @@ void name(struct its_node *its,						\
 	}								\
 									\
 post:									\
+	rd_idx = readl_relaxed(its->base + GITS_CREADR);		\
 	next_cmd = its_post_commands(its);				\
 	raw_spin_unlock_irqrestore(&its->lock, flags);			\
 									\
-	if (its_wait_for_range_completion(its, cmd, next_cmd))		\
+	if (its_wait_for_range_completion(its, rd_idx, next_cmd))	\
 		pr_err_ratelimited("ITS cmd %ps failed\n", builder);	\
 }
 
-- 
2.20.1




  parent reply	other threads:[~2019-07-18  3:07 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-18  3:01 [PATCH 4.19 00/47] 4.19.60-stable review Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 01/47] Revert "e1000e: fix cyclic resets at link up with active tx" Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 02/47] e1000e: start network tx queue only when link is up Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 03/47] Input: synaptics - enable SMBUS on T480 thinkpad trackpad Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 04/47] nilfs2: do not use unexported cpu_to_le32()/le32_to_cpu() in uapi header Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 05/47] drivers: base: cacheinfo: Ensure cpu hotplug work is done before Intel RDT Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 06/47] firmware: improve LSM/IMA security behaviour Greg Kroah-Hartman
2019-07-18  3:01 ` Greg Kroah-Hartman [this message]
2019-07-18  3:01 ` [PATCH 4.19 08/47] clk: ti: clkctrl: Fix returning uninitialized data Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 09/47] efi/bgrt: Drop BGRT status field reserved bits check Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 10/47] perf/core: Fix perf_sample_regs_user() mm check Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 11/47] ARM: dts: gemini Fix up DNS-313 compatible string Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 12/47] ARM: omap2: remove incorrect __init annotation Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 13/47] afs: Fix uninitialised spinlock afs_volume::cb_break_lock Greg Kroah-Hartman
2019-07-19 22:33   ` Pavel Machek
2019-07-18  3:01 ` [PATCH 4.19 14/47] x86/apic: Fix integer overflow on 10 bit left shift of cpu_khz Greg Kroah-Hartman
2019-07-18  3:01   ` Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 15/47] be2net: fix link failure after ethtool offline test Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 16/47] ppp: mppe: Add softdep to arc4 Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 17/47] sis900: fix TX completion Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 18/47] ARM: dts: imx6ul: fix PWM[1-4] interrupts Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 19/47] pinctrl: mcp23s08: Fix add_data and irqchip_add_nested call order Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 20/47] dm table: dont copy from a NULL pointer in realloc_argv() Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 21/47] dm verity: use message limit for data block corruption message Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 22/47] x86/boot/64: Fix crash if kernel image crosses page table boundary Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 23/47] x86/boot/64: Add missing fixup_pointer() for next_early_pgt access Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 24/47] HID: chicony: add another quirk for PixArt mouse Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 25/47] HID: multitouch: Add pointstick support for ALPS Touchpad Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 26/47] pinctrl: mediatek: Ignore interrupts that are wake only during resume Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 27/47] cpu/hotplug: Fix out-of-bounds read when setting fail state Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 28/47] pinctrl: mediatek: Update cur_mask in mask/mask ops Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 29/47] linux/kernel.h: fix overflow for DIV_ROUND_UP_ULL Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 30/47] genirq: Delay deactivation in free_irq() Greg Kroah-Hartman
2019-07-19 19:58   ` Pavel Machek
2019-07-20 10:09     ` Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 31/47] genirq: Fix misleading synchronize_irq() documentation Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 32/47] genirq: Add optional hardware synchronization for shutdown Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 33/47] x86/ioapic: Implement irq_get_irqchip_state() callback Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 34/47] x86/irq: Handle spurious interrupt after shutdown gracefully Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 35/47] x86/irq: Seperate unused system vectors from spurious entry again Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 36/47] ARC: hide unused function unw_hdr_alloc Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 37/47] s390: fix stfle zero padding Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 38/47] s390/qdio: (re-)initialize tiqdio list entries Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 39/47] s390/qdio: dont touch the dsci in tiqdio_add_input_queues() Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 40/47] crypto: talitos - move struct talitos_edesc into talitos.h Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 41/47] crypto: talitos - fix hash on SEC1 Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 42/47] crypto/NX: Set receive window credits to max number of CRBs in RxFIFO Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 43/47] regmap-irq: do not write mask register if mask_base is zero Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 44/47] drm/udl: introduce a macro to convert dev to udl Greg Kroah-Hartman
2019-07-18  3:01 ` [PATCH 4.19 45/47] drm/udl: Replace drm_dev_unref with drm_dev_put Greg Kroah-Hartman
2019-07-18  3:02 ` [PATCH 4.19 46/47] drm/udl: move to embedding drm device inside udl device Greg Kroah-Hartman
2019-07-18  3:02 ` [PATCH 4.19 47/47] x86/entry/32: Fix ENDPROC of common_spurious Greg Kroah-Hartman
2019-07-18  8:13 ` [PATCH 4.19 00/47] 4.19.60-stable review kernelci.org bot
2019-07-18  9:20 ` Jon Hunter
2019-07-18  9:20   ` Jon Hunter
2019-07-18  9:47 ` Naresh Kamboju
2019-07-18 19:48 ` Guenter Roeck
2019-07-18 20:57 ` Kelsey Skunberg

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=20190718030049.020637230@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=guoheyi@huawei.com \
    --cc=jason@lakedaemon.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.