[PATCH] Revert "hw/block/pflash_cfi0{1, 2}: Error out if device length isn't a power of two"

[PATCH] Revert "hw/block/pflash_cfi0{1, 2}: Error out if device length isn't a power of two"

[Daniel Henrique Barboza]

This commit caused a regression [1] that prevents machines that uses
Open Virtual Machine Firmware (OVMF) to boot.

This is a long standing behavior with how pflash handles images. More
information about why this happens can be found in [2] and commit
06f1521795 ("pflash: Require backend size to match device, improve
errors").

This reverts commit 334c388f25707a234c4a0dea05b9df08d7746638.

[1] https://gitlab.com/qemu-project/qemu/-/issues/1294
[2] https://lore.kernel.org/qemu-devel/20190308062455.29755-1-armbru@redhat.com/

Cc: Bernhard Beschow <shentey@gmail.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Stefan Hajnoczi <stefanha@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1294
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/block/pflash_cfi01.c | 8 ++------
 hw/block/pflash_cfi02.c | 5 -----
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 9c235bf66e..0cbc2fb4cb 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, Error **errp)
+static void pflash_cfi01_fill_cfi_table(PFlashCFI01 *pfl)
 {
     uint64_t blocks_per_device, sector_len_per_device, device_len;
     int num_devices;
@@ -708,10 +708,6 @@ static void pflash_cfi01_fill_cfi_table(PFlashCFI01 *pfl, Error **errp)
         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 */
@@ -869,7 +865,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
      */
     pfl->cmd = 0x00;
     pfl->status = 0x80; /* WSM ready */
-    pflash_cfi01_fill_cfi_table(pfl, errp);
+    pflash_cfi01_fill_cfi_table(pfl);
 }
 
 static void pflash_cfi01_system_reset(DeviceState *dev)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index ff2fe154c1..2a99b286b0 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -880,11 +880,6 @@ 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);
-- 
2.37.3

Re: [PATCH] Revert "hw/block/pflash_cfi0{1, 2}: Error out if device length isn't a power of two"

[Philippe Mathieu-Daudé]

On 8/11/22 18:26, Daniel Henrique Barboza wrote:
> This commit caused a regression [1] that prevents machines that uses
> Open Virtual Machine Firmware (OVMF) to boot.
> 
> This is a long standing behavior with how pflash handles images. More
> information about why this happens can be found in [2] and commit
> 06f1521795 ("pflash: Require backend size to match device, improve
> errors").
> 
> This reverts commit 334c388f25707a234c4a0dea05b9df08d7746638.
> 
> [1] https://gitlab.com/qemu-project/qemu/-/issues/1294
> [2] https://lore.kernel.org/qemu-devel/20190308062455.29755-1-armbru@redhat.com/
> 
> Cc: Bernhard Beschow <shentey@gmail.com>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Stefan Hajnoczi <stefanha@gmail.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1294
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>   hw/block/pflash_cfi01.c | 8 ++------
>   hw/block/pflash_cfi02.c | 5 -----
>   2 files changed, 2 insertions(+), 11 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks, our patches crossed :)
https://lore.kernel.org/qemu-devel/20221108175755.95141-1-philmd@linaro.org/

I'm queuing yours which is first and will amend the description
(if you don't disagree).

Regards,

Phil.

Re: [PATCH] Revert "hw/block/pflash_cfi0{1, 2}: Error out if device length isn't a power of two"

[Stefan Hajnoczi]

On Tue, 8 Nov 2022 at 13:10, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 8/11/22 18:26, Daniel Henrique Barboza wrote:
> > This commit caused a regression [1] that prevents machines that uses
> > Open Virtual Machine Firmware (OVMF) to boot.
> >
> > This is a long standing behavior with how pflash handles images. More
> > information about why this happens can be found in [2] and commit
> > 06f1521795 ("pflash: Require backend size to match device, improve
> > errors").
> >
> > This reverts commit 334c388f25707a234c4a0dea05b9df08d7746638.
> >
> > [1] https://gitlab.com/qemu-project/qemu/-/issues/1294
> > [2] https://lore.kernel.org/qemu-devel/20190308062455.29755-1-armbru@redhat.com/
> >
> > Cc: Bernhard Beschow <shentey@gmail.com>
> > Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Cc: Stefan Hajnoczi <stefanha@gmail.com>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1294
> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> > ---
> >   hw/block/pflash_cfi01.c | 8 ++------
> >   hw/block/pflash_cfi02.c | 5 -----
> >   2 files changed, 2 insertions(+), 11 deletions(-)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Thanks, our patches crossed :)
> https://lore.kernel.org/qemu-devel/20221108175755.95141-1-philmd@linaro.org/
>
> I'm queuing yours which is first and will amend the description
> (if you don't disagree).

I've already applied yours, Philippe, because the description is more
comprehensive.

Daniel, thank you for sending your version of the patch!

Stefan

Re: [PATCH] Revert "hw/block/pflash_cfi0{1, 2}: Error out if device length isn't a power of two"

[Daniel Henrique Barboza]

On 11/8/22 15:13, Stefan Hajnoczi wrote:
> On Tue, 8 Nov 2022 at 13:10, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 8/11/22 18:26, Daniel Henrique Barboza wrote:
>>> This commit caused a regression [1] that prevents machines that uses
>>> Open Virtual Machine Firmware (OVMF) to boot.
>>>
>>> This is a long standing behavior with how pflash handles images. More
>>> information about why this happens can be found in [2] and commit
>>> 06f1521795 ("pflash: Require backend size to match device, improve
>>> errors").
>>>
>>> This reverts commit 334c388f25707a234c4a0dea05b9df08d7746638.
>>>
>>> [1] https://gitlab.com/qemu-project/qemu/-/issues/1294
>>> [2] https://lore.kernel.org/qemu-devel/20190308062455.29755-1-armbru@redhat.com/
>>>
>>> Cc: Bernhard Beschow <shentey@gmail.com>
>>> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Cc: Stefan Hajnoczi <stefanha@gmail.com>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1294
>>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>>> ---
>>>    hw/block/pflash_cfi01.c | 8 ++------
>>>    hw/block/pflash_cfi02.c | 5 -----
>>>    2 files changed, 2 insertions(+), 11 deletions(-)
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> Thanks, our patches crossed :)
>> https://lore.kernel.org/qemu-devel/20221108175755.95141-1-philmd@linaro.org/

Well of course that we both decided to send the PR almost at the same time :D

>>
>> I'm queuing yours which is first and will amend the description
>> (if you don't disagree).
> 
> I've already applied yours, Philippe, because the description is more
> comprehensive.
> 
> Daniel, thank you for sending your version of the patch!

Thank you Phil for sending the revert and Stefan for quickly queueing
it. The sooner we get rid of the regression the better.


Daniel



> 
> Stefan