Re: [RFC v2 05/13] vhost: Route guest->host notification through shadow virtqueue
在 2021/3/16 上午3:48, Eugenio Pérez 写道:
Shadow virtqueue notifications forwarding is disabled when vhost_dev
stops, so code flow follows usual cleanup.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.h | 7 ++
include/hw/virtio/vhost.h | 4 +
hw/virtio/vhost-shadow-virtqueue.c | 113 ++-
hw/virtio/vhost.c | 143 -
4 files changed, 265 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.h
b/hw/virtio/vhost-shadow-virtqueue.h
index 6cc18d6acb..c891c6510d 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -17,6 +17,13 @@
typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
+bool vhost_shadow_vq_start(struct vhost_dev *dev,
+ unsigned idx,
+ VhostShadowVirtqueue *svq);
+void vhost_shadow_vq_stop(struct vhost_dev *dev,
+ unsigned idx,
+ VhostShadowVirtqueue *svq);
+
VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx);
void vhost_shadow_vq_free(VhostShadowVirtqueue *vq);
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index ac963bf23d..7ffdf9aea0 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -55,6 +55,8 @@ struct vhost_iommu {
QLIST_ENTRY(vhost_iommu) iommu_next;
};
+typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
+
typedef struct VhostDevConfigOps {
/* Vhost device config space changed callback
*/
@@ -83,7 +85,9 @@ struct vhost_dev {
uint64_t backend_cap;
bool started;
bool log_enabled;
+bool shadow_vqs_enabled;
uint64_t log_size;
+VhostShadowVirtqueue **shadow_vqs;
Any reason that you don't embed the shadow virtqueue into
vhost_virtqueue structure?
(Note that there's a masked_notifier in struct vhost_virtqueue).
Error *migration_blocker;
const VhostOps *vhost_ops;
void *opaque;
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index 4512e5b058..3e43399e9c 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -8,9 +8,12 @@
*/
#include "hw/virtio/vhost-shadow-virtqueue.h"
+#include "hw/virtio/vhost.h"
+
+#include "standard-headers/linux/vhost_types.h"
#include "qemu/error-report.h"
-#include "qemu/event_notifier.h"
+#include "qemu/main-loop.h"
/* Shadow virtqueue to relay notifications */
typedef struct VhostShadowVirtqueue {
@@ -18,14 +21,121 @@ typedef struct VhostShadowVirtqueue {
EventNotifier kick_notifier;
/* Shadow call notifier, sent to vhost */
EventNotifier call_notifier;
+
+/*
+ * Borrowed virtqueue's guest to host notifier.
+ * To borrow it in this event notifier allows to register on the event
+ * loop and access the associated shadow virtqueue easily. If we use the
+ * VirtQueue, we don't have an easy way to retrieve it.
So this is something that worries me. It looks like a layer violation
that makes the codes harder to work correctly.
I wonder if it would be simpler to start from a vDPA dedicated shadow
virtqueue implementation:
1) have the above fields embeded in vhost_vdpa structure
2) Work at the level of
vhost_vdpa_set_vring_kick()/vhost_vdpa_set_vring_call()
Then the layer is still isolated and you have a much simpler context to
work that you don't need to care a lot of synchornization:
1) vq masking
2) vhost dev start and stop
?
+ *
+ * So shadow virtqueue must not clean it, or we would lose VirtQueue one.
+ */
+EventNotifier host_notifier;
+
+/* Virtio queue shadowing */
+VirtQueue *vq;
} VhostShadowVirtqueue;
+/* Forward guest notifications */
+static void vhost_handle_guest_kick(EventNotifier *n)
+{
+VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
+ host_notifier);
+
+if (unlikely(!event_notifier_test_and_clear(n))) {
+return;
+}
+
+event_notifier_set(&svq->kick_notifier);
+}
+
+/*
+ * Restore the vhost guest to host notifier, i.e., disables svq effect.
+ */
+static int vhost_shadow_vq_restore_vdev_host_notifier(struct vhost_dev *dev,
+ unsigned vhost_index,
+ VhostShadowVirtqueue *svq)
+{
+EventNotifier *vq_host_notifier = virtio_queue_get_host_notifier(svq->vq);
+struct vhost_vring_file file = {
+.index = vhost_index,
+.fd = event_notifier_get_fd(vq_host_notifier),
+};
+int r;
+
+/* Restore vhost kick */
+r = dev->vhost_ops->vhost_set_vring_kick(dev, &file);
+return r ? -errno : 0;
+}
+
+/*
+ * Start shadow virtqueue operation.
+ * @dev vhost device
+ * @hidx vhost virtqueue index
+ * @svq Shadow Virtqueue
+ */
+bool vhost_
Re: [PATCH v5 09/13] hw/block/nvme: parameterize nvme_ns_nlbas
On Mar 16 15:53, Minwoo Im wrote:
> On 21-03-10 10:53:43, Klaus Jensen wrote:
> > From: Klaus Jensen
> >
> > Provide a more flexible nlbas helper.
> >
> > Signed-off-by: Klaus Jensen
> > ---
> > hw/block/nvme-ns.h | 14 ++
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
> > index 07e16880801d..34f9474a1cd1 100644
> > --- a/hw/block/nvme-ns.h
> > +++ b/hw/block/nvme-ns.h
> > @@ -136,12 +136,18 @@ static inline bool nvme_ns_ext(NvmeNamespace *ns)
> > }
> >
> > /* calculate the number of LBAs that the namespace can accomodate */
> > +static inline uint64_t __nvme_nlbas(size_t size, uint8_t lbads, uint16_t
> > ms)
> > +{
> > +if (ms) {
> > +return size / ((1 << lbads) + ms);
> > +}
> > +
> > +return size >> lbads;
> > +}
> > +
> > static inline uint64_t nvme_ns_nlbas(NvmeNamespace *ns)
> > {
> > -if (nvme_msize(ns)) {
> > -return ns->size / (nvme_lsize(ns) + nvme_msize(ns));
> > -}
> > -return ns->size >> nvme_ns_lbads(ns);
> > +return __nvme_nlbas(ns->size, nvme_ns_lbads(ns), nvme_msize(ns));
> > }
>
> Hmm.. I think it looks like __nvme_nlbas does the same with the
> nvme_ns_nlbas, but flexible argument attributes. But I think those
> three attributes are all for ns-specific fields which is not that
> generic so that I don't think we are going to take the helper from much
> more general perspective with __nvme_nlbas.
>
This patch should be moved two patches forward in the series - it is
used in [12/13] to check the zone geometry before the values are set on
the namespace proper. This is also used in Format NVM to verify the
format before formatting ("commiting" the values on the NvmeNamespace
structure).
signature.asc
Description: PGP signature
Re: [PULL v2 0/5] Meson version update
On 15/03/21 23:04, Peter Maydell wrote: ../../meson.build:1:0: ERROR: Value "true" (of type "string") for combo option "Localization of the GTK+ user interface" is not one of the choices. Possible choices are (as string): "enabled", "disabled", "auto". This probably dates back to commit 0e8e77d487b3d8ae33158e61c30e1fe5c753a114 Author: Alex Bennée Date: Thu Dec 10 19:04:12 2020 + configure: move gettext detection to meson.build This will allow meson to honour -Dauto_features=disabled later. Suggested-by: Paolo Bonzini Signed-off-by: Alex Bennée Acked-by: Paolo Bonzini Message-Id: <20201210190417.31673-4-alex.ben...@linaro.org> but it should be possible to work around it. Paolo
Re: [RFC v2 06/13] vhost: Route host->guest notification through shadow virtqueue
在 2021/3/16 上午3:48, Eugenio Pérez 写道:
On one hand it uses a mutex to synchronize guest masking with SVQ start
and stop, because otherwise guest mask could race with the SVQ
stop code, sending an incorrect call notifier to vhost device. This
would prevent further communication.
Is this becuase of the OOB monitor? If yes, can we simply drop the QMP
command and introduce cli paramter for vhost backend?
Thanks
On the other hand it needs to add an event to synchronize guest
unmasking with call handling. Not doing that way could cause the guest
to receive notifications after its unmask call. This could be done
through the mutex but the event solution is cheaper for the buffer
forwarding.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.h | 3 +
include/hw/virtio/vhost.h | 1 +
hw/virtio/vhost-shadow-virtqueue.c | 127 +
hw/virtio/vhost.c | 29 ++-
4 files changed, 157 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.h
b/hw/virtio/vhost-shadow-virtqueue.h
index c891c6510d..2ca4b92b12 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -17,6 +17,9 @@
typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
+void vhost_shadow_vq_mask(VhostShadowVirtqueue *svq, EventNotifier *masked);
+void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq);
+
bool vhost_shadow_vq_start(struct vhost_dev *dev,
unsigned idx,
VhostShadowVirtqueue *svq);
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 7ffdf9aea0..2f556bd3d5 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -29,6 +29,7 @@ struct vhost_virtqueue {
unsigned long long used_phys;
unsigned used_size;
bool notifier_is_masked;
+QemuRecMutex masked_mutex;
EventNotifier masked_notifier;
struct vhost_dev *dev;
};
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index 3e43399e9c..8f6ffa729a 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -32,8 +32,22 @@ typedef struct VhostShadowVirtqueue {
*/
EventNotifier host_notifier;
+/* (Possible) masked notifier */
+struct {
+EventNotifier *n;
+
+/*
+ * Event to confirm unmasking.
+ * set when the masked notifier has no uses
+ */
+QemuEvent is_free;
+} masked_notifier;
+
/* Virtio queue shadowing */
VirtQueue *vq;
+
+/* Virtio device */
+VirtIODevice *vdev;
} VhostShadowVirtqueue;
/* Forward guest notifications */
@@ -49,6 +63,70 @@ static void vhost_handle_guest_kick(EventNotifier *n)
event_notifier_set(&svq->kick_notifier);
}
+/* Forward vhost notifications */
+static void vhost_shadow_vq_handle_call_no_test(EventNotifier *n)
+{
+VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
+ call_notifier);
+EventNotifier *masked_notifier;
+
+/* Signal start of using masked notifier */
+qemu_event_reset(&svq->masked_notifier.is_free);
+masked_notifier = qatomic_load_acquire(&svq->masked_notifier.n);
+if (!masked_notifier) {
+qemu_event_set(&svq->masked_notifier.is_free);
+}
+
+if (!masked_notifier) {
+unsigned n = virtio_get_queue_index(svq->vq);
+virtio_queue_invalidate_signalled_used(svq->vdev, n);
+virtio_notify_irqfd(svq->vdev, svq->vq);
+} else {
+event_notifier_set(svq->masked_notifier.n);
+}
+
+if (masked_notifier) {
+/* Signal not using it anymore */
+qemu_event_set(&svq->masked_notifier.is_free);
+}
+}
+
+static void vhost_shadow_vq_handle_call(EventNotifier *n)
+{
+
+if (likely(event_notifier_test_and_clear(n))) {
+vhost_shadow_vq_handle_call_no_test(n);
+}
+}
+
+/*
+ * Mask the shadow virtqueue.
+ *
+ * It can be called from a guest masking vmexit or shadow virtqueue start
+ * through QMP.
+ *
+ * @vq Shadow virtqueue
+ * @masked Masked notifier to signal instead of guest
+ */
+void vhost_shadow_vq_mask(VhostShadowVirtqueue *svq, EventNotifier *masked)
+{
+qatomic_store_release(&svq->masked_notifier.n, masked);
+}
+
+/*
+ * Unmask the shadow virtqueue.
+ *
+ * It can be called from a guest unmasking vmexit or shadow virtqueue start
+ * through QMP.
+ *
+ * @vq Shadow virtqueue
+ */
+void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq)
+{
+qatomic_store_release(&svq->masked_notifier.n, NULL);
+qemu_event_wait(&svq->masked_notifier.is_free);
+}
+
/*
* Restore the vhost guest to host notifier, i.e., disables svq effect.
*/
@@ -103,8 +181,39 @@ bool vhost_shadow_vq_start(struct vhost_dev *dev,
goto err_set_vring_kick;
}
+/* Set vhost call */
+file.fd = event_notifier_get_fd(&svq->call_notifier),
+r =
Re: [RFC v2 10/13] vhost: add vhost_kernel_set_vring_enable
在 2021/3/16 上午3:48, Eugenio Pérez 写道:
This method is already present in vhost-user. This commit adapts it to
vhost-net, so SVQ can use.
vhost_kernel_set_enable stops the device, so qemu can ask for its status
(next available idx the device was going to consume). When SVQ starts it
can resume consuming the guest's driver ring, without notice from the
latter. Not stopping the device before of the swapping could imply that
it process more buffers than reported, what would duplicate the device
action.
Note that it might not be the case of vDPA (virtio) or at least virtio
needs some extension to achieve something similar like this. One example
is virtio-pci which forbids 0 to be wrote to queue_enable.
This is another reason to start from vhost-vDPA.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-backend.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 31b33bde37..1ac5c574a9 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -201,6 +201,34 @@ static int vhost_kernel_get_vq_index(struct vhost_dev
*dev, int idx)
return idx - dev->vq_index;
}
+static int vhost_kernel_set_vq_enable(struct vhost_dev *dev, unsigned idx,
+ bool enable)
+{
+struct vhost_vring_file file = {
+.index = idx,
+};
+
+if (!enable) {
+file.fd = -1; /* Pass -1 to unbind from file. */
+} else {
+struct vhost_net *vn_dev = container_of(dev, struct vhost_net, dev);
+file.fd = vn_dev->backend;
This can only work with vhost-net devices but not vsock/scsi etc.
Thanks
+}
+
+return vhost_kernel_net_set_backend(dev, &file);
+}
+
+static int vhost_kernel_set_vring_enable(struct vhost_dev *dev, int enable)
+{
+int i;
+
+for (i = 0; i < dev->nvqs; ++i) {
+vhost_kernel_set_vq_enable(dev, i, enable);
+}
+
+return 0;
+}
+
#ifdef CONFIG_VHOST_VSOCK
static int vhost_kernel_vsock_set_guest_cid(struct vhost_dev *dev,
uint64_t guest_cid)
@@ -317,6 +345,7 @@ static const VhostOps kernel_ops = {
.vhost_set_owner = vhost_kernel_set_owner,
.vhost_reset_device = vhost_kernel_reset_device,
.vhost_get_vq_index = vhost_kernel_get_vq_index,
+.vhost_set_vring_enable = vhost_kernel_set_vring_enable,
#ifdef CONFIG_VHOST_VSOCK
.vhost_vsock_set_guest_cid = vhost_kernel_vsock_set_guest_cid,
.vhost_vsock_set_running = vhost_kernel_vsock_set_running,
Re: [PATCH v5 09/13] hw/block/nvme: parameterize nvme_ns_nlbas
On 21-03-16 08:19:08, Klaus Jensen wrote:
> On Mar 16 15:53, Minwoo Im wrote:
> > On 21-03-10 10:53:43, Klaus Jensen wrote:
> > > From: Klaus Jensen
> > >
> > > Provide a more flexible nlbas helper.
> > >
> > > Signed-off-by: Klaus Jensen
> > > ---
> > > hw/block/nvme-ns.h | 14 ++
> > > 1 file changed, 10 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
> > > index 07e16880801d..34f9474a1cd1 100644
> > > --- a/hw/block/nvme-ns.h
> > > +++ b/hw/block/nvme-ns.h
> > > @@ -136,12 +136,18 @@ static inline bool nvme_ns_ext(NvmeNamespace *ns)
> > > }
> > >
> > > /* calculate the number of LBAs that the namespace can accomodate */
> > > +static inline uint64_t __nvme_nlbas(size_t size, uint8_t lbads, uint16_t
> > > ms)
> > > +{
> > > +if (ms) {
> > > +return size / ((1 << lbads) + ms);
> > > +}
> > > +
> > > +return size >> lbads;
> > > +}
> > > +
> > > static inline uint64_t nvme_ns_nlbas(NvmeNamespace *ns)
> > > {
> > > -if (nvme_msize(ns)) {
> > > -return ns->size / (nvme_lsize(ns) + nvme_msize(ns));
> > > -}
> > > -return ns->size >> nvme_ns_lbads(ns);
> > > +return __nvme_nlbas(ns->size, nvme_ns_lbads(ns), nvme_msize(ns));
> > > }
> >
> > Hmm.. I think it looks like __nvme_nlbas does the same with the
> > nvme_ns_nlbas, but flexible argument attributes. But I think those
> > three attributes are all for ns-specific fields which is not that
> > generic so that I don't think we are going to take the helper from much
> > more general perspective with __nvme_nlbas.
> >
>
> This patch should be moved two patches forward in the series - it is
> used in [12/13] to check the zone geometry before the values are set on
> the namespace proper. This is also used in Format NVM to verify the
> format before formatting ("commiting" the values on the NvmeNamespace
> structure).
Checked [12/13] right before. Thanks for pointing that out!
Reviewed-by: Minwoo Im
Re: [RFC v2 08/13] virtio: Add vhost_shadow_vq_get_vring_addr
在 2021/3/16 上午3:48, Eugenio Pérez 写道:
It reports the shadow virtqueue address from qemu virtual address space
Note that to be used by vDPA, we can't use qemu VA directly here.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.h | 2 ++
hw/virtio/vhost-shadow-virtqueue.c | 24 +++-
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.h
b/hw/virtio/vhost-shadow-virtqueue.h
index 2ca4b92b12..d82c35bccf 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -19,6 +19,8 @@ typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
void vhost_shadow_vq_mask(VhostShadowVirtqueue *svq, EventNotifier *masked);
void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq);
+void vhost_shadow_vq_get_vring_addr(const VhostShadowVirtqueue *svq,
+struct vhost_vring_addr *addr);
bool vhost_shadow_vq_start(struct vhost_dev *dev,
unsigned idx,
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index b6bab438d6..1460d1d5d1 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -17,6 +17,9 @@
/* Shadow virtqueue to relay notifications */
typedef struct VhostShadowVirtqueue {
+/* Shadow vring */
+struct vring vring;
+
/* Shadow kick notifier, sent to vhost */
EventNotifier kick_notifier;
/* Shadow call notifier, sent to vhost */
@@ -51,6 +54,9 @@ typedef struct VhostShadowVirtqueue {
/* Virtio device */
VirtIODevice *vdev;
+
+/* Descriptors copied from guest */
+vring_desc_t descs[];
} VhostShadowVirtqueue;
/* Forward guest notifications */
@@ -132,6 +138,19 @@ void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq)
qemu_event_wait(&svq->masked_notifier.is_free);
}
+/*
+ * Get the shadow vq vring address.
+ * @svq Shadow virtqueue
+ * @addr Destination to store address
+ */
+void vhost_shadow_vq_get_vring_addr(const VhostShadowVirtqueue *svq,
+struct vhost_vring_addr *addr)
+{
+addr->desc_user_addr = (uint64_t)svq->vring.desc;
+addr->avail_user_addr = (uint64_t)svq->vring.avail;
+addr->used_user_addr = (uint64_t)svq->vring.used;
+}
+
/*
* Restore the vhost guest to host notifier, i.e., disables svq effect.
*/
@@ -262,7 +281,9 @@ void vhost_shadow_vq_stop(struct vhost_dev *dev,
VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx)
{
int vq_idx = dev->vq_index + idx;
-g_autofree VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);
+unsigned num = virtio_queue_get_num(dev->vdev, vq_idx);
+size_t ring_size = vring_size(num, VRING_DESC_ALIGN_SIZE);
+g_autofree VhostShadowVirtqueue *svq = g_malloc0(sizeof(*svq) + ring_size);
int r;
r = event_notifier_init(&svq->kick_notifier, 0);
@@ -279,6 +300,7 @@ VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev
*dev, int idx)
goto err_init_call_notifier;
}
+vring_init(&svq->vring, num, svq->descs, VRING_DESC_ALIGN_SIZE);
We had some dicussion in the past. Exporting vring_init() is wrong but
too late to fix (assumes a legacy split layout). Let's not depend on
this buggy uAPI.
Thanks
svq->vq = virtio_get_queue(dev->vdev, vq_idx);
svq->vdev = dev->vdev;
event_notifier_set_handler(&svq->call_notifier,
[PATCH v8 0/3] vnc: support reload x509 certificates
This series supports reload x509 certificates for vnc
1. Support reload x509 certificates.
2. Support reload vnc certificates.
3. Add new qmp display-reload and implement reload x509 certificates for vnc.
Example:
{"execute": "display-reload", "arguments":{"type": "vnc", "tls-certs": true}}
Zihao Chang (3):
crypto: add reload for QCryptoTLSCredsClass
vnc: support reload x509 certificates for vnc
qmp: add new qmp display-reload
crypto/tlscredsx509.c | 48 ++
include/crypto/tlscreds.h | 8 +++--
include/ui/console.h | 1 +
monitor/qmp-cmds.c| 17 +++
qapi/ui.json | 61 +++
ui/vnc.c | 28 ++
6 files changed, 160 insertions(+), 3 deletions(-)
--
2.28.0
[PATCH v8 1/3] crypto: add reload for QCryptoTLSCredsClass
This patch adds reload interface for QCryptoTLSCredsClass and implements
the interface for QCryptoTLSCredsX509.
Signed-off-by: Zihao Chang
Acked-by: Daniel P. Berrang??
---
crypto/tlscredsx509.c | 48 +++
include/crypto/tlscreds.h | 8 ---
2 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index dbadad4df28e..bc503bab5585 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -770,6 +770,51 @@ qcrypto_tls_creds_x509_prop_get_sanity(Object *obj,
}
+#ifdef CONFIG_GNUTLS
+
+
+static bool
+qcrypto_tls_creds_x509_reload(QCryptoTLSCreds *creds, Error **errp)
+{
+QCryptoTLSCredsX509 *x509_creds = QCRYPTO_TLS_CREDS_X509(creds);
+Error *local_err = NULL;
+gnutls_certificate_credentials_t creds_data = x509_creds->data;
+gnutls_dh_params_t creds_dh_params = x509_creds->parent_obj.dh_params;
+
+x509_creds->data = NULL;
+x509_creds->parent_obj.dh_params = NULL;
+qcrypto_tls_creds_x509_load(x509_creds, &local_err);
+if (local_err) {
+qcrypto_tls_creds_x509_unload(x509_creds);
+x509_creds->data = creds_data;
+x509_creds->parent_obj.dh_params = creds_dh_params;
+error_propagate(errp, local_err);
+return false;
+}
+
+if (creds_data) {
+gnutls_certificate_free_credentials(creds_data);
+}
+if (creds_dh_params) {
+gnutls_dh_params_deinit(creds_dh_params);
+}
+return true;
+}
+
+
+#else /* ! CONFIG_GNUTLS */
+
+
+static bool
+qcrypto_tls_creds_x509_reload(QCryptoTLSCreds *creds, Error **errp)
+{
+return false;
+}
+
+
+#endif /* ! CONFIG_GNUTLS */
+
+
static void
qcrypto_tls_creds_x509_complete(UserCreatable *uc, Error **errp)
{
@@ -800,6 +845,9 @@ static void
qcrypto_tls_creds_x509_class_init(ObjectClass *oc, void *data)
{
UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+QCryptoTLSCredsClass *ctcc = QCRYPTO_TLS_CREDS_CLASS(oc);
+
+ctcc->reload = qcrypto_tls_creds_x509_reload;
ucc->complete = qcrypto_tls_creds_x509_complete;
diff --git a/include/crypto/tlscreds.h b/include/crypto/tlscreds.h
index 079e37604784..d0808e391e91 100644
--- a/include/crypto/tlscreds.h
+++ b/include/crypto/tlscreds.h
@@ -30,14 +30,15 @@
#define TYPE_QCRYPTO_TLS_CREDS "tls-creds"
typedef struct QCryptoTLSCreds QCryptoTLSCreds;
-DECLARE_INSTANCE_CHECKER(QCryptoTLSCreds, QCRYPTO_TLS_CREDS,
- TYPE_QCRYPTO_TLS_CREDS)
-
typedef struct QCryptoTLSCredsClass QCryptoTLSCredsClass;
+DECLARE_OBJ_CHECKERS(QCryptoTLSCreds, QCryptoTLSCredsClass, QCRYPTO_TLS_CREDS,
+ TYPE_QCRYPTO_TLS_CREDS)
+
#define QCRYPTO_TLS_CREDS_DH_PARAMS "dh-params.pem"
+typedef bool (*CryptoTLSCredsReload)(QCryptoTLSCreds *, Error **);
/**
* QCryptoTLSCreds:
*
@@ -61,6 +62,7 @@ struct QCryptoTLSCreds {
struct QCryptoTLSCredsClass {
ObjectClass parent_class;
+CryptoTLSCredsReload reload;
};
--
2.28.0
[PATCH v8 3/3] qmp: add new qmp display-reload
This patch provides a new qmp to reload display configuration
without restart VM, but only reloading the vnc tls certificates
is implemented.
Example:
{"execute": "display-reload", "arguments":{"type": "vnc", "tls-certs": true}}
Signed-off-by: Zihao Chang
---
monitor/qmp-cmds.c | 17 +
qapi/ui.json | 61 ++
2 files changed, 78 insertions(+)
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index c7df8c0ee268..f7d64a64577a 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -334,3 +334,20 @@ MemoryInfo *qmp_query_memory_size_summary(Error **errp)
return mem_info;
}
+
+void qmp_display_reload(DisplayReloadOptions *arg, Error **errp)
+{
+switch (arg->type) {
+case DISPLAY_RELOAD_TYPE_VNC:
+#ifdef CONFIG_VNC
+if (arg->u.vnc.has_tls_certs && arg->u.vnc.tls_certs) {
+vnc_display_reload_certs(NULL, errp);
+}
+#else
+error_setg(errp, "vnc is invalid, missing 'CONFIG_VNC'");
+#endif
+break;
+default:
+abort();
+}
+}
diff --git a/qapi/ui.json b/qapi/ui.json
index d08d72b43923..e39159eae022 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1179,3 +1179,64 @@
##
{ 'command': 'query-display-options',
'returns': 'DisplayOptions' }
+
+##
+# @DisplayReloadType:
+#
+# Available DisplayReload types.
+#
+# @vnc: VNC display
+#
+# Since: 6.0
+#
+##
+{ 'enum': 'DisplayReloadType',
+ 'data': ['vnc'] }
+
+##
+# @DisplayReloadOptionsVNC:
+#
+# Specify the VNC reload options.
+#
+# @tls-certs: reload tls certs or not.
+#
+# Since: 6.0
+#
+##
+{ 'struct': 'DisplayReloadOptionsVNC',
+ 'data': { '*tls-certs': 'bool' } }
+
+##
+# @DisplayReloadOptions:
+#
+# Options of the display configuration reload.
+#
+# @type: Specify the display type.
+#
+# Since: 6.0
+#
+##
+{ 'union': 'DisplayReloadOptions',
+ 'base': {'type': 'DisplayReloadType'},
+ 'discriminator': 'type',
+ 'data': { 'vnc': 'DisplayReloadOptionsVNC' } }
+
+##
+# @display-reload:
+#
+# Reload display configuration.
+#
+# Returns: Nothing on success.
+#
+# Since: 6.0
+#
+# Example:
+#
+# -> { "execute": "display-reload",
+# "arguments": { "type": "vnc", "tls-certs": true } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'display-reload',
+ 'data': 'DisplayReloadOptions',
+ 'boxed' : true }
--
2.28.0
[PATCH v8 2/3] vnc: support reload x509 certificates for vnc
This patch add vnc_display_reload_certs() to support
update x509 certificates.
Signed-off-by: Zihao Chang
Reviewed-by: Daniel P. Berrang??
---
include/ui/console.h | 1 +
ui/vnc.c | 28
2 files changed, 29 insertions(+)
diff --git a/include/ui/console.h b/include/ui/console.h
index c960b7066ccd..2714038a0fae 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -476,6 +476,7 @@ int vnc_display_password(const char *id, const char
*password);
int vnc_display_pw_expire(const char *id, time_t expires);
void vnc_parse(const char *str);
int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp);
+bool vnc_display_reload_certs(const char *id, Error **errp);
/* input.c */
int index_from_key(const char *key, size_t key_length);
diff --git a/ui/vnc.c b/ui/vnc.c
index 310abc937812..381e21a87563 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -584,6 +584,34 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp)
return prev;
}
+bool vnc_display_reload_certs(const char *id, Error **errp)
+{
+VncDisplay *vd = vnc_display_find(id);
+QCryptoTLSCredsClass *creds = NULL;
+
+if (!vd) {
+error_setg(errp, "Can not find vnc display");
+return false;
+}
+
+if (!vd->tlscreds) {
+error_setg(errp, "vnc tls is not enable");
+return false;
+}
+
+creds = QCRYPTO_TLS_CREDS_GET_CLASS(OBJECT(vd->tlscreds));
+if (creds->reload == NULL) {
+error_setg(errp, "%s doesn't support to reload TLS credential",
+ object_get_typename(OBJECT(vd->tlscreds)));
+return false;
+}
+if (!creds->reload(vd->tlscreds, errp)) {
+return false;
+}
+
+return true;
+}
+
/* TODO
1) Get the queue working for IO.
2) there is some weirdness when using the -S option (the screen is grey
--
2.28.0
Re: [RFC v2 12/13] vhost: Check for device VRING_USED_F_NO_NOTIFY at shadow virtqueue kick
在 2021/3/16 上午3:48, Eugenio Pérez 写道:
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index 68ed0f2740..7df98fc43f 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -145,6 +145,15 @@ static void vhost_shadow_vq_add(VhostShadowVirtqueue *svq,
svq->ring_id_maps[qemu_head] = elem;
}
+static void vhost_shadow_vq_kick(VhostShadowVirtqueue *svq)
+{
+/* Make sure we are reading updated device flag */
+smp_rmb();
smp_mb() actually? Or it's better to explain this following read needs
to be orderd with what read before.
Thanks
+if (!(svq->vring.used->flags & VRING_USED_F_NO_NOTIFY)) {
+event_notifier_set(&svq->kick_notifier);
+}
+}
+
/* Handle guest->device notifications */
static void vhost_handle_guest_kick(EventNotifier *n)
{
@@ -174,7 +183,7 @@ static void vhost_handle_guest_kick(EventNotifier *n)
}
vhost_shadow_vq_add(svq, elem);
-event_notifier_set(&svq->kick_notifier);
+vhost_shadow_vq_kick(svq);
}
virtio_queue_set_notification(svq->vq, true);
Re: [RFC v2 13/13] vhost: Use VRING_AVAIL_F_NO_INTERRUPT at device call on shadow virtqueue
在 2021/3/16 上午3:48, Eugenio Pérez 写道:
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index 7df98fc43f..e3879a4622 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -71,10 +71,35 @@ typedef struct VhostShadowVirtqueue {
/* Next head to consume from device */
uint16_t used_idx;
+/* Cache for the exposed notification flag */
+bool notification;
+
/* Descriptors copied from guest */
vring_desc_t descs[];
} VhostShadowVirtqueue;
+static void vhost_shadow_vq_set_notification(VhostShadowVirtqueue *svq,
+ bool enable)
+{
+uint16_t notification_flag;
+
+if (svq->notification == enable) {
+return;
+}
+
+notification_flag = virtio_tswap16(svq->vdev, VRING_AVAIL_F_NO_INTERRUPT);
+
+svq->notification = enable;
+if (enable) {
+svq->vring.avail->flags &= ~notification_flag;
+} else {
+svq->vring.avail->flags |= notification_flag;
+}
+
+/* Make sure device reads our flag */
+smp_mb();
This is a hint, so we don't need memory barrier here.
Thanks
+}
+
static void vhost_vring_write_descs(VhostShadowVirtqueue *svq,
const struct iovec *iovec,
size_t num, bool more_descs, bool write)
@@ -251,7 +276,7 @@ static void
vhost_shadow_vq_handle_call_no_test(EventNotifier *n)
do {
unsigned i = 0;
-/* TODO: Use VRING_AVAIL_F_NO_INTERRUPT */
+vhost_shadow_vq_set_notification(svq, false);
while (true) {
g_autofree VirtQueueElement *elem = vhost_shadow_vq_get_buf(svq);
if (!elem) {
@@ -269,6 +294,7 @@ static void
vhost_shadow_vq_handle_call_no_test(EventNotifier *n)
svq->masked_notifier.signaled = true;
event_notifier_set(svq->masked_notifier.n);
}
+vhost_shadow_vq_set_notification(svq, true);
} while (vhost_shadow_vq_more_used(svq));
if (masked_notifier) {
Re: [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags
Hello,
I gave this series a try on some PPC machines : mac99, g3beige,
sam460ex, pseries, powernv, with linux, macos, darwin, aix and
didn't see any regression. Migration seems to work for pseries.
C.
On 3/15/21 7:45 PM, Richard Henderson wrote:
> Changes for v4:
> * Use hregs_recompute_hflags for hw/ppc/ reset.
>-- Incorporate Cedric's feedback.
>
> Changes for v3:
> * Fixes for linux-user, signal handling and startup.
>-- Oops, the directory in which I did testing for v2
> had a reduced set of targets.
>
> Changes for v2:
> * Do not put tcg internal state into migration, except to
>retain backward compatibility.
> * Do not touch anything in env in ppc_tr_init_disas_context.
> * Do make sure that hflags contains everything that it should.
> * Do verify that hflags is properly updated.
>
>
> r~
>
>
> Richard Henderson (17):
> target/ppc: Move helper_regs.h functions out-of-line
> target/ppc: Move 601 hflags adjustment to hreg_compute_hflags
> target/ppc: Properly sync cpu state with new msr in cpu_load_old
> target/ppc: Do not call hreg_compute_mem_idx after ppc_store_msr
> target/ppc: Retain hflags_nmsr only for migration
> target/ppc: Fix comment for MSR_FE{0,1}
> target/ppc: Disconnect hflags from MSR
> target/ppc: Reduce env->hflags to uint32_t
> target/ppc: Put dbcr0 single-step bits into hflags
> target/ppc: Create helper_scv
> target/ppc: Put LPCR[GTSE] in hflags
> target/ppc: Remove MSR_SA and MSR_AP from hflags
> target/ppc: Remove env->immu_idx and env->dmmu_idx
> hw/ppc/pnv_core: Update hflags after setting msr
> hw/ppc/spapr_rtas: Update hflags after setting msr
> linux-user/ppc: Fix msr updates for signal handling
> target/ppc: Validate hflags with CONFIG_DEBUG_TCG
>
> target/ppc/cpu.h| 50 +-
> target/ppc/helper.h | 1 +
> target/ppc/helper_regs.h| 183 +
> hw/ppc/pnv_core.c | 3 +-
> hw/ppc/spapr_rtas.c | 2 +
> linux-user/ppc/cpu_loop.c | 5 +-
> linux-user/ppc/signal.c | 23 ++-
> target/ppc/excp_helper.c| 9 ++
> target/ppc/helper_regs.c| 272
> target/ppc/int_helper.c | 1 +
> target/ppc/machine.c| 27 ++--
> target/ppc/mem_helper.c | 2 +-
> target/ppc/misc_helper.c| 13 +-
> target/ppc/mmu-hash64.c | 3 +
> target/ppc/translate.c | 98
> target/ppc/translate_init.c.inc | 4 +-
> target/ppc/meson.build | 1 +
> 17 files changed, 410 insertions(+), 287 deletions(-)
> create mode 100644 target/ppc/helper_regs.c
>
Re: [PATCH v4 01/17] target/ppc: Move helper_regs.h functions out-of-line
On 3/15/21 7:45 PM, Richard Henderson wrote:
> Move the functions to a new file, helper_regs.c.
>
> Note int_helper.c was relying on helper_regs.h to
> indirectly include qemu/log.h.
>
> Signed-off-by: Richard Henderson
Reviewed-by: Cédric Le Goater
Thanks,
C.
> ---
> target/ppc/helper_regs.h | 184 ++--
> target/ppc/helper_regs.c | 197 +++
> target/ppc/int_helper.c | 1 +
> target/ppc/meson.build | 1 +
> 4 files changed, 207 insertions(+), 176 deletions(-)
> create mode 100644 target/ppc/helper_regs.c
>
> diff --git a/target/ppc/helper_regs.h b/target/ppc/helper_regs.h
> index efcc903427..4148a442b3 100644
> --- a/target/ppc/helper_regs.h
> +++ b/target/ppc/helper_regs.h
> @@ -20,184 +20,16 @@
> #ifndef HELPER_REGS_H
> #define HELPER_REGS_H
>
> -#include "qemu/main-loop.h"
> -#include "exec/exec-all.h"
> -#include "sysemu/kvm.h"
> +void hreg_swap_gpr_tgpr(CPUPPCState *env);
> +void hreg_compute_mem_idx(CPUPPCState *env);
> +void hreg_compute_hflags(CPUPPCState *env);
> +void cpu_interrupt_exittb(CPUState *cs);
> +int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv);
>
> -/* Swap temporary saved registers with GPRs */
> -static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
> -{
> -target_ulong tmp;
> -
> -tmp = env->gpr[0];
> -env->gpr[0] = env->tgpr[0];
> -env->tgpr[0] = tmp;
> -tmp = env->gpr[1];
> -env->gpr[1] = env->tgpr[1];
> -env->tgpr[1] = tmp;
> -tmp = env->gpr[2];
> -env->gpr[2] = env->tgpr[2];
> -env->tgpr[2] = tmp;
> -tmp = env->gpr[3];
> -env->gpr[3] = env->tgpr[3];
> -env->tgpr[3] = tmp;
> -}
> -
> -static inline void hreg_compute_mem_idx(CPUPPCState *env)
> -{
> -/*
> - * This is our encoding for server processors. The architecture
> - * specifies that there is no such thing as userspace with
> - * translation off, however it appears that MacOS does it and some
> - * 32-bit CPUs support it. Weird...
> - *
> - * 0 = Guest User space virtual mode
> - * 1 = Guest Kernel space virtual mode
> - * 2 = Guest User space real mode
> - * 3 = Guest Kernel space real mode
> - * 4 = HV User space virtual mode
> - * 5 = HV Kernel space virtual mode
> - * 6 = HV User space real mode
> - * 7 = HV Kernel space real mode
> - *
> - * For BookE, we need 8 MMU modes as follow:
> - *
> - * 0 = AS 0 HV User space
> - * 1 = AS 0 HV Kernel space
> - * 2 = AS 1 HV User space
> - * 3 = AS 1 HV Kernel space
> - * 4 = AS 0 Guest User space
> - * 5 = AS 0 Guest Kernel space
> - * 6 = AS 1 Guest User space
> - * 7 = AS 1 Guest Kernel space
> - */
> -if (env->mmu_model & POWERPC_MMU_BOOKE) {
> -env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> -env->immu_idx += msr_is ? 2 : 0;
> -env->dmmu_idx += msr_ds ? 2 : 0;
> -env->immu_idx += msr_gs ? 4 : 0;
> -env->dmmu_idx += msr_gs ? 4 : 0;
> -} else {
> -env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> -env->immu_idx += msr_ir ? 0 : 2;
> -env->dmmu_idx += msr_dr ? 0 : 2;
> -env->immu_idx += msr_hv ? 4 : 0;
> -env->dmmu_idx += msr_hv ? 4 : 0;
> -}
> -}
> -
> -static inline void hreg_compute_hflags(CPUPPCState *env)
> -{
> -target_ulong hflags_mask;
> -
> -/* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */
> -hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |
> -(1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |
> -(1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);
> -hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
> -hreg_compute_mem_idx(env);
> -env->hflags = env->msr & hflags_mask;
> -/* Merge with hflags coming from other registers */
> -env->hflags |= env->hflags_nmsr;
> -}
> -
> -static inline void cpu_interrupt_exittb(CPUState *cs)
> -{
> -if (!kvm_enabled()) {
> -return;
> -}
> -
> -if (!qemu_mutex_iothread_locked()) {
> -qemu_mutex_lock_iothread();
> -cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> -qemu_mutex_unlock_iothread();
> -} else {
> -cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> -}
> -}
> -
> -static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
> - int alter_hv)
> -{
> -int excp;
> -#if !defined(CONFIG_USER_ONLY)
> -CPUState *cs = env_cpu(env);
> -#endif
> -
> -excp = 0;
> -value &= env->msr_mask;
> -#if !defined(CONFIG_USER_ONLY)
> -/* Neither mtmsr nor guest state can alter HV */
> -if (!alter_hv || !(env->msr & MSR_HVB)) {
> -value &= ~MSR_HVB;
> -value |= env->msr & MSR_HVB;
> -}
> -if (((value >> MSR_IR) & 1) != msr_ir ||
> -((value >> MSR_DR) & 1) != msr_dr) {
> -cpu_interrupt_exittb(cs);
> -
[PATCH v3 00/13] net: Pad short frames for network backends
The minimum Ethernet frame length is 60 bytes. For short frames with smaller length like ARP packets (only 42 bytes), on a real world NIC it can choose either padding its length to the minimum required 60 bytes, or sending it out directly to the wire. Such behavior can be hardcoded or controled by a register bit. Similarly on the receive path, NICs can choose either dropping such short frames directly or handing them over to software to handle. On the other hand, for the network backends like SLiRP/TAP, they don't expose a way to control the short frame behavior. As of today they just send/receive data from/to the other end connected to them, which means any sized packet is acceptable. So they can send and receive short frames without any problem. It is observed that ARP packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send these ARP packets to the other end which might be a NIC model that does not allow short frames to pass through. To provide better compatibility, for packets sent from QEMU network backends like SLiRP/TAP, we change to pad short frames before sending it out to the other end, if the other end does not forbid it via the nc->do_not_pad flag. This ensures a backend as an Ethernet sender does not violate the spec. But with this change, the behavior of dropping short frames from SLiRP/TAP interfaces in the NIC model cannot be emulated because it always receives a packet that is spec complaint. The capability of sending short frames from NIC models is still supported and short frames can still pass through SLiRP/TAP. This series should be able to fix the issue as reported with some NIC models before, that ARP requests get dropped, preventing the guest from becoming visible on the network. It was workarounded in these NIC models on the receive path, that when a short frame is received, it is padded up to 60 bytes. Changes in v3: - use 'without' instead of 'sans' - add a helper to pad short frames - add a comment to 'do_not_pad' - use the pad_short_frame() helper for slirp - use the pad_short_frame() helper for tap Bin Meng (13): net: eth: Add a helper to pad a short ethernet frame net: Add a 'do_not_pad" to NetClientState net: slirp: Pad short frames to minimum size before send net: tap: Pad short frames to minimum size before send hw/net: virtio-net: Initialize nc->do_not_pad to true hw/net: e1000: Remove the logic of padding short frames in the receive path hw/net: vmxnet3: Remove the logic of padding short frames in the receive path hw/net: i82596: Remove the logic of padding short frames in the receive path hw/net: ne2000: Remove the logic of padding short frames in the receive path hw/net: pcnet: Remove the logic of padding short frames in the receive path hw/net: rtl8139: Remove the logic of padding short frames in the receive path hw/net: sungem: Remove the logic of padding short frames in the receive path hw/net: sunhme: Remove the logic of padding short frames in the receive path hw/net/e1000.c | 11 +-- hw/net/i82596.c | 18 -- hw/net/ne2000.c | 12 hw/net/pcnet.c | 9 - hw/net/rtl8139.c| 12 hw/net/sungem.c | 14 -- hw/net/sunhme.c | 11 --- hw/net/virtio-net.c | 4 hw/net/vmxnet3.c| 10 -- include/net/eth.h | 25 + include/net/net.h | 1 + net/slirp.c | 9 + net/tap-win32.c | 9 + net/tap.c | 9 + 14 files changed, 58 insertions(+), 96 deletions(-) -- 2.17.1
[PATCH v3 02/13] net: Add a 'do_not_pad" to NetClientState
This adds a flag in NetClientState, so that a net client can tell
its peer that the packets do not need to be padded to the minimum
size of an Ethernet frame (60 bytes) before sending to it.
Signed-off-by: Bin Meng
Reviewed-by: Philippe Mathieu-Daudé
---
Changes in v3:
- add a comment to 'do_not_pad'
include/net/net.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/net/net.h b/include/net/net.h
index 919facaad2..f944731c18 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -100,6 +100,7 @@ struct NetClientState {
int vring_enable;
int vnet_hdr_len;
bool is_netdev;
+bool do_not_pad; /* do not pad to the minimum ethernet frame length */
QTAILQ_HEAD(, NetFilterState) filters;
};
--
2.17.1
[PATCH v3 03/13] net: slirp: Pad short frames to minimum size before send
The minimum Ethernet frame length is 60 bytes. For short frames with
smaller length like ARP packets (only 42 bytes), on a real world NIC
it can choose either padding its length to the minimum required 60
bytes, or sending it out directly to the wire. Such behavior can be
hardcoded or controled by a register bit. Similarly on the receive
path, NICs can choose either dropping such short frames directly or
handing them over to software to handle.
On the other hand, for the network backends like SLiRP/TAP, they
don't expose a way to control the short frame behavior. As of today
they just send/receive data from/to the other end connected to them,
which means any sized packet is acceptable. So they can send and
receive short frames without any problem. It is observed that ARP
packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
these ARP packets to the other end which might be a NIC model that
does not allow short frames to pass through.
To provide better compatibility, for packets sent from QEMU network
backends like SLiRP/TAP, we change to pad short frames before sending
it out to the other end, if the other end does not forbid it via the
nc->do_not_pad flag. This ensures a backend as an Ethernet sender
does not violate the spec. But with this change, the behavior of
dropping short frames from SLiRP/TAP interfaces in the NIC model
cannot be emulated because it always receives a packet that is spec
complaint. The capability of sending short frames from NIC models is
still supported and short frames can still pass through SLiRP/TAP.
This commit should be able to fix the issue as reported with some
NIC models before, that ARP requests get dropped, preventing the
guest from becoming visible on the network. It was workarounded in
these NIC models on the receive path, that when a short frame is
received, it is padded up to 60 bytes.
The following 2 commits seem to be the one to workaround this issue
in e1000 and vmxenet3 before, and should probably be reverted.
commit 78aeb23eded2 ("e1000: Pad short frames to minimum size (60 bytes)")
commit 40a87c6c9b11 ("vmxnet3: Pad short frames to minimum size (60 bytes)")
Signed-off-by: Bin Meng
---
Changes in v3:
- use the pad_short_frame() helper for slirp
net/slirp.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/net/slirp.c b/net/slirp.c
index be914c0be0..62a54d45dc 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -31,6 +31,7 @@
#include
#include
#endif
+#include "net/eth.h"
#include "net/net.h"
#include "clients.h"
#include "hub.h"
@@ -115,6 +116,14 @@ static ssize_t net_slirp_send_packet(const void *pkt,
size_t pkt_len,
void *opaque)
{
SlirpState *s = opaque;
+uint8_t min_pkt[ETH_ZLEN];
+
+if (!s->nc.peer->do_not_pad) {
+if (pad_short_frame(min_pkt, pkt, pkt_len)) {
+pkt = min_pkt;
+pkt_len = ETH_ZLEN;
+}
+}
return qemu_send_packet(&s->nc, pkt, pkt_len);
}
--
2.17.1
Re: [PATCH v4 02/17] target/ppc: Move 601 hflags adjustment to hreg_compute_hflags
On 3/15/21 7:46 PM, Richard Henderson wrote:
> Keep all hflags computation in one place, as this will be
> especially important later.
>
> Introduce a new POWERPC_FLAG_HID0_LE bit to indicate when
> LE should be taken from HID0. This appears to be set if
> and only if POWERPC_FLAG_RTC_CLK is set, but we're not
> short of bits and having both names will avoid confusion.
>
> Note that this was the only user of hflags_nmsr, so we can
> perform a straight assignment rather than mask and set.
>
> Signed-off-by: Richard Henderson
Reviewed-by: Cédric Le Goater
Thanks,
C.
> ---
> target/ppc/cpu.h| 2 ++
> target/ppc/helper_regs.c| 13 +++--
> target/ppc/misc_helper.c| 8 +++-
> target/ppc/translate_init.c.inc | 4 ++--
> 4 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index e73416da68..061d2eed1b 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -581,6 +581,8 @@ enum {
> POWERPC_FLAG_TM = 0x0010,
> /* Has SCV (ISA 3.00)
> */
> POWERPC_FLAG_SCV = 0x0020,
> +/* Has HID0 for LE bit (601)
> */
> +POWERPC_FLAG_HID0_LE = 0x0040,
> };
>
>
> /*/
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 5e18232b84..95b9aca61f 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -96,8 +96,17 @@ void hreg_compute_hflags(CPUPPCState *env)
> hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
> hreg_compute_mem_idx(env);
> env->hflags = env->msr & hflags_mask;
> -/* Merge with hflags coming from other registers */
> -env->hflags |= env->hflags_nmsr;
> +
> +if (env->flags & POWERPC_FLAG_HID0_LE) {
> +/*
> + * Note that MSR_LE is not set in env->msr_mask for this cpu,
> + * and so will never be set in msr or hflags at this point.
> + */
> +uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);
> +env->hflags |= le << MSR_LE;
> +/* Retain for backward compatibility with migration. */
> +env->hflags_nmsr = le << MSR_LE;
> +}
> }
>
> void cpu_interrupt_exittb(CPUState *cs)
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index 5d6e0de396..63e3147eb4 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -194,16 +194,14 @@ void helper_store_hid0_601(CPUPPCState *env,
> target_ulong val)
> target_ulong hid0;
>
> hid0 = env->spr[SPR_HID0];
> +env->spr[SPR_HID0] = (uint32_t)val;
> +
> if ((val ^ hid0) & 0x0008) {
> /* Change current endianness */
> -env->hflags &= ~(1 << MSR_LE);
> -env->hflags_nmsr &= ~(1 << MSR_LE);
> -env->hflags_nmsr |= (1 << MSR_LE) & (((val >> 3) & 1) << MSR_LE);
> -env->hflags |= env->hflags_nmsr;
> +hreg_compute_hflags(env);
> qemu_log("%s: set endianness to %c => " TARGET_FMT_lx "\n", __func__,
> val & 0x8 ? 'l' : 'b', env->hflags);
> }
> -env->spr[SPR_HID0] = (uint32_t)val;
> }
>
> void helper_store_403_pbr(CPUPPCState *env, uint32_t num, target_ulong value)
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index c03a7c4f52..049d76cfd1 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -5441,7 +5441,7 @@ POWERPC_FAMILY(601)(ObjectClass *oc, void *data)
> pcc->excp_model = POWERPC_EXCP_601;
> pcc->bus_model = PPC_FLAGS_INPUT_6xx;
> pcc->bfd_mach = bfd_mach_ppc_601;
> -pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK;
> +pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK |
> POWERPC_FLAG_HID0_LE;
> }
>
> #define POWERPC_MSRR_601v(0x1040ULL)
> @@ -5485,7 +5485,7 @@ POWERPC_FAMILY(601v)(ObjectClass *oc, void *data)
> #endif
> pcc->bus_model = PPC_FLAGS_INPUT_6xx;
> pcc->bfd_mach = bfd_mach_ppc_601;
> -pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK;
> +pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK |
> POWERPC_FLAG_HID0_LE;
> }
>
> static void init_proc_602(CPUPPCState *env)
>
[PATCH v3 04/13] net: tap: Pad short frames to minimum size before send
Do the same for tap backend as what we did for slirp.
Signed-off-by: Bin Meng
---
Changes in v3:
- use the pad_short_frame() helper for tap
net/tap-win32.c | 9 +
net/tap.c | 9 +
2 files changed, 18 insertions(+)
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 2b5dcda36e..e044a5ca35 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -31,6 +31,7 @@
#include "qemu-common.h"
#include "clients.h"/* net_init_tap */
+#include "net/eth.h"
#include "net/net.h"
#include "net/tap.h"/* tap_has_ufo, ... */
#include "qemu/error-report.h"
@@ -688,9 +689,17 @@ static void tap_win32_send(void *opaque)
uint8_t *buf;
int max_size = 4096;
int size;
+uint8_t min_pkt[ETH_ZLEN];
size = tap_win32_read(s->handle, &buf, max_size);
if (size > 0) {
+if (!s->nc.peer->do_not_pad) {
+if (pad_short_frame(min_pkt, buf, size)) {
+buf = min_pkt;
+size = ETH_ZLEN;
+}
+}
+
qemu_send_packet(&s->nc, buf, size);
tap_win32_free_buffer(s->handle, buf);
}
diff --git a/net/tap.c b/net/tap.c
index b7512853f4..aa69cf1c73 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -32,6 +32,7 @@
#include
#include
+#include "net/eth.h"
#include "net/net.h"
#include "clients.h"
#include "monitor/monitor.h"
@@ -189,6 +190,7 @@ static void tap_send(void *opaque)
while (true) {
uint8_t *buf = s->buf;
+uint8_t min_pkt[ETH_ZLEN];
size = tap_read_packet(s->fd, s->buf, sizeof(s->buf));
if (size <= 0) {
@@ -200,6 +202,13 @@ static void tap_send(void *opaque)
size -= s->host_vnet_hdr_len;
}
+if (!s->nc.peer->do_not_pad) {
+if (pad_short_frame(min_pkt, buf, size)) {
+buf = min_pkt;
+size = ETH_ZLEN;
+}
+}
+
size = qemu_send_packet_async(&s->nc, buf, size, tap_send_completed);
if (size == 0) {
tap_read_poll(s, false);
--
2.17.1
[PATCH v3 05/13] hw/net: virtio-net: Initialize nc->do_not_pad to true
For virtio-net, there is no need to pad the Ethernet frame size to
60 bytes before sending to it.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/virtio-net.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 96a3cc8357..66b9ff4511 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3314,6 +3314,10 @@ static void virtio_net_device_realize(DeviceState *dev,
Error **errp)
object_get_typename(OBJECT(dev)), dev->id, n);
}
+for (i = 0; i < n->max_queues; i++) {
+n->nic->ncs[i].do_not_pad = true;
+}
+
peer_test_vnet_hdr(n);
if (peer_has_vnet_hdr(n)) {
for (i = 0; i < n->max_queues; i++) {
--
2.17.1
[PATCH v3 08/13] hw/net: i82596: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/i82596.c | 18 --
1 file changed, 18 deletions(-)
diff --git a/hw/net/i82596.c b/hw/net/i82596.c
index 055c3a1470..1eca2e2d81 100644
--- a/hw/net/i82596.c
+++ b/hw/net/i82596.c
@@ -73,10 +73,6 @@ enum commands {
#define I596_EOF0x8000
#define SIZE_MASK 0x3fff
-#define ETHER_TYPE_LEN 2
-#define VLAN_TCI_LEN 2
-#define VLAN_HLEN (ETHER_TYPE_LEN + VLAN_TCI_LEN)
-
/* various flags in the chip config registers */
#define I596_PREFETCH (s->config[0] & 0x80)
#define I596_PROMISC(s->config[8] & 0x01)
@@ -489,8 +485,6 @@ bool i82596_can_receive(NetClientState *nc)
return true;
}
-#define MIN_BUF_SIZE 60
-
ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz)
{
I82596State *s = qemu_get_nic_opaque(nc);
@@ -501,7 +495,6 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t
*buf, size_t sz)
size_t bufsz = sz; /* length of data in buf */
uint32_t crc;
uint8_t *crc_ptr;
-uint8_t buf1[MIN_BUF_SIZE + VLAN_HLEN];
static const uint8_t broadcast_macaddr[6] = {
0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
@@ -584,17 +577,6 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t
*buf, size_t sz)
}
}
-/* if too small buffer, then expand it */
-if (len < MIN_BUF_SIZE + VLAN_HLEN) {
-memcpy(buf1, buf, len);
-memset(buf1 + len, 0, MIN_BUF_SIZE + VLAN_HLEN - len);
-buf = buf1;
-if (len < MIN_BUF_SIZE) {
-len = MIN_BUF_SIZE;
-}
-bufsz = len;
-}
-
/* Calculate the ethernet checksum (4 bytes) */
len += 4;
crc = cpu_to_be32(crc32(~0, buf, sz));
--
2.17.1
[PATCH v3 01/13] net: eth: Add a helper to pad a short ethernet frame
Add a helper to pad a short ethernet frame to the minimum required
length, which can be used by backend codes.
Signed-off-by: Bin Meng
---
Changes in v3:
- use 'without' instead of 'sans'
- add a helper to pad short frames
include/net/eth.h | 25 +
1 file changed, 25 insertions(+)
diff --git a/include/net/eth.h b/include/net/eth.h
index 0671be6916..bc064f8e52 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -31,6 +31,31 @@
#define ETH_ALEN 6
#define ETH_HLEN 14
+#define ETH_ZLEN 60 /* Min. octets in frame without FCS */
+
+/**
+ * pad_short_frame - pad a short frame to the minimum ethernet frame length
+ *
+ * If the ethernet frame size is shorter than 60 bytes, it will be padded to
+ * 60 bytes at the address @min_pkt.
+ *
+ * @min_pkt: buffer address to hold the padded frame
+ * @pkt: address to hold the original ethernet frame
+ * @size: size of the original ethernet frame
+ * @return true if the frame is padded, otherwise false
+ */
+static inline bool pad_short_frame(uint8_t *min_pkt, const uint8_t *pkt,
+ int size)
+{
+if (size < ETH_ZLEN) {
+/* pad to minimum ethernet frame length */
+memcpy(min_pkt, pkt, size);
+memset(&min_pkt[size], 0, ETH_ZLEN - size);
+return true;
+}
+
+return false;
+}
struct eth_header {
uint8_t h_dest[ETH_ALEN]; /* destination eth addr */
--
2.17.1
[PATCH v3 06/13] hw/net: e1000: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
This actually reverts commit 78aeb23eded2d0b765bf9145c71f80025b568acd.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/e1000.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index d8da2f6528..a53ba9052b 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -882,7 +882,6 @@ e1000_receive_iov(NetClientState *nc, const struct iovec
*iov, int iovcnt)
uint16_t vlan_special = 0;
uint8_t vlan_status = 0;
uint8_t min_buf[MIN_BUF_SIZE];
-struct iovec min_iov;
uint8_t *filter_buf = iov->iov_base;
size_t size = iov_size(iov, iovcnt);
size_t iov_ofs = 0;
@@ -898,15 +897,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec
*iov, int iovcnt)
return 0;
}
-/* Pad to minimum Ethernet frame length */
-if (size < sizeof(min_buf)) {
-iov_to_buf(iov, iovcnt, 0, min_buf, size);
-memset(&min_buf[size], 0, sizeof(min_buf) - size);
-min_iov.iov_base = filter_buf = min_buf;
-min_iov.iov_len = size = sizeof(min_buf);
-iovcnt = 1;
-iov = &min_iov;
-} else if (iov->iov_len < MAXIMUM_ETHERNET_HDR_LEN) {
+if (iov->iov_len < MAXIMUM_ETHERNET_HDR_LEN) {
/* This is very unlikely, but may happen. */
iov_to_buf(iov, iovcnt, 0, min_buf, MAXIMUM_ETHERNET_HDR_LEN);
filter_buf = min_buf;
--
2.17.1
[PATCH v3 07/13] hw/net: vmxnet3: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
This actually reverts commit 40a87c6c9b11ef9c14e0301f76abf0eb2582f08e.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/vmxnet3.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index eff299f629..d993cce097 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -39,7 +39,6 @@
#define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
#define VMXNET3_MSIX_BAR_SIZE 0x2000
-#define MIN_BUF_SIZE 60
/* Compatibility flags for migration */
#define VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT 0
@@ -1951,7 +1950,6 @@ vmxnet3_receive(NetClientState *nc, const uint8_t *buf,
size_t size)
{
VMXNET3State *s = qemu_get_nic_opaque(nc);
size_t bytes_indicated;
-uint8_t min_buf[MIN_BUF_SIZE];
if (!vmxnet3_can_receive(nc)) {
VMW_PKPRN("Cannot receive now");
@@ -1964,14 +1962,6 @@ vmxnet3_receive(NetClientState *nc, const uint8_t *buf,
size_t size)
size -= sizeof(struct virtio_net_hdr);
}
-/* Pad to minimum Ethernet frame length */
-if (size < sizeof(min_buf)) {
-memcpy(min_buf, buf, size);
-memset(&min_buf[size], 0, sizeof(min_buf) - size);
-buf = min_buf;
-size = sizeof(min_buf);
-}
-
net_rx_pkt_set_packet_type(s->rx_pkt,
get_eth_packet_type(PKT_GET_ETH_HDR(buf)));
--
2.17.1
Re: [PATCH v7 0/3] vnc: support reload x509 certificates
On 2021/3/16 2:07, Gerd Hoffmann wrote:
> On Mon, Mar 15, 2021 at 09:16:06PM +0800, Zihao Chang wrote:
>> This series supports reload x509 certificates for vnc
>> 1. Support reload x509 certificates.
>> 2. Support reload vnc certificates.
>> 3. Add new qmp display-reload and implement reload x509 certificates
>> for vnc.
>>
>> Example:
>> {"execute": "display-reload", "arguments":{"type": "vnc", "tls-certs": true}}
>>
>> Zihao Chang (3):
>> crypto: add reload for QCryptoTLSCredsClass
>> vnc: support reload x509 certificates for vnc
>> qmp: add new qmp display-reload
>
> fails gitlab.com ci (build-disabled test, possibly more, still running).
>
> take care,
> Gerd
>
Sorry for the problem, I've fixed this in the patch v8.
Thanks,
Zihao
>
>
[PATCH v3 09/13] hw/net: ne2000: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/ne2000.c | 12
1 file changed, 12 deletions(-)
diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index 6c17ee1ae2..b0a120ece6 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -167,15 +167,12 @@ static int ne2000_buffer_full(NE2000State *s)
return 0;
}
-#define MIN_BUF_SIZE 60
-
ssize_t ne2000_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
{
NE2000State *s = qemu_get_nic_opaque(nc);
size_t size = size_;
uint8_t *p;
unsigned int total_len, next, avail, len, index, mcast_idx;
-uint8_t buf1[60];
static const uint8_t broadcast_macaddr[6] =
{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
@@ -213,15 +210,6 @@ ssize_t ne2000_receive(NetClientState *nc, const uint8_t
*buf, size_t size_)
}
}
-
-/* if too small buffer, then expand it */
-if (size < MIN_BUF_SIZE) {
-memcpy(buf1, buf, size);
-memset(buf1 + size, 0, MIN_BUF_SIZE - size);
-buf = buf1;
-size = MIN_BUF_SIZE;
-}
-
index = s->curpag << 8;
if (index >= NE2000_PMEM_END) {
index = s->start;
--
2.17.1
[PATCH v3 10/13] hw/net: pcnet: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/pcnet.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index f3f18d8598..16330335cd 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -987,7 +987,6 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t
*buf, size_t size_)
{
PCNetState *s = qemu_get_nic_opaque(nc);
int is_padr = 0, is_bcast = 0, is_ladr = 0;
-uint8_t buf1[60];
int remaining;
int crc_err = 0;
size_t size = size_;
@@ -1000,14 +999,6 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t
*buf, size_t size_)
printf("pcnet_receive size=%zu\n", size);
#endif
-/* if too small buffer, then expand it */
-if (size < MIN_BUF_SIZE) {
-memcpy(buf1, buf, size);
-memset(buf1 + size, 0, MIN_BUF_SIZE - size);
-buf = buf1;
-size = MIN_BUF_SIZE;
-}
-
if (CSR_PROM(s)
|| (is_padr=padr_match(s, buf, size))
|| (is_bcast=padr_bcast(s, buf, size))
--
2.17.1
Re: [PATCH v4 08/17] target/ppc: Reduce env->hflags to uint32_t
On 3/15/21 7:46 PM, Richard Henderson wrote:
> It will be stored in tb->flags, which is also uint32_t,
> so let's use the correct size.
>
> Signed-off-by: Richard Henderson
Reviewed-by: Cédric Le Goater
Thanks,
C.
> ---
> target/ppc/cpu.h | 4 ++--
> target/ppc/misc_helper.c | 2 +-
> target/ppc/translate.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 39f35b570c..2abaf56869 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1128,8 +1128,8 @@ struct CPUPPCState {
> bool resume_as_sreset;
> #endif
>
> -/* These resources are used only in QEMU core */
> -target_ulong hflags;
> +/* These resources are used only in TCG */
> +uint32_t hflags;
> target_ulong hflags_compat_nmsr; /* for migration compatibility */
> int immu_idx; /* precomputed MMU index to speed up insn accesses */
> int dmmu_idx; /* precomputed MMU index to speed up data accesses */
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index 63e3147eb4..b04b4d7c6e 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -199,7 +199,7 @@ void helper_store_hid0_601(CPUPPCState *env, target_ulong
> val)
> if ((val ^ hid0) & 0x0008) {
> /* Change current endianness */
> hreg_compute_hflags(env);
> -qemu_log("%s: set endianness to %c => " TARGET_FMT_lx "\n", __func__,
> +qemu_log("%s: set endianness to %c => %08x\n", __func__,
> val & 0x8 ? 'l' : 'b', env->hflags);
> }
> }
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index a9325a12e5..a85b890bb0 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7657,7 +7657,7 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int
> flags)
> env->nip, env->lr, env->ctr, cpu_read_xer(env),
> cs->cpu_index);
> qemu_fprintf(f, "MSR " TARGET_FMT_lx " HID0 " TARGET_FMT_lx " HF "
> - TARGET_FMT_lx " iidx %d didx %d\n",
> + "%08x iidx %d didx %d\n",
> env->msr, env->spr[SPR_HID0],
> env->hflags, env->immu_idx, env->dmmu_idx);
> #if !defined(NO_TIMER_DUMP)
>
Re: [PATCH v4 03/17] target/ppc: Properly sync cpu state with new msr in cpu_load_old
On 3/15/21 7:46 PM, Richard Henderson wrote:
> Match cpu_post_load in using ppc_store_msr to set all of
> the cpu state implied by the value of msr. Do not restore
> hflags or hflags_nmsr, as we recompute them in ppc_store_msr.
>
> Signed-off-by: Richard Henderson
Could we add a common routine used by cpu_post_load() and cpu_load_old() ?
Reviewed-by: Cédric Le Goater
Thanks,
C.
> ---
> target/ppc/machine.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 283db1d28a..87d7bffb86 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -21,6 +21,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int
> version_id)
> int32_t slb_nr;
> #endif
> target_ulong xer;
> +target_ulong msr;
>
> for (i = 0; i < 32; i++) {
> qemu_get_betls(f, &env->gpr[i]);
> @@ -111,11 +112,19 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int
> version_id)
> qemu_get_betls(f, &env->ivpr_mask);
> qemu_get_betls(f, &env->hreset_vector);
> qemu_get_betls(f, &env->nip);
> -qemu_get_betls(f, &env->hflags);
> -qemu_get_betls(f, &env->hflags_nmsr);
> +qemu_get_sbetl(f); /* Discard unused hflags */
> +qemu_get_sbetl(f); /* Discard unused hflags_nmsr */
> qemu_get_sbe32(f); /* Discard unused mmu_idx */
> qemu_get_sbe32(f); /* Discard unused power_mode */
>
> +/*
> + * Invalidate all supported msr bits except MSR_TGPR/MSR_HVB
> + * before restoring. Note that this recomputes hflags and mem_idx.
> + */
> +msr = env->msr;
> +env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
> +ppc_store_msr(env, msr);
> +
> /* Recompute mmu indices */
> hreg_compute_mem_idx(env);
>
>
[PATCH v3 12/13] hw/net: sungem: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/sungem.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/hw/net/sungem.c b/hw/net/sungem.c
index 33c3722df6..3fa83168db 100644
--- a/hw/net/sungem.c
+++ b/hw/net/sungem.c
@@ -550,7 +550,6 @@ static ssize_t sungem_receive(NetClientState *nc, const
uint8_t *buf,
PCIDevice *d = PCI_DEVICE(s);
uint32_t mac_crc, done, kick, max_fsize;
uint32_t fcs_size, ints, rxdma_cfg, rxmac_cfg, csum, coff;
-uint8_t smallbuf[60];
struct gem_rxd desc;
uint64_t dbase, baddr;
unsigned int rx_cond;
@@ -584,19 +583,6 @@ static ssize_t sungem_receive(NetClientState *nc, const
uint8_t *buf,
return size;
}
-/* We don't drop too small frames since we get them in qemu, we pad
- * them instead. We should probably use the min frame size register
- * but I don't want to use a variable size staging buffer and I
- * know both MacOS and Linux use the default 64 anyway. We use 60
- * here to account for the non-existent FCS.
- */
-if (size < 60) {
-memcpy(smallbuf, buf, size);
-memset(&smallbuf[size], 0, 60 - size);
-buf = smallbuf;
-size = 60;
-}
-
/* Get MAC crc */
mac_crc = net_crc32_le(buf, ETH_ALEN);
--
2.17.1
[PATCH v3 11/13] hw/net: rtl8139: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/rtl8139.c | 12
1 file changed, 12 deletions(-)
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 4675ac878e..cbfe29a286 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -827,7 +827,6 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const
uint8_t *buf, size_t
uint32_t packet_header = 0;
-uint8_t buf1[MIN_BUF_SIZE + VLAN_HLEN];
static const uint8_t broadcast_macaddr[6] =
{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
@@ -939,17 +938,6 @@ static ssize_t rtl8139_do_receive(NetClientState *nc,
const uint8_t *buf, size_t
}
}
-/* if too small buffer, then expand it
- * Include some tailroom in case a vlan tag is later removed. */
-if (size < MIN_BUF_SIZE + VLAN_HLEN) {
-memcpy(buf1, buf, size);
-memset(buf1 + size, 0, MIN_BUF_SIZE + VLAN_HLEN - size);
-buf = buf1;
-if (size < MIN_BUF_SIZE) {
-size = MIN_BUF_SIZE;
-}
-}
-
if (rtl8139_cp_receiver_enabled(s))
{
if (!rtl8139_cp_rx_valid(s)) {
--
2.17.1
Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command
On 16/03/21 07:51, Markus Armbruster wrote: The query command could be made more useful than introspection if it reflected run time state, i.e. it showed an accelerator only when the host system actually supports it. Can't say how practical that would be. At least for KVM there is runtime state that can be included in query-accels. Paolo
[PATCH v3 13/13] hw/net: sunhme: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/sunhme.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/hw/net/sunhme.c b/hw/net/sunhme.c
index fc34905f87..6971796e57 100644
--- a/hw/net/sunhme.c
+++ b/hw/net/sunhme.c
@@ -714,8 +714,6 @@ static inline void sunhme_set_rx_ring_nr(SunHMEState *s,
int i)
s->erxregs[HME_ERXI_RING >> 2] = ring;
}
-#define MIN_BUF_SIZE 60
-
static ssize_t sunhme_receive(NetClientState *nc, const uint8_t *buf,
size_t size)
{
@@ -724,7 +722,6 @@ static ssize_t sunhme_receive(NetClientState *nc, const
uint8_t *buf,
dma_addr_t rb, addr;
uint32_t intstatus, status, buffer, buffersize, sum;
uint16_t csum;
-uint8_t buf1[60];
int nr, cr, len, rxoffset, csum_offset;
trace_sunhme_rx_incoming(size);
@@ -775,14 +772,6 @@ static ssize_t sunhme_receive(NetClientState *nc, const
uint8_t *buf,
trace_sunhme_rx_filter_accept();
-/* If too small buffer, then expand it */
-if (size < MIN_BUF_SIZE) {
-memcpy(buf1, buf, size);
-memset(buf1 + size, 0, MIN_BUF_SIZE - size);
-buf = buf1;
-size = MIN_BUF_SIZE;
-}
-
rb = s->erxregs[HME_ERXI_RING >> 2] & HME_ERXI_RING_ADDR;
nr = sunhme_get_rx_ring_count(s);
cr = sunhme_get_rx_ring_nr(s);
--
2.17.1
[PATCH] hw/net: fsl_etsec: Tx padding length should exclude CRC
As the comment of tx_padding_and_crc() says: "Never add CRC in QEMU",
min_frame_len should excluce CRC, so it should be 60 instead of 64.
Signed-off-by: Bin Meng
---
hw/net/fsl_etsec/rings.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
index d6be0d7d18..8f08446415 100644
--- a/hw/net/fsl_etsec/rings.c
+++ b/hw/net/fsl_etsec/rings.c
@@ -259,7 +259,7 @@ static void process_tx_bd(eTSEC *etsec,
|| etsec->regs[MACCFG2].value & MACCFG2_PADCRC) {
/* Padding and CRC (Padding implies CRC) */
-tx_padding_and_crc(etsec, 64);
+tx_padding_and_crc(etsec, 60);
} else if (etsec->first_bd.flags & BD_TX_TC
|| etsec->regs[MACCFG2].value & MACCFG2_CRC_EN) {
--
2.17.1
Re: [PATCH v4 06/17] target/ppc: Fix comment for MSR_FE{0,1}
On 3/15/21 7:46 PM, Richard Henderson wrote:
> As per hreg_compute_hflags:
>
> We 'forget' FE0 & FE1: we'll never generate imprecise exceptions
>
> remove the hflags marker from the respective comments.
>
> Signed-off-by: Richard Henderson
Reviewed-by: Cédric Le Goater
Thanks,
C.
> ---
> target/ppc/cpu.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 79c4033a42..fd13489dce 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -322,13 +322,13 @@ typedef struct ppc_v3_pate_t {
> #define MSR_PR 14 /* Problem state hflags
> */
> #define MSR_FP 13 /* Floating point available hflags
> */
> #define MSR_ME 12 /* Machine check interrupt enable
> */
> -#define MSR_FE0 11 /* Floating point exception mode 0hflags
> */
> +#define MSR_FE0 11 /* Floating point exception mode 0
> */
> #define MSR_SE 10 /* Single-step trace enable x hflags
> */
> #define MSR_DWE 10 /* Debug wait enable on 405 x
> */
> #define MSR_UBLE 10 /* User BTB lock enable on e500 x
> */
> #define MSR_BE 9 /* Branch trace enable x hflags
> */
> #define MSR_DE 9 /* Debug interrupts enable on embedded PowerPC x
> */
> -#define MSR_FE1 8 /* Floating point exception mode 1hflags
> */
> +#define MSR_FE1 8 /* Floating point exception mode 1
> */
> #define MSR_AL 7 /* AL bit on POWER
> */
> #define MSR_EP 6 /* Exception prefix on 601
> */
> #define MSR_IR 5 /* Instruction relocate
> */
>
Re: [RFC v2 11/13] vhost: Shadow virtqueue buffers forwarding
在 2021/3/16 上午3:48, Eugenio Pérez 写道:
Initial version of shadow virtqueue that actually forward buffers.
It reuses the VirtQueue code for the device part. The driver part is
based on Linux's virtio_ring driver, but with stripped functionality
and optimizations so it's easier to review.
These will be added in later commits.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.c | 212 +++--
hw/virtio/vhost.c | 113 ++-
2 files changed, 312 insertions(+), 13 deletions(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index 1460d1d5d1..68ed0f2740 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -9,6 +9,7 @@
#include "hw/virtio/vhost-shadow-virtqueue.h"
#include "hw/virtio/vhost.h"
+#include "hw/virtio/virtio-access.h"
#include "standard-headers/linux/vhost_types.h"
@@ -55,11 +56,96 @@ typedef struct VhostShadowVirtqueue {
/* Virtio device */
VirtIODevice *vdev;
+/* Map for returning guest's descriptors */
+VirtQueueElement **ring_id_maps;
+
+/* Next head to expose to device */
+uint16_t avail_idx_shadow;
+
+/* Next free descriptor */
+uint16_t free_head;
+
+/* Last seen used idx */
+uint16_t shadow_used_idx;
+
+/* Next head to consume from device */
+uint16_t used_idx;
+
/* Descriptors copied from guest */
vring_desc_t descs[];
} VhostShadowVirtqueue;
-/* Forward guest notifications */
+static void vhost_vring_write_descs(VhostShadowVirtqueue *svq,
+const struct iovec *iovec,
+size_t num, bool more_descs, bool write)
+{
+uint16_t i = svq->free_head, last = svq->free_head;
+unsigned n;
+uint16_t flags = write ? virtio_tswap16(svq->vdev, VRING_DESC_F_WRITE) : 0;
+vring_desc_t *descs = svq->vring.desc;
+
+if (num == 0) {
+return;
+}
+
+for (n = 0; n < num; n++) {
+if (more_descs || (n + 1 < num)) {
+descs[i].flags = flags | virtio_tswap16(svq->vdev,
+VRING_DESC_F_NEXT);
+} else {
+descs[i].flags = flags;
+}
+descs[i].addr = virtio_tswap64(svq->vdev, (hwaddr)iovec[n].iov_base);
So unsing virtio_tswap() is probably not correct since we're talking
with vhost backends which has its own endiness.
For vhost-vDPA, we can assume that it's a 1.0 device.
+descs[i].len = virtio_tswap32(svq->vdev, iovec[n].iov_len);
+
+last = i;
+i = virtio_tswap16(svq->vdev, descs[i].next);
+}
+
+svq->free_head = virtio_tswap16(svq->vdev, descs[last].next);
+}
+
+static unsigned vhost_shadow_vq_add_split(VhostShadowVirtqueue *svq,
+ VirtQueueElement *elem)
+{
+int head;
+unsigned avail_idx;
+vring_avail_t *avail = svq->vring.avail;
+
+head = svq->free_head;
+
+/* We need some descriptors here */
+assert(elem->out_num || elem->in_num);
+
+vhost_vring_write_descs(svq, elem->out_sg, elem->out_num,
+elem->in_num > 0, false);
+vhost_vring_write_descs(svq, elem->in_sg, elem->in_num, false, true);
+
+/*
+ * Put entry in available array (but don't update avail->idx until they
+ * do sync).
+ */
+avail_idx = svq->avail_idx_shadow & (svq->vring.num - 1);
+avail->ring[avail_idx] = virtio_tswap16(svq->vdev, head);
+svq->avail_idx_shadow++;
+
+/* Expose descriptors to device */
+smp_wmb();
+avail->idx = virtio_tswap16(svq->vdev, svq->avail_idx_shadow);
+
+return head;
+
+}
+
+static void vhost_shadow_vq_add(VhostShadowVirtqueue *svq,
+VirtQueueElement *elem)
+{
+unsigned qemu_head = vhost_shadow_vq_add_split(svq, elem);
+
+svq->ring_id_maps[qemu_head] = elem;
+}
+
+/* Handle guest->device notifications */
static void vhost_handle_guest_kick(EventNotifier *n)
{
VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
@@ -69,7 +155,72 @@ static void vhost_handle_guest_kick(EventNotifier *n)
return;
}
-event_notifier_set(&svq->kick_notifier);
+/* Make available as many buffers as possible */
+do {
+if (virtio_queue_get_notification(svq->vq)) {
+/* No more notifications until process all available */
+virtio_queue_set_notification(svq->vq, false);
+}
+
+while (true) {
+VirtQueueElement *elem;
+if (virtio_queue_full(svq->vq)) {
+break;
So we've disabled guest notification. If buffer has been consumed, we
need to retry the handle_guest_kick here. But I didn't find the code?
+}
+
+elem = virtqueue_pop(svq->vq, sizeof(*elem));
+if (!elem) {
+break;
+}
+
+vhost_
Re: [PATCH v4 04/17] target/ppc: Do not call hreg_compute_mem_idx after ppc_store_msr
On 3/15/21 7:46 PM, Richard Henderson wrote: > In ppc_store_msr we call hreg_compute_hflags, which itself > calls hreg_compute_mem_idx. Rely on ppc_store_msr to update > everything required by the msr update. > > Signed-off-by: Richard Henderson Reviewed-by: Cédric Le Goater Thanks, C. > --- > target/ppc/machine.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/target/ppc/machine.c b/target/ppc/machine.c > index 87d7bffb86..f6eeda9642 100644 > --- a/target/ppc/machine.c > +++ b/target/ppc/machine.c > @@ -125,9 +125,6 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int > version_id) > env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB); > ppc_store_msr(env, msr); > > -/* Recompute mmu indices */ > -hreg_compute_mem_idx(env); > - > return 0; > } > > @@ -418,14 +415,12 @@ static int cpu_post_load(void *opaque, int version_id) > > /* > * Invalidate all supported msr bits except MSR_TGPR/MSR_HVB > - * before restoring > + * before restoring. Note that this recomputes hflags and mem_idx. > */ > msr = env->msr; > env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB); > ppc_store_msr(env, msr); > > -hreg_compute_mem_idx(env); > - > return 0; > } > >
Re: [RFC v2 00/13] vDPA software assisted live migration
在 2021/3/16 上午3:48, Eugenio Pérez 写道:
This series enable shadow virtqueue for vhost-net devices. This is a
new method of vhost devices migration: Instead of relay on vhost
device's dirty logging capability, SW assisted LM intercepts dataplane,
forwarding the descriptors between VM and device. Is intended for vDPA
devices with no logging, but this address the basic platform to build
that support on.
In this migration mode, qemu offers a new vring to the device to
read and write into, and disable vhost notifiers, processing guest and
vhost notifications in qemu. On used buffer relay, qemu will mark the
dirty memory as with plain virtio-net devices. This way, devices does
not need to have dirty page logging capability.
This series is a POC doing SW LM for vhost-net devices, which already
have dirty page logging capabilities. For qemu to use shadow virtqueues
these vhost-net devices need to be instantiated:
* With IOMMU (iommu_platform=on,ats=on)
* Without event_idx (event_idx=off)
And shadow virtqueue needs to be enabled for them with QMP command
like:
{
"execute": "x-vhost-enable-shadow-vq",
"arguments": {
"name": "virtio-net",
"enable": false
}
}
Just the notification forwarding (with no descriptor relay) can be
achieved with patches 5 and 6, and starting SVQ. Previous commits
are cleanup ones and declaration of QMP command.
Commit 11 introduces the buffer forwarding. Previous one are for
preparations again, and laters are for enabling some obvious
optimizations.
It is based on the ideas of DPDK SW assisted LM, in the series of
DPDK's https://patchwork.dpdk.org/cover/48370/ . However, these does
not map the shadow vq in guest's VA, but in qemu's.
Comments are welcome! Especially on:
* Different/improved way of synchronization, particularly on the race
of masking.
TODO:
* Event, indirect, packed, and others features of virtio - Waiting for
confirmation of the big picture.
So two things in my mind after reviewing the seires:
1) which layer should we implement the shadow virtqueue. E.g if you want
to do that at virtio level, you need to deal with a lot of
synchronizations. I prefer to do it in vhost-vDPA.
2) Using VA as IOVA which can not work for vhost-vDPA
* vDPA devices:
So I think we can start from a vhost-vDPA specific shadow virtqueue
first, then extending it to be a general one which might be much easier.
Developing solutions for tracking the available IOVA
space for all devices.
For vhost-net, you can assume that [0, ULLONG_MAX] is valid so you can
simply use VA as IOVA.
Small POC available, skipping the get/set
status (since vDPA does not support it) and just allocating more and
more IOVA addresses in a hardcoded range available for the device.
I'm not sure this can work but you need make sure that range can fit the
size of the all memory regions and need to deal with memory region add
and del.
I guess you probably need a full functional tree based IOVA allocator.
Thanks
* To sepparate buffers forwarding in its own AIO context, so we can
throw more threads to that task and we don't need to stop the main
event loop.
* IOMMU optimizations, so bacthing and bigger chunks of IOVA can be
sent to device.
* Automatic kick-in on live-migration.
* Proper documentation.
Thanks!
Changes from v1 RFC:
* Use QMP instead of migration to start SVQ mode.
* Only accepting IOMMU devices, closer behavior with target devices
(vDPA)
* Fix invalid masking/unmasking of vhost call fd.
* Use of proper methods for synchronization.
* No need to modify VirtIO device code, all of the changes are
contained in vhost code.
* Delete superfluous code.
* An intermediate RFC was sent with only the notifications forwarding
changes. It can be seen in
https://patchew.org/QEMU/20210129205415.876290-1-epere...@redhat.com/
* v1 at
https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg05372.html
Eugenio Pérez (13):
virtio: Add virtio_queue_is_host_notifier_enabled
vhost: Save masked_notifier state
vhost: Add VhostShadowVirtqueue
vhost: Add x-vhost-enable-shadow-vq qmp
vhost: Route guest->host notification through shadow virtqueue
vhost: Route host->guest notification through shadow virtqueue
vhost: Avoid re-set masked notifier in shadow vq
virtio: Add vhost_shadow_vq_get_vring_addr
virtio: Add virtio_queue_full
vhost: add vhost_kernel_set_vring_enable
vhost: Shadow virtqueue buffers forwarding
vhost: Check for device VRING_USED_F_NO_NOTIFY at shadow virtqueue
kick
vhost: Use VRING_AVAIL_F_NO_INTERRUPT at device call on shadow
virtqueue
qapi/net.json | 22 ++
hw/virtio/vhost-shadow-virtqueue.h | 36 ++
include/hw/virtio/vhost.h | 6 +
include/hw/virtio/virtio.h | 3 +
hw/virtio/vhost-backend.c | 29 ++
hw/virtio/vhost-shadow-virtqueue.c | 551 +
hw/virtio/vhost.c
Re: [PATCH v4 05/17] target/ppc: Retain hflags_nmsr only for migration
On 3/15/21 7:46 PM, Richard Henderson wrote:
> We have eliminated all normal uses of hflags_nmsr. We need
> not even compute it except when we want to migrate. Rename
> the field to emphasize this.
>
> Remove the fixme comment for migrating access_type. This value
> is only ever used with the current executing instruction, and
> is never live when the cpu is halted for migration.
>
> Signed-off-by: Richard Henderson
Reviewed-by: Cédric Le Goater
Thanks,
C.
> ---
> target/ppc/cpu.h | 4 ++--
> target/ppc/helper_regs.c | 2 --
> target/ppc/machine.c | 9 ++---
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 061d2eed1b..79c4033a42 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1105,8 +1105,8 @@ struct CPUPPCState {
> #endif
>
> /* These resources are used only in QEMU core */
> -target_ulong hflags; /* hflags is MSR & HFLAGS_MASK */
> -target_ulong hflags_nmsr; /* specific hflags, not coming from MSR */
> +target_ulong hflags;
> +target_ulong hflags_compat_nmsr; /* for migration compatibility */
> int immu_idx; /* precomputed MMU index to speed up insn accesses */
> int dmmu_idx; /* precomputed MMU index to speed up data accesses */
>
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 95b9aca61f..a87e354ca2 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -104,8 +104,6 @@ void hreg_compute_hflags(CPUPPCState *env)
> */
> uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);
> env->hflags |= le << MSR_LE;
> -/* Retain for backward compatibility with migration. */
> -env->hflags_nmsr = le << MSR_LE;
> }
> }
>
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index f6eeda9642..1f7a353c78 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -310,6 +310,10 @@ static int cpu_pre_save(void *opaque)
> }
> }
>
> +/* Retain migration compatibility for pre 6.0 for 601 machines. */
> +env->hflags_compat_nmsr = (env->flags & POWERPC_FLAG_HID0_LE
> + ? env->hflags & MSR_LE : 0);
> +
> return 0;
> }
>
> @@ -829,9 +833,8 @@ const VMStateDescription vmstate_ppc_cpu = {
> /* Supervisor mode architected state */
> VMSTATE_UINTTL(env.msr, PowerPCCPU),
>
> -/* Internal state */
> -VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
> -/* FIXME: access_type? */
> +/* Backward compatible internal state */
> +VMSTATE_UINTTL(env.hflags_compat_nmsr, PowerPCCPU),
>
> /* Sanity checking */
> VMSTATE_UINTTL_TEST(mig_msr_mask, PowerPCCPU, cpu_pre_2_8_migration),
>
Re: [PATCH v3 01/13] net: eth: Add a helper to pad a short ethernet frame
On 3/16/21 9:12 AM, Bin Meng wrote:
> Add a helper to pad a short ethernet frame to the minimum required
> length, which can be used by backend codes.
>
> Signed-off-by: Bin Meng
>
> ---
>
> Changes in v3:
> - use 'without' instead of 'sans'
> - add a helper to pad short frames
>
> include/net/eth.h | 25 +
> 1 file changed, 25 insertions(+)
>
> diff --git a/include/net/eth.h b/include/net/eth.h
> index 0671be6916..bc064f8e52 100644
> --- a/include/net/eth.h
> +++ b/include/net/eth.h
> @@ -31,6 +31,31 @@
>
> #define ETH_ALEN 6
> #define ETH_HLEN 14
> +#define ETH_ZLEN 60 /* Min. octets in frame without FCS */
> +
> +/**
> + * pad_short_frame - pad a short frame to the minimum ethernet frame length
> + *
> + * If the ethernet frame size is shorter than 60 bytes, it will be padded to
> + * 60 bytes at the address @min_pkt.
> + *
> + * @min_pkt: buffer address to hold the padded frame
> + * @pkt: address to hold the original ethernet frame
> + * @size: size of the original ethernet frame
> + * @return true if the frame is padded, otherwise false
> + */
> +static inline bool pad_short_frame(uint8_t *min_pkt, const uint8_t *pkt,
> + int size)
> +{
> +if (size < ETH_ZLEN) {
> +/* pad to minimum ethernet frame length */
> +memcpy(min_pkt, pkt, size);
> +memset(&min_pkt[size], 0, ETH_ZLEN - size);
> +return true;
> +}
> +
> +return false;
> +}
I don't want to be too nitpicky but since I'm Cc'ed...
- 'ethernet' -> 'Ethernet'
- I'm not sure inlining is justified
- The same function is used for 2 different operations,
. check if padding is required
. do the padding
- If we provide a function a buffer to fill, we need to check the
buffer size is big enough to avoid overflow
What about something like:
bool pad_short_frame(char *padded_pkt, size_t *padded_buflen,
const void *pkt, size_t pkt_size);
{
assert(padded_buflen && *padded_buflen >= ETH_ZLEN);
if (src_size >= ETH_ZLEN) {
return false;
}
/* pad to minimum ethernet frame length */
memcpy(padded_pkt, pkt, pkt_size);
memset(&padded_pkt[pkt_size], 0, ETH_ZLEN - padded_buflen);
*padded_buflen = ETH_ZLEN;
return true;
}
What do you think?
Re: [PATCH v3 01/13] net: eth: Add a helper to pad a short ethernet frame
On Tue, Mar 16, 2021 at 4:49 PM Philippe Mathieu-Daudé
wrote:
>
> On 3/16/21 9:12 AM, Bin Meng wrote:
> > Add a helper to pad a short ethernet frame to the minimum required
> > length, which can be used by backend codes.
> >
> > Signed-off-by: Bin Meng
> >
> > ---
> >
> > Changes in v3:
> > - use 'without' instead of 'sans'
> > - add a helper to pad short frames
> >
> > include/net/eth.h | 25 +
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/include/net/eth.h b/include/net/eth.h
> > index 0671be6916..bc064f8e52 100644
> > --- a/include/net/eth.h
> > +++ b/include/net/eth.h
> > @@ -31,6 +31,31 @@
> >
> > #define ETH_ALEN 6
> > #define ETH_HLEN 14
> > +#define ETH_ZLEN 60 /* Min. octets in frame without FCS */
> > +
> > +/**
> > + * pad_short_frame - pad a short frame to the minimum ethernet frame length
> > + *
> > + * If the ethernet frame size is shorter than 60 bytes, it will be padded
> > to
> > + * 60 bytes at the address @min_pkt.
> > + *
> > + * @min_pkt: buffer address to hold the padded frame
> > + * @pkt: address to hold the original ethernet frame
> > + * @size: size of the original ethernet frame
> > + * @return true if the frame is padded, otherwise false
> > + */
> > +static inline bool pad_short_frame(uint8_t *min_pkt, const uint8_t *pkt,
> > + int size)
> > +{
> > +if (size < ETH_ZLEN) {
> > +/* pad to minimum ethernet frame length */
> > +memcpy(min_pkt, pkt, size);
> > +memset(&min_pkt[size], 0, ETH_ZLEN - size);
> > +return true;
> > +}
> > +
> > +return false;
> > +}
>
> I don't want to be too nitpicky but since I'm Cc'ed...
>
> - 'ethernet' -> 'Ethernet'
>
> - I'm not sure inlining is justified
>
> - The same function is used for 2 different operations,
> . check if padding is required
> . do the padding
>
> - If we provide a function a buffer to fill, we need to check the
> buffer size is big enough to avoid overflow
>
> What about something like:
>
> bool pad_short_frame(char *padded_pkt, size_t *padded_buflen,
>const void *pkt, size_t pkt_size);
> {
> assert(padded_buflen && *padded_buflen >= ETH_ZLEN);
> if (src_size >= ETH_ZLEN) {
> return false;
> }
> /* pad to minimum ethernet frame length */
> memcpy(padded_pkt, pkt, pkt_size);
> memset(&padded_pkt[pkt_size], 0, ETH_ZLEN - padded_buflen);
> *padded_buflen = ETH_ZLEN;
>
> return true;
> }
>
> What do you think?
Looks good to me. Will update in the next version.
Regards,
Bin
Re: [PATCH v7 0/8] Pegasos2 emulation
Le 15/03/2021 à 13:33, BALATON Zoltan a écrit : > On Sat, 13 Mar 2021, BALATON Zoltan wrote: >> On Wed, 10 Mar 2021, BALATON Zoltan wrote: >>> Hello, >> >> I've started posting this series well in advance to get it into 6.0 and yet >> it seems like it may >> be missing it due to organisational issues (no real complaints were found >> with patches but >> Philippe seems to like more review that does not seem to happen as nobody is >> interested). Looks >> like David is waiting for an ack from Philippe but will be away next week so >> if this is not >> resolved now it may be too late on Monday. To avoid that: >> >> David, could you please send an ack before you leave for the last two >> patches so it could get >> committed via some other tree while you're away? >> >> Philippe, if you can't ack the vt82c686 patches now are you OK with taking >> the whole series via >> your tree before the freeze? That would give you some more days to review >> and it could always be >> reverted during the freeze but if it's not merged now I'll have to wait >> until the summer to get it >> in again which would be another long delay. I don't think this will get more >> reviews unless it's >> in master and people can start using and testing it better. > > Hello, > > Since David seems to be away for this week before seeing my mail asking for > an ack from him, now > this can only get in by Philippe or Peter. (David said before he'd be OK with > the series if Philippe > acked it so I think that can count as an implicit ack and it could always be > reverted before the > releease.) > > Philippe, do you have anything against this to get merged now? If not please > send a pull or ack it > so it has a chance to be in 6.0 or tell if you still intend to do anything > about it before the > freeze. This series was on the list since January and the remaining parts you > did not take are here > since February 22nd and the version after your first review since two weeks > so it would be nice to > sort this out and not block it any further without a good reason. Pegasos looks like a New World PowerMac, so perhaps Mark can help? Thanks, Laurent
Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command
On 3/16/21 7:51 AM, Markus Armbruster wrote:
> Eric Blake writes:
>
>> On 3/11/21 5:11 PM, Philippe Mathieu-Daudé wrote:
> [...]
>>> diff --git a/accel/accel-qmp.c b/accel/accel-qmp.c
>>> new file mode 100644
>>> index 000..f16e49b8956
>>> --- /dev/null
>>> +++ b/accel/accel-qmp.c
>>> @@ -0,0 +1,47 @@
>>> +/*
>>> + * QEMU accelerators, QMP commands
>>> + *
>>> + * Copyright (c) 2021 Red Hat Inc.
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "qapi/qapi-commands-machine.h"
>>> +
>>> +static const Accelerator accel_list[] = {
>>> +ACCELERATOR_QTEST,
>>> +#ifdef CONFIG_TCG
>>> +ACCELERATOR_TCG,
>>> +#endif
>>> +#ifdef CONFIG_KVM
>>> +ACCELERATOR_KVM,
>>> +#endif
>>
>> ...would it be worth compiling the enum to only list enum values that
>> were actually compiled in? That would change it to:
>>
>> { 'enum': 'Accelerator',
>> 'data': [ 'qtest',
>> { 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
>> ...
These accelerator definitions are supposed to be poisoned in generic
code... But I like the simplicity of your suggestion, so I'll give it
a try and see what happens with removing the poisoned definitions.
> Makes introspection more useful. Management applications can get the
> information the list of compiled-in accelerators from query-qmp-schema.
> They don't have to be taught to use query-accels.
>
> In fact, query-accels becomes useless except as a tool to force
> visibility of Accelerator in query-qmp-schema. We wouldn't have to
> force if we had CLI introspection that shows the type of -accel's
> parameter @accel. Adding a query command is a common work-around for
> our anemic CLI introspection capabilities.
>
> The query command could be made more useful than introspection if it
> reflected run time state, i.e. it showed an accelerator only when the
> host system actually supports it. Can't say how practical that would
> be.
>
>>>
>>> +AcceleratorInfoList *qmp_query_accels(Error **errp)
>>> +{
>>> +AcceleratorInfoList *list = NULL, **tail = &list;
>>> +
>>> +for (unsigned i = 0; i < ARRAY_SIZE(accel_list); i++) {
>>> +AcceleratorInfo *info = g_new0(AcceleratorInfo, 1);
>>> +
>>> +info->name = accel_list[i];
>>> +
>>> +QAPI_LIST_APPEND(tail, info);
>>> +}
>>> +
>>> +return list;
>>> +}
>
> You could then use something like
>
> for (accel = 0; accel < ACCELERATOR__MAX; accel++) {
> AcceleratorInfo *info = g_new0(AcceleratorInfo, 1);
>
> info->name = Accelerator_str(accel);
>
> QAPI_LIST_APPEND(tail, info);
> }
>
libqemuutil
I rebased my "[PATCH v6 00/10] Configurable policy for handling
deprecated interfaces" to master, and it surprisingly fails to link
several utility programs. Here's the first error:
gcc -o tests/bench/benchmark-crypto-hmac
tests/bench/benchmark-crypto-hmac.p/benchmark-crypto-hmac.c.o -Wl,--as-needed
-Wl,--no-undefined -pie -Wl,--whole-archive libcrypto.fa libauthz.fa libqom.fa
-Wl,--no-whole-archive -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -m64
-fstack-protector-strong -Wl,--start-group libqemuutil.a
subprojects/libvhost-user/libvhost-user-glib.a
subprojects/libvhost-user/libvhost-user.a libcrypto.fa libauthz.fa libqom.fa
-pthread -lgthread-2.0 -lglib-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lgio-2.0
-lgobject-2.0 -lglib-2.0 -lgnutls -lutil -lm -lgthread-2.0 -lglib-2.0 -lnettle
-lgnutls -lpam -Wl,--end-group
/usr/bin/ld: libqemuutil.a(util_main-loop.c.o): in function
`qemu_set_fd_handler':
/work/armbru/qemu/bld-x86/../util/main-loop.c:581: multiple definition of
`qemu_set_fd_handler';
libqemuutil.a(stubs_set-fd-handler.c.o):/work/armbru/qemu/bld-x86/../stubs/set-fd-handler.c:8:
first defined here
collect2: error: ld returned 1 exit status
Both master and PATCH 01 still link fine, PATCH 02 doesn't. PATCH 02
doesn't go anywhere near qemu_set_fd_handler().
Turns out libqemuutil.a contains two definitions of
qemu_set_fd_handler(). In master:
$ nm --defined-only libqemuutil.a | awk '/:$/ { f=$0 } /
qemu_set_fd_handler/ { if (f) { print f; f="" } print $0 }'
util_main-loop.c.o:
07fe T qemu_set_fd_handler
stubs_set-fd-handler.c.o:
T qemu_set_fd_handler
This is obviously unhealthy.
I suspect the linker happens to pick the one that makes things work,
until something in my patch makes it pick the other one.
Is qemu_set_fd_handler() the only one? Nope:
$ nm --defined-only bld-x86/libqemuutil.a | awk '/ T / { print $NF }' |
sort | uniq -c | grep -v '^ *1 '
2 qemu_set_fd_handler
2 yank_generic_iochannel
2 yank_register_function
2 yank_register_instance
2 yank_unregister_function
2 yank_unregister_instance
I didn't run into this issue when I posted my series last Friday. The
issue now blocks its merge, and today is the soft freeze. Help!
Re: [PATCH] sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog
On Wed, Mar 10, 2021 at 05:30:04PM +, Stefan Hajnoczi wrote:
> socket_get_fd() fails with the error "socket_get_fd: too many
> connections" if the given listen backlog value is not 1.
>
> Not all callers set the backlog to 1. For example, commit
> 582d4210eb2f2ab5baac328fe4b479cd86da1647 ("qemu-nbd: Use SOMAXCONN for
> socket listen() backlog") uses SOMAXCONN. This will always fail with in
> socket_get_fd().
>
> This patch calls listen(2) on the fd to update the backlog value. The
> socket may already be in the listen state. I have tested that this works
> on Linux 5.10 and macOS Catalina.
>
> As a bonus this allows us to detect when the fd cannot listen. Now we'll
> be able to catch unbound or connected fds in socket_listen().
>
> Drop the num argument from socket_get_fd() since this function is also
> called by socket_connect() where a listen backlog value does not make
> sense.
>
> Fixes: e5b6353cf25c99c3f08bf51e29933352f7140e8f ("socket: Add backlog
> parameter to socket_listen")
> Reported-by: Richard W.M. Jones
> Cc: Juan Quintela
> Cc: Eric Blake
> Signed-off-by: Stefan Hajnoczi
> ---
> util/qemu-sockets.c | 29 ++---
> 1 file changed, 22 insertions(+), 7 deletions(-)
Dan and Gerd: Can this go via one of your trees?
Thanks,
Stefan
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 8af0278f15..2463c49773 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1116,14 +1116,10 @@ fail:
> return NULL;
> }
>
> -static int socket_get_fd(const char *fdstr, int num, Error **errp)
> +static int socket_get_fd(const char *fdstr, Error **errp)
> {
> Monitor *cur_mon = monitor_cur();
> int fd;
> -if (num != 1) {
> -error_setg_errno(errp, EINVAL, "socket_get_fd: too many
> connections");
> -return -1;
> -}
> if (cur_mon) {
> fd = monitor_get_fd(cur_mon, fdstr, errp);
> if (fd < 0) {
> @@ -1159,7 +1155,7 @@ int socket_connect(SocketAddress *addr, Error **errp)
> break;
>
> case SOCKET_ADDRESS_TYPE_FD:
> -fd = socket_get_fd(addr->u.fd.str, 1, errp);
> +fd = socket_get_fd(addr->u.fd.str, errp);
> break;
>
> case SOCKET_ADDRESS_TYPE_VSOCK:
> @@ -1187,7 +1183,26 @@ int socket_listen(SocketAddress *addr, int num, Error
> **errp)
> break;
>
> case SOCKET_ADDRESS_TYPE_FD:
> -fd = socket_get_fd(addr->u.fd.str, num, errp);
> +fd = socket_get_fd(addr->u.fd.str, errp);
> +if (fd < 0) {
> +return -1;
> +}
> +
> +/*
> + * If the socket is not yet in the listen state, then transition it
> to
> + * the listen state now.
> + *
> + * If it's already listening then this updates the backlog value as
> + * requested.
> + *
> + * If this socket cannot listen because it's already in another state
> + * (e.g. unbound or connected) then we'll catch the error here.
> + */
> +if (listen(fd, num) != 0) {
> +error_setg_errno(errp, errno, "Failed to listen on fd socket");
> +closesocket(fd);
> +return -1;
> +}
> break;
>
> case SOCKET_ADDRESS_TYPE_VSOCK:
> --
> 2.29.2
>
signature.asc
Description: PGP signature
Re: [PATCH] hw/i8254: fix vmstate load
* Pavel Dovgalyuk (pavel.dovgal...@ispras.ru) wrote:
> On 15.03.2021 23:13, Dr. David Alan Gilbert wrote:
> > * Pavel Dovgalyuk (pavel.dovgal...@ispras.ru) wrote:
> > > QEMU timer of channel 0 in i8254 is used to raise irq
> > > at the specified moment of time. This irq can be disabled
> > > with irq_disabled flag. But when vmstate of the pit is
> > > loaded, timer may be rearmed despite the disabled interrupts.
> > > This patch adds irq_disabled flag check to fix that.
> > >
> > > Signed-off-by: Pavel Dovgalyuk
> >
> > Hi Pavel,
> >I'm curious, did you see this cause a problem on a particular guest
> > OS?
>
> That was Windows 7 on i386.
> I found this when tested reverse debugging.
Thanks; I like to know of migration fixes for when someone comes asking
why something obscure breaks under migration :-)
Dave
> >
> > Dave
> >
> > > ---
> > > hw/timer/i8254.c |2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/hw/timer/i8254.c b/hw/timer/i8254.c
> > > index c01ee2c72a..c8388ea432 100644
> > > --- a/hw/timer/i8254.c
> > > +++ b/hw/timer/i8254.c
> > > @@ -324,7 +324,7 @@ static void pit_post_load(PITCommonState *s)
> > > {
> > > PITChannelState *sc = &s->channels[0];
> > > -if (sc->next_transition_time != -1) {
> > > +if (sc->next_transition_time != -1 && !sc->irq_disabled) {
> > > timer_mod(sc->irq_timer, sc->next_transition_time);
> > > } else {
> > > timer_del(sc->irq_timer);
> > >
> > >
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: libqemuutil
On 16/03/2021 10.07, Markus Armbruster wrote:
I rebased my "[PATCH v6 00/10] Configurable policy for handling
deprecated interfaces" to master, and it surprisingly fails to link
several utility programs. Here's the first error:
gcc -o tests/bench/benchmark-crypto-hmac
tests/bench/benchmark-crypto-hmac.p/benchmark-crypto-hmac.c.o -Wl,--as-needed
-Wl,--no-undefined -pie -Wl,--whole-archive libcrypto.fa libauthz.fa libqom.fa
-Wl,--no-whole-archive -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -m64
-fstack-protector-strong -Wl,--start-group libqemuutil.a
subprojects/libvhost-user/libvhost-user-glib.a
subprojects/libvhost-user/libvhost-user.a libcrypto.fa libauthz.fa libqom.fa
-pthread -lgthread-2.0 -lglib-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lgio-2.0
-lgobject-2.0 -lglib-2.0 -lgnutls -lutil -lm -lgthread-2.0 -lglib-2.0 -lnettle
-lgnutls -lpam -Wl,--end-group
/usr/bin/ld: libqemuutil.a(util_main-loop.c.o): in function
`qemu_set_fd_handler':
/work/armbru/qemu/bld-x86/../util/main-loop.c:581: multiple definition of
`qemu_set_fd_handler';
libqemuutil.a(stubs_set-fd-handler.c.o):/work/armbru/qemu/bld-x86/../stubs/set-fd-handler.c:8:
first defined here
collect2: error: ld returned 1 exit status
Both master and PATCH 01 still link fine, PATCH 02 doesn't. PATCH 02
doesn't go anywhere near qemu_set_fd_handler().
Turns out libqemuutil.a contains two definitions of
qemu_set_fd_handler(). In master:
$ nm --defined-only libqemuutil.a | awk '/:$/ { f=$0 } / qemu_set_fd_handler/ { if
(f) { print f; f="" } print $0 }'
util_main-loop.c.o:
07fe T qemu_set_fd_handler
stubs_set-fd-handler.c.o:
T qemu_set_fd_handler
This is obviously unhealthy.
I suspect the linker happens to pick the one that makes things work,
until something in my patch makes it pick the other one.
Is qemu_set_fd_handler() the only one? Nope:
$ nm --defined-only bld-x86/libqemuutil.a | awk '/ T / { print $NF }' |
sort | uniq -c | grep -v '^ *1 '
2 qemu_set_fd_handler
2 yank_generic_iochannel
2 yank_register_function
2 yank_register_instance
2 yank_unregister_function
2 yank_unregister_instance
I didn't run into this issue when I posted my series last Friday. The
issue now blocks its merge, and today is the soft freeze. Help!
A very, very quick-n-dirty band-aid is likely to mark the function in stubs
as weak:
diff --git a/stubs/set-fd-handler.c b/stubs/set-fd-handler.c
--- a/stubs/set-fd-handler.c
+++ b/stubs/set-fd-handler.c
@@ -1,6 +1,7 @@
#include "qemu/osdep.h"
#include "qemu/main-loop.h"
+__attribute__((weak))
void qemu_set_fd_handler(int fd,
IOHandler *fd_read,
IOHandler *fd_write,
... should IMHO be good enough for the soft freeze. In the long run, you
might want to analyze the problem more thoroughly, of course. I had similar
problems in the past already, and solved them by moving the stubs around. See:
b0476d6602adbf818132dc896b585e01f47eaf96
stubs: Move qemu_timer_notify_cb() and remove
qemu_notify_event() stub
8c2787629eee73ca8ce4f100cff4f4946583b4e8
stubs: Move qemu_fd_register stub to util/main-loop.c
HTH,
Thomas
Re: libqemuutil
On 16/03/21 10:07, Markus Armbruster wrote:
I suspect the linker happens to pick the one that makes things work,
until something in my patch makes it pick the other one.
Ouch. Fortunately the stub is unnecessary and can be removed.
--- 8<
From fe45350cc11434efe3461c540bb0f258bbe010f7 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini
Date: Tue, 16 Mar 2021 05:25:48 -0400
Subject: [PATCH] qemuutil: remove qemu_set_fd_handler duplicate symbol
libqemuutil has two definitions of qemu_set_fd_handler. This
is not needed since the only users of the function are
qemu-io.c and the emulators, both of which already include
util/main-loop.c.
Signed-off-by: Paolo Bonzini
diff --git a/stubs/meson.build b/stubs/meson.build
index a054d5877f..8a3e804cf0 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -34,7 +34,6 @@ stub_ss.add(files('ram-block.c'))
stub_ss.add(files('ramfb.c'))
stub_ss.add(files('replay.c'))
stub_ss.add(files('runstate-check.c'))
-stub_ss.add(files('set-fd-handler.c'))
stub_ss.add(files('sysbus.c'))
stub_ss.add(files('target-get-monitor-def.c'))
stub_ss.add(files('target-monitor-defs.c'))
diff --git a/stubs/set-fd-handler.c b/stubs/set-fd-handler.c
deleted file mode 100644
index bff7e0a45a..00
--- a/stubs/set-fd-handler.c
+++ /dev/null
@@ -1,10 +0,0 @@
-#include "qemu/osdep.h"
-#include "qemu/main-loop.h"
-
-void qemu_set_fd_handler(int fd,
- IOHandler *fd_read,
- IOHandler *fd_write,
- void *opaque)
-{
-abort();
-}
Is qemu_set_fd_handler() the only one? Nope:
$ nm --defined-only bld-x86/libqemuutil.a | awk '/ T / { print $NF }' |
sort | uniq -c | grep -v '^ *1 '
2 qemu_set_fd_handler
2 yank_generic_iochannel
2 yank_register_function
2 yank_register_instance
2 yank_unregister_function
2 yank_unregister_instance
I didn't run into this issue when I posted my series last Friday. The
issue now blocks its merge, and today is the soft freeze. Help!
For yank_*, I suggest moving the non-stub version to monitor/ and adding
it to the qmp_ss sourceset.
Paolo
Re: [RFC PATCH] docs/devel: expand style section of memory management
Daniel P. Berrangé writes: > On Mon, Mar 15, 2021 at 06:04:10PM +0100, Thomas Huth wrote: >> On 15/03/2021 17.57, Peter Maydell wrote: >> > On Mon, 15 Mar 2021 at 16:53, Alex Bennée wrote: >> > > -Prefer g_new(T, n) instead of g_malloc(sizeof(T) ``*`` n) for the >> > > following >> > > +Care should be taken to avoid introducing places where the guest could >> > > +trigger an exit. For example using ``g_malloc`` on start-up is fine >> > > +if the result of a failure is going to be a fatal exit anyway. There >> > > +may be some start-up cases where failing is unreasonable (for example >> > > +speculatively loading debug symbols). >> > > + >> > > +However if we are doing an allocation because of something the guest >> > > +has done we should never trigger an exit. The code may deal with this >> > > +by trying to allocate less memory and continue or re-designed to >> > > allocate >> > > +buffers on start-up. >> > >> > I think this is overly strong. We want to avoid malloc-or-die for >> > cases where the guest gets to decide how big the allocation is; >> > but if we're doing a single small fixed-size allocation that happens >> > to be triggered by a guest action we should be OK to g_malloc() that >> > I think. >> >> I agree with Peter. If the host is so much out-of-memory that we even can't >> allocate some few bytes anymore (let's say less than 4k), the system is >> pretty much dead anyway and it might be better to terminate the program >> immediately instead of continuing with the out-of-memory situation. > > On a Linux host you're almost certainly not going to see g_malloc > fail for small allocations at least. Instead at some point the host > will be under enough memory pressure that the OOM killer activates > and reaps arbitrary processes based on some criteria it has, freeing > up memory for malloc to succeed (unless OOM killer picked you as the > victim). This happens even for large allocations. In a prior iteration of the "When it's okay to treat OOM as fatal?" discussion[1], I showed that Linux malloc() and g_malloc() happily give me a terabyte of memory I don't have in 1024 chunks of 1 GiB each. I just reran the test, same results. See also [2]. [1] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg04229.html [2] http://turnoff.us/geek/bad-malloc/
Re: libqemuutil
On 16/03/21 10:24, Thomas Huth wrote: A very, very quick-n-dirty band-aid is likely to mark the function in stubs as weak: This probably does not work on some of our porting targets (OS X)? The static library was introduced as a low-tech alternative to weak symbols, IIRC. Paolo
Re: [PATCH 1/2] Fix the segment fault when calling yank_register_instance
Hi Marc-André,
The new chardev can get the same label. It is assigned after the function
ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
Error **errp)
{
.
chr_new = chardev_new(NULL, object_class_get_name(OBJECT_CLASS(cc)),
backend, chr->gcontext, errp);
if (!chr_new) {
return NULL;
}
chr_new->label = g_strdup(id);
if (chr->be_open && !chr_new->be_open) {
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
closed_sent = true;
}
chr->be = NULL;
qemu_chr_fe_init(be, chr_new, &error_abort);
.
}
It passes parameter NULL in chardev_new, I think it may be because the old
chardev isn't released yet.
It will cause duplicated problems. I need to consider this logic to see if
it can be changed.
Thanks
Li
On Mon, Mar 15, 2021 at 7:51 PM Marc-André Lureau <
marcandre.lur...@gmail.com> wrote:
> Hi
>
> On Mon, Mar 15, 2021 at 9:22 PM Li Zhang wrote:
>
>> From: Li Zhang
>>
>> When executing the QMP commands "chardev-change" to change the
>> backend device to socket, it will cause a segment fault because
>> it assumes chr->label as non-NULL in function yank_register_instance.
>> The function qmp_chardev_change calls chardev_new, which label
>> is NULL when creating a new chardev. The label will be passed to
>> yank_register_instance which causes a segment fault. The callchain
>> is as the following:
>> chardev_new ->
>> qemu_char_open ->
>> cc->open ->
>> qmp_chardev_open_socket ->
>> yank_register_instance
>>
>> Signed-off-by: Li Zhang
>> ---
>> chardev/char-socket.c | 8 +---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>> index c8bced76b7..26d5172682 100644
>> --- a/chardev/char-socket.c
>> +++ b/chardev/char-socket.c
>> @@ -1421,10 +1421,12 @@ static void qmp_chardev_open_socket(Chardev *chr,
>> qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS);
>> }
>>
>> -if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->label),
>> errp)) {
>> -return;
>> +if (chr->label) {
>> +if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->label),
>> errp)) {
>> +return;
>> +}
>> +s->registered_yank = true;
>> }
>> -s->registered_yank = true;
>>
>> /* be isn't opened until we get a connection */
>> *be_opened = false
>>
>
> Looks wrong to me, the new chardev will get the same label, and it should
> still be possible to call the yank functions then. The registration logic
> needs to be reworked during chardev-change.
>
> --
> Marc-André Lureau
>
Re: libqemuutil
Paolo Bonzini writes:
> On 16/03/21 10:07, Markus Armbruster wrote:
>> I suspect the linker happens to pick the one that makes things work,
>> until something in my patch makes it pick the other one.
>
> Ouch. Fortunately the stub is unnecessary and can be removed.
>
> --- 8<
> From fe45350cc11434efe3461c540bb0f258bbe010f7 Mon Sep 17 00:00:00 2001
> From: Paolo Bonzini
> Date: Tue, 16 Mar 2021 05:25:48 -0400
> Subject: [PATCH] qemuutil: remove qemu_set_fd_handler duplicate symbol
>
> libqemuutil has two definitions of qemu_set_fd_handler. This
> is not needed since the only users of the function are
> qemu-io.c and the emulators, both of which already include
> util/main-loop.c.
>
> Signed-off-by: Paolo Bonzini
>
> diff --git a/stubs/meson.build b/stubs/meson.build
> index a054d5877f..8a3e804cf0 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -34,7 +34,6 @@ stub_ss.add(files('ram-block.c'))
> stub_ss.add(files('ramfb.c'))
> stub_ss.add(files('replay.c'))
> stub_ss.add(files('runstate-check.c'))
> -stub_ss.add(files('set-fd-handler.c'))
> stub_ss.add(files('sysbus.c'))
> stub_ss.add(files('target-get-monitor-def.c'))
> stub_ss.add(files('target-monitor-defs.c'))
> diff --git a/stubs/set-fd-handler.c b/stubs/set-fd-handler.c
> deleted file mode 100644
> index bff7e0a45a..00
> --- a/stubs/set-fd-handler.c
> +++ /dev/null
> @@ -1,10 +0,0 @@
> -#include "qemu/osdep.h"
> -#include "qemu/main-loop.h"
> -
> -void qemu_set_fd_handler(int fd,
> - IOHandler *fd_read,
> - IOHandler *fd_write,
> - void *opaque)
> -{
> -abort();
> -}
Tested-by: Markus Armbruster
I'll include this in my pull request, if you don't mind.
>> Is qemu_set_fd_handler() the only one? Nope:
>> $ nm --defined-only bld-x86/libqemuutil.a | awk '/ T / { print
>> $NF }' | sort | uniq -c | grep -v '^ *1 '
>>2 qemu_set_fd_handler
>>2 yank_generic_iochannel
>>2 yank_register_function
>>2 yank_register_instance
>>2 yank_unregister_function
>>2 yank_unregister_instance
>> I didn't run into this issue when I posted my series last Friday.
>> The
>> issue now blocks its merge, and today is the soft freeze. Help!
>
> For yank_*, I suggest moving the non-stub version to monitor/ and
> adding it to the qmp_ss sourceset.
I can give it a try after the soft freeze.
[PULL 0/6] s390x patches for 6.0 softfreeze
The following changes since commit 6157b0e19721aadb4c7fdcfe57b2924af6144b14: Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-6.0-pull-request' into staging (2021-03-14 17:47:49 +) are available in the Git repository at: https://gitlab.com/cohuck/qemu.git tags/s390x-20210316 for you to fetch changes up to 5793f5aafb05dae30e9dcb57d0d1c8f1a9633f6d: s390x/pci: Add missing initialization for g_autofree variables (2021-03-15 15:47:18 +0100) s390x updates: - get rid of legacy_s390_alloc() and phys_mem_set_alloc() - tcg: implement the MVPG condition-code-option bit - fix g_autofree variable handing in the pci vfio code - use official z15 names in the cpu model definitions Cornelia Huck (1): s390x/cpu_model: use official name for 8562 David Hildenbrand (3): s390x/kvm: Get rid of legacy_s390_alloc() exec: Get rid of phys_mem_set_alloc() target/s390x: Store r1/r2 for page-translation exceptions during MVPG Miroslav Rezanina (1): s390x/pci: Add missing initialization for g_autofree variables Richard Henderson (1): target/s390x: Implement the MVPG condition-code-option bit hw/s390x/s390-pci-vfio.c | 9 +-- include/sysemu/kvm.h | 4 - softmmu/physmem.c | 36 + target/s390x/cpu.h | 5 ++ target/s390x/cpu_models.c | 4 +- target/s390x/excp_helper.c | 3 + target/s390x/helper.h | 2 +- target/s390x/insn-data.def | 2 +- target/s390x/kvm.c | 43 ++ target/s390x/mem_helper.c | 160 ++--- target/s390x/translate.c | 7 +- 11 files changed, 159 insertions(+), 116 deletions(-) -- 2.26.3
[PULL 5/6] target/s390x: Store r1/r2 for page-translation exceptions during MVPG
From: David Hildenbrand
The PoP states:
When EDAT-1 does not apply, and a program interruption due to a
page-translation exception is recognized by the MOVE PAGE
instruction, the contents of the R1 field of the instruction are
stored in bit positions 0-3 of location 162, and the contents of
the R2 field are stored in bit positions 4-7.
If [...] an ASCE-type, region-first-translation,
region-second-translation, region-third-translation, or
segment-translation exception was recognized, the contents of
location 162 are unpredictable.
So we have to write r1/r2 into the lowcore on page-translation
exceptions. Simply handle all exceptions inside our mvpg helper now.
Reviewed-by: Thomas Huth
Reviewed-by: Richard Henderson
Signed-off-by: David Hildenbrand
Tested-by: Thomas Huth
Message-Id: <20210315085449.34676-3-da...@redhat.com>
Signed-off-by: Cornelia Huck
---
target/s390x/helper.h | 2 +-
target/s390x/insn-data.def | 2 +-
target/s390x/mem_helper.c | 46 +++---
target/s390x/translate.c | 7 +-
4 files changed, 36 insertions(+), 21 deletions(-)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 55bd1551e604..d4e4f3388f81 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -18,7 +18,7 @@ DEF_HELPER_3(srstu, void, env, i32, i32)
DEF_HELPER_4(clst, i64, env, i64, i64, i64)
DEF_HELPER_FLAGS_4(mvn, TCG_CALL_NO_WG, void, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(mvo, TCG_CALL_NO_WG, void, env, i32, i64, i64)
-DEF_HELPER_FLAGS_4(mvpg, TCG_CALL_NO_WG, i32, env, i64, i64, i64)
+DEF_HELPER_FLAGS_4(mvpg, TCG_CALL_NO_WG, i32, env, i64, i32, i32)
DEF_HELPER_FLAGS_4(mvz, TCG_CALL_NO_WG, void, env, i32, i64, i64)
DEF_HELPER_3(mvst, i32, env, i32, i32)
DEF_HELPER_4(ex, void, env, i32, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index e5b6efabf323..0bb1886a2e16 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -641,7 +641,7 @@
/* MOVE NUMERICS */
C(0xd100, MVN, SS_a, Z, la1, a2, 0, 0, mvn, 0)
/* MOVE PAGE */
-C(0xb254, MVPG,RRE, Z, r1_o, r2_o, 0, 0, mvpg, 0)
+C(0xb254, MVPG,RRE, Z, 0, 0, 0, 0, mvpg, 0)
/* MOVE STRING */
C(0xb255, MVST,RRE, Z, 0, 0, 0, 0, mvst, 0)
/* MOVE WITH OPTIONAL SPECIFICATION */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index bab872dcad02..12e84a42855e 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -915,8 +915,10 @@ uint64_t HELPER(clst)(CPUS390XState *env, uint64_t c,
uint64_t s1, uint64_t s2)
}
/* move page */
-uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t
r2)
+uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint32_t r1, uint32_t
r2)
{
+const uint64_t src = get_address(env, r2) & TARGET_PAGE_MASK;
+const uint64_t dst = get_address(env, r1) & TARGET_PAGE_MASK;
const int mmu_idx = cpu_mmu_index(env, false);
const bool f = extract64(r0, 11, 1);
const bool s = extract64(r0, 10, 1);
@@ -929,34 +931,42 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0,
uint64_t r1, uint64_t r2)
tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
}
-r1 = wrap_address(env, r1 & TARGET_PAGE_MASK);
-r2 = wrap_address(env, r2 & TARGET_PAGE_MASK);
-
/*
- * TODO:
- * - Access key handling
- * - Store r1/r2 register identifiers at real location 162
+ * We always manually handle exceptions such that we can properly store
+ * r1/r2 to the lowcore on page-translation exceptions.
+ *
+ * TODO: Access key handling
*/
-exc = access_prepare_nf(&srca, env, cco, r2, TARGET_PAGE_SIZE,
+exc = access_prepare_nf(&srca, env, true, src, TARGET_PAGE_SIZE,
MMU_DATA_LOAD, mmu_idx, ra);
if (exc) {
-return 2;
+if (cco) {
+return 2;
+}
+goto inject_exc;
}
-exc = access_prepare_nf(&desta, env, cco, r1, TARGET_PAGE_SIZE,
+exc = access_prepare_nf(&desta, env, true, dst, TARGET_PAGE_SIZE,
MMU_DATA_STORE, mmu_idx, ra);
if (exc) {
-if (exc == PGM_PROTECTION) {
-#if !defined(CONFIG_USER_ONLY)
-stq_phys(env_cpu(env)->as,
- env->psa + offsetof(LowCore, trans_exc_code),
- env->tlb_fill_tec);
-#endif
-tcg_s390_program_interrupt(env, PGM_PROTECTION, ra);
+if (cco && exc != PGM_PROTECTION) {
+return 1;
}
-return 1;
+goto inject_exc;
}
access_memmove(env, &desta, &srca, ra);
return 0; /* data moved */
+inject_exc:
+#if !defined(CONFIG_USER_ONLY)
+if (exc != PGM_ADDRESSING) {
+stq_phys(env_cpu(env)->as, env->psa + offsetof(LowCore,
trans_exc_code),
+ env->tlb_fill_tec);
+}
+if (exc == PGM_PAGE_TRANS) {
+stb_phys(env_cpu(env)->as, e
[PULL 6/6] s390x/pci: Add missing initialization for g_autofree variables
From: Miroslav Rezanina
When declaring g_autofree variable without initialization, compiler
will raise "may be used uninitialized in this function" warning due
to automatic free handling.
This is mentioned in docs/devel/style.rst (quote from section
"Automatic memory deallocation"):
* Variables declared with g_auto* MUST always be initialized,
otherwise the cleanup function will use uninitialized stack memory
Add initialization for these declarations to prevent the warning and
comply with coding style.
Signed-off-by: Miroslav Rezanina
Reviewed-by: Philippe Mathieu-Daudé
Fixes: cd7498d07fbb ("s390x/pci: Add routine to get the vfio dma available
count")
Fixes: 1e7552ff5c34 ("s390x/pci: get zPCI function info from host")
Reviewed-by: Thomas Huth
Tested-by: Matthew Rosato
Message-Id: <20210315101352.152888-1-mreza...@redhat.com>
Signed-off-by: Cornelia Huck
---
hw/s390x/s390-pci-vfio.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index ead4f222d55a..2a153fa8c9e2 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -29,14 +29,11 @@
*/
bool s390_pci_update_dma_avail(int fd, unsigned int *avail)
{
-g_autofree struct vfio_iommu_type1_info *info;
-uint32_t argsz;
+uint32_t argsz = sizeof(struct vfio_iommu_type1_info);
+g_autofree struct vfio_iommu_type1_info *info = g_malloc0(argsz);
assert(avail);
-argsz = sizeof(struct vfio_iommu_type1_info);
-info = g_malloc0(argsz);
-
/*
* If the specified argsz is not large enough to contain all capabilities
* it will be updated upon return from the ioctl. Retry until we have
@@ -230,7 +227,7 @@ static void s390_pci_read_pfip(S390PCIBusDevice *pbdev,
*/
void s390_pci_get_clp_info(S390PCIBusDevice *pbdev)
{
-g_autofree struct vfio_device_info *info;
+g_autofree struct vfio_device_info *info = NULL;
VFIOPCIDevice *vfio_pci;
uint32_t argsz;
int fd;
--
2.26.3
[PULL 1/6] s390x/kvm: Get rid of legacy_s390_alloc()
From: David Hildenbrand
legacy_s390_alloc() was required for dealing with the absence of the ESOP
feature -- on old HW (< gen 10) and old z/VM versions (< 6.3).
As z/VM v6.2 (and even v6.3) is no longer supported since 2017 [1]
and we don't expect to have real users on such old hardware, let's drop
legacy_s390_alloc().
Still check+report an error just in case someone still runs on
such old z/VM environments, or someone runs under weird nested KVM
setups (where we can manually disable ESOP via the CPU model).
No need to check for KVM_CAP_GMAP - that should always be around on
kernels that also have KVM_CAP_DEVICE_CTRL (>= v3.15).
[1] https://www.ibm.com/support/lifecycle/search?q=z%2FVM
Suggested-by: Cornelia Huck
Suggested-by: Thomas Huth
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Halil Pasic
Cc: Cornelia Huck
Cc: Christian Borntraeger
Cc: Thomas Huth
Cc: Igor Mammedov
Cc: Peter Xu
Signed-off-by: David Hildenbrand
Reviewed-by: Thomas Huth
Reviewed-by: Igor Mammedov
Message-Id: <20210303130916.22553-2-da...@redhat.com>
Signed-off-by: Cornelia Huck
---
target/s390x/kvm.c | 43 +--
1 file changed, 5 insertions(+), 38 deletions(-)
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 73f816a72227..4fb3bbfef506 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -161,8 +161,6 @@ static int cap_protected;
static int active_cmma;
-static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared);
-
static int kvm_s390_query_mem_limit(uint64_t *memory_limit)
{
struct kvm_device_attr attr = {
@@ -349,6 +347,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
"please use kernel 3.15 or newer");
return -1;
}
+if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
+error_report("KVM is missing capability KVM_CAP_S390_COW - "
+ "unsupported environment");
+return -1;
+}
cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
@@ -357,11 +360,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
cap_vcpu_resets = kvm_check_extension(s, KVM_CAP_S390_VCPU_RESETS);
cap_protected = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
-if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
-|| !kvm_check_extension(s, KVM_CAP_S390_COW)) {
-phys_mem_set_alloc(legacy_s390_alloc);
-}
-
kvm_vm_enable_cap(s, KVM_CAP_S390_USER_SIGP, 0);
kvm_vm_enable_cap(s, KVM_CAP_S390_VECTOR_REGISTERS, 0);
kvm_vm_enable_cap(s, KVM_CAP_S390_USER_STSI, 0);
@@ -889,37 +887,6 @@ int kvm_s390_mem_op_pv(S390CPU *cpu, uint64_t offset, void
*hostbuf,
return ret;
}
-/*
- * Legacy layout for s390:
- * Older S390 KVM requires the topmost vma of the RAM to be
- * smaller than an system defined value, which is at least 256GB.
- * Larger systems have larger values. We put the guest between
- * the end of data segment (system break) and this value. We
- * use 32GB as a base to have enough room for the system break
- * to grow. We also have to use MAP parameters that avoid
- * read-only mapping of guest pages.
- */
-static void *legacy_s390_alloc(size_t size, uint64_t *align, bool shared)
-{
-static void *mem;
-
-if (mem) {
-/* we only support one allocation, which is enough for initial ram */
-return NULL;
-}
-
-mem = mmap((void *) 0x8ULL, size,
- PROT_EXEC|PROT_READ|PROT_WRITE,
- MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
-if (mem == MAP_FAILED) {
-mem = NULL;
-}
-if (mem && align) {
-*align = QEMU_VMALLOC_ALIGN;
-}
-return mem;
-}
-
static uint8_t const *sw_bp_inst;
static uint8_t sw_bp_ilen;
--
2.26.3
[PULL 2/6] exec: Get rid of phys_mem_set_alloc()
From: David Hildenbrand
As the last user is gone, we can get rid of phys_mem_set_alloc() and
simplify.
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Halil Pasic
Cc: Cornelia Huck
Cc: Christian Borntraeger
Cc: Thomas Huth
Cc: Igor Mammedov
Cc: Peter Xu
Signed-off-by: David Hildenbrand
Reviewed-by: Thomas Huth
Reviewed-by: Igor Mammedov
Message-Id: <20210303130916.22553-3-da...@redhat.com>
Signed-off-by: Cornelia Huck
---
include/sysemu/kvm.h | 4
softmmu/physmem.c| 36 +++-
2 files changed, 3 insertions(+), 37 deletions(-)
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 687c598be9b3..a1ab1ee12d32 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -249,10 +249,6 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long
reinject_trap);
int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
int kvm_on_sigbus(int code, void *addr);
-/* interface with exec.c */
-
-void phys_mem_set_alloc(void *(*alloc)(size_t, uint64_t *align, bool shared));
-
/* internal API */
int kvm_ioctl(KVMState *s, int type, ...);
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 7e8b0fab89a9..9e5ef4828e9e 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -1144,19 +1144,6 @@ static int subpage_register(subpage_t *mmio, uint32_t
start, uint32_t end,
uint16_t section);
static subpage_t *subpage_init(FlatView *fv, hwaddr base);
-static void *(*phys_mem_alloc)(size_t size, uint64_t *align, bool shared) =
- qemu_anon_ram_alloc;
-
-/*
- * Set a custom physical guest memory alloator.
- * Accelerators with unusual needs may need this. Hopefully, we can
- * get rid of it eventually.
- */
-void phys_mem_set_alloc(void *(*alloc)(size_t, uint64_t *align, bool shared))
-{
-phys_mem_alloc = alloc;
-}
-
static uint16_t phys_section_add(PhysPageMap *map,
MemoryRegionSection *section)
{
@@ -1962,8 +1949,9 @@ static void ram_block_add(RAMBlock *new_block, Error
**errp, bool shared)
return;
}
} else {
-new_block->host = phys_mem_alloc(new_block->max_length,
- &new_block->mr->align, shared);
+new_block->host = qemu_anon_ram_alloc(new_block->max_length,
+ &new_block->mr->align,
+ shared);
if (!new_block->host) {
error_setg_errno(errp, errno,
"cannot set up guest memory '%s'",
@@ -2047,17 +2035,6 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size,
MemoryRegion *mr,
return NULL;
}
-if (phys_mem_alloc != qemu_anon_ram_alloc) {
-/*
- * file_ram_alloc() needs to allocate just like
- * phys_mem_alloc, but we haven't bothered to provide
- * a hook there.
- */
-error_setg(errp,
- "-mem-path not supported with this accelerator");
-return NULL;
-}
-
size = HOST_PAGE_ALIGN(size);
file_size = get_file_size(fd);
if (file_size > 0 && file_size < size) {
@@ -2247,13 +2224,6 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
flags, block->fd, offset);
} else {
-/*
- * Remap needs to match alloc. Accelerators that
- * set phys_mem_alloc never remap. If they did,
- * we'd need a remap hook here.
- */
-assert(phys_mem_alloc == qemu_anon_ram_alloc);
-
flags |= MAP_PRIVATE | MAP_ANONYMOUS;
area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
flags, -1, 0);
--
2.26.3
[PULL 4/6] target/s390x: Implement the MVPG condition-code-option bit
From: Richard Henderson
If the CCO bit is set, MVPG should not generate an exception but
report page translation faults via a CC code.
Create a new helper, access_prepare_nf, which can use probe_access_flags
in non-faulting mode, and then handle watchpoints.
Reviewed-by: Richard Henderson
Signed-off-by: Richard Henderson
[thuth: Added logic to still inject protection exceptions]
Signed-off-by: Thomas Huth
[david: Look at env->tlb_fill_exc to determine if there was an exception]
Signed-off-by: David Hildenbrand
Tested-by: Thomas Huth
Message-Id: <20210315085449.34676-2-da...@redhat.com>
Signed-off-by: Cornelia Huck
---
target/s390x/cpu.h | 5 ++
target/s390x/excp_helper.c | 3 +
target/s390x/mem_helper.c | 136 ++---
3 files changed, 121 insertions(+), 23 deletions(-)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 60d434d5edd5..468b4430f339 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -114,6 +114,11 @@ struct CPUS390XState {
uint64_t diag318_info;
+#if !defined(CONFIG_USER_ONLY)
+uint64_t tlb_fill_tec; /* translation exception code during tlb_fill */
+int tlb_fill_exc;/* exception number seen during tlb_fill */
+#endif
+
/* Fields up to this point are cleared by a CPU reset */
struct {} end_reset_fields;
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index ce16af394b1f..c48cd6b46f49 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -164,6 +164,9 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
tec = 0; /* unused */
}
+env->tlb_fill_exc = excp;
+env->tlb_fill_tec = tec;
+
if (!excp) {
qemu_log_mask(CPU_LOG_MMU,
"%s: set tlb %" PRIx64 " -> %" PRIx64 " (%x)\n",
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 25cfede806af..bab872dcad02 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -130,28 +130,103 @@ typedef struct S390Access {
int mmu_idx;
} S390Access;
-static S390Access access_prepare(CPUS390XState *env, vaddr vaddr, int size,
- MMUAccessType access_type, int mmu_idx,
- uintptr_t ra)
+/*
+ * With nonfault=1, return the PGM_ exception that would have been injected
+ * into the guest; return 0 if no exception was detected.
+ *
+ * For !CONFIG_USER_ONLY, the TEC is stored stored to env->tlb_fill_tec.
+ * For CONFIG_USER_ONLY, the faulting address is stored to env->__excp_addr.
+ */
+static int s390_probe_access(CPUArchState *env, target_ulong addr, int size,
+ MMUAccessType access_type, int mmu_idx,
+ bool nonfault, void **phost, uintptr_t ra)
{
-S390Access access = {
-.vaddr1 = vaddr,
-.size1 = MIN(size, -(vaddr | TARGET_PAGE_MASK)),
-.mmu_idx = mmu_idx,
-};
+int flags;
-g_assert(size > 0 && size <= 4096);
-access.haddr1 = probe_access(env, access.vaddr1, access.size1, access_type,
- mmu_idx, ra);
+#if defined(CONFIG_USER_ONLY)
+flags = page_get_flags(addr);
+if (!(flags & (access_type == MMU_DATA_LOAD ? PAGE_READ : PAGE_WRITE))) {
+env->__excp_addr = addr;
+flags = (flags & PAGE_VALID) ? PGM_PROTECTION : PGM_ADDRESSING;
+if (nonfault) {
+return flags;
+}
+tcg_s390_program_interrupt(env, flags, ra);
+}
+*phost = g2h(env_cpu(env), addr);
+#else
+/*
+ * For !CONFIG_USER_ONLY, we cannot rely on TLB_INVALID_MASK or haddr==NULL
+ * to detect if there was an exception during tlb_fill().
+ */
+env->tlb_fill_exc = 0;
+flags = probe_access_flags(env, addr, access_type, mmu_idx, nonfault,
phost,
+ ra);
+if (env->tlb_fill_exc) {
+return env->tlb_fill_exc;
+}
-if (unlikely(access.size1 != size)) {
-/* The access crosses page boundaries. */
-access.vaddr2 = wrap_address(env, vaddr + access.size1);
-access.size2 = size - access.size1;
-access.haddr2 = probe_access(env, access.vaddr2, access.size2,
- access_type, mmu_idx, ra);
+if (unlikely(flags & TLB_WATCHPOINT)) {
+/* S390 does not presently use transaction attributes. */
+cpu_check_watchpoint(env_cpu(env), addr, size,
+ MEMTXATTRS_UNSPECIFIED,
+ (access_type == MMU_DATA_STORE
+ ? BP_MEM_WRITE : BP_MEM_READ), ra);
}
-return access;
+#endif
+return 0;
+}
+
+static int access_prepare_nf(S390Access *access, CPUS390XState *env,
+ bool nonfault, vaddr vaddr1, int size,
+ MMUAccessType access_type,
+ int mmu_idx, uintptr_t ra)
+{
+void *haddr1, *haddr2 = NULL;
+i
[PULL 3/6] s390x/cpu_model: use official name for 8562
The single-frame z15 is called "z15 T02" (and the multi-frame z15
"z15 T01").
Signed-off-by: Cornelia Huck
Reviewed-by: David Hildenbrand
Acked-by: Christian Borntraeger
Message-Id: <20210311132746.154-1-coh...@redhat.com>
---
target/s390x/cpu_models.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index dd474c5e9ad1..050dcf2d42d2 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -86,8 +86,8 @@ static S390CPUDef s390_cpu_defs[] = {
CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model ZR1
GA1"),
-CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "gen15a", "IBM z15 GA1"),
-CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "gen15b", "IBM 8562 GA1"),
+CPUDEF_INIT(0x8561, 15, 1, 47, 0x0800U, "gen15a", "IBM z15 T01 GA1"),
+CPUDEF_INIT(0x8562, 15, 1, 47, 0x0800U, "gen15b", "IBM z15 T02 GA1"),
};
#define QEMU_MAX_CPU_TYPE 0x2964
--
2.26.3
Re: libqemuutil
On 16/03/21 11:09, Markus Armbruster wrote:
Paolo Bonzini writes:
On 16/03/21 10:07, Markus Armbruster wrote:
I suspect the linker happens to pick the one that makes things work,
until something in my patch makes it pick the other one.
Ouch. Fortunately the stub is unnecessary and can be removed.
--- 8<
From fe45350cc11434efe3461c540bb0f258bbe010f7 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini
Date: Tue, 16 Mar 2021 05:25:48 -0400
Subject: [PATCH] qemuutil: remove qemu_set_fd_handler duplicate symbol
libqemuutil has two definitions of qemu_set_fd_handler. This
is not needed since the only users of the function are
qemu-io.c and the emulators, both of which already include
util/main-loop.c.
Signed-off-by: Paolo Bonzini
diff --git a/stubs/meson.build b/stubs/meson.build
index a054d5877f..8a3e804cf0 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -34,7 +34,6 @@ stub_ss.add(files('ram-block.c'))
stub_ss.add(files('ramfb.c'))
stub_ss.add(files('replay.c'))
stub_ss.add(files('runstate-check.c'))
-stub_ss.add(files('set-fd-handler.c'))
stub_ss.add(files('sysbus.c'))
stub_ss.add(files('target-get-monitor-def.c'))
stub_ss.add(files('target-monitor-defs.c'))
diff --git a/stubs/set-fd-handler.c b/stubs/set-fd-handler.c
deleted file mode 100644
index bff7e0a45a..00
--- a/stubs/set-fd-handler.c
+++ /dev/null
@@ -1,10 +0,0 @@
-#include "qemu/osdep.h"
-#include "qemu/main-loop.h"
-
-void qemu_set_fd_handler(int fd,
- IOHandler *fd_read,
- IOHandler *fd_write,
- void *opaque)
-{
-abort();
-}
Tested-by: Markus Armbruster
I'll include this in my pull request, if you don't mind.
Yes, of course.
Paolo
Re: [PATCH v3 04/13] net: tap: Pad short frames to minimum size before send
On 3/16/21 9:12 AM, Bin Meng wrote:
> Do the same for tap backend as what we did for slirp.
You explained SLiRP/TAP in the previous patch. IMO these
changes could be squashed there directly (besides, same
maintainer entry).
>
> Signed-off-by: Bin Meng
>
> ---
>
> Changes in v3:
> - use the pad_short_frame() helper for tap
>
> net/tap-win32.c | 9 +
> net/tap.c | 9 +
> 2 files changed, 18 insertions(+)
>
> diff --git a/net/tap-win32.c b/net/tap-win32.c
> index 2b5dcda36e..e044a5ca35 100644
> --- a/net/tap-win32.c
> +++ b/net/tap-win32.c
> @@ -31,6 +31,7 @@
>
> #include "qemu-common.h"
> #include "clients.h"/* net_init_tap */
> +#include "net/eth.h"
> #include "net/net.h"
> #include "net/tap.h"/* tap_has_ufo, ... */
> #include "qemu/error-report.h"
> @@ -688,9 +689,17 @@ static void tap_win32_send(void *opaque)
> uint8_t *buf;
> int max_size = 4096;
> int size;
> +uint8_t min_pkt[ETH_ZLEN];
>
> size = tap_win32_read(s->handle, &buf, max_size);
> if (size > 0) {
> +if (!s->nc.peer->do_not_pad) {
> +if (pad_short_frame(min_pkt, buf, size)) {
> +buf = min_pkt;
> +size = ETH_ZLEN;
> +}
> +}
> +
> qemu_send_packet(&s->nc, buf, size);
> tap_win32_free_buffer(s->handle, buf);
> }
> diff --git a/net/tap.c b/net/tap.c
> index b7512853f4..aa69cf1c73 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -32,6 +32,7 @@
> #include
> #include
>
> +#include "net/eth.h"
> #include "net/net.h"
> #include "clients.h"
> #include "monitor/monitor.h"
> @@ -189,6 +190,7 @@ static void tap_send(void *opaque)
>
> while (true) {
> uint8_t *buf = s->buf;
> +uint8_t min_pkt[ETH_ZLEN];
>
> size = tap_read_packet(s->fd, s->buf, sizeof(s->buf));
> if (size <= 0) {
> @@ -200,6 +202,13 @@ static void tap_send(void *opaque)
> size -= s->host_vnet_hdr_len;
> }
>
> +if (!s->nc.peer->do_not_pad) {
> +if (pad_short_frame(min_pkt, buf, size)) {
> +buf = min_pkt;
> +size = ETH_ZLEN;
> +}
> +}
> +
> size = qemu_send_packet_async(&s->nc, buf, size, tap_send_completed);
> if (size == 0) {
> tap_read_poll(s, false);
>
Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command
On 3/16/21 10:02 AM, Philippe Mathieu-Daudé wrote:
> On 3/16/21 7:51 AM, Markus Armbruster wrote:
>> Eric Blake writes:
>>
>>> On 3/11/21 5:11 PM, Philippe Mathieu-Daudé wrote:
>> [...]
diff --git a/accel/accel-qmp.c b/accel/accel-qmp.c
new file mode 100644
index 000..f16e49b8956
--- /dev/null
+++ b/accel/accel-qmp.c
@@ -0,0 +1,47 @@
+/*
+ * QEMU accelerators, QMP commands
+ *
+ * Copyright (c) 2021 Red Hat Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qapi-commands-machine.h"
+
+static const Accelerator accel_list[] = {
+ACCELERATOR_QTEST,
+#ifdef CONFIG_TCG
+ACCELERATOR_TCG,
+#endif
+#ifdef CONFIG_KVM
+ACCELERATOR_KVM,
+#endif
>>>
>>> ...would it be worth compiling the enum to only list enum values that
>>> were actually compiled in? That would change it to:
>>>
>>> { 'enum': 'Accelerator',
>>> 'data': [ 'qtest',
>>> { 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
>>> ...
>
> These accelerator definitions are supposed to be poisoned in generic
> code... But I like the simplicity of your suggestion, so I'll give it
> a try and see what happens with removing the poisoned definitions.
This is actually quite interesting :) Accelerator definitions are
declared in config-target.h, but acceleration is host specific...
We certainly don't want to make qapi target-specific.
Re: [RFC v2 05/13] vhost: Route guest->host notification through shadow virtqueue
On Tue, Mar 16, 2021 at 8:18 AM Jason Wang wrote:
>
>
> 在 2021/3/16 上午3:48, Eugenio Pérez 写道:
> > Shadow virtqueue notifications forwarding is disabled when vhost_dev
> > stops, so code flow follows usual cleanup.
> >
> > Signed-off-by: Eugenio Pérez
> > ---
> > hw/virtio/vhost-shadow-virtqueue.h | 7 ++
> > include/hw/virtio/vhost.h | 4 +
> > hw/virtio/vhost-shadow-virtqueue.c | 113 ++-
> > hw/virtio/vhost.c | 143 -
> > 4 files changed, 265 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.h
> > b/hw/virtio/vhost-shadow-virtqueue.h
> > index 6cc18d6acb..c891c6510d 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > @@ -17,6 +17,13 @@
> >
> > typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> >
> > +bool vhost_shadow_vq_start(struct vhost_dev *dev,
> > + unsigned idx,
> > + VhostShadowVirtqueue *svq);
> > +void vhost_shadow_vq_stop(struct vhost_dev *dev,
> > + unsigned idx,
> > + VhostShadowVirtqueue *svq);
> > +
> > VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx);
> >
> > void vhost_shadow_vq_free(VhostShadowVirtqueue *vq);
> > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> > index ac963bf23d..7ffdf9aea0 100644
> > --- a/include/hw/virtio/vhost.h
> > +++ b/include/hw/virtio/vhost.h
> > @@ -55,6 +55,8 @@ struct vhost_iommu {
> > QLIST_ENTRY(vhost_iommu) iommu_next;
> > };
> >
> > +typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> > +
> > typedef struct VhostDevConfigOps {
> > /* Vhost device config space changed callback
> >*/
> > @@ -83,7 +85,9 @@ struct vhost_dev {
> > uint64_t backend_cap;
> > bool started;
> > bool log_enabled;
> > +bool shadow_vqs_enabled;
> > uint64_t log_size;
> > +VhostShadowVirtqueue **shadow_vqs;
>
>
> Any reason that you don't embed the shadow virtqueue into
> vhost_virtqueue structure?
>
Not really, it could be relatively big and I would prefer SVQ
members/methods to remain hidden from any other part that includes
vhost.h. But it could be changed, for sure.
> (Note that there's a masked_notifier in struct vhost_virtqueue).
>
They are used differently: in SVQ the masked notifier is a pointer,
and if it's NULL the SVQ code knows that device is not masked. The
vhost_virtqueue is the real owner.
It could be replaced by a boolean in SVQ or something like that, I
experimented with a tri-state too (UNMASKED, MASKED, MASKED_NOTIFIED)
and let vhost.c code to manage all the transitions. But I find clearer
the pointer use, since it's the more natural for the
vhost_virtqueue_mask, vhost_virtqueue_pending existing functions.
This masking/unmasking is the part I dislike the most from this
series, so I'm very open to alternatives.
>
> > Error *migration_blocker;
> > const VhostOps *vhost_ops;
> > void *opaque;
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> > b/hw/virtio/vhost-shadow-virtqueue.c
> > index 4512e5b058..3e43399e9c 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > @@ -8,9 +8,12 @@
> >*/
> >
> > #include "hw/virtio/vhost-shadow-virtqueue.h"
> > +#include "hw/virtio/vhost.h"
> > +
> > +#include "standard-headers/linux/vhost_types.h"
> >
> > #include "qemu/error-report.h"
> > -#include "qemu/event_notifier.h"
> > +#include "qemu/main-loop.h"
> >
> > /* Shadow virtqueue to relay notifications */
> > typedef struct VhostShadowVirtqueue {
> > @@ -18,14 +21,121 @@ typedef struct VhostShadowVirtqueue {
> > EventNotifier kick_notifier;
> > /* Shadow call notifier, sent to vhost */
> > EventNotifier call_notifier;
> > +
> > +/*
> > + * Borrowed virtqueue's guest to host notifier.
> > + * To borrow it in this event notifier allows to register on the event
> > + * loop and access the associated shadow virtqueue easily. If we use
> > the
> > + * VirtQueue, we don't have an easy way to retrieve it.
>
>
> So this is something that worries me. It looks like a layer violation
> that makes the codes harder to work correctly.
>
I don't follow you here.
The vhost code already depends on virtqueue in the same sense:
virtio_queue_get_host_notifier is called on vhost_virtqueue_start. So
if this behavior ever changes it is unlikely for vhost to keep working
without changes. vhost_virtqueue has a kick/call int where I think it
should be stored actually, but they are never used as far as I see.
Previous RFC did rely on vhost_dev_disable_notifiers. From its documentation:
/* Stop processing guest IO notifications in vhost.
* Start processing them in qemu.
...
But it was easier for this mode to miss a notification, since they
create a new host_notifier in virtio_bus_set_host_
[PULL 01/11] qemuutil: remove qemu_set_fd_handler duplicate symbol
From: Paolo Bonzini
libqemuutil has two definitions of qemu_set_fd_handler. This
is not needed since the only users of the function are
qemu-io.c and the emulators, both of which already include
util/main-loop.c.
Signed-off-by: Paolo Bonzini
Message-Id:
Tested-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
stubs/set-fd-handler.c | 10 --
stubs/meson.build | 1 -
2 files changed, 11 deletions(-)
delete mode 100644 stubs/set-fd-handler.c
diff --git a/stubs/set-fd-handler.c b/stubs/set-fd-handler.c
deleted file mode 100644
index bff7e0a45a..00
--- a/stubs/set-fd-handler.c
+++ /dev/null
@@ -1,10 +0,0 @@
-#include "qemu/osdep.h"
-#include "qemu/main-loop.h"
-
-void qemu_set_fd_handler(int fd,
- IOHandler *fd_read,
- IOHandler *fd_write,
- void *opaque)
-{
-abort();
-}
diff --git a/stubs/meson.build b/stubs/meson.build
index a054d5877f..8a3e804cf0 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -34,7 +34,6 @@ stub_ss.add(files('ram-block.c'))
stub_ss.add(files('ramfb.c'))
stub_ss.add(files('replay.c'))
stub_ss.add(files('runstate-check.c'))
-stub_ss.add(files('set-fd-handler.c'))
stub_ss.add(files('sysbus.c'))
stub_ss.add(files('target-get-monitor-def.c'))
stub_ss.add(files('target-monitor-defs.c'))
--
2.26.2
[PULL 05/11] qapi: Implement deprecated-output=hide for QMP event data
This policy suppresses deprecated bits in output, and thus permits
"testing the future". Implement it for QMP event data: suppress
deprecated members.
No QMP event data is deprecated right now.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210312153210.2810514-5-arm...@redhat.com>
---
tests/unit/test-qmp-event.c | 21 +
scripts/qapi/events.py | 8 ++--
tests/qapi-schema/qapi-schema-test.json | 3 +++
tests/qapi-schema/qapi-schema-test.out | 2 ++
4 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c
index ab059fb5c2..047f44ff9a 100644
--- a/tests/unit/test-qmp-event.c
+++ b/tests/unit/test-qmp-event.c
@@ -159,6 +159,26 @@ static void test_event_deprecated(TestEventData *data,
const void *unused)
qobject_unref(data->expect);
}
+static void test_event_deprecated_data(TestEventData *data, const void *unused)
+{
+memset(&compat_policy, 0, sizeof(compat_policy));
+
+data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0',"
+ " 'data': { 'foo': 42 } }");
+qapi_event_send_test_event_features0(42);
+g_assert(data->emitted);
+
+qobject_unref(data->expect);
+
+compat_policy.has_deprecated_output = true;
+compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;
+data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0'
}");
+qapi_event_send_test_event_features0(42);
+g_assert(data->emitted);
+
+qobject_unref(data->expect);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -168,6 +188,7 @@ int main(int argc, char **argv)
event_test_add("/event/event_c", test_event_c);
event_test_add("/event/event_d", test_event_d);
event_test_add("/event/deprecated", test_event_deprecated);
+event_test_add("/event/deprecated_data", test_event_deprecated_data);
g_test_run();
return 0;
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index f6e1e76f64..8335c2bdfc 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -126,7 +126,7 @@ def gen_event_send(name: str,
if have_args:
assert arg_type is not None
ret += mcgen('''
-v = qobject_output_visitor_new(&obj);
+v = qobject_output_visitor_new_qmp(&obj);
''')
if not arg_type.is_implicit():
ret += mcgen('''
@@ -145,7 +145,11 @@ def gen_event_send(name: str,
ret += mcgen('''
visit_complete(v, &obj);
-qdict_put_obj(qmp, "data", obj);
+if (qdict_size(qobject_to(QDict, obj))) {
+qdict_put_obj(qmp, "data", obj);
+} else {
+qobject_unref(obj);
+}
''')
ret += mcgen('''
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index 48a0adabae..12ec588b52 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -324,5 +324,8 @@
'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
'defined(TEST_IF_COND_2)'] } ] }
+{ 'event': 'TEST-EVENT-FEATURES0',
+ 'data': 'FeatureStruct1' }
+
{ 'event': 'TEST-EVENT-FEATURES1',
'features': [ 'deprecated' ] }
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index 776d737891..f5741df97f 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -440,6 +440,8 @@ command test-command-cond-features3 None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
feature feature1
if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
+event TEST-EVENT-FEATURES0 FeatureStruct1
+boxed=False
event TEST-EVENT-FEATURES1 None
boxed=False
feature deprecated
--
2.26.2
[PULL 06/11] monitor: Drop query-qmp-schema 'gen': false hack
QMP commands return their response as a generated QAPI type, which the
monitor core converts to JSON via QObject.
query-qmp-schema's response is the generated introspection data. This
is a QLitObject since commit 7d0f982bfb "qapi: generate a literal
qobject for introspection", v2.12). Before, it was a string. Instead
of converting QLitObject / string -> QObject -> QAPI type
SchemaInfoList -> QObject -> JSON, we take a shortcut: the command is
'gen': false, so it can return the QObject instead of the QAPI type.
Slightly simpler and more efficient.
The next commit will filter the response for output policy, and this
is easier in the SchemaInfoList representation. Drop the shortcut.
This replaces the manual command registration by a generated one. The
manual registration makes the command available before the machine is
built by passing flag QCO_ALLOW_PRECONFIG. To keep it available
there, we need need to add 'allow-preconfig': true to its definition
in the schema.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210312153210.2810514-6-arm...@redhat.com>
[Commit message typo fixed]
---
qapi/introspect.json | 2 +-
monitor/monitor-internal.h | 3 ---
monitor/misc.c | 2 --
monitor/qmp-cmds-control.c | 29
storage-daemon/qemu-storage-daemon.c | 2 --
5 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/qapi/introspect.json b/qapi/introspect.json
index 944bb87a20..39bd303778 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -49,7 +49,7 @@
##
{ 'command': 'query-qmp-schema',
'returns': [ 'SchemaInfo' ],
- 'gen': false }# just to simplify qmp_query_json()
+ 'allow-preconfig': true }
##
# @SchemaMetaType:
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 40903d6386..9c3a09cb01 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -183,7 +183,4 @@ void help_cmd(Monitor *mon, const char *name);
void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
int hmp_compare_cmd(const char *name, const char *list);
-void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
- Error **errp);
-
#endif
diff --git a/monitor/misc.c b/monitor/misc.c
index a7650ed747..0b46006e42 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -231,8 +231,6 @@ static void monitor_init_qmp_commands(void)
qmp_init_marshal(&qmp_commands);
-qmp_register_command(&qmp_commands, "query-qmp-schema",
- qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
qmp_register_command(&qmp_commands, "device_add", qmp_device_add,
QCO_NO_OPTIONS);
qmp_register_command(&qmp_commands, "object-add", qmp_object_add,
diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
index 509ae870bd..25afd0867f 100644
--- a/monitor/qmp-cmds-control.c
+++ b/monitor/qmp-cmds-control.c
@@ -26,10 +26,14 @@
#include "monitor-internal.h"
#include "qemu-version.h"
+#include "qapi/compat-policy.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-control.h"
+#include "qapi/qapi-commands-introspect.h"
#include "qapi/qapi-emit-events.h"
#include "qapi/qapi-introspect.h"
+#include "qapi/qapi-visit-introspect.h"
+#include "qapi/qobject-input-visitor.h"
/*
* Accept QMP capabilities in @list for @mon.
@@ -154,17 +158,18 @@ EventInfoList *qmp_query_events(Error **errp)
return ev_list;
}
-/*
- * Minor hack: generated marshalling suppressed for this command
- * ('gen': false in the schema) so we can parse the JSON string
- * directly into QObject instead of first parsing it with
- * visit_type_SchemaInfoList() into a SchemaInfoList, then marshal it
- * to QObject with generated output marshallers, every time. Instead,
- * we do it in test-qobject-input-visitor.c, just to make sure
- * qapi-gen.py's output actually conforms to the schema.
- */
-void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
- Error **errp)
+SchemaInfoList *qmp_query_qmp_schema(Error **errp)
{
-*ret_data = qobject_from_qlit(&qmp_schema_qlit);
+QObject *obj = qobject_from_qlit(&qmp_schema_qlit);
+Visitor *v = qobject_input_visitor_new(obj);
+SchemaInfoList *schema = NULL;
+
+/* test_visitor_in_qmp_introspect() ensures this can't fail */
+visit_type_SchemaInfoList(v, NULL, &schema, &error_abort);
+g_assert(schema);
+
+qobject_unref(obj);
+visit_free(v);
+
+return schema;
}
diff --git a/storage-daemon/qemu-storage-daemon.c
b/storage-daemon/qemu-storage-daemon.c
index 23756fc8e5..02afdbeafc 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -146,8 +146,6 @@ static QemuOptsList qemu_object_opts = {
static void init_qmp_commands(void)
{
qmp_init_marshal(&qmp_commands);
-qmp_register_command(&qmp_commands, "query-qmp-schema",
-
[PULL 03/11] qapi: Implement deprecated-output=hide for QMP command results
This policy suppresses deprecated bits in output, and thus permits
"testing the future". Implement it for QMP command results. Example:
when QEMU is run with -compat deprecated-output=hide, then
{"execute": "query-cpus-fast"}
yields
{"return": [{"thread-id": 9805, "props": {"core-id": 0, "thread-id": 0,
"socket-id": 0}, "qom-path": "/machine/unattached/device[0]", "cpu-index": 0,
"target": "x86_64"}]}
instead of
{"return": [{"arch": "x86", "thread-id": 22436, "props": {"core-id": 0,
"thread-id": 0, "socket-id": 0}, "qom-path": "/machine/unattached/device[0]",
"cpu-index": 0, "target": "x86_64"}]}
Note the suppression of deprecated member "arch".
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210312153210.2810514-3-arm...@redhat.com>
---
include/qapi/qobject-output-visitor.h | 9 ++
include/qapi/visitor-impl.h | 3 ++
include/qapi/visitor.h | 9 ++
qapi/qapi-visit-core.c | 9 ++
qapi/qobject-output-visitor.c | 19 +++
tests/unit/test-qmp-cmds.c | 42 ++---
qapi/trace-events | 1 +
scripts/qapi/commands.py| 2 +-
scripts/qapi/visit.py | 12 +++
tests/qapi-schema/qapi-schema-test.json | 17 +-
tests/qapi-schema/qapi-schema-test.out | 18 +--
11 files changed, 118 insertions(+), 23 deletions(-)
diff --git a/include/qapi/qobject-output-visitor.h
b/include/qapi/qobject-output-visitor.h
index 2b1726baf5..29f4ea6aad 100644
--- a/include/qapi/qobject-output-visitor.h
+++ b/include/qapi/qobject-output-visitor.h
@@ -53,4 +53,13 @@ typedef struct QObjectOutputVisitor QObjectOutputVisitor;
*/
Visitor *qobject_output_visitor_new(QObject **result);
+/*
+ * Create a QObject output visitor for @obj for use with QMP
+ *
+ * This is like qobject_output_visitor_new(), except it obeys the
+ * policy for handling deprecated management interfaces set with
+ * -compat.
+ */
+Visitor *qobject_output_visitor_new_qmp(QObject **result);
+
#endif
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 7362c043be..2d853255a3 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -113,6 +113,9 @@ struct Visitor
The core takes care of the return type in the public interface. */
void (*optional)(Visitor *v, const char *name, bool *present);
+/* Optional */
+bool (*deprecated)(Visitor *v, const char *name);
+
/* Must be set */
VisitorType type;
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index ebc19ede7f..4d23b59853 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -459,6 +459,15 @@ void visit_end_alternate(Visitor *v, void **obj);
*/
bool visit_optional(Visitor *v, const char *name, bool *present);
+/*
+ * Should we visit deprecated member @name?
+ *
+ * @name must not be NULL. This function is only useful between
+ * visit_start_struct() and visit_end_struct(), since only objects
+ * have deprecated members.
+ */
+bool visit_deprecated(Visitor *v, const char *name);
+
/*
* Visit an enum value.
*
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 7e5f40e7f0..d9726ecaa1 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -135,6 +135,15 @@ bool visit_optional(Visitor *v, const char *name, bool
*present)
return *present;
}
+bool visit_deprecated(Visitor *v, const char *name)
+{
+trace_visit_deprecated(v, name);
+if (v->deprecated) {
+return v->deprecated(v, name);
+}
+return true;
+}
+
bool visit_is_input(Visitor *v)
{
return v->type == VISITOR_INPUT;
diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
index ba6f6ac8a7..5c4aa0f64d 100644
--- a/qapi/qobject-output-visitor.c
+++ b/qapi/qobject-output-visitor.c
@@ -13,6 +13,7 @@
*/
#include "qemu/osdep.h"
+#include "qapi/compat-policy.h"
#include "qapi/qobject-output-visitor.h"
#include "qapi/visitor-impl.h"
#include "qemu/queue.h"
@@ -31,6 +32,8 @@ typedef struct QStackEntry {
struct QObjectOutputVisitor {
Visitor visitor;
+CompatPolicyOutput deprecated_policy;
+
QSLIST_HEAD(, QStackEntry) stack; /* Stack of unfinished containers */
QObject *root; /* Root of the output visit */
QObject **result; /* User's storage location for result */
@@ -207,6 +210,13 @@ static bool qobject_output_type_null(Visitor *v, const
char *name,
return true;
}
+static bool qobject_output_deprecated(Visitor *v, const char *name)
+{
+QObjectOutputVisitor *qov = to_qov(v);
+
+return qov->deprecated_policy != COMPAT_POLICY_OUTPUT_HIDE;
+}
+
/* Finish building, and return the root object.
* The root object is never null. The caller becomes the object's
* owner, and should use qobject_unref() when done with it. */
@@ -256,6 +266,7 @@ Visitor *qobject_output_visitor_new(QObject **result)
v->visit
[PULL 04/11] qapi: Implement deprecated-output=hide for QMP events
This policy suppresses deprecated bits in output, and thus permits
"testing the future". Implement it for QMP events: suppress
deprecated ones.
No QMP event is deprecated right now.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210312153210.2810514-4-arm...@redhat.com>
---
tests/unit/test-qmp-event.c | 20
scripts/qapi/events.py | 12 +++-
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c
index 7dd0053190..ab059fb5c2 100644
--- a/tests/unit/test-qmp-event.c
+++ b/tests/unit/test-qmp-event.c
@@ -14,6 +14,7 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
+#include "qapi/compat-policy.h"
#include "qapi/error.h"
#include "qapi/qmp/qbool.h"
#include "qapi/qmp/qdict.h"
@@ -140,6 +141,24 @@ static void test_event_d(TestEventData *data,
qobject_unref(data->expect);
}
+static void test_event_deprecated(TestEventData *data, const void *unused)
+{
+data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1'
}");
+
+memset(&compat_policy, 0, sizeof(compat_policy));
+
+qapi_event_send_test_event_features1();
+g_assert(data->emitted);
+
+compat_policy.has_deprecated_output = true;
+compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;
+data->emitted = false;
+qapi_event_send_test_event_features1();
+g_assert(!data->emitted);
+
+qobject_unref(data->expect);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -148,6 +167,7 @@ int main(int argc, char **argv)
event_test_add("/event/event_b", test_event_b);
event_test_add("/event/event_c", test_event_c);
event_test_add("/event/event_d", test_event_d);
+event_test_add("/event/deprecated", test_event_deprecated);
g_test_run();
return 0;
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 90d2f6156d..f6e1e76f64 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -79,6 +79,7 @@ def gen_param_var(typ: QAPISchemaObjectType) -> str:
def gen_event_send(name: str,
arg_type: Optional[QAPISchemaObjectType],
+ features: List[QAPISchemaFeature],
boxed: bool,
event_enum_name: str,
event_emit: str) -> str:
@@ -107,6 +108,14 @@ def gen_event_send(name: str,
if not boxed:
ret += gen_param_var(arg_type)
+if 'deprecated' in [f.name for f in features]:
+ret += mcgen('''
+
+if (compat_policy.deprecated_output == COMPAT_POLICY_OUTPUT_HIDE) {
+return;
+}
+''')
+
ret += mcgen('''
qmp = qmp_event_build_dict("%(name)s");
@@ -176,6 +185,7 @@ def _begin_user_module(self, name: str) -> None:
#include "%(prefix)sqapi-emit-events.h"
#include "%(events)s.h"
#include "%(visit)s.h"
+#include "qapi/compat-policy.h"
#include "qapi/error.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qobject-output-visitor.h"
@@ -220,7 +230,7 @@ def visit_event(self,
boxed: bool) -> None:
with ifcontext(ifcond, self._genh, self._genc):
self._genh.add(gen_event_send_decl(name, arg_type, boxed))
-self._genc.add(gen_event_send(name, arg_type, boxed,
+self._genc.add(gen_event_send(name, arg_type, features, boxed,
self._event_enum_name,
self._event_emit_name))
# Note: we generate the enum member regardless of @ifcond, to
--
2.26.2
[PULL 02/11] qemu-options: New -compat to set policy for deprecated interfaces
New option -compat lets you configure what to do when deprecated
interfaces get used. This is intended for testing users of the
management interfaces. It is experimental.
-compat deprecated-input= configures what to do when
deprecated input is received. Input policy can be "accept" (accept
silently), or "reject" (reject the request with an error).
-compat deprecated-output= configures what to do when
deprecated output is sent. Output policy can be "accept" (pass on
unchanged), or "hide" (filter out the deprecated parts).
Default is "accept". Policies other than "accept" are implemented
later in this series.
For now, -compat covers only syntactic aspects of QMP, i.e. stuff
tagged with feature 'deprecated'. We may want to extend it to cover
semantic aspects, CLI, and experimental features.
Note that there is no good way for management application to detect
presence of -compat: it's not visible output of query-qmp-schema or
query-command-line-options. Tolerable, because it's meant for
testing. If running with -compat fails, skip the test.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210312153210.2810514-2-arm...@redhat.com>
[Since: comments in qapi/compat.json fixed up]
---
qapi/compat.json | 51
qapi/qapi-schema.json| 1 +
include/qapi/compat-policy.h | 20 ++
qapi/qmp-dispatch.c | 3 +++
softmmu/vl.c | 17
qapi/meson.build | 1 +
qemu-options.hx | 20 ++
7 files changed, 113 insertions(+)
create mode 100644 qapi/compat.json
create mode 100644 include/qapi/compat-policy.h
diff --git a/qapi/compat.json b/qapi/compat.json
new file mode 100644
index 00..fc24a58a9e
--- /dev/null
+++ b/qapi/compat.json
@@ -0,0 +1,51 @@
+# -*- Mode: Python -*-
+
+##
+# = Compatibility policy
+##
+
+##
+# @CompatPolicyInput:
+#
+# Policy for handling "funny" input.
+#
+# @accept: Accept silently
+# @reject: Reject with an error
+#
+# Since: 6.0
+##
+{ 'enum': 'CompatPolicyInput',
+ 'data': [ 'accept', 'reject' ] }
+
+##
+# @CompatPolicyOutput:
+#
+# Policy for handling "funny" output.
+#
+# @accept: Pass on unchanged
+# @hide: Filter out
+#
+# Since: 6.0
+##
+{ 'enum': 'CompatPolicyOutput',
+ 'data': [ 'accept', 'hide' ] }
+
+##
+# @CompatPolicy:
+#
+# Policy for handling deprecated management interfaces.
+#
+# This is intended for testing users of the management interfaces.
+#
+# Limitation: covers only syntactic aspects of QMP, i.e. stuff tagged
+# with feature 'deprecated'. We may want to extend it to cover
+# semantic aspects, CLI, and experimental features.
+#
+# @deprecated-input: how to handle deprecated input (default 'accept')
+# @deprecated-output: how to handle deprecated output (default 'accept')
+#
+# Since: 6.0
+##
+{ 'struct': 'CompatPolicy',
+ 'data': { '*deprecated-input': 'CompatPolicyInput',
+'*deprecated-output': 'CompatPolicyOutput' } }
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 3441c9a9ae..4912b9744e 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -79,6 +79,7 @@
{ 'include': 'migration.json' }
{ 'include': 'transaction.json' }
{ 'include': 'trace.json' }
+{ 'include': 'compat.json' }
{ 'include': 'control.json' }
{ 'include': 'introspect.json' }
{ 'include': 'qom.json' }
diff --git a/include/qapi/compat-policy.h b/include/qapi/compat-policy.h
new file mode 100644
index 00..b8c6638156
--- /dev/null
+++ b/include/qapi/compat-policy.h
@@ -0,0 +1,20 @@
+/*
+ * Policy for handling "funny" management interfaces
+ *
+ * Copyright (C) 2020 Red Hat, Inc.
+ *
+ * Authors:
+ * Markus Armbruster
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#ifndef QAPI_COMPAT_POLICY_H
+#define QAPI_COMPAT_POLICY_H
+
+#include "qapi/qapi-types-compat.h"
+
+extern CompatPolicy compat_policy;
+
+#endif
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 0a2b20a4e4..45090f881a 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -14,6 +14,7 @@
#include "qemu/osdep.h"
#include "block/aio.h"
+#include "qapi/compat-policy.h"
#include "qapi/error.h"
#include "qapi/qmp/dispatch.h"
#include "qapi/qmp/qdict.h"
@@ -23,6 +24,8 @@
#include "qemu/coroutine.h"
#include "qemu/main-loop.h"
+CompatPolicy compat_policy;
+
static QDict *qmp_dispatch_check_obj(QDict *dict, bool allow_oob,
Error **errp)
{
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b7673b9613..4c53b2940c 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -29,6 +29,7 @@
#include "exec/cpu-common.h"
#include "hw/boards.h"
#include "hw/qdev-properties.h"
+#include "qapi/compat-policy.h"
#include "qapi/error.h"
#include "qapi/qmp/qdict.h"
#include "qemu-version.h"
@@ -113,6 +114,7 @@
#include "sysemu/replay.h"
#include "qapi/qapi-events-run-state.h"
#inclu
[PULL 11/11] qapi: New -compat deprecated-input=crash
Policy "crash" calls abort() when deprecated input is received.
Bugs in integration tests may mask the error from policy "reject".
Provide a larger hammer: crash outright. Masking that seems unlikely.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210312153210.2810514-11-arm...@redhat.com>
---
qapi/compat.json | 3 ++-
qapi/qmp-dispatch.c | 1 +
qapi/qobject-input-visitor.c | 1 +
qemu-options.hx | 4 +++-
4 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/qapi/compat.json b/qapi/compat.json
index fc24a58a9e..ae3afc22df 100644
--- a/qapi/compat.json
+++ b/qapi/compat.json
@@ -11,11 +11,12 @@
#
# @accept: Accept silently
# @reject: Reject with an error
+# @crash: abort() the process
#
# Since: 6.0
##
{ 'enum': 'CompatPolicyInput',
- 'data': [ 'accept', 'reject' ] }
+ 'data': [ 'accept', 'reject', 'crash' ] }
##
# @CompatPolicyOutput:
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index cbc4452341..12657d635e 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -167,6 +167,7 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject
*request,
"Deprecated command %s disabled by policy",
command);
goto out;
+case COMPAT_POLICY_INPUT_CRASH:
default:
abort();
}
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 1b8fa1f2f6..baad0dcd3c 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -676,6 +676,7 @@ static bool qobject_input_deprecated_accept(Visitor *v,
const char *name,
error_setg(errp, "Deprecated parameter '%s' disabled by policy",
name);
return false;
+case COMPAT_POLICY_INPUT_CRASH:
default:
abort();
}
diff --git a/qemu-options.hx b/qemu-options.hx
index f34d592eb1..2b5df64cdf 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3471,7 +3471,7 @@ DEFHEADING()
DEFHEADING(Debug/Expert options:)
DEF("compat", HAS_ARG, QEMU_OPTION_compat,
-"-compat
[deprecated-input=accept|reject][,deprecated-output=accept|hide]\n"
+"-compat
[deprecated-input=accept|reject|crash][,deprecated-output=accept|hide]\n"
"Policy for handling deprecated management interfaces\n",
QEMU_ARCH_ALL)
SRST
@@ -3482,6 +3482,8 @@ SRST
Accept deprecated commands and arguments
``deprecated-input=reject``
Reject deprecated commands and arguments
+``deprecated-input=crash``
+Crash on deprecated commands and arguments
``deprecated-output=accept`` (default)
Emit deprecated command results and events
``deprecated-output=hide``
--
2.26.2
[PULL 07/11] qapi: Implement deprecated-output=hide for QMP introspection
This policy suppresses deprecated bits in output, and thus permits
"testing the future". Implement it for QMP command query-qmp-schema:
suppress information on deprecated commands, events and object type
members, i.e. anything that has the special feature flag "deprecated".
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210312153210.2810514-7-arm...@redhat.com>
---
monitor/qmp-cmds-control.c | 71 ++
1 file changed, 71 insertions(+)
diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
index 25afd0867f..bcfccc4ac4 100644
--- a/monitor/qmp-cmds-control.c
+++ b/monitor/qmp-cmds-control.c
@@ -158,6 +158,74 @@ EventInfoList *qmp_query_events(Error **errp)
return ev_list;
}
+static void *split_off_generic_list(void *list,
+bool (*splitp)(void *elt),
+void **part)
+{
+GenericList *keep = NULL, **keep_tailp = &keep;
+GenericList *split = NULL, **split_tailp = &split;
+GenericList *tail;
+
+for (tail = list; tail; tail = tail->next) {
+if (splitp(tail)) {
+*split_tailp = tail;
+split_tailp = &tail->next;
+} else {
+*keep_tailp = tail;
+keep_tailp = &tail->next;
+}
+}
+
+*keep_tailp = *split_tailp = NULL;
+*part = split;
+return keep;
+}
+
+static bool is_in(const char *s, strList *list)
+{
+strList *tail;
+
+for (tail = list; tail; tail = tail->next) {
+if (!strcmp(tail->value, s)) {
+return true;
+}
+}
+return false;
+}
+
+static bool is_entity_deprecated(void *link)
+{
+return is_in("deprecated", ((SchemaInfoList *)link)->value->features);
+}
+
+static bool is_member_deprecated(void *link)
+{
+return is_in("deprecated",
+ ((SchemaInfoObjectMemberList *)link)->value->features);
+}
+
+static SchemaInfoList *zap_deprecated(SchemaInfoList *schema)
+{
+void *to_zap;
+SchemaInfoList *tail;
+SchemaInfo *ent;
+
+schema = split_off_generic_list(schema, is_entity_deprecated, &to_zap);
+qapi_free_SchemaInfoList(to_zap);
+
+for (tail = schema; tail; tail = tail->next) {
+ent = tail->value;
+if (ent->meta_type == SCHEMA_META_TYPE_OBJECT) {
+ent->u.object.members
+= split_off_generic_list(ent->u.object.members,
+ is_member_deprecated, &to_zap);
+qapi_free_SchemaInfoObjectMemberList(to_zap);
+}
+}
+
+return schema;
+}
+
SchemaInfoList *qmp_query_qmp_schema(Error **errp)
{
QObject *obj = qobject_from_qlit(&qmp_schema_qlit);
@@ -171,5 +239,8 @@ SchemaInfoList *qmp_query_qmp_schema(Error **errp)
qobject_unref(obj);
visit_free(v);
+if (compat_policy.deprecated_output == COMPAT_POLICY_OUTPUT_HIDE) {
+return zap_deprecated(schema);
+}
return schema;
}
--
2.26.2
[PULL 10/11] qapi: Implement deprecated-input=reject for QMP command arguments
This policy rejects deprecated input, and thus permits "testing the
future". Implement it for QMP command arguments: reject commands with
deprecated ones. Example: when QEMU is run with -compat
deprecated-input=reject, then
{"execute": "eject", "arguments": {"device": "cd"}}
fails like this
{"error": {"class": "GenericError", "desc": "Deprecated parameter 'device'
disabled by policy"}}
When the deprecated parameter is removed, the error will change to
{"error": {"class": "GenericError", "desc": "Parameter 'device' is
unexpected"}}
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210312153210.2810514-10-arm...@redhat.com>
---
include/qapi/qobject-input-visitor.h | 9 +
include/qapi/visitor-impl.h | 3 +++
include/qapi/visitor.h | 9 +
qapi/qapi-visit-core.c | 9 +
qapi/qobject-input-visitor.c | 28
tests/unit/test-qmp-cmds.c | 25 +
qapi/trace-events| 1 +
scripts/qapi/commands.py | 2 +-
scripts/qapi/visit.py| 3 +++
9 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/include/qapi/qobject-input-visitor.h
b/include/qapi/qobject-input-visitor.h
index 95985e25e5..cbc54de4ac 100644
--- a/include/qapi/qobject-input-visitor.h
+++ b/include/qapi/qobject-input-visitor.h
@@ -58,6 +58,15 @@ typedef struct QObjectInputVisitor QObjectInputVisitor;
*/
Visitor *qobject_input_visitor_new(QObject *obj);
+/*
+ * Create a QObject input visitor for @obj for use with QMP
+ *
+ * This is like qobject_input_visitor_new(), except it obeys the
+ * policy for handling deprecated management interfaces set with
+ * -compat.
+ */
+Visitor *qobject_input_visitor_new_qmp(QObject *obj);
+
/*
* Create a QObject input visitor for @obj for use with keyval_parse()
*
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 2d853255a3..3b950f6e3d 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -113,6 +113,9 @@ struct Visitor
The core takes care of the return type in the public interface. */
void (*optional)(Visitor *v, const char *name, bool *present);
+/* Optional */
+bool (*deprecated_accept)(Visitor *v, const char *name, Error **errp);
+
/* Optional */
bool (*deprecated)(Visitor *v, const char *name);
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 4d23b59853..b3c9ef7a81 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -459,6 +459,15 @@ void visit_end_alternate(Visitor *v, void **obj);
*/
bool visit_optional(Visitor *v, const char *name, bool *present);
+/*
+ * Should we reject deprecated member @name?
+ *
+ * @name must not be NULL. This function is only useful between
+ * visit_start_struct() and visit_end_struct(), since only objects
+ * have deprecated members.
+ */
+bool visit_deprecated_accept(Visitor *v, const char *name, Error **errp);
+
/*
* Should we visit deprecated member @name?
*
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index d9726ecaa1..a641adec51 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -135,6 +135,15 @@ bool visit_optional(Visitor *v, const char *name, bool
*present)
return *present;
}
+bool visit_deprecated_accept(Visitor *v, const char *name, Error **errp)
+{
+trace_visit_deprecated_accept(v, name);
+if (v->deprecated_accept) {
+return v->deprecated_accept(v, name, errp);
+}
+return true;
+}
+
bool visit_deprecated(Visitor *v, const char *name)
{
trace_visit_deprecated(v, name);
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 23843b242e..1b8fa1f2f6 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -14,6 +14,7 @@
#include "qemu/osdep.h"
#include
+#include "qapi/compat-policy.h"
#include "qapi/error.h"
#include "qapi/qobject-input-visitor.h"
#include "qapi/visitor-impl.h"
@@ -43,6 +44,7 @@ typedef struct StackObject {
struct QObjectInputVisitor {
Visitor visitor;
+CompatPolicyInput deprecated_policy;
/* Root of visit at visitor creation. */
QObject *root;
@@ -662,6 +664,23 @@ static void qobject_input_optional(Visitor *v, const char
*name, bool *present)
*present = true;
}
+static bool qobject_input_deprecated_accept(Visitor *v, const char *name,
+Error **errp)
+{
+QObjectInputVisitor *qiv = to_qiv(v);
+
+switch (qiv->deprecated_policy) {
+case COMPAT_POLICY_INPUT_ACCEPT:
+return true;
+case COMPAT_POLICY_INPUT_REJECT:
+error_setg(errp, "Deprecated parameter '%s' disabled by policy",
+ name);
+return false;
+default:
+abort();
+}
+}
+
static void qobject_input_free(Visitor *v)
{
QObjectInputVisitor *qiv = to_qiv(v);
@@ -696,6 +715,7
[PULL 00/11] QAPI patches patches for 2021-03-16
The following changes since commit 2615a5e433aeb812c300d3a48e1a88e1303e2339: Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2021-03-15 19:23:00 +) are available in the Git repository at: git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-03-16 for you to fetch changes up to 5b728a7754e32ff6dac3501ded3ba820ef2edc7b: qapi: New -compat deprecated-input=crash (2021-03-16 11:10:38 +0100) QAPI patches patches for 2021-03-16 Markus Armbruster (10): qemu-options: New -compat to set policy for deprecated interfaces qapi: Implement deprecated-output=hide for QMP command results qapi: Implement deprecated-output=hide for QMP events qapi: Implement deprecated-output=hide for QMP event data monitor: Drop query-qmp-schema 'gen': false hack qapi: Implement deprecated-output=hide for QMP introspection test-util-sockets: Add stub for monitor_set_cur() qapi: Implement deprecated-input=reject for QMP commands qapi: Implement deprecated-input=reject for QMP command arguments qapi: New -compat deprecated-input=crash Paolo Bonzini (1): qemuutil: remove qemu_set_fd_handler duplicate symbol qapi/compat.json| 52 + qapi/introspect.json| 2 +- qapi/qapi-schema.json | 1 + include/qapi/compat-policy.h| 20 +++ include/qapi/qmp/dispatch.h | 1 + include/qapi/qobject-input-visitor.h| 9 +++ include/qapi/qobject-output-visitor.h | 9 +++ include/qapi/visitor-impl.h | 6 ++ include/qapi/visitor.h | 18 ++ monitor/monitor-internal.h | 3 - monitor/misc.c | 2 - monitor/qmp-cmds-control.c | 100 qapi/qapi-visit-core.c | 18 ++ qapi/qmp-dispatch.c | 17 ++ qapi/qobject-input-visitor.c| 29 + qapi/qobject-output-visitor.c | 19 ++ softmmu/vl.c| 17 ++ storage-daemon/qemu-storage-daemon.c| 2 - stubs/set-fd-handler.c | 10 tests/unit/test-qmp-cmds.c | 91 +++-- tests/unit/test-qmp-event.c | 41 + tests/unit/test-util-sockets.c | 1 + qapi/meson.build| 1 + qapi/trace-events | 2 + qemu-options.hx | 22 +++ scripts/qapi/commands.py| 14 +++-- scripts/qapi/events.py | 20 ++- scripts/qapi/visit.py | 15 + stubs/meson.build | 1 - tests/qapi-schema/qapi-schema-test.json | 20 --- tests/qapi-schema/qapi-schema-test.out | 20 --- 31 files changed, 522 insertions(+), 61 deletions(-) create mode 100644 qapi/compat.json create mode 100644 include/qapi/compat-policy.h delete mode 100644 stubs/set-fd-handler.c -- 2.26.2
[PULL 08/11] test-util-sockets: Add stub for monitor_set_cur()
Without this stub, the next commit fails to link. I suspect the real
cause is 947e47448d "monitor: Use getter/setter functions for
cur_mon".
Cc: Kevin Wolf
Signed-off-by: Markus Armbruster
Message-Id: <20210312153210.2810514-8-arm...@redhat.com>
Reviewed-by: Eric Blake
---
tests/unit/test-util-sockets.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/unit/test-util-sockets.c b/tests/unit/test-util-sockets.c
index 67486055ed..72b9246529 100644
--- a/tests/unit/test-util-sockets.c
+++ b/tests/unit/test-util-sockets.c
@@ -73,6 +73,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error
**errp)
* otherwise we get duplicate syms at link time.
*/
Monitor *monitor_cur(void) { return cur_mon; }
+Monitor *monitor_set_cur(Coroutine *co, Monitor *mon) { abort(); }
int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) { abort(); }
#ifndef _WIN32
--
2.26.2
[PULL 09/11] qapi: Implement deprecated-input=reject for QMP commands
This policy rejects deprecated input, and thus permits "testing the
future". Implement it for QMP commands: make deprecated ones fail.
Example: when QEMU is run with -compat deprecated-input=reject, then
{"execute": "query-cpus"}
fails like this
{"error": {"class": "CommandNotFound", "desc": "Deprecated command
query-cpus disabled by policy"}}
When the deprecated command is removed, the error will change to
{"error": {"class": "CommandNotFound", "desc": "The command query-cpus has
not been found"}}
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210312153210.2810514-9-arm...@redhat.com>
---
include/qapi/qmp/dispatch.h | 1 +
qapi/qmp-dispatch.c | 13 +
tests/unit/test-qmp-cmds.c | 24
scripts/qapi/commands.py| 10 +++---
4 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 1486cac3ef..8b974b570e 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -26,6 +26,7 @@ typedef enum QmpCommandOptions
QCO_ALLOW_OOB = (1U << 1),
QCO_ALLOW_PRECONFIG = (1U << 2),
QCO_COROUTINE = (1U << 3),
+QCO_DEPRECATED= (1U << 4),
} QmpCommandOptions;
typedef struct QmpCommand
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 45090f881a..cbc4452341 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -158,6 +158,19 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject
*request,
"The command %s has not been found", command);
goto out;
}
+if (cmd->options & QCO_DEPRECATED) {
+switch (compat_policy.deprecated_input) {
+case COMPAT_POLICY_INPUT_ACCEPT:
+break;
+case COMPAT_POLICY_INPUT_REJECT:
+error_set(&err, ERROR_CLASS_COMMAND_NOT_FOUND,
+ "Deprecated command %s disabled by policy",
+ command);
+goto out;
+default:
+abort();
+}
+}
if (!cmd->enabled) {
error_set(&err, ERROR_CLASS_COMMAND_NOT_FOUND,
"The command %s has been disabled for this instance",
diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c
index 1079d35122..cba982154b 100644
--- a/tests/unit/test-qmp-cmds.c
+++ b/tests/unit/test-qmp-cmds.c
@@ -281,6 +281,28 @@ static void test_dispatch_cmd_io(void)
qobject_unref(ret3);
}
+static void test_dispatch_cmd_deprecated(void)
+{
+const char *cmd = "{ 'execute': 'test-command-features1' }";
+QDict *ret;
+
+memset(&compat_policy, 0, sizeof(compat_policy));
+
+/* accept */
+ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+assert(ret && qdict_size(ret) == 0);
+qobject_unref(ret);
+
+compat_policy.has_deprecated_input = true;
+compat_policy.deprecated_input = COMPAT_POLICY_INPUT_ACCEPT;
+ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+assert(ret && qdict_size(ret) == 0);
+qobject_unref(ret);
+
+compat_policy.deprecated_input = COMPAT_POLICY_INPUT_REJECT;
+do_qmp_dispatch_error(false, ERROR_CLASS_COMMAND_NOT_FOUND, cmd);
+}
+
static void test_dispatch_cmd_ret_deprecated(void)
{
const char *cmd = "{ 'execute': 'test-features0' }";
@@ -379,6 +401,8 @@ int main(int argc, char **argv)
g_test_add_func("/qmp/dispatch_cmd_io", test_dispatch_cmd_io);
g_test_add_func("/qmp/dispatch_cmd_success_response",
test_dispatch_cmd_success_response);
+g_test_add_func("/qmp/dispatch_cmd_deprecated",
+test_dispatch_cmd_deprecated);
g_test_add_func("/qmp/dispatch_cmd_ret_deprecated",
test_dispatch_cmd_ret_deprecated);
g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 7adeda917b..f5d97454af 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -210,12 +210,16 @@ def gen_marshal(name: str,
def gen_register_command(name: str,
+ features: List[QAPISchemaFeature],
success_response: bool,
allow_oob: bool,
allow_preconfig: bool,
coroutine: bool) -> str:
options = []
+if 'deprecated' in [f.name for f in features]:
+options += ['QCO_DEPRECATED']
+
if not success_response:
options += ['QCO_NO_SUCCESS_RESP']
if allow_oob:
@@ -326,9 +330,9 @@ def visit_command(self,
self._genc.add(gen_marshal(name, arg_type, boxed, ret_type))
with self._temp_module('./init'):
with ifcontext(ifcond, self._genh, self._genc):
-self._genc.add(gen_register_command(name, success_response,
-allow_oob, allow_preconfig,
-
Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command
Philippe Mathieu-Daudé writes:
> On 3/16/21 10:02 AM, Philippe Mathieu-Daudé wrote:
>> On 3/16/21 7:51 AM, Markus Armbruster wrote:
>>> Eric Blake writes:
>>>
On 3/11/21 5:11 PM, Philippe Mathieu-Daudé wrote:
>>> [...]
> diff --git a/accel/accel-qmp.c b/accel/accel-qmp.c
> new file mode 100644
> index 000..f16e49b8956
> --- /dev/null
> +++ b/accel/accel-qmp.c
> @@ -0,0 +1,47 @@
> +/*
> + * QEMU accelerators, QMP commands
> + *
> + * Copyright (c) 2021 Red Hat Inc.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/qapi-commands-machine.h"
> +
> +static const Accelerator accel_list[] = {
> +ACCELERATOR_QTEST,
> +#ifdef CONFIG_TCG
> +ACCELERATOR_TCG,
> +#endif
> +#ifdef CONFIG_KVM
> +ACCELERATOR_KVM,
> +#endif
...would it be worth compiling the enum to only list enum values that
were actually compiled in? That would change it to:
{ 'enum': 'Accelerator',
'data': [ 'qtest',
{ 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
...
>>
>> These accelerator definitions are supposed to be poisoned in generic
>> code... But I like the simplicity of your suggestion, so I'll give it
>> a try and see what happens with removing the poisoned definitions.
>
> This is actually quite interesting :) Accelerator definitions are
> declared in config-target.h, but acceleration is host specific...
>
> We certainly don't want to make qapi target-specific.
We certainly don't want to make all the generated QAPI headers
target-specific.
We have made *selected* QAPI sub-modules target-specific, namely the
ones ending with "-target", currently qapi/machine-target.json and
qapi/misc-target.json. Only these may use poisoned symbols in 'if'
conditions. Example:
{ 'command': 'query-sev', 'returns': 'SevInfo',
'if': 'defined(TARGET_I386)' }
Re: [RFC v2 10/13] vhost: add vhost_kernel_set_vring_enable
On Tue, Mar 16, 2021 at 8:30 AM Jason Wang wrote:
>
>
> 在 2021/3/16 上午3:48, Eugenio Pérez 写道:
> > This method is already present in vhost-user. This commit adapts it to
> > vhost-net, so SVQ can use.
> >
> > vhost_kernel_set_enable stops the device, so qemu can ask for its status
> > (next available idx the device was going to consume). When SVQ starts it
> > can resume consuming the guest's driver ring, without notice from the
> > latter. Not stopping the device before of the swapping could imply that
> > it process more buffers than reported, what would duplicate the device
> > action.
>
>
> Note that it might not be the case of vDPA (virtio) or at least virtio
> needs some extension to achieve something similar like this. One example
> is virtio-pci which forbids 0 to be wrote to queue_enable.
>
> This is another reason to start from vhost-vDPA.
>
>
> >
> > Signed-off-by: Eugenio Pérez
> > ---
> > hw/virtio/vhost-backend.c | 29 +
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> > index 31b33bde37..1ac5c574a9 100644
> > --- a/hw/virtio/vhost-backend.c
> > +++ b/hw/virtio/vhost-backend.c
> > @@ -201,6 +201,34 @@ static int vhost_kernel_get_vq_index(struct vhost_dev
> > *dev, int idx)
> > return idx - dev->vq_index;
> > }
> >
> > +static int vhost_kernel_set_vq_enable(struct vhost_dev *dev, unsigned idx,
> > + bool enable)
> > +{
> > +struct vhost_vring_file file = {
> > +.index = idx,
> > +};
> > +
> > +if (!enable) {
> > +file.fd = -1; /* Pass -1 to unbind from file. */
> > +} else {
> > +struct vhost_net *vn_dev = container_of(dev, struct vhost_net,
> > dev);
> > +file.fd = vn_dev->backend;
>
>
> This can only work with vhost-net devices but not vsock/scsi etc.
>
Right. Shadow virtqueue code should also check the return value of the
vhost_set_vring_enable call.
I'm not sure how to solve it without resorting to some ifelse/switch
chain, checking for specific net/vsock/... features, or relaying on
some other qemu class facilities. However, since the main use case is
vDPA live migration, this commit could be left out and SVQ operation
would only be valid for vhost-vdpa and vhost-user.
> Thanks
>
>
> > +}
> > +
> > +return vhost_kernel_net_set_backend(dev, &file);
> > +}
> > +
> > +static int vhost_kernel_set_vring_enable(struct vhost_dev *dev, int enable)
> > +{
> > +int i;
> > +
> > +for (i = 0; i < dev->nvqs; ++i) {
> > +vhost_kernel_set_vq_enable(dev, i, enable);
> > +}
> > +
> > +return 0;
> > +}
> > +
> > #ifdef CONFIG_VHOST_VSOCK
> > static int vhost_kernel_vsock_set_guest_cid(struct vhost_dev *dev,
> > uint64_t guest_cid)
> > @@ -317,6 +345,7 @@ static const VhostOps kernel_ops = {
> > .vhost_set_owner = vhost_kernel_set_owner,
> > .vhost_reset_device = vhost_kernel_reset_device,
> > .vhost_get_vq_index = vhost_kernel_get_vq_index,
> > +.vhost_set_vring_enable = vhost_kernel_set_vring_enable,
> > #ifdef CONFIG_VHOST_VSOCK
> > .vhost_vsock_set_guest_cid = vhost_kernel_vsock_set_guest_cid,
> > .vhost_vsock_set_running = vhost_kernel_vsock_set_running,
>
[PULL 3/3] coreaudio: Handle output device change
From: Akihiko Odaki
An output device change can occur when plugging or unplugging an
earphone.
Signed-off-by: Akihiko Odaki
Message-Id: <20210311151512.22096-3-akihiko.od...@gmail.com>
Signed-off-by: Gerd Hoffmann
---
audio/coreaudio.c | 206 --
1 file changed, 164 insertions(+), 42 deletions(-)
diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index ab43cdcac199..578ec9b8b2e6 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -41,19 +41,21 @@ typedef struct coreaudioVoiceOut {
UInt32 audioDevicePropertyBufferFrameSize;
AudioStreamBasicDescription outputStreamBasicDescription;
AudioDeviceIOProcID ioprocid;
+bool enabled;
} coreaudioVoiceOut;
+static const AudioObjectPropertyAddress voice_addr = {
+kAudioHardwarePropertyDefaultOutputDevice,
+kAudioObjectPropertyScopeGlobal,
+kAudioObjectPropertyElementMaster
+};
+
static OSStatus coreaudio_get_voice(AudioDeviceID *id)
{
UInt32 size = sizeof(*id);
-AudioObjectPropertyAddress addr = {
-kAudioHardwarePropertyDefaultOutputDevice,
-kAudioObjectPropertyScopeGlobal,
-kAudioObjectPropertyElementMaster
-};
return AudioObjectGetPropertyData(kAudioObjectSystemObject,
- &addr,
+ &voice_addr,
0,
NULL,
&size,
@@ -258,18 +260,6 @@ static void GCC_FMT_ATTR (3, 4) coreaudio_logerr2 (
#define coreaudio_playback_logerr(status, ...) \
coreaudio_logerr2(status, "playback", __VA_ARGS__)
-static inline UInt32 isPlaying (AudioDeviceID outputDeviceID)
-{
-OSStatus status;
-UInt32 result = 0;
-status = coreaudio_get_isrunning(outputDeviceID, &result);
-if (status != kAudioHardwareNoError) {
-coreaudio_logerr(status,
- "Could not determine whether Device is playing\n");
-}
-return result;
-}
-
static int coreaudio_lock (coreaudioVoiceOut *core, const char *fn_name)
{
int err;
@@ -341,6 +331,11 @@ static OSStatus audioDeviceIOProc(
return 0;
}
+if (inDevice != core->outputDeviceID) {
+coreaudio_unlock (core, "audioDeviceIOProc(old device)");
+return 0;
+}
+
frameCount = core->audioDevicePropertyBufferFrameSize;
pending_frames = hw->pending_emul / hw->info.bytes_per_frame;
@@ -392,6 +387,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
/* get minimum and maximum buffer frame sizes */
status = coreaudio_get_framesizerange(core->outputDeviceID,
&frameRange);
+if (status == kAudioHardwareBadObjectError) {
+return 0;
+}
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr (status,
"Could not get device buffer frame
range\n");
@@ -411,6 +409,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
/* set Buffer Frame Size */
status = coreaudio_set_framesize(core->outputDeviceID,
&core->audioDevicePropertyBufferFrameSize);
+if (status == kAudioHardwareBadObjectError) {
+return 0;
+}
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr (status,
"Could not set device buffer frame size %"
PRIu32 "\n",
@@ -421,6 +422,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
/* get Buffer Frame Size */
status = coreaudio_get_framesize(core->outputDeviceID,
&core->audioDevicePropertyBufferFrameSize);
+if (status == kAudioHardwareBadObjectError) {
+return 0;
+}
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr (status,
"Could not get device buffer frame
size\n");
@@ -431,6 +435,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
/* get StreamFormat */
status = coreaudio_get_streamformat(core->outputDeviceID,
&core->outputStreamBasicDescription);
+if (status == kAudioHardwareBadObjectError) {
+return 0;
+}
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr (status,
"Could not get Device Stream
properties\n");
@@ -441,6 +448,9 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
/* set Samplerate */
status = coreaudio_set_streamformat(core->outputDeviceID,
&core->outputStreamBasicDescription);
+if (status == kAudioHardwareBadObjectError) {
+return 0;
+}
if (status != kAudioHardwareNoError) {
coreaudio_playback_logerr (status,
"Could not set samplerate %lf\n",
@@ -455,6 +465,9 @@ static
[PULL 2/3] coreaudio: Extract device operations
From: Akihiko Odaki
This change prepare to support dynamic device changes, which requires to
perform device initialization/deinitialization multiple times.
Signed-off-by: Akihiko Odaki
Message-Id: <20210311151512.22096-2-akihiko.od...@gmail.com>
Signed-off-by: Gerd Hoffmann
---
audio/coreaudio.c | 137 +++---
1 file changed, 80 insertions(+), 57 deletions(-)
diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index c5f0b615d643..ab43cdcac199 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -36,6 +36,8 @@ typedef struct coreaudioVoiceOut {
HWVoiceOut hw;
pthread_mutex_t mutex;
AudioDeviceID outputDeviceID;
+int frameSizeSetting;
+uint32_t bufferCount;
UInt32 audioDevicePropertyBufferFrameSize;
AudioStreamBasicDescription outputStreamBasicDescription;
AudioDeviceIOProcID ioprocid;
@@ -253,6 +255,9 @@ static void GCC_FMT_ATTR (3, 4) coreaudio_logerr2 (
coreaudio_logstatus (status);
}
+#define coreaudio_playback_logerr(status, ...) \
+coreaudio_logerr2(status, "playback", __VA_ARGS__)
+
static inline UInt32 isPlaying (AudioDeviceID outputDeviceID)
{
OSStatus status;
@@ -368,126 +373,100 @@ static OSStatus audioDeviceIOProc(
return 0;
}
-static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as,
- void *drv_opaque)
+static OSStatus init_out_device(coreaudioVoiceOut *core)
{
OSStatus status;
-coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
-int err;
-const char *typ = "playback";
AudioValueRange frameRange;
-Audiodev *dev = drv_opaque;
-AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out;
-int frames;
-struct audsettings obt_as;
-
-/* create mutex */
-err = pthread_mutex_init(&core->mutex, NULL);
-if (err) {
-dolog("Could not create mutex\nReason: %s\n", strerror (err));
-return -1;
-}
-
-obt_as = *as;
-as = &obt_as;
-as->fmt = AUDIO_FORMAT_F32;
-audio_pcm_init_info (&hw->info, as);
status = coreaudio_get_voice(&core->outputDeviceID);
if (status != kAudioHardwareNoError) {
-coreaudio_logerr2 (status, typ,
- "Could not get default output Device\n");
-return -1;
+coreaudio_playback_logerr (status,
+ "Could not get default output Device\n");
+return status;
}
if (core->outputDeviceID == kAudioDeviceUnknown) {
-dolog ("Could not initialize %s - Unknown Audiodevice\n", typ);
-return -1;
+dolog ("Could not initialize playback - Unknown Audiodevice\n");
+return status;
}
/* get minimum and maximum buffer frame sizes */
status = coreaudio_get_framesizerange(core->outputDeviceID,
&frameRange);
if (status != kAudioHardwareNoError) {
-coreaudio_logerr2 (status, typ,
- "Could not get device buffer frame range\n");
-return -1;
+coreaudio_playback_logerr (status,
+"Could not get device buffer frame
range\n");
+return status;
}
-frames = audio_buffer_frames(
-qapi_AudiodevCoreaudioPerDirectionOptions_base(cpdo), as, 11610);
-if (frameRange.mMinimum > frames) {
+if (frameRange.mMinimum > core->frameSizeSetting) {
core->audioDevicePropertyBufferFrameSize = (UInt32)
frameRange.mMinimum;
dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
-} else if (frameRange.mMaximum < frames) {
+} else if (frameRange.mMaximum < core->frameSizeSetting) {
core->audioDevicePropertyBufferFrameSize = (UInt32)
frameRange.mMaximum;
dolog ("warning: Downsizing Buffer Frames to %f\n",
frameRange.mMaximum);
} else {
-core->audioDevicePropertyBufferFrameSize = frames;
+core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
}
/* set Buffer Frame Size */
status = coreaudio_set_framesize(core->outputDeviceID,
&core->audioDevicePropertyBufferFrameSize);
if (status != kAudioHardwareNoError) {
-coreaudio_logerr2 (status, typ,
- "Could not set device buffer frame size %" PRIu32
"\n",
- (uint32_t)core->audioDevicePropertyBufferFrameSize);
-return -1;
+coreaudio_playback_logerr (status,
+"Could not set device buffer frame size %"
PRIu32 "\n",
+
(uint32_t)core->audioDevicePropertyBufferFrameSize);
+return status;
}
/* get Buffer Frame Size */
status = coreaudio_get_framesize(core->outputDeviceID,
&core->audioDevicePropertyBufferFrameSize);
if (status != kAudioHardwareNoError) {
-coreaudio_logerr2
[PULL 0/3] Audio 20210316 patches
The following changes since commit 2615a5e433aeb812c300d3a48e1a88e1303e2339: Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/block-pull-reque= st' into staging (2021-03-15 19:23:00 +) are available in the Git repository at: git://git.kraxel.org/qemu tags/audio-20210316-pull-request for you to fetch changes up to 3ba6e3f6888d2825709eba2f623f0615069c036c: coreaudio: Handle output device change (2021-03-16 07:17:50 +0100) coreaudio fixes and cleanups. Akihiko Odaki (3): coreaudio: Drop support for macOS older than 10.6 coreaudio: Extract device operations coreaudio: Handle output device change audio/coreaudio.c | 428 +- 1 file changed, 235 insertions(+), 193 deletions(-) --=20 2.30.2
[PULL 1/3] coreaudio: Drop support for macOS older than 10.6
From: Akihiko Odaki
Mac OS X 10.6 was released in 2009.
Signed-off-by: Akihiko Odaki
Reviewed-by: Peter Maydell
Message-Id: <20210311151512.22096-1-akihiko.od...@gmail.com>
Signed-off-by: Gerd Hoffmann
---
audio/coreaudio.c | 103 --
1 file changed, 103 deletions(-)
diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index b7c02e0e516d..c5f0b615d643 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -32,10 +32,6 @@
#define AUDIO_CAP "coreaudio"
#include "audio_int.h"
-#ifndef MAC_OS_X_VERSION_10_6
-#define MAC_OS_X_VERSION_10_6 1060
-#endif
-
typedef struct coreaudioVoiceOut {
HWVoiceOut hw;
pthread_mutex_t mutex;
@@ -45,9 +41,6 @@ typedef struct coreaudioVoiceOut {
AudioDeviceIOProcID ioprocid;
} coreaudioVoiceOut;
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_6
-/* The APIs used here only become available from 10.6 */
-
static OSStatus coreaudio_get_voice(AudioDeviceID *id)
{
UInt32 size = sizeof(*id);
@@ -169,102 +162,6 @@ static OSStatus coreaudio_get_isrunning(AudioDeviceID id,
UInt32 *result)
&size,
result);
}
-#else
-/* Legacy versions of functions using deprecated APIs */
-
-static OSStatus coreaudio_get_voice(AudioDeviceID *id)
-{
-UInt32 size = sizeof(*id);
-
-return AudioHardwareGetProperty(
-kAudioHardwarePropertyDefaultOutputDevice,
-&size,
-id);
-}
-
-static OSStatus coreaudio_get_framesizerange(AudioDeviceID id,
- AudioValueRange *framerange)
-{
-UInt32 size = sizeof(*framerange);
-
-return AudioDeviceGetProperty(
-id,
-0,
-0,
-kAudioDevicePropertyBufferFrameSizeRange,
-&size,
-framerange);
-}
-
-static OSStatus coreaudio_get_framesize(AudioDeviceID id, UInt32 *framesize)
-{
-UInt32 size = sizeof(*framesize);
-
-return AudioDeviceGetProperty(
-id,
-0,
-false,
-kAudioDevicePropertyBufferFrameSize,
-&size,
-framesize);
-}
-
-static OSStatus coreaudio_set_framesize(AudioDeviceID id, UInt32 *framesize)
-{
-UInt32 size = sizeof(*framesize);
-
-return AudioDeviceSetProperty(
-id,
-NULL,
-0,
-false,
-kAudioDevicePropertyBufferFrameSize,
-size,
-framesize);
-}
-
-static OSStatus coreaudio_get_streamformat(AudioDeviceID id,
- AudioStreamBasicDescription *d)
-{
-UInt32 size = sizeof(*d);
-
-return AudioDeviceGetProperty(
-id,
-0,
-false,
-kAudioDevicePropertyStreamFormat,
-&size,
-d);
-}
-
-static OSStatus coreaudio_set_streamformat(AudioDeviceID id,
- AudioStreamBasicDescription *d)
-{
-UInt32 size = sizeof(*d);
-
-return AudioDeviceSetProperty(
-id,
-0,
-0,
-0,
-kAudioDevicePropertyStreamFormat,
-size,
-d);
-}
-
-static OSStatus coreaudio_get_isrunning(AudioDeviceID id, UInt32 *result)
-{
-UInt32 size = sizeof(*result);
-
-return AudioDeviceGetProperty(
-id,
-0,
-0,
-kAudioDevicePropertyDeviceIsRunning,
-&size,
-result);
-}
-#endif
static void coreaudio_logstatus (OSStatus status)
{
--
2.30.2
Re: [PATCH] hw/core: Only build guest-loader if libfdt is available
Philippe Mathieu-Daudé writes:
> Add a Kconfig entry for guest-loader so we can optionally deselect
> it (default is built in), and add a Meson dependency on libfdt.
>
> This fixes when building with --disable-fdt:
>
> /usr/bin/ld: libcommon.fa.p/hw_core_guest-loader.c.o: in function
> `loader_insert_platform_data':
> hw/core/guest-loader.c:56: undefined reference to `qemu_fdt_add_subnode'
> /usr/bin/ld: hw/core/guest-loader.c:57: undefined reference to
> `qemu_fdt_setprop'
> /usr/bin/ld: hw/core/guest-loader.c:61: undefined reference to
> `qemu_fdt_setprop_string_array'
> /usr/bin/ld: hw/core/guest-loader.c:68: undefined reference to
> `qemu_fdt_setprop_string'
> /usr/bin/ld: hw/core/guest-loader.c:74: undefined reference to
> `qemu_fdt_setprop_string_array'
> collect2: error: ld returned 1 exit status
>
> Fixes: a33ff6d2c6b ("hw/core: implement a guest-loader to support static
> hypervisor guests")
> Reported-by: Christian Borntraeger
> Tested-by: Christian Borntraeger
> Signed-off-by: Philippe Mathieu-Daudé
Queued to for-6.0/assorted-fixes, thanks.
--
Alex Bennée
Re: [PATCH] target/mips/mxu: Rewrite D16MIN / D16MAX opcodes
On Mon, 15 Mar 2021 at 22:39, Philippe Mathieu-Daudé wrote:
>
> Coverity reported (CID 1450831) an array overrun in
> gen_mxu_D16MAX_D16MIN():
>
> 1103 } else if (unlikely((XRb == 0) || (XRa == 0))) {
>
> 1112 if (opc == OPC_MXU_D16MAX) {
> 1113 tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> 1114 } else {
> 1115 tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> 1116 }
>
> >>> Overrunning array "mxu_gpr" of 15 8-byte elements at element
> index 4294967295 (byte offset 34359738367) using index "XRa - 1U"
> (which evaluates to 4294967295).
>
> Because we check if 'XRa == 0' then access 'XRa - 1' in array.
>
> I figured it could be easier to rewrite this function to something
> simpler rather than trying to understand it.
This seems to drop half of the optimised cases the old
code had. Wouldn't it be simpler just to fix the bugs
in the conditions?
That is:
> if (unlikely(pad != 0)) {
> /* opcode padding incorrect -> do nothing */
> -} else if (unlikely(XRc == 0)) {
> -/* destination is zero register -> do nothing */
This should be "XRa == 0"
> -} else if (unlikely((XRb == 0) && (XRa == 0))) {
> -/* both operands zero registers -> just set destination to zero */
This should be "XRb == 0 && XRc == 0"
> -tcg_gen_movi_i32(mxu_gpr[XRc - 1], 0);
This should set mxu_gpr[XRa - 1]
> -} else if (unlikely((XRb == 0) || (XRa == 0))) {
This should be "XRb == 0 || XRc == 0"
And everything else in the function looks OK to me.
thanks
-- PMM
Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command
On 3/16/21 11:26 AM, Philippe Mathieu-Daudé wrote:
> On 3/16/21 10:02 AM, Philippe Mathieu-Daudé wrote:
>> On 3/16/21 7:51 AM, Markus Armbruster wrote:
>>> Eric Blake writes:
>>>
On 3/11/21 5:11 PM, Philippe Mathieu-Daudé wrote:
>>> [...]
> diff --git a/accel/accel-qmp.c b/accel/accel-qmp.c
> new file mode 100644
> index 000..f16e49b8956
> --- /dev/null
> +++ b/accel/accel-qmp.c
> @@ -0,0 +1,47 @@
> +/*
> + * QEMU accelerators, QMP commands
> + *
> + * Copyright (c) 2021 Red Hat Inc.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/qapi-commands-machine.h"
> +
> +static const Accelerator accel_list[] = {
> +ACCELERATOR_QTEST,
> +#ifdef CONFIG_TCG
> +ACCELERATOR_TCG,
> +#endif
> +#ifdef CONFIG_KVM
> +ACCELERATOR_KVM,
> +#endif
...would it be worth compiling the enum to only list enum values that
were actually compiled in? That would change it to:
{ 'enum': 'Accelerator',
'data': [ 'qtest',
{ 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
...
>>
>> These accelerator definitions are supposed to be poisoned in generic
>> code... But I like the simplicity of your suggestion, so I'll give it
>> a try and see what happens with removing the poisoned definitions.
>
> This is actually quite interesting :) Accelerator definitions are
> declared in config-target.h, but acceleration is host specific...
Thomas, I guess I hit Claudio's reported bug again...
1/ generic libqemuutil.a is built without any CONFIG_accel definition.
So this qapi-generated enum ... :
typedef enum Accelerator {
ACCELERATOR_QTEST,
#if defined(CONFIG_TCG)
ACCELERATOR_TCG,
#endif /* defined(CONFIG_TCG) */
#if defined(CONFIG_KVM)
ACCELERATOR_KVM,
#endif /* defined(CONFIG_KVM) */
#if defined(CONFIG_HAX)
ACCELERATOR_HAX,
#endif /* defined(CONFIG_HAX) */
#if defined(CONFIG_HVF)
ACCELERATOR_HVF,
#endif /* defined(CONFIG_HVF) */
#if defined(CONFIG_WHPX)
ACCELERATOR_WHPX,
#endif /* defined(CONFIG_WHPX) */
#if defined(CONFIG_XEN_BACKEND)
ACCELERATOR_XEN,
#endif /* defined(CONFIG_XEN_BACKEND) */
ACCELERATOR__MAX,
} Accelerator;
... is expanded to:
typedef enum Accelerator {
ACCELERATOR_QTEST,
ACCELERATOR__MAX,
} Accelerator;
2/ softmmu code and qtest do get the definition, the enum
is different:
typedef enum Accelerator {
ACCELERATOR_QTEST,
ACCELERATOR_KVM,
ACCELERATOR__MAX,
} Accelerator;
qmp_query_accels() fills AcceleratorInfoList with 2 entries
3/ trying to understand what's happening, query-qmp-schema
returns:
{
"name": "206",
"tag": "name",
"variants": [
{
"case": "qtest",
"type": "0"
},
{
"case": "kvm",
"type": "0"
}
],
"members": [
{
"name": "name",
"type": "403"
}
],
"meta-type": "object"
},
{
"name": "403",
"meta-type": "enum",
"values": [
"qtest",
"kvm"
]
},
So accelerators are listed, but with the same enum index?
4/ Running 'query-accels' aborts in qapi_enum_lookup():
The first entry 'qtest' is visited correctly.
When the second entry 'kvm' is visited, this assertion fires:
assert(val >= 0 && val < lookup->size);
because lookup->size = 1 (remember from 1/ Accelerator_lookup
has been compiled without definitions, in libqemuutil.a).
I'll keep the current patch, as it works, and address the rest
of this series comments.
Thanks,
Phil.
Re: [PULL 0/3] Audio 20210316 patches
Patchew URL:
https://patchew.org/QEMU/20210316104745.2196286-1-kra...@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210316104745.2196286-1-kra...@redhat.com
Subject: [PULL 0/3] Audio 20210316 patches
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20210316104745.2196286-1-kra...@redhat.com ->
patchew/20210316104745.2196286-1-kra...@redhat.com
Switched to a new branch 'test'
54c97ba coreaudio: Handle output device change
3013a57 coreaudio: Extract device operations
45d1cb4 coreaudio: Drop support for macOS older than 10.6
=== OUTPUT BEGIN ===
1/3 Checking commit 45d1cb4bb7c7 (coreaudio: Drop support for macOS older than
10.6)
2/3 Checking commit 3013a5778d0a (coreaudio: Extract device operations)
ERROR: space prohibited between function name and open parenthesis '('
#76: FILE: audio/coreaudio.c:383:
+coreaudio_playback_logerr (status,
ERROR: space prohibited between function name and open parenthesis '('
#83: FILE: audio/coreaudio.c:388:
+dolog ("Could not initialize playback - Unknown Audiodevice\n");
ERROR: space prohibited between function name and open parenthesis '('
#94: FILE: audio/coreaudio.c:396:
+coreaudio_playback_logerr (status,
ERROR: space prohibited between function name and open parenthesis '('
#122: FILE: audio/coreaudio.c:415:
+coreaudio_playback_logerr (status,
ERROR: line over 90 characters
#123: FILE: audio/coreaudio.c:416:
+"Could not set device buffer frame size %"
PRIu32 "\n",
WARNING: line over 80 characters
#124: FILE: audio/coreaudio.c:417:
+
(uint32_t)core->audioDevicePropertyBufferFrameSize);
ERROR: space prohibited between function name and open parenthesis '('
#135: FILE: audio/coreaudio.c:425:
+coreaudio_playback_logerr (status,
WARNING: line over 80 characters
#141: FILE: audio/coreaudio.c:429:
+core->hw.samples = core->bufferCount *
core->audioDevicePropertyBufferFrameSize;
ERROR: space prohibited between function name and open parenthesis '('
#149: FILE: audio/coreaudio.c:435:
+coreaudio_playback_logerr (status,
ERROR: space prohibited between function name and open parenthesis '('
#164: FILE: audio/coreaudio.c:445:
+coreaudio_playback_logerr (status,
WARNING: line over 80 characters
#166: FILE: audio/coreaudio.c:447:
+
core->outputStreamBasicDescription.mSampleRate);
ERROR: space prohibited between function name and open parenthesis '('
#181: FILE: audio/coreaudio.c:459:
+coreaudio_playback_logerr (status, "Could not set IOProc\n");
ERROR: space prohibited between function name and open parenthesis '('
#218: FILE: audio/coreaudio.c:501:
+dolog("Could not create mutex\nReason: %s\n", strerror (err));
ERROR: space prohibited between function name and open parenthesis '('
#225: FILE: audio/coreaudio.c:508:
+audio_pcm_init_info (&hw->info, as);
ERROR: space prohibited between function name and open parenthesis '('
#240: FILE: audio/coreaudio.c:523:
+static void coreaudio_fini_out (HWVoiceOut *hw)
total: 12 errors, 3 warnings, 224 lines checked
Patch 2/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/3 Checking commit 54c97bac0510 (coreaudio: Handle output device change)
ERROR: space prohibited between function name and open parenthesis '('
#74: FILE: audio/coreaudio.c:335:
+coreaudio_unlock (core, "audioDeviceIOProc(old device)");
ERROR: line over 90 characters
#160: FILE: audio/coreaudio.c:495:
+if (status != kAudioHardwareBadDeviceError && status !=
kAudioHardwareNoError) {
WARNING: line over 80 characters
#170: FILE: audio/coreaudio.c:504:
+if (status != kAudioHardwareBadDeviceError && status !=
kAudioHardwareNoError) {
ERROR: line over 90 characters
#195: FILE: audio/coreaudio.c:529:
+if (status != kAudioHardwareBadDeviceError && status !=
kAudioHardwareNoError) {
ERROR: space prohibited between function name and open parenthesis '('
#196: FILE: audio/coreaudio.c:530:
+coreaudio_logerr (status, "Could not resume playback\n");
ERROR: line over 90 characters
#204: FILE: audio/coreaudio.c:538:
+if (status != kAudioHa
Re: [PULL v2 0/5] Meson version update
On Mon, 15 Mar 2021 at 22:04, Peter Maydell wrote: > > On Mon, 15 Mar 2021 at 17:47, Paolo Bonzini wrote: > > > > The following changes since commit 51204c2f188ec1e2a38f14718d38a3772f850a4b: > > > > Merge remote-tracking branch > > 'remotes/bkoppelmann2/tags/pull-tricore-20210314' into staging (2021-03-15 > > 15:34:27 +) > > > > are available in the Git repository at: > > > > https://gitlab.com/bonzini/qemu.git tags/for-upstream-meson-0.57 > > > > for you to fetch changes up to 57d42c3b774d0716b9ad1a5a576480521edc7201: > > > > hexagon: use env keyword argument to pass PYTHONPATH (2021-03-15 18:06:21 > > +0100) > > > > v1->v2: rebased > > > > > > Update Meson to 0.57. > > > > > > Paolo Bonzini (5): > > hexagon: do not specify executables as inputs > > hexagon: do not specify Python scripts as inputs > > meson: bump submodule to 0.57.1 > > meson: switch minimum meson version to 0.57.0 > > hexagon: use env keyword argument to pass PYTHONPATH > > Fails to build, on at least x86-64, s390, ppc, aarch32, aarch64 > Linux hosts: This pullreq also caused a build failure on OSX when trying to build something else after rolling back the merge (but only on OSX, oddly): make: Entering directory '/Users/pm215/src/qemu-for-merges/build/all' config-host.mak is out-of-date, running configure GIT ui/keycodemapdb tests/fp/berkeley-testfloat-3 tests/fp/berkeley-softfloat-3 meson dtc capstone slirp Disabling PIE due to missing toolchain support /usr/local/bin/ninja build.ninja && touch build.ninja.stamp [0/1] Regenerating build files. Traceback (most recent call last): File "/Users/pm215/src/qemu-for-merges/meson/mesonbuild/mesonmain.py", line 140, in run return options.run_func(options) File "/Users/pm215/src/qemu-for-merges/meson/mesonbuild/msetup.py", line 245, in run app.generate() File "/Users/pm215/src/qemu-for-merges/meson/mesonbuild/msetup.py", line 154, in generate env = environment.Environment(self.source_dir, self.build_dir, self.options) File "/Users/pm215/src/qemu-for-merges/meson/mesonbuild/environment.py", line 523, in __init__ self.coredata = coredata.load(self.get_build_dir()) File "/Users/pm215/src/qemu-for-merges/meson/mesonbuild/coredata.py", line 1016, in load obj = pickle.load(f) ModuleNotFoundError: No module named 'mesonbuild.mesonlib.universal'; 'mesonbuild.mesonlib' is not a package FAILED: build.ninja /usr/local/opt/python@3.8/bin/python3.8 /Users/pm215/src/qemu-for-merges/meson/meson.py --internal regenerate /Users/pm215/src/qemu-for-merges /Users/pm215/src/qemu-for-merges/build/all --backend ninja ninja: error: rebuilding 'build.ninja': subcommand failed This kind of "breaks the build tree such that you can't just 'git reset' back to master" bug is irritating. Doesn't meson have an "is this the right version" check on the cached data it's loading out of the build tree ? thanks -- PMM
[PATCH v2] hw/isa/Kconfig: Add missing dependency VIA VT82C686 -> APM
TYPE_VIA_PM calls apm_init() in via_pm_realize(), so requires APM to be selected. Reported-by: BALATON Zoltan Signed-off-by: Philippe Mathieu-Daudé --- Rebased on usb-20210315-pull-request Based-on: <20210315180240.1597240-1-kra...@redhat.com> --- hw/isa/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig index 2691eae2f0c..55e0003ce40 100644 --- a/hw/isa/Kconfig +++ b/hw/isa/Kconfig @@ -48,6 +48,7 @@ config VT82C686 select SERIAL_ISA select FDC select USB_UHCI +select APM config SMC37C669 bool -- 2.26.2
Re: [PATCH] migration: Move populate_vfio_info() into a separate file
* Thomas Huth (th...@redhat.com) wrote:
> On 15/03/2021 22.05, Philippe Mathieu-Daudé wrote:
> > Hi Thomas,
> >
> > +Alex
> >
> > On 3/15/21 8:07 PM, Thomas Huth wrote:
> > > The CONFIG_VFIO switch only works in target specific code. Since
> > > migration/migration.c is common code, the #ifdef does not have
> > > the intended behavior here. Move the related code to a separate
> > > file now which gets compiled via specific_ss instead.
> > >
> > > Fixes: 3710586caa ("qapi: Add VFIO devices migration stats in Migration
> > > stats")
> > > Signed-off-by: Thomas Huth
> > > ---
> > > migration/meson.build | 3 ++-
> > > migration/migration.c | 15 ---
> > > migration/migration.h | 2 ++
> > > migration/special.c | 25 +
> > > 4 files changed, 29 insertions(+), 16 deletions(-)
> > > create mode 100644 migration/special.c
> > >
> > > diff --git a/migration/meson.build b/migration/meson.build
> > > index 9645f44005..e1f72f6ba0 100644
> > > --- a/migration/meson.build
> > > +++ b/migration/meson.build
> > > @@ -30,4 +30,5 @@ softmmu_ss.add(when: ['CONFIG_RDMA', rdma], if_true:
> > > files('rdma.c'))
> > > softmmu_ss.add(when: 'CONFIG_LIVE_BLOCK_MIGRATION', if_true:
> > > files('block.c'))
> > > softmmu_ss.add(when: zstd, if_true: files('multifd-zstd.c'))
> > > -specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: files('dirtyrate.c',
> > > 'ram.c'))
> > > +specific_ss.add(when: 'CONFIG_SOFTMMU',
> > > +if_true: files('dirtyrate.c', 'ram.c', 'special.c'))
> >
> > Why not simply name this migration/vfio.c? That way we do not start
> > mixed bag of everything target specific.
>
> I don't mind ... well, if we have other small functions there in the future
> that depend on CONFIG switches, a mixed bag file might not be such a bad
> idea instead of having lots and lots of small other files ... OTOH, if there
> is more vfio migration code in the works that might fit here, a name like
> vfio.c would be better, of course. What do the maintainers think?
Could this be done with stubs instead of an ifdef; i.e. a stub of
'vfio_mig_active' and 'vfio_mig_bytes_transferred'?
As for naming 'special' is too generic.
'vfio' is too specific (especially since most vfio code ends up under
hw/vfio)
how about migration/target.c as something which is explicit about why
it's done that way.
Dave
> Thomas
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[PULL 0/1] 9p queue 2021-03-16
The following changes since commit 2615a5e433aeb812c300d3a48e1a88e1303e2339: Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2021-03-15 19:23:00 +) are available in the Git repository at: https://github.com/cschoenebeck/qemu.git tags/pull-9p-20210316 for you to fetch changes up to e4fd889f51094a8e76274ca1e9e0ed70375166f0: hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD (2021-03-16 11:41:49 +0100) 9pfs: code cleanup * Use lock-guard design pattern instead of manual lock/unlock. Mahmoud Mandour (1): hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD hw/9pfs/9p-synth.c | 12 1 file changed, 4 insertions(+), 8 deletions(-)
[PULL 1/1] hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD
From: Mahmoud Mandour
Replaced a call to qemu_mutex_lock and its respective call to
qemu_mutex_unlock and used QEMU_LOCK_GUARD macro in their place.
This simplifies the code by removing the call required to unlock
and also eliminates goto paths.
Signed-off-by: Mahmoud Mandour
Acked-by: Greg Kurz
Reviewed-by: Christian Schoenebeck
Message-Id: <20210311031538.5325-9-ma.mando...@gmail.com>
Signed-off-by: Christian Schoenebeck
---
hw/9pfs/9p-synth.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index 7eb210ffa8..473ef914b0 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -79,11 +79,11 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
if (!parent) {
parent = &synth_root;
}
-qemu_mutex_lock(&synth_mutex);
+QEMU_LOCK_GUARD(&synth_mutex);
QLIST_FOREACH(tmp, &parent->child, sibling) {
if (!strcmp(tmp->name, name)) {
ret = EEXIST;
-goto err_out;
+return ret;
}
}
/* Add the name */
@@ -94,8 +94,6 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
node->attr, node->attr->inode);
*result = node;
ret = 0;
-err_out:
-qemu_mutex_unlock(&synth_mutex);
return ret;
}
@@ -116,11 +114,11 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int
mode,
parent = &synth_root;
}
-qemu_mutex_lock(&synth_mutex);
+QEMU_LOCK_GUARD(&synth_mutex);
QLIST_FOREACH(tmp, &parent->child, sibling) {
if (!strcmp(tmp->name, name)) {
ret = EEXIST;
-goto err_out;
+return ret;
}
}
/* Add file type and remove write bits */
@@ -136,8 +134,6 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int
mode,
pstrcpy(node->name, sizeof(node->name), name);
QLIST_INSERT_HEAD_RCU(&parent->child, node, sibling);
ret = 0;
-err_out:
-qemu_mutex_unlock(&synth_mutex);
return ret;
}
--
2.20.1
Re: [PATCH v7 0/8] Pegasos2 emulation
On 3/16/21 10:01 AM, Laurent Vivier wrote: > Le 15/03/2021 à 13:33, BALATON Zoltan a écrit : >> On Sat, 13 Mar 2021, BALATON Zoltan wrote: >>> On Wed, 10 Mar 2021, BALATON Zoltan wrote: Hello, >>> >>> I've started posting this series well in advance to get it into 6.0 and yet >>> it seems like it may >>> be missing it due to organisational issues (no real complaints were found >>> with patches but >>> Philippe seems to like more review that does not seem to happen as nobody >>> is interested). Looks >>> like David is waiting for an ack from Philippe but will be away next week >>> so if this is not >>> resolved now it may be too late on Monday. To avoid that: >>> >>> David, could you please send an ack before you leave for the last two >>> patches so it could get >>> committed via some other tree while you're away? >>> >>> Philippe, if you can't ack the vt82c686 patches now are you OK with taking >>> the whole series via >>> your tree before the freeze? That would give you some more days to review >>> and it could always be >>> reverted during the freeze but if it's not merged now I'll have to wait >>> until the summer to get it >>> in again which would be another long delay. I don't think this will get >>> more reviews unless it's >>> in master and people can start using and testing it better. >> >> Hello, >> >> Since David seems to be away for this week before seeing my mail asking for >> an ack from him, now >> this can only get in by Philippe or Peter. (David said before he'd be OK >> with the series if Philippe >> acked it so I think that can count as an implicit ack and it could always be >> reverted before the >> releease.) >> >> Philippe, do you have anything against this to get merged now? If not please >> send a pull or ack it >> so it has a chance to be in 6.0 or tell if you still intend to do anything >> about it before the >> freeze. This series was on the list since January and the remaining parts >> you did not take are here >> since February 22nd and the version after your first review since two weeks >> so it would be nice to >> sort this out and not block it any further without a good reason. > > Pegasos looks like a New World PowerMac, so perhaps Mark can help? The PPC part is mostly reviewed. The problem is the first patch: "vt82c686: Implement control of serial port io ranges via config regs". I don't understand it. Zoltan said Paolo isn't acking it because he doesn't mind. I prefer to be cautious and think than Paolo is rather too busy. Laurent, Peter, can you have a look at it? Thanks, Phil.
[Bug 1880518] Re: issue while installing docker inside s390x container
Any pointers about this issue? Any other steps needs to be followed to
resolve the issue?
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1880518
Title:
issue while installing docker inside s390x container
Status in QEMU:
New
Bug description:
This is in reference to
https://github.com/multiarch/qemu-user-static/issues/108.
I am facing issue while installing docker inside s390x container under qemu
on Ubuntu 18.04 host running on amd64.
Following are the contents of /proc/sys/fs/binfmt_misc/qemu-s390x on Intel
host after running
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
enabled
interpreter /usr/bin/qemu-s390x-static
flags: F
offset 0
magic 7f454c46020201020016
mask ff00fffe
I could get docker service up with the following trick.
printf '{"iptables": false,"ip-masq": false,"bridge": "none" }' >
/etc/docker/daemon.json
But even though docker service comes up, images are not getting pulled,
docker pull fails with the following error.
failed to register layer: Error processing tar file(exit status 1):
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1880518/+subscriptions
Re: [PATCH] target/mips/mxu: Rewrite D16MIN / D16MAX opcodes
On 3/16/21 11:51 AM, Peter Maydell wrote:
> On Mon, 15 Mar 2021 at 22:39, Philippe Mathieu-Daudé wrote:
>>
>> Coverity reported (CID 1450831) an array overrun in
>> gen_mxu_D16MAX_D16MIN():
>>
>> 1103 } else if (unlikely((XRb == 0) || (XRa == 0))) {
>>
>> 1112 if (opc == OPC_MXU_D16MAX) {
>> 1113 tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
>> 1114 } else {
>> 1115 tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
>> 1116 }
>>
> Overrunning array "mxu_gpr" of 15 8-byte elements at element
>> index 4294967295 (byte offset 34359738367) using index "XRa - 1U"
>> (which evaluates to 4294967295).
>>
>> Because we check if 'XRa == 0' then access 'XRa - 1' in array.
>>
>> I figured it could be easier to rewrite this function to something
>> simpler rather than trying to understand it.
>
> This seems to drop half of the optimised cases the old
> code had. Wouldn't it be simpler just to fix the bugs
> in the conditions?
>
> That is:
>
>> if (unlikely(pad != 0)) {
>> /* opcode padding incorrect -> do nothing */
>> -} else if (unlikely(XRc == 0)) {
>> -/* destination is zero register -> do nothing */
>
> This should be "XRa == 0"
>
>> -} else if (unlikely((XRb == 0) && (XRa == 0))) {
>> -/* both operands zero registers -> just set destination to zero */
>
> This should be "XRb == 0 && XRc == 0"
>
>> -tcg_gen_movi_i32(mxu_gpr[XRc - 1], 0);
>
> This should set mxu_gpr[XRa - 1]
>
>> -} else if (unlikely((XRb == 0) || (XRa == 0))) {
>
> This should be "XRb == 0 || XRc == 0"
>
> And everything else in the function looks OK to me.
If you have the changes clear, do you mind sending a patch?
Thanks,
Phil.
[PATCH v4 01/12] net: eth: Add a helper to pad a short Ethernet frame
Add a helper to pad a short Ethernet frame to the minimum required
length, which can be used by backend codes.
Signed-off-by: Bin Meng
---
Changes in v4:
- change 'ethernet' to 'Ethernet'
- do not inline the helper
- check the padded buffer size to avoid buffer overflow
Changes in v3:
- use 'without' instead of 'sans'
- add a helper to pad short frames
include/net/eth.h | 17 +
net/eth.c | 17 +
2 files changed, 34 insertions(+)
diff --git a/include/net/eth.h b/include/net/eth.h
index 0671be6916..6aabbdd0d3 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -31,6 +31,7 @@
#define ETH_ALEN 6
#define ETH_HLEN 14
+#define ETH_ZLEN 60 /* Min. octets in frame without FCS */
struct eth_header {
uint8_t h_dest[ETH_ALEN]; /* destination eth addr */
@@ -422,4 +423,20 @@ bool
eth_parse_ipv6_hdr(const struct iovec *pkt, int pkt_frags,
size_t ip6hdr_off, eth_ip6_hdr_info *info);
+/**
+ * eth_pad_short_frame - pad a short frame to the minimum Ethernet frame length
+ *
+ * If the Ethernet frame size is shorter than 60 bytes, it will be padded to
+ * 60 bytes at the address @padded_pkt.
+ *
+ * @padded_pkt: buffer address to hold the padded frame
+ * @padded_buflen: buffer length of @padded_pkt. If the frame is padded, it is
+ * written to ETH_ZLEN, otherwise it remains unchanged.
+ * @pkt: address to hold the original Ethernet frame
+ * @pkt_size: size of the original Ethernet frame
+ * @return true if the frame is padded, otherwise false
+ */
+bool eth_pad_short_frame(uint8_t *padded_pkt, size_t *padded_buflen,
+ const void *pkt, size_t pkt_size);
+
#endif
diff --git a/net/eth.c b/net/eth.c
index 1e0821c5f8..f913e4396f 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -548,3 +548,20 @@ bool eth_parse_ipv6_hdr(const struct iovec *pkt, int
pkt_frags,
info->l4proto = ext_hdr.ip6r_nxt;
return true;
}
+
+bool eth_pad_short_frame(uint8_t *padded_pkt, size_t *padded_buflen,
+ const void *pkt, size_t pkt_size)
+{
+assert(padded_buflen && *padded_buflen >= ETH_ZLEN);
+
+if (pkt_size >= ETH_ZLEN) {
+return false;
+}
+
+/* pad to minimum Ethernet frame length */
+memcpy(padded_pkt, pkt, pkt_size);
+memset(&padded_pkt[pkt_size], 0, ETH_ZLEN - pkt_size);
+*padded_buflen = ETH_ZLEN;
+
+return true;
+}
--
2.25.1
[PATCH v4 00/12] net: Pad short frames for network backends
The minimum Ethernet frame length is 60 bytes. For short frames with smaller length like ARP packets (only 42 bytes), on a real world NIC it can choose either padding its length to the minimum required 60 bytes, or sending it out directly to the wire. Such behavior can be hardcoded or controled by a register bit. Similarly on the receive path, NICs can choose either dropping such short frames directly or handing them over to software to handle. On the other hand, for the network backends like SLiRP/TAP, they don't expose a way to control the short frame behavior. As of today they just send/receive data from/to the other end connected to them, which means any sized packet is acceptable. So they can send and receive short frames without any problem. It is observed that ARP packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send these ARP packets to the other end which might be a NIC model that does not allow short frames to pass through. To provide better compatibility, for packets sent from QEMU network backends like SLiRP/TAP, we change to pad short frames before sending it out to the other end, if the other end does not forbid it via the nc->do_not_pad flag. This ensures a backend as an Ethernet sender does not violate the spec. But with this change, the behavior of dropping short frames from SLiRP/TAP interfaces in the NIC model cannot be emulated because it always receives a packet that is spec complaint. The capability of sending short frames from NIC models is still supported and short frames can still pass through SLiRP/TAP. This series should be able to fix the issue as reported with some NIC models before, that ARP requests get dropped, preventing the guest from becoming visible on the network. It was workarounded in these NIC models on the receive path, that when a short frame is received, it is padded up to 60 bytes. Changes in v4: - change 'ethernet' to 'Ethernet' - do not inline the helper - check the padded buffer size to avoid buffer overflow - squash slirp/tap commits into one Changes in v3: - use 'without' instead of 'sans' - add a helper to pad short frames - add a comment to 'do_not_pad' - use the pad_short_frame() helper Bin Meng (12): net: eth: Add a helper to pad a short Ethernet frame net: Add a 'do_not_pad" to NetClientState net: Pad short frames to minimum size before sending from SLiRP/TAP hw/net: virtio-net: Initialize nc->do_not_pad to true hw/net: e1000: Remove the logic of padding short frames in the receive path hw/net: vmxnet3: Remove the logic of padding short frames in the receive path hw/net: i82596: Remove the logic of padding short frames in the receive path hw/net: ne2000: Remove the logic of padding short frames in the receive path hw/net: pcnet: Remove the logic of padding short frames in the receive path hw/net: rtl8139: Remove the logic of padding short frames in the receive path hw/net: sungem: Remove the logic of padding short frames in the receive path hw/net: sunhme: Remove the logic of padding short frames in the receive path include/net/eth.h | 17 + include/net/net.h | 1 + hw/net/e1000.c | 11 +-- hw/net/i82596.c | 18 -- hw/net/ne2000.c | 12 hw/net/pcnet.c | 9 - hw/net/rtl8139.c| 12 hw/net/sungem.c | 14 -- hw/net/sunhme.c | 11 --- hw/net/virtio-net.c | 4 hw/net/vmxnet3.c| 10 -- net/eth.c | 17 + net/slirp.c | 10 ++ net/tap-win32.c | 10 ++ net/tap.c | 10 ++ 15 files changed, 70 insertions(+), 96 deletions(-) -- 2.25.1
[PATCH v4 02/12] net: Add a 'do_not_pad" to NetClientState
This adds a flag in NetClientState, so that a net client can tell
its peer that the packets do not need to be padded to the minimum
size of an Ethernet frame (60 bytes) before sending to it.
Signed-off-by: Bin Meng
Reviewed-by: Philippe Mathieu-Daudé
---
(no changes since v3)
Changes in v3:
- add a comment to 'do_not_pad'
include/net/net.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/net/net.h b/include/net/net.h
index 919facaad2..f944731c18 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -100,6 +100,7 @@ struct NetClientState {
int vring_enable;
int vnet_hdr_len;
bool is_netdev;
+bool do_not_pad; /* do not pad to the minimum ethernet frame length */
QTAILQ_HEAD(, NetFilterState) filters;
};
--
2.25.1
[PATCH v4 07/12] hw/net: i82596: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/i82596.c | 18 --
1 file changed, 18 deletions(-)
diff --git a/hw/net/i82596.c b/hw/net/i82596.c
index 055c3a1470..1eca2e2d81 100644
--- a/hw/net/i82596.c
+++ b/hw/net/i82596.c
@@ -73,10 +73,6 @@ enum commands {
#define I596_EOF0x8000
#define SIZE_MASK 0x3fff
-#define ETHER_TYPE_LEN 2
-#define VLAN_TCI_LEN 2
-#define VLAN_HLEN (ETHER_TYPE_LEN + VLAN_TCI_LEN)
-
/* various flags in the chip config registers */
#define I596_PREFETCH (s->config[0] & 0x80)
#define I596_PROMISC(s->config[8] & 0x01)
@@ -489,8 +485,6 @@ bool i82596_can_receive(NetClientState *nc)
return true;
}
-#define MIN_BUF_SIZE 60
-
ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz)
{
I82596State *s = qemu_get_nic_opaque(nc);
@@ -501,7 +495,6 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t
*buf, size_t sz)
size_t bufsz = sz; /* length of data in buf */
uint32_t crc;
uint8_t *crc_ptr;
-uint8_t buf1[MIN_BUF_SIZE + VLAN_HLEN];
static const uint8_t broadcast_macaddr[6] = {
0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
@@ -584,17 +577,6 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t
*buf, size_t sz)
}
}
-/* if too small buffer, then expand it */
-if (len < MIN_BUF_SIZE + VLAN_HLEN) {
-memcpy(buf1, buf, len);
-memset(buf1 + len, 0, MIN_BUF_SIZE + VLAN_HLEN - len);
-buf = buf1;
-if (len < MIN_BUF_SIZE) {
-len = MIN_BUF_SIZE;
-}
-bufsz = len;
-}
-
/* Calculate the ethernet checksum (4 bytes) */
len += 4;
crc = cpu_to_be32(crc32(~0, buf, sz));
--
2.25.1
[PATCH v4 03/12] net: Pad short frames to minimum size before sending from SLiRP/TAP
The minimum Ethernet frame length is 60 bytes. For short frames with
smaller length like ARP packets (only 42 bytes), on a real world NIC
it can choose either padding its length to the minimum required 60
bytes, or sending it out directly to the wire. Such behavior can be
hardcoded or controled by a register bit. Similarly on the receive
path, NICs can choose either dropping such short frames directly or
handing them over to software to handle.
On the other hand, for the network backends like SLiRP/TAP, they
don't expose a way to control the short frame behavior. As of today
they just send/receive data from/to the other end connected to them,
which means any sized packet is acceptable. So they can send and
receive short frames without any problem. It is observed that ARP
packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
these ARP packets to the other end which might be a NIC model that
does not allow short frames to pass through.
To provide better compatibility, for packets sent from QEMU network
backends like SLiRP/TAP, we change to pad short frames before sending
it out to the other end, if the other end does not forbid it via the
nc->do_not_pad flag. This ensures a backend as an Ethernet sender
does not violate the spec. But with this change, the behavior of
dropping short frames from SLiRP/TAP interfaces in the NIC model
cannot be emulated because it always receives a packet that is spec
complaint. The capability of sending short frames from NIC models is
still supported and short frames can still pass through SLiRP/TAP.
This commit should be able to fix the issue as reported with some
NIC models before, that ARP requests get dropped, preventing the
guest from becoming visible on the network. It was workarounded in
these NIC models on the receive path, that when a short frame is
received, it is padded up to 60 bytes.
The following 2 commits seem to be the one to workaround this issue
in e1000 and vmxenet3 before, and should probably be reverted.
commit 78aeb23eded2 ("e1000: Pad short frames to minimum size (60 bytes)")
commit 40a87c6c9b11 ("vmxnet3: Pad short frames to minimum size (60 bytes)")
Signed-off-by: Bin Meng
---
Changes in v4:
- squash slirp/tap commits into one
Changes in v3:
- use the pad_short_frame() helper
net/slirp.c | 10 ++
net/tap-win32.c | 10 ++
net/tap.c | 10 ++
3 files changed, 30 insertions(+)
diff --git a/net/slirp.c b/net/slirp.c
index be914c0be0..a01a0fccd3 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -31,6 +31,7 @@
#include
#include
#endif
+#include "net/eth.h"
#include "net/net.h"
#include "clients.h"
#include "hub.h"
@@ -115,6 +116,15 @@ static ssize_t net_slirp_send_packet(const void *pkt,
size_t pkt_len,
void *opaque)
{
SlirpState *s = opaque;
+uint8_t min_pkt[ETH_ZLEN];
+size_t min_pktsz = sizeof(min_pkt);
+
+if (!s->nc.peer->do_not_pad) {
+if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pkt_len)) {
+pkt = min_pkt;
+pkt_len = min_pktsz;
+}
+}
return qemu_send_packet(&s->nc, pkt, pkt_len);
}
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 2b5dcda36e..fb92b55768 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -31,6 +31,7 @@
#include "qemu-common.h"
#include "clients.h"/* net_init_tap */
+#include "net/eth.h"
#include "net/net.h"
#include "net/tap.h"/* tap_has_ufo, ... */
#include "qemu/error-report.h"
@@ -688,9 +689,18 @@ static void tap_win32_send(void *opaque)
uint8_t *buf;
int max_size = 4096;
int size;
+uint8_t min_pkt[ETH_ZLEN];
+size_t min_pktsz = sizeof(min_pkt);
size = tap_win32_read(s->handle, &buf, max_size);
if (size > 0) {
+if (!s->nc.peer->do_not_pad) {
+if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
+buf = min_pkt;
+size = min_pktsz;
+}
+}
+
qemu_send_packet(&s->nc, buf, size);
tap_win32_free_buffer(s->handle, buf);
}
diff --git a/net/tap.c b/net/tap.c
index b7512853f4..dd42ac6134 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -32,6 +32,7 @@
#include
#include
+#include "net/eth.h"
#include "net/net.h"
#include "clients.h"
#include "monitor/monitor.h"
@@ -189,6 +190,8 @@ static void tap_send(void *opaque)
while (true) {
uint8_t *buf = s->buf;
+uint8_t min_pkt[ETH_ZLEN];
+size_t min_pktsz = sizeof(min_pkt);
size = tap_read_packet(s->fd, s->buf, sizeof(s->buf));
if (size <= 0) {
@@ -200,6 +203,13 @@ static void tap_send(void *opaque)
size -= s->host_vnet_hdr_len;
}
+if (!s->nc.peer->do_not_pad) {
+if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
+buf = min_pkt;
+size = min_pktsz;
+}
+}
+
size = qemu_send_packet_async(&s->nc, buf
[PATCH v4 04/12] hw/net: virtio-net: Initialize nc->do_not_pad to true
For virtio-net, there is no need to pad the Ethernet frame size to
60 bytes before sending to it.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/virtio-net.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 96a3cc8357..66b9ff4511 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3314,6 +3314,10 @@ static void virtio_net_device_realize(DeviceState *dev,
Error **errp)
object_get_typename(OBJECT(dev)), dev->id, n);
}
+for (i = 0; i < n->max_queues; i++) {
+n->nic->ncs[i].do_not_pad = true;
+}
+
peer_test_vnet_hdr(n);
if (peer_has_vnet_hdr(n)) {
for (i = 0; i < n->max_queues; i++) {
--
2.25.1
