[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/3] pci: ROM preallocation for incoming migration
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH 3/3] pci: ROM preallocation for incoming migration |
Date: |
Tue, 25 Apr 2023 08:43:10 -0400 |
On Tue, Apr 25, 2023 at 01:56:03PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 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.
>
> 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
>
> This commit brings new behavior for romfile="",romsize=SIZE combination
> of options. Current behavior is just ignore romsize and not load or
> create any ROM.
>
> Let's instead preallocate ROM, not loading any file. This way we can
> migrate old vm to new environment not thinking about ROM files on
> destination host:
>
> 1. specify romfile="",romsize=SIZE on target, with correct SIZE
> (actually, size of romfile on source aligned up to power of two, or
> just original romsize option on source)
>
> 2. On device realize we just preallocate ROM, and not load any file
>
> 3. On incoming migration ROM is filled from the migration stream
>
> As a bonus we avoid extra reading from ROM file on target.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
why is this a bad idea:
- on source presumably user overrides romfile
- we have a general rule that source and destination flags must match
I propose instead to ignore romfile if qemu is incoming migration
and romsize has been specified.
> ---
> hw/pci/pci.c | 76 +++++++++++++++++++++++++++++-----------------------
> 1 file changed, 43 insertions(+), 33 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index a442f8fce1..039e762b66 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2293,17 +2293,21 @@ static void pci_add_option_rom(PCIDevice *pdev, bool
> is_default_rom,
> {
> int64_t size;
> g_autofree char *path = NULL;
> - void *ptr;
> char name[32];
> const VMStateDescription *vmsd;
> + bool load_file;
>
> if (!pdev->romfile) {
> return;
> }
> - if (strlen(pdev->romfile) == 0) {
> +
> + load_file = strlen(pdev->romfile) > 0;
> + if (!load_file && pdev->romsize == -1) {
> return;
> }
>
> + assert(load_file || !is_default_rom);
> +
> if (!pdev->rom_bar) {
> /*
> * Load rom via fw_cfg instead of creating a rom bar,
> @@ -2329,32 +2333,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) {
> + 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));
> @@ -2365,15 +2372,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);
> --
> 2.34.1
- [PATCH 0/3] ROM migration, Vladimir Sementsov-Ogievskiy, 2023/04/25
- [PATCH 3/3] pci: ROM preallocation for incoming migration, Vladimir Sementsov-Ogievskiy, 2023/04/25
- Re: [PATCH 3/3] pci: ROM preallocation for incoming migration,
Michael S. Tsirkin <=
- Re: [PATCH 3/3] pci: ROM preallocation for incoming migration, Vladimir Sementsov-Ogievskiy, 2023/04/25
- Re: [PATCH 3/3] pci: ROM preallocation for incoming migration, Vladimir Sementsov-Ogievskiy, 2023/04/25
- Re: [PATCH 3/3] pci: ROM preallocation for incoming migration, Michael S. Tsirkin, 2023/04/25
- Re: [PATCH 3/3] pci: ROM preallocation for incoming migration, Igor Mammedov, 2023/04/25
- Re: [PATCH 3/3] pci: ROM preallocation for incoming migration, Vladimir Sementsov-Ogievskiy, 2023/04/25
[PATCH 2/3] pci: pci_add_option_rom(): refactor: use g_autofree for path variable, Vladimir Sementsov-Ogievskiy, 2023/04/25
[PATCH 1/3] pci: pci_add_option_rom(): improve style, Vladimir Sementsov-Ogievskiy, 2023/04/25