On 10/25/24 21:47, Philippe Mathieu-Daudé wrote:
On 25/10/24 12:25, Jan Lübbe wrote:
On Fri, 2024-10-25 at 06:59 -0700, Guenter Roeck wrote:
On 10/25/24 02:57, Jan Lübbe wrote:
On Fri, 2024-10-25 at 08:55 +0200, Cédric Le Goater wrote:
On 10/24/24 19:59, Philippe Mathieu-Daudé wrote:
Cc'ing Jan.

On 22/10/24 12:04, Guenter Roeck wrote:

I have no idea how this is supposed to work, and I don't think it works
as implemented. I'll revert commit e32ac563b83 in my local copy of qemu.
Jan, can you have a look at this bug report please? Otherwise I'll
have a look during soft freeze.

Guenter, just to make sure: In your case [1], you had "boot-emmc" (see
aspeed_machine_ast2600_class_emmc_init) enabled, right? Otherwise the

I tried both enabled and disabled.

boot partition size would be 0, meaning that the eMMC has no boot
partitions.

That is what I would have expected, but it does not reflect reality.

In that configuration, your backing file needs to have space for the
boot partitions at the start (2*1MiB) and the rootfs starts at the 2
MiB offset.


boot-emmc doesn't make a difference, because

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

in hw/arm/aspeed.c sets the boot partition size independently of the
boot-emmc flag.

Ah, yes. :/

So you can have SD, eMMC with boot from boot partition *disabled* or
eMMC with boot from boot partition *enabled*.

I suspect that the expectation is that I always provide
an emmc image with a 2 MB gap at the beginning, but in my opinion that is
really hyper-clumsy, and I simply won't do it, sorry.

I was surprised that the boot partitions' contents are stored in the
same backing file as the main area (instead of being separate files).
But I've not researched why it was designed this way.

Yeah I'd have preferred separate files too, but when it seemed
to be simpler for the single user case:
https://lore.kernel.org/qemu-devel/d48b6357-c839-4971-aa28-bdbd5b1ba...@kaod.org/


I don't mind a single file. What bothers me is that the partitioning is made
mandatory for ast2600 even if not used.

Guenter


I can work around
the problem by changing the above code to only set boot-partition-size if
boot_emmc is set, or I can revert commit e32ac563b83. For now I chose the
latter.

With e32ac563b83 reverted, your backing file layout will change *at
runtime* depending on whether booting from the boot partition is
enabled via EXT CSD registers. You should be able to try that using
'mmc bootpart enable <boot_part> 0 /dev/mmcblk0' (with boot_part=0 for
disabled and boot_part=1/2 for enabled).


I'm not sure what the best way forward is... perhaps make the boot-
partition-size zero if boot_emmc is false? e.g.

@@ -339,7 +339,12 @@ static void sdhci_attach_drive(SDHCIState *sdhci, 
DriveInfo *dinfo, bool emmc,
          }
          card = qdev_new(emmc ? TYPE_EMMC : TYPE_SD_CARD);
          if (emmc) {
-            qdev_prop_set_uint64(card, "boot-partition-size", 1 * MiB);
+            /*
+             * Avoid the requirement for a padded disk image if booting from
+             * eMMC boot partitions is not needed.
+             */
+            qdev_prop_set_uint64(card, "boot-partition-size",
+                                boot_emmc ? 1 * MiB : 0);
              qdev_prop_set_uint8(card, "boot-config",
                                  boot_emmc ? 0x1 << 3 : 0x0);
          }


Then you'd have the choice between:
- an eMMC, but without boot partitions (and simple backing file layout)
- an eMMC with boot partitions and need a backing file with
   the boot partitions at the beginning

That might be the lesser evil. :)

Jan




Reply via email to