Message ID | 20221028163951.810456-60-danielhb413@gmail.com |
---|---|
State | New |
Headers | show |
Series | [PULL,01/62] target/ppc: fix msgclr/msgsnd insns flags | expand |
There is a report that this commit breaks an existing OVMF setup: https://gitlab.com/qemu-project/qemu/-/issues/1290#note_1156507334 I'm not familiar with pflash. Please find a way to avoid a regression in QEMU 7.2 here. Thank you! Stefan
On 1/11/22 23:23, Stefan Hajnoczi wrote: > There is a report that this commit breaks an existing OVMF setup: > https://gitlab.com/qemu-project/qemu/-/issues/1290#note_1156507334 > > I'm not familiar with pflash. Please find a way to avoid a regression > in QEMU 7.2 here. Long-standing problem with pflash and underlying images... i.e: https://lore.kernel.org/qemu-devel/20190308062455.29755-1-armbru@redhat.com/ Let's revert for 7.2. Daniel, I can prepare a patch explaining.
On 11/1/22 19:49, Philippe Mathieu-Daudé wrote: > On 1/11/22 23:23, Stefan Hajnoczi wrote: >> There is a report that this commit breaks an existing OVMF setup: >> https://gitlab.com/qemu-project/qemu/-/issues/1290#note_1156507334 >> >> I'm not familiar with pflash. Please find a way to avoid a regression >> in QEMU 7.2 here. > > Long-standing problem with pflash and underlying images... i.e: > https://lore.kernel.org/qemu-devel/20190308062455.29755-1-armbru@redhat.com/ > > Let's revert for 7.2. Daniel, I can prepare a patch explaining. I appreciate if you can send a revert with the proper explanation. I can make a PR with it. Daniel
Phil, On 11/1/22 19:49, Philippe Mathieu-Daudé wrote: > On 1/11/22 23:23, Stefan Hajnoczi wrote: >> There is a report that this commit breaks an existing OVMF setup: >> https://gitlab.com/qemu-project/qemu/-/issues/1290#note_1156507334 >> >> I'm not familiar with pflash. Please find a way to avoid a regression >> in QEMU 7.2 here. > > Long-standing problem with pflash and underlying images... i.e: > https://lore.kernel.org/qemu-devel/20190308062455.29755-1-armbru@redhat.com/ > > Let's revert for 7.2. Daniel, I can prepare a patch explaining. Just sent a revert. I'm not sure if the explanation I provided is good enough. I appreciate if you can review it. If it's plausible I'll send a pull request ASAP. Thanks, Daniel
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 0cbc2fb4cb..9c235bf66e 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -690,7 +690,7 @@ static const MemoryRegionOps pflash_cfi01_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static void pflash_cfi01_fill_cfi_table(PFlashCFI01 *pfl) +static void pflash_cfi01_fill_cfi_table(PFlashCFI01 *pfl, Error **errp) { uint64_t blocks_per_device, sector_len_per_device, device_len; int num_devices; @@ -708,6 +708,10 @@ static void pflash_cfi01_fill_cfi_table(PFlashCFI01 *pfl) sector_len_per_device = pfl->sector_len / num_devices; } device_len = sector_len_per_device * blocks_per_device; + if (!is_power_of_2(device_len)) { + error_setg(errp, "Device size must be a power of two."); + return; + } /* Hardcoded CFI table */ /* Standard "QRY" string */ @@ -865,7 +869,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp) */ pfl->cmd = 0x00; pfl->status = 0x80; /* WSM ready */ - pflash_cfi01_fill_cfi_table(pfl); + pflash_cfi01_fill_cfi_table(pfl, errp); } static void pflash_cfi01_system_reset(DeviceState *dev) diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c index 2a99b286b0..ff2fe154c1 100644 --- a/hw/block/pflash_cfi02.c +++ b/hw/block/pflash_cfi02.c @@ -880,6 +880,11 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp) return; } + if (!is_power_of_2(pfl->chip_len)) { + error_setg(errp, "Device size must be a power of two."); + return; + } + memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl), &pflash_cfi02_ops, pfl, pfl->name, pfl->chip_len, errp);