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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 2/9/25 18:00, Cédric Le Goater wrote:
> On 9/2/25 17:55, Philippe Mathieu-Daudé wrote:
>> On 2/9/25 17:47, Cédric Le Goater wrote:
>>> On 9/2/25 17:45, Philippe Mathieu-Daudé wrote:
>>>> On 2/9/25 17:43, Philippe Mathieu-Daudé wrote:
>>>>> 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/

[*]

>>>>
>>>> Digging further:
>>>> https://lore.kernel.org/qemu- 
>>>> devel/9046a4327336d4425f1e7e7a973edef9e9948e80.camel@pengutronix.de/
>>>>
>>>
>>> yes commit c078298301a8 might have some impact there.
>>
>> With Jan patch, your script doesn't need anymore the
>>
>>    echo "Fixing size to keep qemu happy..."
>>
>> kludge.
> 
> which script ?

The one you pasted in [*]:

--
#!/bin/sh

URLBASE=https://jenkins.openbmc.org/view/latest/job/latest-master/label=docker-builder,target=witherspoon-tacoma/lastSuccessfulBuild/artifact/openbmc/build/tmp/deploy/images/witherspoon-tacoma/

IMAGESIZE=128
OUTFILE=mmc.img

FILES="u-boot.bin u-boot-spl.bin 
obmc-phosphor-image-witherspoon-tacoma.wic.xz"

for file in ${FILES}; do

	if test -f ${file}; then
		echo "${file}: Already downloaded"
	else
		echo "${file}: Downloading"
		wget -nv ${URLBASE}/${file}
	fi
done

echo

echo "Creating empty image..."
dd status=none if=/dev/zero of=${OUTFILE} bs=1M count=${IMAGESIZE}
echo "Adding SPL..."
dd status=none if=u-boot-spl.bin of=${OUTFILE} conv=notrunc
echo "Adding u-boot..."
dd status=none if=u-boot.bin of=${OUTFILE} conv=notrunc bs=1K seek=64
echo "Adding userdata..."
unxz -c obmc-phosphor-image-witherspoon-tacoma.wic.xz | dd 
status=progress of=${OUTFILE} conv=notrunc bs=1M seek=2
echo "Fixing size to keep qemu happy..."
truncate --size 16G ${OUTFILE}

echo "Done!"
echo
echo " qemu-system-arm -M tacoma-bmc -nographic -drive 
file=mmc.img,if=sd,index=2,format=raw"
---