On Sun, 2024-11-03 at 22:08 +0100, Cédric Le Goater wrote:
> Commit e554e45b4478 ("aspeed: Tune eMMC device properties to reflect
> HW strapping") added support to boot from an eMMC device by setting
> the boot properties of the eMMC device. This change made the
> assumption that the device always has boot areas.
>
> However, if the machine boots from the flash device (or -kernel) and
> uses an eMMC device without boot areas, support would be broken. This
> impacts the ast2600-evb machine which can choose to boot from flash or
> eMMC using the "boot-emmc" machine option.
>
> To provide some flexibility for Aspeed machine users to use different
> flavors of eMMC devices (with or without boot areas), do not set the
> eMMC device boot properties when the machine is not configured to boot
> from eMMC. However, this approach makes another assumption about eMMC
> devices, namely that eMMC devices from which the machine does not boot
> do not have boot areas.
>
> A preferable alternative would be to add support for user creatable
> eMMC devices and define the device boot properties on the QEMU command
> line :
>
> -blockdev node-name=emmc0,driver=file,filename=mmc-ast2600-evb.raw \
> -device
> emmc,bus=sdhci-bus.2,drive=emmc0,boot-partition-size=1048576,boot-config=8
>
> This is a global change requiring more thinking. Nevertheless, in the
> case of the ast2600-evb machine booting from an eMMC device and when
> default devices are created, the proposed change still makes sense
> since the device is required to have boot areas.
>
> Cc: Jan Luebbe <j...@pengutronix.de>
> Fixes: e554e45b4478 ("aspeed: Tune eMMC device properties to reflect
> HW strapping")
> Signed-off-by: Cédric Le Goater <c...@redhat.com>
> Tested-by: Guenter Roeck <li...@roeck-us.net>
> ---
>
> Changes in v2:
>
> - Simplified "boot-config setting
>
> hw/arm/aspeed.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index e447923536b4..6ca145362cbd 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -338,10 +338,20 @@ static void sdhci_attach_drive(SDHCIState *sdhci,
> DriveInfo *dinfo, bool emmc,
> return;
> }
> card = qdev_new(emmc ? TYPE_EMMC : TYPE_SD_CARD);
> - if (emmc) {
> +
> + /*
> + * Force the boot properties of the eMMC device only when the
> + * machine is strapped to boot from eMMC. Without these
> + * settings, the machine would not boot.
> + *
> + * This also allows the machine to use an eMMC device without
> + * boot areas when booting from the flash device (or -kernel)
> + * Ideally, the device and its properties should be defined on
> + * the command line.
> + */
> + if (emmc && boot_emmc) {
> qdev_prop_set_uint64(card, "boot-partition-size", 1 * MiB);
> - qdev_prop_set_uint8(card, "boot-config",
> - boot_emmc ? 0x1 << 3 : 0x0);
> + qdev_prop_set_uint8(card, "boot-config", 0x1 << 3);
> }
> qdev_prop_set_drive_err(card, "drive", blk_by_legacy_dinfo(dinfo),
> &error_fatal);
Reviewed-by: Jan Luebbe <j...@pengutronix.de>
Thanks,
Jan
