[PATCH v2 1/8] hw/sd/sdcard: Fix size check for backing block image

Jan Kiszka posted 8 patches 3 weeks, 6 days ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Bin Meng <bmeng.cn@gmail.com>
There is a newer version of this series
[PATCH v2 1/8] hw/sd/sdcard: Fix size check for backing block image
Posted by Jan Kiszka 3 weeks, 6 days ago
From: Jan Kiszka <jan.kiszka@siemens.com>

The power-of-2 rule applies to the user data area, not the complete
block image. The latter can be concatenation of boot partition images
and the user data.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8c290595f0..16aee210b4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2789,7 +2789,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
             return;
         }
 
-        blk_size = blk_getlength(sd->blk);
+        blk_size = blk_getlength(sd->blk) - sd->boot_part_size * 2;
         if (blk_size > 0 && !is_power_of_2(blk_size)) {
             int64_t blk_size_aligned = pow2ceil(blk_size);
             char *blk_size_str;
-- 
2.43.0


Re: [PATCH v2 1/8] hw/sd/sdcard: Fix size check for backing block image
Posted by Philippe Mathieu-Daudé 3 weeks, 5 days ago
On 1/9/25 07:56, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> The power-of-2 rule applies to the user data area, not the complete
> block image. The latter can be concatenation of boot partition images
> and the user data.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/sd/sd.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 8c290595f0..16aee210b4 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -2789,7 +2789,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
>               return;
>           }
>   
> -        blk_size = blk_getlength(sd->blk);
> +        blk_size = blk_getlength(sd->blk) - sd->boot_part_size * 2;
>           if (blk_size > 0 && !is_power_of_2(blk_size)) {
>               int64_t blk_size_aligned = pow2ceil(blk_size);
>               char *blk_size_str;

This seems to break the tests/functional/arm/test_aspeed_rainier.py
test due to mmc-p10bmc-20240617.qcow2 size:

Command: /builds/qemu-project/qemu/build/qemu-system-arm -display none 
-vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control 
-machine rainier-bmc -chardev socket,id=console,fd=10 -serial 
chardev:console -drive 
file=/builds/qemu-project/qemu/functional-cache/download/d523fb478d2b84d5adc5658d08502bc64b1486955683814f89c6137518acd90b,if=sd,id=sd2,index=2 
-net nic -net user -snapshot
Output: qemu-system-arm: Invalid SD card size: 16 GiB
SD card size has to be a power of 2, e.g. 16 GiB.

https://gitlab.com/qemu-project/qemu/-/jobs/11217561316


Re: [PATCH v2 1/8] hw/sd/sdcard: Fix size check for backing block image
Posted by Jan Kiszka 3 weeks, 4 days ago
On 02.09.25 17:06, Philippe Mathieu-Daudé wrote:
> On 1/9/25 07:56, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> The power-of-2 rule applies to the user data area, not the complete
>> block image. The latter can be concatenation of boot partition images
>> and the user data.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/sd/sd.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 8c290595f0..16aee210b4 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -2789,7 +2789,7 @@ static void sd_realize(DeviceState *dev, Error
>> **errp)
>>               return;
>>           }
>>   -        blk_size = blk_getlength(sd->blk);
>> +        blk_size = blk_getlength(sd->blk) - sd->boot_part_size * 2;
>>           if (blk_size > 0 && !is_power_of_2(blk_size)) {
>>               int64_t blk_size_aligned = pow2ceil(blk_size);
>>               char *blk_size_str;
> 
> This seems to break the tests/functional/arm/test_aspeed_rainier.py
> test due to mmc-p10bmc-20240617.qcow2 size:
> 
> Command: /builds/qemu-project/qemu/build/qemu-system-arm -display none -
> vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -
> machine rainier-bmc -chardev socket,id=console,fd=10 -serial
> chardev:console -drive file=/builds/qemu-project/qemu/functional-cache/
> download/
> d523fb478d2b84d5adc5658d08502bc64b1486955683814f89c6137518acd90b,if=sd,id=sd2,index=2 -net nic -net user -snapshot
> Output: qemu-system-arm: Invalid SD card size: 16 GiB
> SD card size has to be a power of 2, e.g. 16 GiB.
> 
> https://gitlab.com/qemu-project/qemu/-/jobs/11217561316
> 

Hmm, then the test was always wrong as well. I suspect the aspeed is
enabling boot partitions by default, and the image was created to pass
the wrong alignment check. Where / by whom is the image maintained?

Jan

-- 
Siemens AG, Foundational Technologies
Linux Expert Center

Re: [PATCH v2 1/8] hw/sd/sdcard: Fix size check for backing block image
Posted by Cédric Le Goater 3 weeks, 4 days ago
On 9/2/25 17:34, Jan Kiszka wrote:
> On 02.09.25 17:06, Philippe Mathieu-Daudé wrote:
>> On 1/9/25 07:56, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> The power-of-2 rule applies to the user data area, not the complete
>>> block image. The latter can be concatenation of boot partition images
>>> and the user data.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    hw/sd/sd.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>> index 8c290595f0..16aee210b4 100644
>>> --- a/hw/sd/sd.c
>>> +++ b/hw/sd/sd.c
>>> @@ -2789,7 +2789,7 @@ static void sd_realize(DeviceState *dev, Error
>>> **errp)
>>>                return;
>>>            }
>>>    -        blk_size = blk_getlength(sd->blk);
>>> +        blk_size = blk_getlength(sd->blk) - sd->boot_part_size * 2;
>>>            if (blk_size > 0 && !is_power_of_2(blk_size)) {
>>>                int64_t blk_size_aligned = pow2ceil(blk_size);
>>>                char *blk_size_str;
>>
>> This seems to break the tests/functional/arm/test_aspeed_rainier.py
>> test due to mmc-p10bmc-20240617.qcow2 size:
>>
>> Command: /builds/qemu-project/qemu/build/qemu-system-arm -display none -
>> vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -
>> machine rainier-bmc -chardev socket,id=console,fd=10 -serial
>> chardev:console -drive file=/builds/qemu-project/qemu/functional-cache/
>> download/
>> d523fb478d2b84d5adc5658d08502bc64b1486955683814f89c6137518acd90b,if=sd,id=sd2,index=2 -net nic -net user -snapshot
>> Output: qemu-system-arm: Invalid SD card size: 16 GiB
>> SD card size has to be a power of 2, e.g. 16 GiB.
>>
>> https://gitlab.com/qemu-project/qemu/-/jobs/11217561316
>>
> 
> Hmm, then the test was always wrong as well. I suspect the aspeed is
> enabling boot partitions by default, 

Yes for the rainier machine which boots from eMMC :

         if (emmc && boot_emmc) {
             qdev_prop_set_uint64(card, "boot-partition-size", 1 * MiB);
             qdev_prop_set_uint8(card, "boot-config", 0x1 << 3);
         }

In this case, the emmc image is tailored to support boot. It's no
different from real HW AFAICT. It's old (6/7y).

Thanks,

C.


Re: [PATCH v2 1/8] hw/sd/sdcard: Fix size check for backing block image
Posted by Cédric Le Goater 3 weeks, 4 days ago
On 9/2/25 17:34, Jan Kiszka wrote:
> On 02.09.25 17:06, Philippe Mathieu-Daudé wrote:
>> On 1/9/25 07:56, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> The power-of-2 rule applies to the user data area, not the complete
>>> block image. The latter can be concatenation of boot partition images
>>> and the user data.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    hw/sd/sd.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>> index 8c290595f0..16aee210b4 100644
>>> --- a/hw/sd/sd.c
>>> +++ b/hw/sd/sd.c
>>> @@ -2789,7 +2789,7 @@ static void sd_realize(DeviceState *dev, Error
>>> **errp)
>>>                return;
>>>            }
>>>    -        blk_size = blk_getlength(sd->blk);
>>> +        blk_size = blk_getlength(sd->blk) - sd->boot_part_size * 2;
>>>            if (blk_size > 0 && !is_power_of_2(blk_size)) {
>>>                int64_t blk_size_aligned = pow2ceil(blk_size);
>>>                char *blk_size_str;
>>
>> This seems to break the tests/functional/arm/test_aspeed_rainier.py
>> test due to mmc-p10bmc-20240617.qcow2 size:
>>
>> Command: /builds/qemu-project/qemu/build/qemu-system-arm -display none -
>> vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -
>> machine rainier-bmc -chardev socket,id=console,fd=10 -serial
>> chardev:console -drive file=/builds/qemu-project/qemu/functional-cache/
>> download/
>> d523fb478d2b84d5adc5658d08502bc64b1486955683814f89c6137518acd90b,if=sd,id=sd2,index=2 -net nic -net user -snapshot
>> Output: qemu-system-arm: Invalid SD card size: 16 GiB
>> SD card size has to be a power of 2, e.g. 16 GiB.
>>
>> https://gitlab.com/qemu-project/qemu/-/jobs/11217561316
>>
> 
> Hmm, then the test was always wrong as well. I suspect the aspeed is
> enabling boot partitions by default, and the image was created to pass
> the wrong alignment check. Where / by whom is the image maintained?

See this commit to know how it is constructed :

   https://gitlab.com/qemu-project/qemu/-/commit/c8cb19876d3e

Thanks,

C.



Re: [PATCH v2 1/8] hw/sd/sdcard: Fix size check for backing block image
Posted by Philippe Mathieu-Daudé 3 weeks, 4 days ago
On 2/9/25 17:43, Cédric Le Goater wrote:
> On 9/2/25 17:34, Jan Kiszka wrote:
>> On 02.09.25 17:06, Philippe Mathieu-Daudé wrote:
>>> On 1/9/25 07:56, Jan Kiszka wrote:
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> The power-of-2 rule applies to the user data area, not the complete
>>>> block image. The latter can be concatenation of boot partition images
>>>> and the user data.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>>    hw/sd/sd.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>>> index 8c290595f0..16aee210b4 100644
>>>> --- a/hw/sd/sd.c
>>>> +++ b/hw/sd/sd.c
>>>> @@ -2789,7 +2789,7 @@ static void sd_realize(DeviceState *dev, Error
>>>> **errp)
>>>>                return;
>>>>            }
>>>>    -        blk_size = blk_getlength(sd->blk);
>>>> +        blk_size = blk_getlength(sd->blk) - sd->boot_part_size * 2;
>>>>            if (blk_size > 0 && !is_power_of_2(blk_size)) {
>>>>                int64_t blk_size_aligned = pow2ceil(blk_size);
>>>>                char *blk_size_str;
>>>
>>> This seems to break the tests/functional/arm/test_aspeed_rainier.py
>>> test due to mmc-p10bmc-20240617.qcow2 size:
>>>
>>> Command: /builds/qemu-project/qemu/build/qemu-system-arm -display none -
>>> vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -
>>> machine rainier-bmc -chardev socket,id=console,fd=10 -serial
>>> chardev:console -drive file=/builds/qemu-project/qemu/functional-cache/
>>> download/
>>> d523fb478d2b84d5adc5658d08502bc64b1486955683814f89c6137518acd90b,if=sd,id=sd2,index=2 -net nic -net user -snapshot
>>> Output: qemu-system-arm: Invalid SD card size: 16 GiB
>>> SD card size has to be a power of 2, e.g. 16 GiB.
>>>
>>> https://gitlab.com/qemu-project/qemu/-/jobs/11217561316
>>>
>>
>> Hmm, then the test was always wrong as well. I suspect the aspeed is
>> enabling boot partitions by default, and the image was created to pass
>> the wrong alignment check. Where / by whom is the image maintained?
> 
> See this commit to know how it is constructed :
> 
>    https://gitlab.com/qemu-project/qemu/-/commit/c8cb19876d3e

Hmm now I remember. I wasn't enthusiastic about that approach and
suggested to use block drive for each partition, but it was not
well received as too complex on the command line.

Re: [PATCH v2 1/8] hw/sd/sdcard: Fix size check for backing block image
Posted by Philippe Mathieu-Daudé 3 weeks, 4 days ago
On 2/9/25 17:34, Jan Kiszka wrote:
> On 02.09.25 17:06, Philippe Mathieu-Daudé wrote:
>> On 1/9/25 07:56, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> The power-of-2 rule applies to the user data area, not the complete
>>> block image. The latter can be concatenation of boot partition images
>>> and the user data.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    hw/sd/sd.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>> index 8c290595f0..16aee210b4 100644
>>> --- a/hw/sd/sd.c
>>> +++ b/hw/sd/sd.c
>>> @@ -2789,7 +2789,7 @@ static void sd_realize(DeviceState *dev, Error
>>> **errp)
>>>                return;
>>>            }
>>>    -        blk_size = blk_getlength(sd->blk);
>>> +        blk_size = blk_getlength(sd->blk) - sd->boot_part_size * 2;
>>>            if (blk_size > 0 && !is_power_of_2(blk_size)) {
>>>                int64_t blk_size_aligned = pow2ceil(blk_size);
>>>                char *blk_size_str;
>>
>> This seems to break the tests/functional/arm/test_aspeed_rainier.py
>> test due to mmc-p10bmc-20240617.qcow2 size:
>>
>> Command: /builds/qemu-project/qemu/build/qemu-system-arm -display none -
>> vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -
>> machine rainier-bmc -chardev socket,id=console,fd=10 -serial
>> chardev:console -drive file=/builds/qemu-project/qemu/functional-cache/
>> download/
>> d523fb478d2b84d5adc5658d08502bc64b1486955683814f89c6137518acd90b,if=sd,id=sd2,index=2 -net nic -net user -snapshot
>> Output: qemu-system-arm: Invalid SD card size: 16 GiB
>> SD card size has to be a power of 2, e.g. 16 GiB.
>>
>> https://gitlab.com/qemu-project/qemu/-/jobs/11217561316
>>
> 
> Hmm, then the test was always wrong as well. I suspect the aspeed is
> enabling boot partitions by default, and the image was created to pass
> the wrong alignment check. Where / by whom is the image maintained?

Cédric Le Goater (Cc'ed).

The test comes from:
https://lore.kernel.org/qemu-devel/4d1777d6-0195-4ecb-a85f-09964268533d@kaod.org/

Maybe also relevant to your suspicion:
https://lore.kernel.org/qemu-devel/e401d119-402e-0edd-c2bf-28950ba48ccb@kaod.org/