On gen9 for blt cmd parser we relied on the magic fence error
propagation which:
- doesn't work on gen7, because there's no scheduler with ringbuffers
there yet
- fence error propagation can be weaponized to attack other things, so
not a good design idea
Instead of magic, do the same thing on gen9 as on gen7.
Kudos to Jason for figuring this out.
Fixes: 9e31c1fe45d5 ("drm/i915: Propagate errors on awaiting already signaled fences")
Cc: <stable(a)vger.kernel.org> # v5.6+
Cc: Jason Ekstrand <jason.ekstrand(a)intel.com>
Cc: Marcin Slusarz <marcin.slusarz(a)intel.com>
Cc: Jon Bloomfield <jon.bloomfield(a)intel.com>
Relates: https://gitlab.freedesktop.org/drm/intel/-/issues/3080
Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com>
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 34 +++++++++++++-------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 5b4b2bd46e7c..2d3336ab7ba3 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1509,6 +1509,12 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
}
}
+ /* Batch unsafe to execute with privileges, cancel! */
+ if (ret) {
+ cmd = page_mask_bits(shadow->obj->mm.mapping);
+ *cmd = MI_BATCH_BUFFER_END;
+ }
+
if (trampoline) {
/*
* With the trampoline, the shadow is executed twice.
@@ -1524,26 +1530,20 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
*/
*batch_end = MI_BATCH_BUFFER_END;
- if (ret) {
- /* Batch unsafe to execute with privileges, cancel! */
- cmd = page_mask_bits(shadow->obj->mm.mapping);
- *cmd = MI_BATCH_BUFFER_END;
+ /* If batch is unsafe but valid, jump to the original */
+ if (ret == -EACCES) {
+ unsigned int flags;
- /* If batch is unsafe but valid, jump to the original */
- if (ret == -EACCES) {
- unsigned int flags;
+ flags = MI_BATCH_NON_SECURE_I965;
+ if (IS_HASWELL(engine->i915))
+ flags = MI_BATCH_NON_SECURE_HSW;
- flags = MI_BATCH_NON_SECURE_I965;
- if (IS_HASWELL(engine->i915))
- flags = MI_BATCH_NON_SECURE_HSW;
+ GEM_BUG_ON(!IS_GEN_RANGE(engine->i915, 6, 7));
+ __gen6_emit_bb_start(batch_end,
+ batch_addr,
+ flags);
- GEM_BUG_ON(!IS_GEN_RANGE(engine->i915, 6, 7));
- __gen6_emit_bb_start(batch_end,
- batch_addr,
- flags);
-
- ret = 0; /* allow execution */
- }
+ ret = 0; /* allow execution */
}
}
--
2.31.0
The mdev remove callback for the vfio_ap device driver bails out with
-EBUSY if the mdev is in use by a KVM guest. The intended purpose was
to prevent the mdev from being removed while in use; however, returning a
non-zero rc does not prevent removal. This could result in a memory leak
of the resources allocated when the mdev was created. In addition, the
KVM guest will still have access to the AP devices assigned to the mdev
even though the mdev no longer exists.
To prevent this scenario, cleanup will be done - including unplugging the
AP adapters, domains and control domains - regardless of whether the mdev
is in use by a KVM guest or not.
Fixes: 258287c994de ("s390: vfio-ap: implement mediated device open callback")
Cc: stable(a)vger.kernel.org
Signed-off-by: Tony Krowiak <akrowiak(a)linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck(a)redhat.com>
---
drivers/s390/crypto/vfio_ap_ops.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index b2c7e10dfdcd..f90c9103dac2 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -26,6 +26,7 @@
static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev);
static struct vfio_ap_queue *vfio_ap_find_queue(int apqn);
+static void vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev *matrix_mdev);
static int match_apqn(struct device *dev, const void *data)
{
@@ -366,17 +367,7 @@ static int vfio_ap_mdev_remove(struct mdev_device *mdev)
struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
mutex_lock(&matrix_dev->lock);
-
- /*
- * If the KVM pointer is in flux or the guest is running, disallow
- * un-assignment of control domain.
- */
- if (matrix_mdev->kvm_busy || matrix_mdev->kvm) {
- mutex_unlock(&matrix_dev->lock);
- return -EBUSY;
- }
-
- vfio_ap_mdev_reset_queues(mdev);
+ vfio_ap_mdev_unset_kvm(matrix_mdev);
list_del(&matrix_mdev->node);
kfree(matrix_mdev);
mdev_set_drvdata(mdev, NULL);
--
2.30.2
From: Magnus Karlsson <magnus.karlsson(a)intel.com>
Hi Greg and Sasha,
Please find attached backports of commit 11cc2d21499c ("xsk: Simplify
detection of empty and full rings") for the 5.4.y and 4.19.y stable
series. It solves a reproducible race between poll() and sendmsg()
when used concurrently by two different processes using the same
AF_XDP socket. Note that the commit above unknowingly (read: by sheer
luck) fixed the bug as it was about simplification and performance
improvement only. I have included two backports that are code wise
equivallent to the commit above, however, they contain a commit message
that describes the race in question and how it is fixed by the
patch. Sorry, but I do not know the correct procedure in these kind of
situations, but if you prefer to pick the original commit, please
ignore the "backports" below.
Please let me know if there are any problems.
Thanks: Magnus
>From aa84d8c8e0ba1e83a3454df04cd6bd417ee2bc8e Mon Sep 17 00:00:00 2001
From: Magnus Karlsson <magnus.karlsson(a)intel.com>
Date: Thu, 19 Dec 2019 13:39:21 +0100
Subject: [PATCH 4.19] xsk: fix race between poll and the driver
commit 11cc2d21499cabe7e7964389634ed1de3ee91d33 upstream
Fix a race between poll() and the driver that can result in one or
more packets being transmitted or received twice. The problem is that
poll() uses the same function the driver uses to access the Rx and Tx
rings in user space, and this function will update the state of one of
these rings under certain conditions. E.g., if the poll() call from
one process updates the Tx ring state while at the same time the
driver in zero-copy mode is processing entries in the ring (invoked by
sendmsg() or an interrupt), some packets will be sent twice. All the
AF_XDP rings are single producer / single consumer, so modifying one
from two places at the same time will corrupt it. Similarly, on the Rx
side packets might be received twice.
Fix this by changing the poll() function to not use the same function
as the driver uses and instead only read state from the ring.
Fixes: 35fcde7f8deb ("xsk: support for Tx")
Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
Signed-off-by: Magnus Karlsson <magnus.karlsson(a)intel.com>
Reported-by: Benoit Ganne <bganne(a)cisco.com>
Signed-off-by: Alexei Starovoitov <ast(a)kernel.org>
Link: https://lore.kernel.org/bpf/BYAPR11MB365382C5DB1E5FCC53242609C1549@BYAPR11M…
---
net/xdp/xsk_queue.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index fe96c0d039f2..cf7cbb5dd918 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -245,12 +245,15 @@ static inline void xskq_produce_flush_desc(struct xsk_queue *q)
static inline bool xskq_full_desc(struct xsk_queue *q)
{
- return xskq_nb_avail(q, q->nentries) == q->nentries;
+ /* No barriers needed since data is not accessed */
+ return READ_ONCE(q->ring->producer) - READ_ONCE(q->ring->consumer) ==
+ q->nentries;
}
static inline bool xskq_empty_desc(struct xsk_queue *q)
{
- return xskq_nb_free(q, q->prod_tail, q->nentries) == q->nentries;
+ /* No barriers needed since data is not accessed */
+ return READ_ONCE(q->ring->consumer) == READ_ONCE(q->ring->producer);
}
void xskq_set_umem(struct xsk_queue *q, struct xdp_umem_props *umem_props);
base-commit: 3c8c23092588a23bf1856a64f58c37f477a413be
--
2.29.0
>From 07f2ccb39bd20e90293c59ffcc977c14cf0ce577 Mon Sep 17 00:00:00 2001
From: Magnus Karlsson <magnus.karlsson(a)intel.com>
Date: Thu, 19 Dec 2019 13:39:21 +0100
Subject: [PATCH 5.4] xsk: fix race between poll and the driver
commit 11cc2d21499cabe7e7964389634ed1de3ee91d33 upstream
Fix a race between poll() and the driver that can result in one or
more packets being transmitted or received twice. The problem is that
poll() uses the same function the driver uses to access the Rx and Tx
rings in user space, and this function will update the state of one of
these rings under certain conditions. E.g., if the poll() call from
one process updates the Tx ring state while at the same time the
driver in zero-copy mode is processing entries in the ring (invoked by
sendmsg() or an interrupt), some packets will be sent twice. All the
AF_XDP rings are single producer / single consumer, so modifying one
from two places at the same time will corrupt it. Similarly, on the Rx
side packets might be received twice.
Fix this by changing the poll() function to not use the same function
as the driver uses and instead only read state from the ring.
Fixes: 35fcde7f8deb ("xsk: support for Tx")
Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
Signed-off-by: Magnus Karlsson <magnus.karlsson(a)intel.com>
Reported-by: Benoit Ganne <bganne(a)cisco.com>
Signed-off-by: Alexei Starovoitov <ast(a)kernel.org>
Link: https://lore.kernel.org/bpf/BYAPR11MB365382C5DB1E5FCC53242609C1549@BYAPR11M…
---
net/xdp/xsk_queue.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index eddae4688862..ee3f8c857dd8 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -363,12 +363,15 @@ static inline void xskq_produce_flush_desc(struct xsk_queue *q)
static inline bool xskq_full_desc(struct xsk_queue *q)
{
- return xskq_nb_avail(q, q->nentries) == q->nentries;
+ /* No barriers needed since data is not accessed */
+ return READ_ONCE(q->ring->producer) - READ_ONCE(q->ring->consumer) ==
+ q->nentries;
}
static inline bool xskq_empty_desc(struct xsk_queue *q)
{
- return xskq_nb_free(q, q->prod_tail, q->nentries) == q->nentries;
+ /* No barriers needed since data is not accessed */
+ return READ_ONCE(q->ring->consumer) == READ_ONCE(q->ring->producer);
}
void xskq_set_umem(struct xsk_queue *q, u64 size, u64 chunk_mask);
base-commit: b82e5721a17325739adef83a029340a63b53d4ad
--
2.29.0