On 01.10.25 08:24, Cédric Le Goater wrote: > On 9/30/25 11:06, Jan Kiszka wrote: >> On 17.09.25 07:53, Cédric Le Goater wrote: >>> On 9/16/25 19:17, Jan Kiszka wrote: >>>> On 16.09.25 18:14, Cédric Le Goater wrote: >>>>> + Jamin >>>>> >>>>> On 9/16/25 13:39, Jan Kiszka wrote: >>>>>> On 14.09.25 14:46, Jan Kiszka wrote: >>>>>>> From: Jan Kiszka <[email protected]> >>>>>>> >>>>>>> Alignment rules apply the the individual partitions (user, boot, >>>>>>> later >>>>>>> on also RPMB) and depend both on the size of the image and the >>>>>>> type of >>>>>>> the device. Up to and including 2GB, the power-of-2 rule applies to >>>>>>> the >>>>>>> user data area. For larger images, multiples of 512 sectors must be >>>>>>> used >>>>>>> for eMMC and multiples of 512K for SD-cards. Fix the check >>>>>>> accordingly >>>>>>> and also detect if the image is too small to even hold the boot >>>>>>> partitions. >>>>>>> >>>>>>> Signed-off-by: Jan Kiszka <[email protected]> >>>>>>> Reviewed-by: Warner Losh <[email protected]> >>>>>>> --- >>>>>>> CC: Warner Losh <[email protected]> >>>>>>> CC: Cédric Le Goater <[email protected]> >>>>>>> CC: Joel Stanley <[email protected]> >>>>>>> CC: Alistair Francis <[email protected]> >>>>>>> CC: Alexander Bulekov <[email protected]> >>>>>>> --- >>>>>>> hw/sd/sd.c | 61 ++++++++++++++++++++++++++++++++++++ >>>>>>> +----------------- >>>>>>> 1 file changed, 42 insertions(+), 19 deletions(-) >>>>>>> >>>>>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c >>>>>>> index d7a496d77c..b42cd01d1f 100644 >>>>>>> --- a/hw/sd/sd.c >>>>>>> +++ b/hw/sd/sd.c >>>>>>> @@ -2759,6 +2759,28 @@ static void sd_instance_finalize(Object *obj) >>>>>>> timer_free(sd->ocr_power_timer); >>>>>>> } >>>>>>> +static void sd_blk_size_error(SDState *sd, int64_t blk_size, >>>>>>> + int64_t blk_size_aligned, const char >>>>>>> *rule, >>>>>>> + Error **errp) >>>>>>> +{ >>>>>>> + const char *dev_type = sd_is_emmc(sd) ? "eMMC" : "SD card"; >>>>>>> + char *blk_size_str; >>>>>>> + >>>>>>> + blk_size_str = size_to_str(blk_size); >>>>>>> + error_setg(errp, "Invalid %s size: %s", dev_type, >>>>>>> blk_size_str); >>>>>>> + g_free(blk_size_str); >>>>>>> + >>>>>>> + blk_size_str = size_to_str(blk_size_aligned); >>>>>>> + error_append_hint(errp, >>>>>>> + "%s size has to be %s, e.g. %s.\n" >>>>>>> + "You can resize disk images with" >>>>>>> + " 'qemu-img resize <imagefile> <new-size>'\n" >>>>>>> + "(note that this will lose data if you make >>>>>>> the" >>>>>>> + " image smaller than it currently is).\n", >>>>>>> + dev_type, rule, blk_size_str); >>>>>>> + g_free(blk_size_str); >>>>>>> +} >>>>>>> + >>>>>>> static void sd_realize(DeviceState *dev, Error **errp) >>>>>>> { >>>>>>> SDState *sd = SDMMC_COMMON(dev); >>>>>>> @@ -2781,25 +2803,26 @@ static void sd_realize(DeviceState *dev, >>>>>>> Error **errp) >>>>>>> return; >>>>>>> } >>>>>>> - blk_size = blk_getlength(sd->blk); >>>>>>> - if (blk_size > 0 && !is_power_of_2(blk_size)) { >>>>>>> - int64_t blk_size_aligned = pow2ceil(blk_size); >>>>>>> - char *blk_size_str; >>>>>>> - >>>>>>> - blk_size_str = size_to_str(blk_size); >>>>>>> - error_setg(errp, "Invalid SD card size: %s", >>>>>>> blk_size_str); >>>>>>> - g_free(blk_size_str); >>>>>>> - >>>>>>> - blk_size_str = size_to_str(blk_size_aligned); >>>>>>> - error_append_hint(errp, >>>>>>> - "SD card size has to be a power of 2, >>>>>>> e.g. %s.\n" >>>>>>> - "You can resize disk images with" >>>>>>> - " 'qemu-img resize <imagefile> <new- >>>>>>> size>'\n" >>>>>>> - "(note that this will lose data if >>>>>>> you >>>>>>> make the" >>>>>>> - " image smaller than it currently >>>>>>> is). >>>>>>> \n", >>>>>>> - blk_size_str); >>>>>>> - g_free(blk_size_str); >>>>>>> - >>>>>>> + blk_size = blk_getlength(sd->blk) - sd->boot_part_size * 2; >>>>>>> + if (blk_size > SDSC_MAX_CAPACITY) { >>>>>>> + if (sd_is_emmc(sd) && blk_size % (1 << >>>>>>> HWBLOCK_SHIFT) != >>>>>>> 0) { >>>>>>> + int64_t blk_size_aligned = >>>>>>> + ((blk_size >> HWBLOCK_SHIFT) + 1) << >>>>>>> HWBLOCK_SHIFT; >>>>>>> + sd_blk_size_error(sd, blk_size, blk_size_aligned, >>>>>>> + "multiples of 512", errp); >>>>>>> + return; >>>>>>> + } else if (!sd_is_emmc(sd) && blk_size % (512 * KiB)) { >>>>>>> + int64_t blk_size_aligned = ((blk_size >> 19) + >>>>>>> 1) << >>>>>>> 19; >>>>>>> + sd_blk_size_error(sd, blk_size, blk_size_aligned, >>>>>>> + "multiples of 512K", errp); >>>>>>> + return; >>>>>>> + } >>>>>>> + } else if (blk_size > 0 && !is_power_of_2(blk_size)) { >>>>>>> + sd_blk_size_error(sd, blk_size, pow2ceil(blk_size), "a >>>>>>> power of 2", >>>>>>> + errp); >>>>>>> + return; >>>>>>> + } else if (blk_size < 0) { >>>>>>> + error_setg(errp, "eMMC image smaller than boot >>>>>>> partitions"); >>>>>> >>>>>> Cedric, I just played with some ast* machines and noticed that >>>>>> they now >>>>>> trigger that error above when no eMMC disk image is specified >>>>>> ("qemu-system-aarch64 -M ast2700a1-evb"). Is that a valid error, >>>>>> i.e. we >>>>>> shouldn't have tried to boot without an eMMC at all so far, or would >>>>>> this be a regression? >>>>> >>>>> Only the ast2600-evb and the rainier-bmc have eMMC support. >>>>> I don't think the ast2700a1-evb has eMMC support. Jamin ? >>>>> >>>>> >>>>> >>>>> The rainier-bmc boots by default from eMMC. Nothing really >>>>> special about the image, the first boot partition includes >>>>> the u-boot-spl.bin and u-boot.bin images at expected offset. >>>>> The machine model loads the u-boot-spl.bin contents as a ROM. >>>>> >>>>> The ast2600-evb machine boots from flash. To add an eMMC drive >>>>> (needs to be the 3rd 'sd' drive), use this command line : >>>>> >>>>> $ qemu-system-arm -M ast2600-evb -net nic,netdev=net0 -netdev >>>>> user,id=net0 \ >>>>> -drive file=./v09.07/ast2600-default/image- >>>>> bmc,format=raw,if=mtd - >>>>> serial mon:stdio \ >>>>> -drive file=mmc-ast2600-evb- >>>>> noboot.qcow2,format=qcow2,if=sd,id=sd2,index=2 >>>>> .... >>>>> U-Boot 2019.04-v00.04.22 (Jun 17 2025 - 08:57:39 +0000) >>>>> SOC: AST2600-A3 >>>>> eSPI Mode: SIO:Enable : SuperIO-2e >>>>> Eth: MAC0: RGMII, MAC1: RGMII, MAC2: RGMII, MAC3: RGMII >>>>> Model: AST2600 EVB >>>>> DRAM: already initialized, 1008 MiB (capacity:1024 MiB, VGA:64 >>>>> MiB, >>>>> ECC:off) >>>>> RC Bridge phy@1e6ed200 : Link up >>>>> MMC: sdhci_slot0@100: 1, sdhci_slot1@200: 2, emmc_slot0@100: 0 >>>>> .... >>>>> [ 4.209117] mmc0: new high speed MMC card at address 0001 >>>>> [ 4.211765] mmcblk0: mmc0:0001 QEMU!! 16.0 GiB >>>>> [ 4.233637] GPT:Primary header thinks Alt. header is not >>>>> at the >>>>> end of the disk. >>>>> [ 4.233995] GPT:29624393 != 33554431 >>>>> [ 4.234161] GPT:Alternate GPT header not at the end of the >>>>> disk. >>>>> [ 4.234399] GPT:29624393 != 33554431 >>>>> [ 4.234549] GPT: Use GNU Parted to correct GPT errors. >>>>> [ 4.235223] mmcblk0: p1 p2 p3 p4 p5 p6 p7 >>>>> >>>> >>>> $ ./qemu-system-arm -M ast2600-evb >>>> qemu-system-arm: eMMC image smaller than boot partitions >>>> $ ./qemu-system-arm -M ast2600-evb -drive file=disk.image,if=sd >>>> <works if disk.image is large enough> >>>> >>>> Is that ok? >>> >>> No. This is wrong. >>> >>> An sd card device is auto created at init time and a 'DriveInfo *' >>> is always available for index 0. 'mc->auto_create_sdcard' should >>> be set to false IMO. >>> >>> commit cdc8d7cadaac ("hw/boards: Rename no_sdcard -> >>> auto_create_sdcard") seems to have changed the behavior of >>> several machines. See 'make check'. >>> >> >> Just to avoid we are in deadlock: My understanding of this issue is that >> it is not a fault of this series. Am I right? Or am I supposed to >> address that as well? > > Could you add to your series : > > https://lore.kernel.org/qemu-devel/20250930142448.1030476-1- > [email protected]/ > > and retry the aspeed machines ?
Yes, that patch resolves the issue above. > > I am afraid more should be done to run 'make check' with your series. > Maybe set 'mc->auto_create_sdcard' to false for all machines ? Who should do that? > > Thanks, > > C. > > > >> >> Regarding the rest of the series, there was another comment on potential >> improvements for the docs. I was waiting for further remarks before >> creating a potential v5. >> >> So, what is needed now to move forward with my series? >> This question is still open for me. Jan -- Siemens AG, Foundational Technologies Linux Expert Center
