[PULL 00/10] pc,virtio: fixes

[PULL 00/10] pc,virtio: fixes

[Michael S. Tsirkin]

The following changes since commit c7208a6e0d049f9e8af15df908168a79b1f99685:

  Update version for v7.1.0-rc3 release (2022-08-16 20:45:19 -0500)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to 019d2530a7cf082af01280a9ad265722a15601f9:

  virtio-pci: don't touch pci on virtio reset (2022-08-17 12:11:05 -0400)

----------------------------------------------------------------
pc,virtio: fixes

Several bugfixes, they all look very safe to me. Revert
seed support since we aren't any closer to a proper fix.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Alex Bennée (3):
      hw/virtio: gracefully handle unset vhost_dev vdev
      hw/virtio: handle un-configured shutdown in virtio-pci
      hw/virtio: fix vhost_user_read tracepoint

Gerd Hoffmann (1):
      x86: disable rng seeding via setup_data

Igor Mammedov (1):
      tests: acpi: silence applesmc warning about invalid key

Jonathan Cameron (3):
      hw/cxl: Add stub write function for RO MemoryRegionOps entries.
      hw/cxl: Fix Get LSA input payload size which should be 8 bytes.
      hw/cxl: Correctly handle variable sized mailbox input payloads.

Michael S. Tsirkin (1):
      virtio-pci: don't touch pci on virtio reset

Stefan Hajnoczi (1):
      virtio-scsi: fix race in virtio_scsi_dataplane_start()

 hw/block/dataplane/virtio-blk.c |  5 +++++
 hw/cxl/cxl-device-utils.c       | 12 +++++++++---
 hw/cxl/cxl-mailbox-utils.c      |  4 ++--
 hw/i386/microvm.c               |  2 +-
 hw/i386/pc_piix.c               |  2 +-
 hw/i386/pc_q35.c                |  2 +-
 hw/scsi/virtio-scsi-dataplane.c | 11 ++++++++---
 hw/virtio/vhost-user.c          |  4 ++--
 hw/virtio/vhost.c               | 10 +++++++---
 hw/virtio/virtio-pci.c          | 19 +++++++++++++++----
 tests/qtest/bios-tables-test.c  |  4 +++-
 11 files changed, 54 insertions(+), 21 deletions(-)

[PULL 01/10] virtio-scsi: fix race in virtio_scsi_dataplane_start()

[Michael S. Tsirkin]

From: Stefan Hajnoczi <stefanha@redhat.com>

As soon as virtio_scsi_data_plane_start() attaches host notifiers the
IOThread may start virtqueue processing. There is a race between
IOThread virtqueue processing and virtio_scsi_data_plane_start() because
it only assigns s->dataplane_started after attaching host notifiers.

When a virtqueue handler function in the IOThread calls
virtio_scsi_defer_to_dataplane() it may see !s->dataplane_started and
attempt to start dataplane even though we're already in the IOThread:

  #0  0x00007f67b360857c __pthread_kill_implementation (libc.so.6 + 0xa257c)
  #1  0x00007f67b35bbd56 raise (libc.so.6 + 0x55d56)
  #2  0x00007f67b358e833 abort (libc.so.6 + 0x28833)
  #3  0x00007f67b358e75b __assert_fail_base.cold (libc.so.6 + 0x2875b)
  #4  0x00007f67b35b4cd6 __assert_fail (libc.so.6 + 0x4ecd6)
  #5  0x000055ca87fd411b memory_region_transaction_commit (qemu-kvm + 0x67511b)
  #6  0x000055ca87e17811 virtio_pci_ioeventfd_assign (qemu-kvm + 0x4b8811)
  #7  0x000055ca87e14836 virtio_bus_set_host_notifier (qemu-kvm + 0x4b5836)
  #8  0x000055ca87f8e14e virtio_scsi_set_host_notifier (qemu-kvm + 0x62f14e)
  #9  0x000055ca87f8dd62 virtio_scsi_dataplane_start (qemu-kvm + 0x62ed62)
  #10 0x000055ca87e14610 virtio_bus_start_ioeventfd (qemu-kvm + 0x4b5610)
  #11 0x000055ca87f8c29a virtio_scsi_handle_ctrl (qemu-kvm + 0x62d29a)
  #12 0x000055ca87fa5902 virtio_queue_host_notifier_read (qemu-kvm + 0x646902)
  #13 0x000055ca882c099e aio_dispatch_handler (qemu-kvm + 0x96199e)
  #14 0x000055ca882c1761 aio_poll (qemu-kvm + 0x962761)
  #15 0x000055ca880e1052 iothread_run (qemu-kvm + 0x782052)
  #16 0x000055ca882c562a qemu_thread_start (qemu-kvm + 0x96662a)

This patch assigns s->dataplane_started before attaching host notifiers
so that virtqueue handler functions that run in the IOThread before
virtio_scsi_data_plane_start() returns correctly identify that dataplane
does not need to be started. This fix is taken from the virtio-blk
dataplane code and it's worth adding a comment in virtio-blk as well to
explain why it works.

Note that s->dataplane_started does not need the AioContext lock because
it is set before attaching host notifiers and cleared after detaching
host notifiers. In other words, the IOThread always sees the value true
and the main loop thread does not modify it while the IOThread is
active.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2099541
Reported-by: Qing Wang <qinwang@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20220808162134.240405-1-stefanha@redhat.com>
Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/block/dataplane/virtio-blk.c |  5 +++++
 hw/scsi/virtio-scsi-dataplane.c | 11 ++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 49276e46f2..26f965cabc 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -219,6 +219,11 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
 
     memory_region_transaction_commit();
 
+    /*
+     * These fields are visible to the IOThread so we rely on implicit barriers
+     * in aio_context_acquire() on the write side and aio_notify_accept() on
+     * the read side.
+     */
     s->starting = false;
     vblk->dataplane_started = true;
     trace_virtio_blk_data_plane_start(s);
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 8bb6e6acfc..20bb91766e 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -136,6 +136,14 @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
 
     memory_region_transaction_commit();
 
+    /*
+     * These fields are visible to the IOThread so we rely on implicit barriers
+     * in aio_context_acquire() on the write side and aio_notify_accept() on
+     * the read side.
+     */
+    s->dataplane_starting = false;
+    s->dataplane_started = true;
+
     aio_context_acquire(s->ctx);
     virtio_queue_aio_attach_host_notifier(vs->ctrl_vq, s->ctx);
     virtio_queue_aio_attach_host_notifier_no_poll(vs->event_vq, s->ctx);
@@ -143,9 +151,6 @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
     for (i = 0; i < vs->conf.num_queues; i++) {
         virtio_queue_aio_attach_host_notifier(vs->cmd_vqs[i], s->ctx);
     }
-
-    s->dataplane_starting = false;
-    s->dataplane_started = true;
     aio_context_release(s->ctx);
     return 0;
 
-- 
MST

[PULL 02/10] hw/virtio: gracefully handle unset vhost_dev vdev

[Michael S. Tsirkin]

From: Alex Bennée <alex.bennee@linaro.org>

I've noticed asserts firing because we query the status of vdev after
a vhost connection is closed down. Rather than faulting on the NULL
indirect just quietly reply false.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220728135503.1060062-3-alex.bennee@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/vhost.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 0827d631c0..f758f177bb 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -306,7 +306,7 @@ static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
     dev->log_size = size;
 }
 
-static int vhost_dev_has_iommu(struct vhost_dev *dev)
+static bool vhost_dev_has_iommu(struct vhost_dev *dev)
 {
     VirtIODevice *vdev = dev->vdev;
 
@@ -316,8 +316,12 @@ static int vhost_dev_has_iommu(struct vhost_dev *dev)
      * does not have IOMMU, there's no need to enable this feature
      * which may cause unnecessary IOTLB miss/update transactions.
      */
-    return virtio_bus_device_iommu_enabled(vdev) &&
-           virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+    if (vdev) {
+        return virtio_bus_device_iommu_enabled(vdev) &&
+            virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+    } else {
+        return false;
+    }
 }
 
 static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
-- 
MST

[PULL 03/10] hw/virtio: handle un-configured shutdown in virtio-pci

[Michael S. Tsirkin]

From: Alex Bennée <alex.bennee@linaro.org>

The assert() protecting against leakage is a little aggressive and
causes needless crashes if a device is shutdown without having been
configured. In this case no descriptors are lost because none have
been assigned.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220728135503.1060062-4-alex.bennee@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-pci.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 45327f0b31..5ce61f9b45 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -996,9 +996,14 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
 
     nvqs = MIN(nvqs, VIRTIO_QUEUE_MAX);
 
-    /* When deassigning, pass a consistent nvqs value
-     * to avoid leaking notifiers.
+    /*
+     * When deassigning, pass a consistent nvqs value to avoid leaking
+     * notifiers. But first check we've actually been configured, exit
+     * early if we haven't.
      */
+    if (!assign && !proxy->nvqs_with_notifiers) {
+        return 0;
+    }
     assert(assign || nvqs == proxy->nvqs_with_notifiers);
 
     proxy->nvqs_with_notifiers = nvqs;
-- 
MST

[PULL 04/10] hw/virtio: fix vhost_user_read tracepoint

[Michael S. Tsirkin]

From: Alex Bennée <alex.bennee@linaro.org>

As reads happen in the callback we were never seeing them. We only
really care about the header so move the tracepoint to when the header
is complete.

Fixes: 6ca6d8ee9d (hw/virtio: add vhost_user_[read|write] trace points)
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Jason Wang <jasowang@redhat.com>
Message-Id: <20220728135503.1060062-5-alex.bennee@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/vhost-user.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 75b8df21a4..bd24741be8 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -295,6 +295,8 @@ static int vhost_user_read_header(struct vhost_dev *dev, VhostUserMsg *msg)
         return -EPROTO;
     }
 
+    trace_vhost_user_read(msg->hdr.request, msg->hdr.flags);
+
     return 0;
 }
 
@@ -544,8 +546,6 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
         }
     }
 
-    trace_vhost_user_read(msg.hdr.request, msg.hdr.flags);
-
     return 0;
 }
 
-- 
MST

[PULL 05/10] x86: disable rng seeding via setup_data

[Michael S. Tsirkin]

From: Gerd Hoffmann <kraxel@redhat.com>

Causes regressions when doing direct kernel boots with OVMF.

At this point in the release cycle the only sensible action
is to just disable this for 7.1 and sort it properly in the
7.2 devel cycle.

Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-Id: <20220817083940.3174933-1-kraxel@redhat.com>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/microvm.c | 2 +-
 hw/i386/pc_piix.c | 2 +-
 hw/i386/pc_q35.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 7fe8cce03e..52cafa003d 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -332,7 +332,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
     rom_set_fw(fw_cfg);
 
     if (machine->kernel_filename != NULL) {
-        x86_load_linux(x86ms, fw_cfg, 0, true, false);
+        x86_load_linux(x86ms, fw_cfg, 0, true, true);
     }
 
     if (mms->option_roms) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a5c65c1c35..20962c34e7 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -439,6 +439,7 @@ static void pc_i440fx_7_1_machine_options(MachineClass *m)
     m->alias = "pc";
     m->is_default = true;
     pcmc->default_cpu_version = 1;
+    pcmc->legacy_no_rng_seed = true;
 }
 
 DEFINE_I440FX_MACHINE(v7_1, "pc-i440fx-7.1", NULL,
@@ -450,7 +451,6 @@ static void pc_i440fx_7_0_machine_options(MachineClass *m)
     pc_i440fx_7_1_machine_options(m);
     m->alias = NULL;
     m->is_default = false;
-    pcmc->legacy_no_rng_seed = true;
     pcmc->enforce_amd_1tb_hole = false;
     compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
     compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 3a35193ff7..2e5dae9a89 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -376,6 +376,7 @@ static void pc_q35_7_1_machine_options(MachineClass *m)
     pc_q35_machine_options(m);
     m->alias = "q35";
     pcmc->default_cpu_version = 1;
+    pcmc->legacy_no_rng_seed = true;
 }
 
 DEFINE_Q35_MACHINE(v7_1, "pc-q35-7.1", NULL,
@@ -386,7 +387,6 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_q35_7_1_machine_options(m);
     m->alias = NULL;
-    pcmc->legacy_no_rng_seed = true;
     pcmc->enforce_amd_1tb_hole = false;
     compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
     compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
-- 
MST

[PULL 06/10] hw/cxl: Add stub write function for RO MemoryRegionOps entries.

[Michael S. Tsirkin]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

There is no checking on the availability of a write callback.
Hence QEMU crashes if a write does occur to one of these regions.

Discovered whilst chasing a Linux kernel bug that incorrectly
wrote into one of these regions.

Fixes: 6364adacdf ("hw/cxl/device: Implement the CAP array (8.2.8.1-2)")
Reported-by: Bobo WL <lmw.bobo@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Message-Id: <20220817145759.32603-2-Jonathan.Cameron@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/cxl/cxl-device-utils.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
index 687759b301..83ce7a8270 100644
--- a/hw/cxl/cxl-device-utils.c
+++ b/hw/cxl/cxl-device-utils.c
@@ -141,9 +141,15 @@ static uint64_t mdev_reg_read(void *opaque, hwaddr offset, unsigned size)
     return retval;
 }
 
+static void ro_reg_write(void *opaque, hwaddr offset, uint64_t value,
+                           unsigned size)
+{
+    /* Many register sets are read only */
+}
+
 static const MemoryRegionOps mdev_ops = {
     .read = mdev_reg_read,
-    .write = NULL, /* memory device register is read only */
+    .write = ro_reg_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
         .min_access_size = 1,
@@ -173,7 +179,7 @@ static const MemoryRegionOps mailbox_ops = {
 
 static const MemoryRegionOps dev_ops = {
     .read = dev_reg_read,
-    .write = NULL, /* status register is read only */
+    .write = ro_reg_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
         .min_access_size = 1,
@@ -188,7 +194,7 @@ static const MemoryRegionOps dev_ops = {
 
 static const MemoryRegionOps caps_ops = {
     .read = caps_reg_read,
-    .write = NULL, /* caps registers are read only */
+    .write = ro_reg_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
         .min_access_size = 1,
-- 
MST

[PULL 07/10] hw/cxl: Fix Get LSA input payload size which should be 8 bytes.

[Michael S. Tsirkin]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Get LSA needs 4 byte offset and 4 byte length arguments.
CXL rev 2.0 Table 178.

Fixes: 3ebe676a34 ("hw/cxl/device: Implement get/set Label Storage Area (LSA)")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Message-Id: <20220817145759.32603-3-Jonathan.Cameron@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/cxl/cxl-mailbox-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index bb66c765a5..3cea8b17a8 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -406,7 +406,7 @@ static struct cxl_cmd cxl_cmd_set[256][256] = {
         cmd_identify_memory_device, 0, 0 },
     [CCLS][GET_PARTITION_INFO] = { "CCLS_GET_PARTITION_INFO",
         cmd_ccls_get_partition_info, 0, 0 },
-    [CCLS][GET_LSA] = { "CCLS_GET_LSA", cmd_ccls_get_lsa, 0, 0 },
+    [CCLS][GET_LSA] = { "CCLS_GET_LSA", cmd_ccls_get_lsa, 8, 0 },
     [CCLS][SET_LSA] = { "CCLS_SET_LSA", cmd_ccls_set_lsa,
         ~0, IMMEDIATE_CONFIG_CHANGE | IMMEDIATE_DATA_CHANGE },
 };
-- 
MST

[PULL 08/10] hw/cxl: Correctly handle variable sized mailbox input payloads.

[Michael S. Tsirkin]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

A placeholder of ~0 is used to indicate variable payload size.
Whilst the checks for output payload correctly took this into
account, those for input payload did not.

This results in failure of the Set LSA command.

Fixes: 464e14ac43 ("hw/cxl/device: Implement basic mailbox (8.2.8.4)")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Message-Id: <20220817145759.32603-4-Jonathan.Cameron@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/cxl/cxl-mailbox-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 3cea8b17a8..bc1bb18844 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -425,7 +425,7 @@ void cxl_process_mailbox(CXLDeviceState *cxl_dstate)
     cxl_cmd = &cxl_cmd_set[set][cmd];
     h = cxl_cmd->handler;
     if (h) {
-        if (len == cxl_cmd->in) {
+        if (len == cxl_cmd->in || cxl_cmd->in == ~0) {
             cxl_cmd->payload = cxl_dstate->mbox_reg_state +
                 A_CXL_DEV_CMD_PAYLOAD;
             ret = (*h)(cxl_cmd, cxl_dstate, &len);
-- 
MST

[PULL 09/10] tests: acpi: silence applesmc warning about invalid key

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

OSK value is irrelevant for ACPI test case.
Supply fake OSK explicitly to prevent QEMU complaining about
invalid key when it fallbacks to default_osk.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20220728133713.1369596-1-imammedo@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/qtest/bios-tables-test.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 359916c228..7c5f736b51 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1632,7 +1632,9 @@ static void test_acpi_q35_applesmc(void)
         .variant = ".applesmc",
     };
 
-    test_acpi_one("-device isa-applesmc", &data);
+    /* supply fake 64-byte OSK to silence missing key warning */
+    test_acpi_one("-device isa-applesmc,osk=any64characterfakeoskisenough"
+                  "topreventinvalidkeywarningsonstderr", &data);
     free_test_data(&data);
 }
 
-- 
MST

[PULL 10/10] virtio-pci: don't touch pci on virtio reset

[Michael S. Tsirkin]

virtio level reset should not affect pci express
registers such as PM, error or link.

Fixes: 27ce0f3afc ("hw/virtio: fix Power Management Control Register for PCI Express virtio devices")
Fixes: d584f1b9ca ("hw/virtio: fix Link Control Register for PCI Express virtio devices")
Fixes: c2cabb3422 ("hw/virtio: fix error enabling flags in Device Control register")
Cc: "Marcel Apfelbaum" <marcel@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-pci.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 5ce61f9b45..a50c5a57d7 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1947,7 +1947,6 @@ static void virtio_pci_reset(DeviceState *qdev)
 {
     VirtIOPCIProxy *proxy = VIRTIO_PCI(qdev);
     VirtioBusState *bus = VIRTIO_BUS(&proxy->bus);
-    PCIDevice *dev = PCI_DEVICE(qdev);
     int i;
 
     virtio_bus_reset(bus);
@@ -1960,6 +1959,13 @@ static void virtio_pci_reset(DeviceState *qdev)
         proxy->vqs[i].avail[0] = proxy->vqs[i].avail[1] = 0;
         proxy->vqs[i].used[0] = proxy->vqs[i].used[1] = 0;
     }
+}
+
+static void virtio_pci_bus_reset(DeviceState *qdev)
+{
+    PCIDevice *dev = PCI_DEVICE(qdev);
+
+    virtio_pci_reset(qdev);
 
     if (pci_is_express(dev)) {
         pcie_cap_deverr_reset(dev);
@@ -2027,7 +2033,7 @@ static void virtio_pci_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_OTHERS;
     device_class_set_parent_realize(dc, virtio_pci_dc_realize,
                                     &vpciklass->parent_dc_realize);
-    dc->reset = virtio_pci_reset;
+    dc->reset = virtio_pci_bus_reset;
 }
 
 static const TypeInfo virtio_pci_info = {
-- 
MST

Re: [PULL 05/10] x86: disable rng seeding via setup_data

[Paolo Bonzini]

On 8/17/22 18:14, Michael S. Tsirkin wrote:
> @@ -332,7 +332,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
>       rom_set_fw(fw_cfg);
>   
>       if (machine->kernel_filename != NULL) {
> -        x86_load_linux(x86ms, fw_cfg, 0, true, false);
> +        x86_load_linux(x86ms, fw_cfg, 0, true, true);
>       }
>   
>       if (mms->option_roms) {
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index a5c65c1c35..20962c34e7 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -439,6 +439,7 @@ static void pc_i440fx_7_1_machine_options(MachineClass *m)
>       m->alias = "pc";
>       m->is_default = true;
>       pcmc->default_cpu_version = 1;
> +    pcmc->legacy_no_rng_seed = true;
>   }
>   
>   DEFINE_I440FX_MACHINE(v7_1, "pc-i440fx-7.1", NULL,
> @@ -450,7 +451,6 @@ static void pc_i440fx_7_0_machine_options(MachineClass *m)
>       pc_i440fx_7_1_machine_options(m);
>       m->alias = NULL;
>       m->is_default = false;
> -    pcmc->legacy_no_rng_seed = true;
>       pcmc->enforce_amd_1tb_hole = false;
>       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
>       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 3a35193ff7..2e5dae9a89 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -376,6 +376,7 @@ static void pc_q35_7_1_machine_options(MachineClass *m)
>       pc_q35_machine_options(m);
>       m->alias = "q35";
>       pcmc->default_cpu_version = 1;
> +    pcmc->legacy_no_rng_seed = true;
>   }
>   
>   DEFINE_Q35_MACHINE(v7_1, "pc-q35-7.1", NULL,
> @@ -386,7 +387,6 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
>       PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>       pc_q35_7_1_machine_options(m);
>       m->alias = NULL;
> -    pcmc->legacy_no_rng_seed = true;
>       pcmc->enforce_amd_1tb_hole = false;
>       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
>       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);

Why not just revert the whole patch?

Paolo

Re: [PULL 05/10] x86: disable rng seeding via setup_data

[Michael S. Tsirkin]

On Thu, Aug 18, 2022 at 11:27:30AM +0200, Paolo Bonzini wrote:
> On 8/17/22 18:14, Michael S. Tsirkin wrote:
> > @@ -332,7 +332,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
> >       rom_set_fw(fw_cfg);
> >       if (machine->kernel_filename != NULL) {
> > -        x86_load_linux(x86ms, fw_cfg, 0, true, false);
> > +        x86_load_linux(x86ms, fw_cfg, 0, true, true);
> >       }
> >       if (mms->option_roms) {
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index a5c65c1c35..20962c34e7 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -439,6 +439,7 @@ static void pc_i440fx_7_1_machine_options(MachineClass *m)
> >       m->alias = "pc";
> >       m->is_default = true;
> >       pcmc->default_cpu_version = 1;
> > +    pcmc->legacy_no_rng_seed = true;
> >   }
> >   DEFINE_I440FX_MACHINE(v7_1, "pc-i440fx-7.1", NULL,
> > @@ -450,7 +451,6 @@ static void pc_i440fx_7_0_machine_options(MachineClass *m)
> >       pc_i440fx_7_1_machine_options(m);
> >       m->alias = NULL;
> >       m->is_default = false;
> > -    pcmc->legacy_no_rng_seed = true;
> >       pcmc->enforce_amd_1tb_hole = false;
> >       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
> >       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 3a35193ff7..2e5dae9a89 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -376,6 +376,7 @@ static void pc_q35_7_1_machine_options(MachineClass *m)
> >       pc_q35_machine_options(m);
> >       m->alias = "q35";
> >       pcmc->default_cpu_version = 1;
> > +    pcmc->legacy_no_rng_seed = true;
> >   }
> >   DEFINE_Q35_MACHINE(v7_1, "pc-q35-7.1", NULL,
> > @@ -386,7 +387,6 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
> >       PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> >       pc_q35_7_1_machine_options(m);
> >       m->alias = NULL;
> > -    pcmc->legacy_no_rng_seed = true;
> >       pcmc->enforce_amd_1tb_hole = false;
> >       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
> >       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
> 
> Why not just revert the whole patch?
> 
> Paolo

At this point I was looking for a minimally intrusive change.

-- 
MST

Re: [PULL 05/10] x86: disable rng seeding via setup_data

[Gerd Hoffmann]

  Hi,

> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 3a35193ff7..2e5dae9a89 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -376,6 +376,7 @@ static void pc_q35_7_1_machine_options(MachineClass *m)
> >       pc_q35_machine_options(m);
> >       m->alias = "q35";
> >       pcmc->default_cpu_version = 1;
> > +    pcmc->legacy_no_rng_seed = true;
> >   }
> >   DEFINE_Q35_MACHINE(v7_1, "pc-q35-7.1", NULL,
> > @@ -386,7 +387,6 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
> >       PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> >       pc_q35_7_1_machine_options(m);
> >       m->alias = NULL;
> > -    pcmc->legacy_no_rng_seed = true;
> >       pcmc->enforce_amd_1tb_hole = false;
> >       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
> >       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
> 
> Why not just revert the whole patch?

Tried that first.  Plain revert not working, there are conflicts.
So just disabling the code looked simpler and safer to me.

take care,
  Gerd

Re: [PULL 05/10] x86: disable rng seeding via setup_data

[Jason A. Donenfeld]

Hi Gerd, Michael, Paolo,

On Thu, Aug 18, 2022 at 01:56:14PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > > index 3a35193ff7..2e5dae9a89 100644
> > > --- a/hw/i386/pc_q35.c
> > > +++ b/hw/i386/pc_q35.c
> > > @@ -376,6 +376,7 @@ static void pc_q35_7_1_machine_options(MachineClass *m)
> > >       pc_q35_machine_options(m);
> > >       m->alias = "q35";
> > >       pcmc->default_cpu_version = 1;
> > > +    pcmc->legacy_no_rng_seed = true;
> > >   }
> > >   DEFINE_Q35_MACHINE(v7_1, "pc-q35-7.1", NULL,
> > > @@ -386,7 +387,6 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
> > >       PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> > >       pc_q35_7_1_machine_options(m);
> > >       m->alias = NULL;
> > > -    pcmc->legacy_no_rng_seed = true;
> > >       pcmc->enforce_amd_1tb_hole = false;
> > >       compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
> > >       compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
> > 
> > Why not just revert the whole patch?
> 
> Tried that first.  Plain revert not working, there are conflicts.
> So just disabling the code looked simpler and safer to me.

Yea, this is fine with me. This commit will be easy enough to revert in
7.2 when things are hopefully working properly in all circumstances.

Jason