[PULL 0/4] pc,virtio,pci: bug fixes

[PULL 0/4] pc,virtio,pci: bug fixes

[Michael S. Tsirkin]

A very small pull but there are a couple of fixes here for
bugs that have been biting us in the field.

The following changes since commit 51db2d7cf26d05a961ec0ee0eb773594b32cc4a1:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2021-02-25 14:02:16 +0000)

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 1d8d014e936a49795de95a35a2a1ab8000ab2e5b:

  vhost: simplify vhost_dev_init() fail_busyloop label (2021-03-02 09:16:18 -0500)

----------------------------------------------------------------
pc,virtio,pci: bug fixes

Fixes all over the place. Specifically this fixes
a bug which made windows guests lose device config
(such as the configured fixed IP) after upgrading
to the new QEMU.

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

----------------------------------------------------------------
Cindy Lu (1):
      virtio-net: handle zero mac for a vdpa peer

Philippe Mathieu-Daudé (1):
      hw/pci: Have safer pcie_bus_realize() by checking error path

Stefan Hajnoczi (1):
      vhost: simplify vhost_dev_init() fail_busyloop label

Vitaly Cheptsov (1):
      i386/acpi: restore device paths for pre-5.1 vms

 include/hw/i386/pc.h |  1 +
 hw/i386/acpi-build.c |  4 ++--
 hw/i386/pc_piix.c    |  2 ++
 hw/i386/pc_q35.c     |  2 ++
 hw/net/virtio-net.c  | 12 ++++++++++++
 hw/pci/pci.c         |  7 ++++++-
 hw/virtio/vhost.c    | 12 +++++-------
 7 files changed, 30 insertions(+), 10 deletions(-)

[PULL 1/4] i386/acpi: restore device paths for pre-5.1 vms

[Michael S. Tsirkin]

From: Vitaly Cheptsov <cheptsov@ispras.ru>

After fixing the _UID value for the primary PCI root bridge in
af1b80ae it was discovered that this change updates Windows
configuration in an incompatible way causing network configuration
failure unless DHCP is used. More details provided on the list:

https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html

This change reverts the _UID update from 1 to 0 for q35 and i440fx
VMs before version 5.2 to maintain the original behaviour when
upgrading.

Cc: qemu-stable@nongnu.org
Cc: qemu-devel@nongnu.org
Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Vitaly Cheptsov <cheptsov@ispras.ru>
Message-Id: <20210301195919.9333-1-cheptsov@ispras.ru>
Tested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Fixes: af1b80ae56c9 ("i386/acpi: fix inconsistent QEMU/OVMF device paths")
---
 include/hw/i386/pc.h | 1 +
 hw/i386/acpi-build.c | 4 ++--
 hw/i386/pc_piix.c    | 2 ++
 hw/i386/pc_q35.c     | 2 ++
 4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index c9d194a5e7..d4c3d73c11 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -105,6 +105,7 @@ struct PCMachineClass {
     int legacy_acpi_table_size;
     unsigned acpi_data_size;
     bool do_not_add_smb_acpi;
+    int pci_root_uid;
 
     /* SMBIOS compat: */
     bool smbios_defaults;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 31a5f6f4a5..442b4629a9 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1277,7 +1277,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         dev = aml_device("PCI0");
         aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
         aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+        aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid)));
         aml_append(sb_scope, dev);
         aml_append(dsdt, sb_scope);
 
@@ -1296,7 +1296,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
         aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
         aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+        aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid)));
         aml_append(dev, build_q35_osc_method());
         aml_append(sb_scope, dev);
         if (mcfg_valid) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2904b40163..46cc951073 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -405,6 +405,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pcmc->default_nic_model = "e1000";
+    pcmc->pci_root_uid = 0;
 
     m->family = "pc_piix";
     m->desc = "Standard PC (i440FX + PIIX, 1996)";
@@ -448,6 +449,7 @@ static void pc_i440fx_5_1_machine_options(MachineClass *m)
     compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len);
     compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len);
     pcmc->kvmclock_create_always = false;
+    pcmc->pci_root_uid = 1;
 }
 
 DEFINE_I440FX_MACHINE(v5_1, "pc-i440fx-5.1", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 0a212443aa..53450190f5 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -329,6 +329,7 @@ static void pc_q35_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pcmc->default_nic_model = "e1000e";
+    pcmc->pci_root_uid = 0;
 
     m->family = "pc_q35";
     m->desc = "Standard PC (Q35 + ICH9, 2009)";
@@ -375,6 +376,7 @@ static void pc_q35_5_1_machine_options(MachineClass *m)
     compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len);
     compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len);
     pcmc->kvmclock_create_always = false;
+    pcmc->pci_root_uid = 1;
 }
 
 DEFINE_Q35_MACHINE(v5_1, "pc-q35-5.1", NULL,
-- 
MST

[PULL 2/4] virtio-net: handle zero mac for a vdpa peer

[Michael S. Tsirkin]

From: Cindy Lu <lulu@redhat.com>

Some mlx vdpa devices with kernels at least up to 5.11 currently present
0 as their MAC address.  This is because they have not been
  pre-configured with a MAC: they have a learning bridge and only learn
the MAC once guest is up.  Kernel patches and tools to allow programming
the MAC from host are being developed. For now - since these
combinations exist in the field - let's detect zero mac and just try to
proceed with the mac from the qemu command line.

This makes the guest use this MAC to send packets in turn teaching
the MAC to the card, and things work.

TODO:
report the actual MAC from QEMU commad line in the info message.
TODO:
detect that a (non-zero) hardware MAC does not match QEMU command line
and fail init.

Signed-off-by: Cindy Lu <lulu@redhat.com>
Message-Id: <20210225165506.18321-2-lulu@redhat.com>

mst: rewritten code comments, message printed and the commit log.

Cc: Eli Cohen <elic@nvidia.com>
Cc: Parav Pandit <parav@nvidia.com>
Tested-by: Adrian Moreno <amorenoz@redhat.com>
Tested-by: Sean Mooney <smooney@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/virtio-net.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 439f823b19..96a3cc8357 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -126,6 +126,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
     VirtIONet *n = VIRTIO_NET(vdev);
     struct virtio_net_config netcfg;
     NetClientState *nc = qemu_get_queue(n->nic);
+    static const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
 
     int ret = 0;
     memset(&netcfg, 0 , sizeof(struct virtio_net_config));
@@ -151,6 +152,17 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
         ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
                                    n->config_size);
         if (ret != -1) {
+            /*
+             * Some NIC/kernel combinations present 0 as the mac address.  As
+             * that is not a legal address, try to proceed with the
+             * address from the QEMU command line in the hope that the
+             * address has been configured correctly elsewhere - just not
+             * reported by the device.
+             */
+            if (memcmp(&netcfg.mac, &zero, sizeof(zero)) == 0) {
+                info_report("Zero hardware mac address detected. Ignoring.");
+                memcpy(netcfg.mac, n->mac, ETH_ALEN);
+            }
             memcpy(config, &netcfg, n->config_size);
         }
     }
-- 
MST

[PULL 3/4] hw/pci: Have safer pcie_bus_realize() by checking error path

[Michael S. Tsirkin]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

While pci_bus_realize() currently does not use the Error* argument,
it would be an error to leave pcie_bus_realize() setting bus->flags
if pci_bus_realize() had failed.

Fix by using a local Error* and return early (propagating the error)
if pci_bus_realize() failed.

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210201153700.618946-1-philmd@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pci.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index fa97a671d1..0eadcdbc9e 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -132,8 +132,13 @@ static void pci_bus_realize(BusState *qbus, Error **errp)
 static void pcie_bus_realize(BusState *qbus, Error **errp)
 {
     PCIBus *bus = PCI_BUS(qbus);
+    Error *local_err = NULL;
 
-    pci_bus_realize(qbus, errp);
+    pci_bus_realize(qbus, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     /*
      * A PCI-E bus can support extended config space if it's the root
-- 
MST

[PULL 4/4] vhost: simplify vhost_dev_init() fail_busyloop label

[Michael S. Tsirkin]

From: Stefan Hajnoczi <stefanha@redhat.com>

Requiring a conditional for every goto is tedious:

  if (busyloop_timeout) {
      goto fail_busyloop;
  } else {
      goto fail;
  }

Move the conditional to into the fail_busyloop label so that it's safe
to jump to this label unconditionally.

This change makes the migrate_add_blocker() error case more consistent.
It jumped to fail_busyloop unconditionally whereas the memslots limits
error case was conditional.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210222114931.272308-1-stefanha@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/vhost.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 6e17d631f7..2a01662b08 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1388,18 +1388,16 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
         error_report("vhost backend memory slots limit is less"
                 " than current number of present memory slots");
         r = -1;
-        if (busyloop_timeout) {
-            goto fail_busyloop;
-        } else {
-            goto fail;
-        }
+        goto fail_busyloop;
     }
 
     return 0;
 
 fail_busyloop:
-    while (--i >= 0) {
-        vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i, 0);
+    if (busyloop_timeout) {
+        while (--i >= 0) {
+            vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i, 0);
+        }
     }
 fail:
     hdev->nvqs = n_initialized_vqs;
-- 
MST

Re: [PULL 0/4] pc,virtio,pci: bug fixes

[Peter Maydell]

On Tue, 2 Mar 2021 at 14:21, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> A very small pull but there are a couple of fixes here for
> bugs that have been biting us in the field.
>
> The following changes since commit 51db2d7cf26d05a961ec0ee0eb773594b32cc4a1:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2021-02-25 14:02:16 +0000)
>
> 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 1d8d014e936a49795de95a35a2a1ab8000ab2e5b:
>
>   vhost: simplify vhost_dev_init() fail_busyloop label (2021-03-02 09:16:18 -0500)
>
> ----------------------------------------------------------------
> pc,virtio,pci: bug fixes
>
> Fixes all over the place. Specifically this fixes
> a bug which made windows guests lose device config
> (such as the configured fixed IP) after upgrading
> to the new QEMU.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM