qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>,
	David Hildenbrand <david@redhat.com>,
	Juan Quintela <quintela@redhat.com>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Subject: [PULL 25/53] pci: ROM preallocation for incoming migration
Date: Mon, 26 Jun 2023 08:29:05 -0400	[thread overview]
Message-ID: <8eb85fb5ac60bfb6ee4c729cc087078d490670c4.1687782442.git.mst@redhat.com> (raw)
In-Reply-To: <cover.1687782442.git.mst@redhat.com>

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

On incoming migration we have the following sequence to load option
ROM:

1. On device realize we do normal load ROM from the file

2. Than, on incoming migration we rewrite ROM from the incoming RAM
   block. If sizes mismatch we fail, like this:

    Size mismatch: 0000:00:03.0/virtio-net-pci.rom: 0x40000 != 0x80000: Invalid argument

This is not ideal when we migrate to updated distribution: we have to
keep old ROM files in new distribution and be careful around romfile
property to load correct ROM file. Which is loaded actually just to
allocate the ROM with correct length.

Note, that romsize property doesn't really help: if we try to specify
it when default romfile is larger, it fails with something like:

    romfile "efi-virtio.rom" (160768 bytes) is too large for ROM size 65536

Let's just ignore ROM file when romsize is specified and we are in
incoming migration state. In other words, we need only to preallocate
ROM of specified size, local ROM file is unrelated.

This way:

If romsize was specified on source, we just use same commandline as on
source, and migration will work independently of local ROM files on
target.

If romsize was not specified on source (and we have mismatching local
ROM file on target host), we have to specify romsize on target to match
source romsize. romfile parameter may be kept same as on source or may
be dropped, the file is not loaded anyway.

As a bonus we avoid extra reading from ROM file on target.

Note: when we don't have romsize parameter on source command line and
need it for target, it may be calculated as aligned up to power of two
size of ROM file on source (if we know, which file is it) or,
alternatively it may be retrieved from source QEMU by QMP qom-get
command, like

  { "execute": "qom-get",
    "arguments": {
      "path": "/machine/peripheral/CARD_ID/virtio-net-pci.rom[0]",
      "property": "size" } }

Note: we have extra initialization of size variable to zero in
      pci_add_option_rom to avoid false-positive
      "error: ‘size’ may be used uninitialized"

Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <20230522201740.88960-2-vsementsov@yandex-team.ru>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pci.c | 79 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 46 insertions(+), 33 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index bf38905b7d..e2eb4c3b4a 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -36,6 +36,7 @@
 #include "migration/vmstate.h"
 #include "net/net.h"
 #include "sysemu/numa.h"
+#include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
 #include "hw/loader.h"
 #include "qemu/error-report.h"
@@ -2308,12 +2309,18 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
 static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
                                Error **errp)
 {
-    int64_t size;
+    int64_t size = 0;
     g_autofree char *path = NULL;
-    void *ptr;
     char name[32];
     const VMStateDescription *vmsd;
 
+    /*
+     * In case of incoming migration ROM will come with migration stream, no
+     * reason to load the file.  Neither we want to fail if local ROM file
+     * mismatches with specified romsize.
+     */
+    bool load_file = !runstate_check(RUN_STATE_INMIGRATE);
+
     if (!pdev->romfile || !strlen(pdev->romfile)) {
         return;
     }
@@ -2343,32 +2350,35 @@ static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
         return;
     }
 
-    path = qemu_find_file(QEMU_FILE_TYPE_BIOS, pdev->romfile);
-    if (path == NULL) {
-        path = g_strdup(pdev->romfile);
-    }
+    if (load_file || pdev->romsize == -1) {
+        path = qemu_find_file(QEMU_FILE_TYPE_BIOS, pdev->romfile);
+        if (path == NULL) {
+            path = g_strdup(pdev->romfile);
+        }
 
-    size = get_image_size(path);
-    if (size < 0) {
-        error_setg(errp, "failed to find romfile \"%s\"", pdev->romfile);
-        return;
-    } else if (size == 0) {
-        error_setg(errp, "romfile \"%s\" is empty", pdev->romfile);
-        return;
-    } else if (size > 2 * GiB) {
-        error_setg(errp, "romfile \"%s\" too large (size cannot exceed 2 GiB)",
-                   pdev->romfile);
-        return;
-    }
-    if (pdev->romsize != -1) {
-        if (size > pdev->romsize) {
-            error_setg(errp, "romfile \"%s\" (%u bytes) "
-                       "is too large for ROM size %u",
-                       pdev->romfile, (uint32_t)size, pdev->romsize);
+        size = get_image_size(path);
+        if (size < 0) {
+            error_setg(errp, "failed to find romfile \"%s\"", pdev->romfile);
+            return;
+        } else if (size == 0) {
+            error_setg(errp, "romfile \"%s\" is empty", pdev->romfile);
+            return;
+        } else if (size > 2 * GiB) {
+            error_setg(errp,
+                       "romfile \"%s\" too large (size cannot exceed 2 GiB)",
+                       pdev->romfile);
             return;
         }
-    } else {
-        pdev->romsize = pow2ceil(size);
+        if (pdev->romsize != -1) {
+            if (size > pdev->romsize) {
+                error_setg(errp, "romfile \"%s\" (%u bytes) "
+                           "is too large for ROM size %u",
+                           pdev->romfile, (uint32_t)size, pdev->romsize);
+                return;
+            }
+        } else {
+            pdev->romsize = pow2ceil(size);
+        }
     }
 
     vmsd = qdev_get_vmsd(DEVICE(pdev));
@@ -2379,15 +2389,18 @@ static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
     memory_region_init_rom(&pdev->rom, OBJECT(pdev), name, pdev->romsize,
                            &error_fatal);
 
-    ptr = memory_region_get_ram_ptr(&pdev->rom);
-    if (load_image_size(path, ptr, size) < 0) {
-        error_setg(errp, "failed to load romfile \"%s\"", pdev->romfile);
-        return;
-    }
+    if (load_file) {
+        void *ptr = memory_region_get_ram_ptr(&pdev->rom);
 
-    if (is_default_rom) {
-        /* Only the default rom images will be patched (if needed). */
-        pci_patch_ids(pdev, ptr, size);
+        if (load_image_size(path, ptr, size) < 0) {
+            error_setg(errp, "failed to load romfile \"%s\"", pdev->romfile);
+            return;
+        }
+
+        if (is_default_rom) {
+            /* Only the default rom images will be patched (if needed). */
+            pci_patch_ids(pdev, ptr, size);
+        }
     }
 
     pci_register_bar(pdev, PCI_ROM_SLOT, 0, &pdev->rom);
-- 
MST



  parent reply	other threads:[~2023-06-26 12:31 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-26 12:27 [PULL 00/53] virtio,pc,pci: fixes, features, cleanups Michael S. Tsirkin
2023-06-26 12:27 ` [PULL 01/53] bswap: Add the ability to store to an unaligned 24 bit field Michael S. Tsirkin
2023-06-26 12:27 ` [PULL 02/53] hw/cxl: QMP based poison injection support Michael S. Tsirkin
2023-06-26 12:27 ` [PULL 03/53] hw/cxl: Add poison injection via the mailbox Michael S. Tsirkin
2023-06-26 12:27 ` [PULL 04/53] hw/cxl: Add clear poison mailbox command support Michael S. Tsirkin
2024-05-03 12:45   ` Peter Maydell
2024-05-31 12:38     ` Peter Maydell
2024-05-31 16:23       ` Ira Weiny
2023-06-26 12:28 ` [PULL 05/53] hw/cxl/events: Add event status register Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 06/53] hw/cxl: Move CXLRetCode definition to cxl_device.h Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 07/53] hw/cxl/events: Wire up get/clear event mailbox commands Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 08/53] hw/cxl/events: Add event interrupt support Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 09/53] hw/cxl/events: Add injection of General Media Events Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 10/53] hw/cxl/events: Add injection of DRAM events Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 11/53] hw/cxl/events: Add injection of Memory Module Events Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 12/53] cryptodev-vhost-user: add asymmetric crypto support Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 13/53] softmmu: Introduce qemu_target_page_mask() helper Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 14/53] hw/scsi: Introduce VHOST_SCSI_COMMON symbol in Kconfig Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 15/53] hw/scsi: Rearrange meson.build Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 16/53] hw/scsi: Rename target-specific source set as 'specific_virtio_scsi_ss' Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 17/53] hw/virtio: Introduce VHOST_VSOCK_COMMON symbol in Kconfig Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 18/53] hw/virtio/virtio-mem: Use qemu_ram_get_fd() helper Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 19/53] hw/virtio/vhost-vsock: Include missing 'virtio/virtio-bus.h' header Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 20/53] hw/virtio/virtio-iommu: Use target-agnostic qemu_target_page_mask() Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 21/53] hw/virtio: Remove unnecessary 'virtio-access.h' header Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 22/53] hw/virtio: Build various target-agnostic objects just once Michael S. Tsirkin
2023-06-26 12:28 ` [PULL 23/53] vhost: release memory_listener object in error path Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 24/53] vhost: release virtqueue objects " Michael S. Tsirkin
2023-06-26 12:29 ` Michael S. Tsirkin [this message]
2023-06-26 12:29 ` [PULL 26/53] virtio-mem: Simplify bitmap handling and virtio_mem_set_block_state() Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 27/53] vdpa: return errno in vhost_vdpa_get_vring_group error Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 28/53] vdpa: move CVQ isolation check to net_init_vhost_vdpa Michael S. Tsirkin
2023-06-27 11:30   ` Peter Maydell
2023-09-15 14:52     ` Peter Maydell
2023-09-15 15:56       ` Eugenio Perez Martin
2023-06-26 12:29 ` [PULL 29/53] cryptodev: fix memory leak during stats query Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 30/53] hw/acpi: Fix PM control register access Michael S. Tsirkin
2023-06-26 13:20   ` Igor Mammedov
2023-06-26 13:49     ` Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 31/53] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models Michael S. Tsirkin
2023-11-28 13:57   ` Fiona Ebner
2023-11-28 14:13     ` Daniel P. Berrangé
2023-11-28 14:53       ` Fiona Ebner
2023-11-28 16:00         ` Michael S. Tsirkin
2023-11-28 16:04           ` Daniel P. Berrangé
2023-11-29 10:01           ` Igor Mammedov
2023-11-30 11:22             ` Igor Mammedov
2023-11-30 11:47               ` Gerd Hoffmann
2023-11-30 12:45                 ` Fiona Ebner
2023-12-29 15:35               ` Igor Mammedov
2023-12-29 15:45                 ` Michael S. Tsirkin
2024-01-03  8:51                   ` Igor Mammedov
2023-06-26 12:29 ` [PULL 32/53] tests/data/acpi: update after SMBIOS 2.0 change Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 33/53] pc: q35: Bump max_cpus to 1024 Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 34/53] vdpa: do not block migration if device has cvq and x-svq=on Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 35/53] vdpa: reorder vhost_vdpa_net_cvq_cmd_page_len function Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 36/53] vdpa: map shadow vrings with MAP_SHARED Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 37/53] include/hw/virtio: make some VirtIODevice const Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 38/53] vdpa: reuse virtio_vdev_has_feature() Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 39/53] hw/net/virtio-net: make some VirtIONet const Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 40/53] virtio-net: expose virtio_net_supported_guest_offloads() Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 41/53] vdpa: Add vhost_vdpa_net_load_offloads() Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 42/53] vdpa: Allow VIRTIO_NET_F_CTRL_GUEST_OFFLOADS in SVQ Michael S. Tsirkin
2023-06-26 12:29 ` [PULL 43/53] vhost: fix vhost_dev_enable_notifiers() error case Michael S. Tsirkin
2023-06-26 12:30 ` [PULL 44/53] vdpa: mask _F_CTRL_GUEST_OFFLOADS for vhost vdpa devices Michael S. Tsirkin
2023-06-26 12:30 ` [PULL 45/53] vdpa: fix not using CVQ buffer in case of error Michael S. Tsirkin
2023-06-26 12:30 ` [PULL 46/53] hw/i386/pc: Clean up pc_machine_initfn Michael S. Tsirkin
2023-06-26 12:30 ` [PULL 47/53] virtio-scsi: avoid dangling host notifier in ->ioeventfd_stop() Michael S. Tsirkin
2023-06-26 12:30 ` [PULL 48/53] vhost-user: fully use new backend/frontend naming Michael S. Tsirkin
2023-06-26 12:30 ` [PULL 49/53] intel_iommu: Fix a potential issue in VFIO dirty page sync Michael S. Tsirkin
2023-06-26 12:30 ` [PULL 50/53] intel_iommu: Fix flag check in replay Michael S. Tsirkin
2023-06-26 12:30 ` [PULL 51/53] intel_iommu: Fix address space unmap Michael S. Tsirkin
2023-06-26 12:30 ` [PULL 52/53] vhost_net: add an assertion for TAP client backends Michael S. Tsirkin
2023-06-28  6:28   ` Cédric Le Goater
2023-06-28  6:45     ` Ani Sinha
2023-06-28  7:30       ` Cédric Le Goater
2023-06-28 10:33         ` Ani Sinha
2023-06-28 10:50           ` Michael S. Tsirkin
2023-06-26 12:30 ` [PULL 53/53] vhost-vdpa: do not cleanup the vdpa/vhost-net structures if peer nic is present Michael S. Tsirkin
2023-06-26 15:53   ` Michael Tokarev
2023-06-27  4:35     ` Ani Sinha
2023-06-26 13:51 ` [PULL 00/53] virtio,pc,pci: fixes, features, cleanups Michael S. Tsirkin
2023-06-26 15:32 ` Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8eb85fb5ac60bfb6ee4c729cc087078d490670c4.1687782442.git.mst@redhat.com \
    --to=mst@redhat.com \
    --cc=david@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=vsementsov@yandex-team.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).