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? 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? Thanks, Jan -- Siemens AG, Foundational Technologies Linux Expert Center
