Message ID | 20230515125229.44836-4-vsementsov@yandex-team.ru (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | ROM migration | expand |
On Mon, May 15, 2023 at 03:52:29PM +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, 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" } } > > 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> Breaks build here: In function ‘pci_add_option_rom’, inlined from ‘pci_qdev_realize’ at ../hw/pci/pci.c:2155:5: ../hw/pci/pci.c:2395:13: error: ‘size’ may be used uninitialized [-Werror=maybe-uninitialized] 2395 | if (load_image_size(path, ptr, size) < 0) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../hw/pci/pci.c: In function ‘pci_qdev_realize’: ../hw/pci/pci.c:2312:13: note: ‘size’ was declared here 2312 | int64_t size; | ^~~~ > --- > hw/pci/pci.c | 77 ++++++++++++++++++++++++++++++---------------------- > 1 file changed, 45 insertions(+), 32 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 3a0107758c..0f0c83c02f 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,10 +2309,16 @@ 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; > > + /* > + * 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; > } > @@ -2341,32 +2348,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)); > @@ -2377,15 +2387,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
On 19.05.23 08:34, Michael S. Tsirkin wrote: > On Mon, May 15, 2023 at 03:52:29PM +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, 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" } } >> >> 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> > > > Breaks build here: > > In function ‘pci_add_option_rom’, > inlined from ‘pci_qdev_realize’ at ../hw/pci/pci.c:2155:5: > ../hw/pci/pci.c:2395:13: error: ‘size’ may be used uninitialized [-Werror=maybe-uninitialized] > 2395 | if (load_image_size(path, ptr, size) < 0) { > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ../hw/pci/pci.c: In function ‘pci_qdev_realize’: > ../hw/pci/pci.c:2312:13: note: ‘size’ was declared here > 2312 | int64_t size; > | ^~~~ > > Hmm, but works for me. Anyway that's obviously false-positive, if we are here, size is initialized in previous block if (load_file || ..). So, may be add simply this: diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 0f0c83c02f..075c998284 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2307,7 +2307,7 @@ 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; /* fix "uninitialized" false-positive */ g_autofree char *path = NULL; char name[32]; const VMStateDescription *vmsd; > >> --- >> hw/pci/pci.c | 77 ++++++++++++++++++++++++++++++---------------------- >> 1 file changed, 45 insertions(+), 32 deletions(-) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index 3a0107758c..0f0c83c02f 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,10 +2309,16 @@ 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; >> >> + /* >> + * 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; >> } >> @@ -2341,32 +2348,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)); >> @@ -2377,15 +2387,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 >
On Mon, May 22, 2023 at 11:44:32AM +0300, Vladimir Sementsov-Ogievskiy wrote: > On 19.05.23 08:34, Michael S. Tsirkin wrote: > > On Mon, May 15, 2023 at 03:52:29PM +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, 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" } } > > > > > > 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> > > > > > > Breaks build here: > > > > In function ‘pci_add_option_rom’, > > inlined from ‘pci_qdev_realize’ at ../hw/pci/pci.c:2155:5: > > ../hw/pci/pci.c:2395:13: error: ‘size’ may be used uninitialized [-Werror=maybe-uninitialized] > > 2395 | if (load_image_size(path, ptr, size) < 0) { > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > ../hw/pci/pci.c: In function ‘pci_qdev_realize’: > > ../hw/pci/pci.c:2312:13: note: ‘size’ was declared here > > 2312 | int64_t size; > > | ^~~~ > > > > > > Hmm, but works for me. Anyway that's obviously false-positive, if we are here, size is initialized in previous block if (load_file || ..). > > So, may be add simply this: > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 0f0c83c02f..075c998284 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -2307,7 +2307,7 @@ 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; /* fix "uninitialized" false-positive */ I'd even drop the comment, we will not remember to remove it. just mention in commit log. OK, pls repost with this fix. Minor so include acks posted so far. Thanks! > g_autofree char *path = NULL; > char name[32]; > const VMStateDescription *vmsd; > > > > > > > --- > > > hw/pci/pci.c | 77 ++++++++++++++++++++++++++++++---------------------- > > > 1 file changed, 45 insertions(+), 32 deletions(-) > > > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > > index 3a0107758c..0f0c83c02f 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,10 +2309,16 @@ 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; > > > + /* > > > + * 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; > > > } > > > @@ -2341,32 +2348,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)); > > > @@ -2377,15 +2387,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 > > > > -- > Best regards, > Vladimir
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 3a0107758c..0f0c83c02f 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,10 +2309,16 @@ 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; + /* + * 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; } @@ -2341,32 +2348,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)); @@ -2377,15 +2387,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);