* [PATCH for 7.1 v1 0/5] virtio fixes (split from virtio-gpio series)
@ 2022-07-27 15:56 Alex Bennée
2022-07-27 15:56 ` [PATCH v1 1/5] block/vhost-user-blk-server: don't expose VHOST_USER_F_PROTOCOL_FEATURES Alex Bennée
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Alex Bennée @ 2022-07-27 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, Alex Bennée
Hi,
This is just a split out series based on:
Subject: [PATCH v3 for 7.2 00/21] virtio-gpio and various virtio cleanups
Date: Tue, 26 Jul 2022 20:21:29 +0100
Message-Id: <20220726192150.2435175-1-alex.bennee@linaro.org>
with the patches identified by mst:
Right. Still I think some are fixes we should merge now.
I am thinking patches 5, 7,8,9 ? 6 if it makes backporting
much easier. WDYT? If you agree pls separate bugfixes in
series I can apply. Thanks!
So here they are ;-)
Alex Bennée (5):
block/vhost-user-blk-server: don't expose
VHOST_USER_F_PROTOCOL_FEATURES
hw/virtio: incorporate backend features in features
hw/virtio: gracefully handle unset vhost_dev vdev
hw/virtio: handle un-configured shutdown in virtio-pci
hw/virtio: fix vhost_user_read tracepoint
block/export/vhost-user-blk-server.c | 3 +--
hw/virtio/vhost-user.c | 13 ++++++++++---
hw/virtio/vhost.c | 10 +++++++---
hw/virtio/virtio-pci.c | 9 +++++++--
4 files changed, 25 insertions(+), 10 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v1 1/5] block/vhost-user-blk-server: don't expose VHOST_USER_F_PROTOCOL_FEATURES
2022-07-27 15:56 [PATCH for 7.1 v1 0/5] virtio fixes (split from virtio-gpio series) Alex Bennée
@ 2022-07-27 15:56 ` Alex Bennée
2022-07-28 8:46 ` Kevin Wolf
2022-07-27 15:56 ` [PATCH v1 2/5] hw/virtio: incorporate backend features in features Alex Bennée
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2022-07-27 15:56 UTC (permalink / raw)
To: qemu-devel
Cc: mst, Alex Bennée, Coiby Xu, Kevin Wolf, Hanna Reitz,
open list:Block layer core
This bit is unused in actual VirtIO feature negotiation and should
only appear in the vhost-user messages between master and slave.
[AJB: experiment, this doesn't break the tests but I'm not super
confident of the range of tests]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220726192150.2435175-6-alex.bennee@linaro.org>
---
block/export/vhost-user-blk-server.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index 3409d9e02e..d31436006d 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -125,8 +125,7 @@ static uint64_t vu_blk_get_features(VuDev *dev)
1ull << VIRTIO_BLK_F_MQ |
1ull << VIRTIO_F_VERSION_1 |
1ull << VIRTIO_RING_F_INDIRECT_DESC |
- 1ull << VIRTIO_RING_F_EVENT_IDX |
- 1ull << VHOST_USER_F_PROTOCOL_FEATURES;
+ 1ull << VIRTIO_RING_F_EVENT_IDX ;
if (!vexp->handler.writable) {
features |= 1ull << VIRTIO_BLK_F_RO;
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v1 2/5] hw/virtio: incorporate backend features in features
2022-07-27 15:56 [PATCH for 7.1 v1 0/5] virtio fixes (split from virtio-gpio series) Alex Bennée
2022-07-27 15:56 ` [PATCH v1 1/5] block/vhost-user-blk-server: don't expose VHOST_USER_F_PROTOCOL_FEATURES Alex Bennée
@ 2022-07-27 15:56 ` Alex Bennée
2022-07-27 15:56 ` [PATCH v1 3/5] hw/virtio: gracefully handle unset vhost_dev vdev Alex Bennée
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Alex Bennée @ 2022-07-27 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, Alex Bennée
There are some extra bits used over a vhost-user connection which are
hidden from the device itself. We need to set them here to ensure we
enable things like the protocol extensions.
Currently net/vhost-user.c has it's own inscrutable way of persisting
this data but it really should live in the core vhost_user code.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220726192150.2435175-7-alex.bennee@linaro.org>
---
hw/virtio/vhost-user.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 75b8df21a4..1936a44e82 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1460,7 +1460,14 @@ static int vhost_user_set_features(struct vhost_dev *dev,
*/
bool log_enabled = features & (0x1ULL << VHOST_F_LOG_ALL);
- return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
+ /*
+ * We need to include any extra backend only feature bits that
+ * might be needed by our device. Currently this includes the
+ * VHOST_USER_F_PROTOCOL_FEATURES bit for enabling protocol
+ * features.
+ */
+ return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES,
+ features | dev->backend_features,
log_enabled);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v1 3/5] hw/virtio: gracefully handle unset vhost_dev vdev
2022-07-27 15:56 [PATCH for 7.1 v1 0/5] virtio fixes (split from virtio-gpio series) Alex Bennée
2022-07-27 15:56 ` [PATCH v1 1/5] block/vhost-user-blk-server: don't expose VHOST_USER_F_PROTOCOL_FEATURES Alex Bennée
2022-07-27 15:56 ` [PATCH v1 2/5] hw/virtio: incorporate backend features in features Alex Bennée
@ 2022-07-27 15:56 ` Alex Bennée
2022-07-27 15:56 ` [PATCH v1 4/5] hw/virtio: handle un-configured shutdown in virtio-pci Alex Bennée
2022-07-27 15:56 ` [PATCH v1 5/5] hw/virtio: fix vhost_user_read tracepoint Alex Bennée
4 siblings, 0 replies; 8+ messages in thread
From: Alex Bennée @ 2022-07-27 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, Alex Bennée
I've noticed asserts firing because we query the status of vdev after
a vhost connection is closed down. Rather than faulting on the NULL
indirect just quietly reply false.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220726192150.2435175-8-alex.bennee@linaro.org>
---
hw/virtio/vhost.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 0827d631c0..f758f177bb 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -306,7 +306,7 @@ static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
dev->log_size = size;
}
-static int vhost_dev_has_iommu(struct vhost_dev *dev)
+static bool vhost_dev_has_iommu(struct vhost_dev *dev)
{
VirtIODevice *vdev = dev->vdev;
@@ -316,8 +316,12 @@ static int vhost_dev_has_iommu(struct vhost_dev *dev)
* does not have IOMMU, there's no need to enable this feature
* which may cause unnecessary IOTLB miss/update transactions.
*/
- return virtio_bus_device_iommu_enabled(vdev) &&
- virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+ if (vdev) {
+ return virtio_bus_device_iommu_enabled(vdev) &&
+ virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+ } else {
+ return false;
+ }
}
static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v1 4/5] hw/virtio: handle un-configured shutdown in virtio-pci
2022-07-27 15:56 [PATCH for 7.1 v1 0/5] virtio fixes (split from virtio-gpio series) Alex Bennée
` (2 preceding siblings ...)
2022-07-27 15:56 ` [PATCH v1 3/5] hw/virtio: gracefully handle unset vhost_dev vdev Alex Bennée
@ 2022-07-27 15:56 ` Alex Bennée
2022-07-27 15:56 ` [PATCH v1 5/5] hw/virtio: fix vhost_user_read tracepoint Alex Bennée
4 siblings, 0 replies; 8+ messages in thread
From: Alex Bennée @ 2022-07-27 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, Alex Bennée
The assert() protecting against leakage is a little aggressive and
causes needless crashes if a device is shutdown without having been
configured. In this case no descriptors are lost because none have
been assigned.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220726192150.2435175-9-alex.bennee@linaro.org>
---
hw/virtio/virtio-pci.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 45327f0b31..5ce61f9b45 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -996,9 +996,14 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
nvqs = MIN(nvqs, VIRTIO_QUEUE_MAX);
- /* When deassigning, pass a consistent nvqs value
- * to avoid leaking notifiers.
+ /*
+ * When deassigning, pass a consistent nvqs value to avoid leaking
+ * notifiers. But first check we've actually been configured, exit
+ * early if we haven't.
*/
+ if (!assign && !proxy->nvqs_with_notifiers) {
+ return 0;
+ }
assert(assign || nvqs == proxy->nvqs_with_notifiers);
proxy->nvqs_with_notifiers = nvqs;
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v1 5/5] hw/virtio: fix vhost_user_read tracepoint
2022-07-27 15:56 [PATCH for 7.1 v1 0/5] virtio fixes (split from virtio-gpio series) Alex Bennée
` (3 preceding siblings ...)
2022-07-27 15:56 ` [PATCH v1 4/5] hw/virtio: handle un-configured shutdown in virtio-pci Alex Bennée
@ 2022-07-27 15:56 ` Alex Bennée
4 siblings, 0 replies; 8+ messages in thread
From: Alex Bennée @ 2022-07-27 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, Alex Bennée
As reads happen in the callback we were never seeing them. We only
really care about the header so move the tracepoint to when the header
is complete.
Fixes: 6ca6d8ee9d (hw/virtio: add vhost_user_[read|write] trace points)
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220726192150.2435175-10-alex.bennee@linaro.org>
---
hw/virtio/vhost-user.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 1936a44e82..c0b50deaf2 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -295,6 +295,8 @@ static int vhost_user_read_header(struct vhost_dev *dev, VhostUserMsg *msg)
return -EPROTO;
}
+ trace_vhost_user_read(msg->hdr.request, msg->hdr.flags);
+
return 0;
}
@@ -544,8 +546,6 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
}
}
- trace_vhost_user_read(msg.hdr.request, msg.hdr.flags);
-
return 0;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/5] block/vhost-user-blk-server: don't expose VHOST_USER_F_PROTOCOL_FEATURES
2022-07-27 15:56 ` [PATCH v1 1/5] block/vhost-user-blk-server: don't expose VHOST_USER_F_PROTOCOL_FEATURES Alex Bennée
@ 2022-07-28 8:46 ` Kevin Wolf
2022-07-28 10:27 ` Alex Bennée
0 siblings, 1 reply; 8+ messages in thread
From: Kevin Wolf @ 2022-07-28 8:46 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, mst, Coiby Xu, Hanna Reitz, open list:Block layer core
Am 27.07.2022 um 17:56 hat Alex Bennée geschrieben:
> This bit is unused in actual VirtIO feature negotiation and should
> only appear in the vhost-user messages between master and slave.
>
> [AJB: experiment, this doesn't break the tests but I'm not super
> confident of the range of tests]
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20220726192150.2435175-6-alex.bennee@linaro.org>
> ---
> block/export/vhost-user-blk-server.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
> index 3409d9e02e..d31436006d 100644
> --- a/block/export/vhost-user-blk-server.c
> +++ b/block/export/vhost-user-blk-server.c
> @@ -125,8 +125,7 @@ static uint64_t vu_blk_get_features(VuDev *dev)
> 1ull << VIRTIO_BLK_F_MQ |
> 1ull << VIRTIO_F_VERSION_1 |
> 1ull << VIRTIO_RING_F_INDIRECT_DESC |
> - 1ull << VIRTIO_RING_F_EVENT_IDX |
> - 1ull << VHOST_USER_F_PROTOCOL_FEATURES;
> + 1ull << VIRTIO_RING_F_EVENT_IDX ;
I didn't see this series yet when I replied to the other series this is
split off from, but of course, my comments are still relevant for this
one.
I asked for a changed commit message (the "experiment" part should
probably go away if we're merging it; instead, it should explain that
in vu_get_features_exec(), libvhost-user adds the vhost-user protocol
level VHOST_USER_F_PROTOCOL_FEATURES flag anyway and the device is the
wrong layer to add it, but the behaviour doesn't change with this patch)
and noted the extra space before the semicolon.
Kevin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/5] block/vhost-user-blk-server: don't expose VHOST_USER_F_PROTOCOL_FEATURES
2022-07-28 8:46 ` Kevin Wolf
@ 2022-07-28 10:27 ` Alex Bennée
0 siblings, 0 replies; 8+ messages in thread
From: Alex Bennée @ 2022-07-28 10:27 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-devel, mst, Coiby Xu, Hanna Reitz, open list:Block layer core
Kevin Wolf <kwolf@redhat.com> writes:
> Am 27.07.2022 um 17:56 hat Alex Bennée geschrieben:
>> This bit is unused in actual VirtIO feature negotiation and should
>> only appear in the vhost-user messages between master and slave.
>>
>> [AJB: experiment, this doesn't break the tests but I'm not super
>> confident of the range of tests]
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20220726192150.2435175-6-alex.bennee@linaro.org>
>> ---
>> block/export/vhost-user-blk-server.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
>> index 3409d9e02e..d31436006d 100644
>> --- a/block/export/vhost-user-blk-server.c
>> +++ b/block/export/vhost-user-blk-server.c
>> @@ -125,8 +125,7 @@ static uint64_t vu_blk_get_features(VuDev *dev)
>> 1ull << VIRTIO_BLK_F_MQ |
>> 1ull << VIRTIO_F_VERSION_1 |
>> 1ull << VIRTIO_RING_F_INDIRECT_DESC |
>> - 1ull << VIRTIO_RING_F_EVENT_IDX |
>> - 1ull << VHOST_USER_F_PROTOCOL_FEATURES;
>> + 1ull << VIRTIO_RING_F_EVENT_IDX ;
>
> I didn't see this series yet when I replied to the other series this is
> split off from, but of course, my comments are still relevant for this
> one.
>
> I asked for a changed commit message (the "experiment" part should
> probably go away if we're merging it; instead, it should explain that
> in vu_get_features_exec(), libvhost-user adds the vhost-user protocol
> level VHOST_USER_F_PROTOCOL_FEATURES flag anyway and the device is the
> wrong layer to add it, but the behaviour doesn't change with this patch)
> and noted the extra space before the semicolon.
OK - mst do you want me to respin or can you tweak the commit?
>
> Kevin
--
Alex Bennée
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-07-28 10:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-27 15:56 [PATCH for 7.1 v1 0/5] virtio fixes (split from virtio-gpio series) Alex Bennée
2022-07-27 15:56 ` [PATCH v1 1/5] block/vhost-user-blk-server: don't expose VHOST_USER_F_PROTOCOL_FEATURES Alex Bennée
2022-07-28 8:46 ` Kevin Wolf
2022-07-28 10:27 ` Alex Bennée
2022-07-27 15:56 ` [PATCH v1 2/5] hw/virtio: incorporate backend features in features Alex Bennée
2022-07-27 15:56 ` [PATCH v1 3/5] hw/virtio: gracefully handle unset vhost_dev vdev Alex Bennée
2022-07-27 15:56 ` [PATCH v1 4/5] hw/virtio: handle un-configured shutdown in virtio-pci Alex Bennée
2022-07-27 15:56 ` [PATCH v1 5/5] hw/virtio: fix vhost_user_read tracepoint Alex Bennée
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.