* [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model
@ 2025-10-17 12:03 Jan Kiszka
2025-10-17 12:03 ` [PATCH v5 1/6] hw/sd/sdcard: Fix size check for backing block image Jan Kiszka
` (7 more replies)
0 siblings, 8 replies; 30+ messages in thread
From: Jan Kiszka @ 2025-10-17 12:03 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Bin Meng, qemu-block,
Ilias Apalodimas, Alex Bennée, Jan Lübbe,
Jerome Forissier, Alexander Bulekov, Alistair Francis,
Cédric Le Goater, Joel Stanley, Warner Losh
Changes in v5 [1]:
- fix regression of patch 1 with unplugged SD cards
- address review comments on documentation
Changes in v4:
- add truncation warning to mkemmc.sh
- fix typos in doc and mkemmc.sh
Changes in v3:
- rebased, dropping merged patches
- rework image alignment rules to match hardware
- improve/fix mkemmc script
- add emmc documentation
Changes in v2:
- handle write counter expiry
- assert() availability of QCRYPTO_HASH_ALGO_SHA256
- add missing SPDX-License-Identifier
This closes an old gap in system integration testing for the very
complex ARM firmware stacks by adding fairly advanced Replay Protected
Memory Block (RPMB) emulation to the eMMC device model. Key programming
and message authentication are working, so is the write counter. Known
users are happy with the result. What is missing, but not only for RPMB-
related registers, is state persistence across QEMU restarts. This is OK
at this stage for most test scenarios, though, and could still be added
later on.
What can already be done with it is demonstrated in the WIP branch of
isar-cip-core at [2]: TF-A + OP-TEE + StandaloneMM TA + fTPM TA, used by
U-Boot and Linux for UEFI variable storage and TPM scenarios. If you
want to try: build qemu-arm64 target for trixie with 6.12-cip *head*
kernel, enable secure boot and disk encryption, then run
$ QEMU_PATH=/path/to/qemu-build/ ./start-qemu.sh
Deploy snakeoil keys into PK, KEK and db after first boot to enable
secure booting:
root@demo:~# cert-to-efi-sig-list PkKek-1-snakeoil.pem PK.esl
root@demo:~# sign-efi-sig-list -k PkKek-1-snakeoil.key -c PkKek-1-snakeoil.pem PK PK.esl PK.auth
root@demo:~# efi-updatevar -f PK.auth db
root@demo:~# efi-updatevar -f PK.auth KEK
root@demo:~# efi-updatevar -f PK.auth PK
Note that emulation is a bit slow in general, and specifically the
partition encryption on first boot is taking 20 min. - we should
probably reduce its size or understand if there is still something to
optimize.
Jan
[1] https://github.com/siemens/qemu/commits/queues/emmc/
[2] https://gitlab.com/cip-project/cip-core/isar-cip-core/-/commits/wip/qemu-rpmb
CC: Alexander Bulekov <alxndr@bu.edu>
CC: Alistair Francis <alistair@alistair23.me>
CC: Cédric Le Goater <clg@kaod.org>
CC: Joel Stanley <joel@jms.id.au>
CC: Warner Losh <imp@bsdimp.com>
Jan Kiszka (6):
hw/sd/sdcard: Fix size check for backing block image
hw/sd/sdcard: Allow user-instantiated eMMC
hw/sd/sdcard: Add basic support for RPMB partition
hw/sd/sdcard: Handle RPMB MAC field
scripts: Add helper script to generate eMMC block device images
docs: Add eMMC device model description
docs/system/device-emulation.rst | 1 +
docs/system/devices/emmc.rst | 53 +++++
hw/sd/sd.c | 352 ++++++++++++++++++++++++++++---
hw/sd/sdmmc-internal.h | 21 ++
hw/sd/trace-events | 2 +
scripts/mkemmc.sh | 218 +++++++++++++++++++
6 files changed, 618 insertions(+), 29 deletions(-)
create mode 100644 docs/system/devices/emmc.rst
create mode 100755 scripts/mkemmc.sh
--
2.51.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH v5 1/6] hw/sd/sdcard: Fix size check for backing block image 2025-10-17 12:03 [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Jan Kiszka @ 2025-10-17 12:03 ` Jan Kiszka 2025-10-20 7:29 ` Cédric Le Goater 2025-10-30 22:10 ` Philippe Mathieu-Daudé 2025-10-17 12:03 ` [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC Jan Kiszka ` (6 subsequent siblings) 7 siblings, 2 replies; 30+ messages in thread From: Jan Kiszka @ 2025-10-17 12:03 UTC (permalink / raw) To: qemu-devel Cc: Philippe Mathieu-Daudé, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Warner Losh, Cédric Le Goater, Joel Stanley, Alistair Francis, Alexander Bulekov From: Jan Kiszka <jan.kiszka@siemens.com> 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 <jan.kiszka@siemens.com> --- CC: Warner Losh <imp@bsdimp.com> CC: Cédric Le Goater <clg@kaod.org> CC: Joel Stanley <joel@jms.id.au> CC: Alistair Francis <alistair@alistair23.me> CC: Alexander Bulekov <alxndr@bu.edu> --- hw/sd/sd.c | 65 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index d7a496d77c..d1e1bb4f0e 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -2759,9 +2759,32 @@ 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); + int64_t blk_size = -ENOMEDIUM; int ret; switch (sd->spec_version) { @@ -2774,32 +2797,34 @@ static void sd_realize(DeviceState *dev, Error **errp) } if (sd->blk) { - int64_t blk_size; - if (!blk_supports_write_perm(sd->blk)) { error_setg(errp, "Cannot use read-only drive as SD card"); 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); - + } + if (blk_size >= 0) { + blk_size -= 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"); return; } -- 2.51.0 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v5 1/6] hw/sd/sdcard: Fix size check for backing block image 2025-10-17 12:03 ` [PATCH v5 1/6] hw/sd/sdcard: Fix size check for backing block image Jan Kiszka @ 2025-10-20 7:29 ` Cédric Le Goater 2025-10-30 22:10 ` Philippe Mathieu-Daudé 1 sibling, 0 replies; 30+ messages in thread From: Cédric Le Goater @ 2025-10-20 7:29 UTC (permalink / raw) To: Jan Kiszka, qemu-devel Cc: Philippe Mathieu-Daudé, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Warner Losh, Joel Stanley, Alistair Francis, Alexander Bulekov On 10/17/25 14:03, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > 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 <jan.kiszka@siemens.com> > --- > CC: Warner Losh <imp@bsdimp.com> > CC: Cédric Le Goater <clg@kaod.org> > CC: Joel Stanley <joel@jms.id.au> > CC: Alistair Francis <alistair@alistair23.me> > CC: Alexander Bulekov <alxndr@bu.edu> > --- > hw/sd/sd.c | 65 +++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 45 insertions(+), 20 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index d7a496d77c..d1e1bb4f0e 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -2759,9 +2759,32 @@ 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); > + int64_t blk_size = -ENOMEDIUM; > int ret; > > switch (sd->spec_version) { > @@ -2774,32 +2797,34 @@ static void sd_realize(DeviceState *dev, Error **errp) > } > > if (sd->blk) { > - int64_t blk_size; > - > if (!blk_supports_write_perm(sd->blk)) { > error_setg(errp, "Cannot use read-only drive as SD card"); > 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); > - > + } > + if (blk_size >= 0) { > + blk_size -= 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"); > return; > } > Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 1/6] hw/sd/sdcard: Fix size check for backing block image 2025-10-17 12:03 ` [PATCH v5 1/6] hw/sd/sdcard: Fix size check for backing block image Jan Kiszka 2025-10-20 7:29 ` Cédric Le Goater @ 2025-10-30 22:10 ` Philippe Mathieu-Daudé 1 sibling, 0 replies; 30+ messages in thread From: Philippe Mathieu-Daudé @ 2025-10-30 22:10 UTC (permalink / raw) To: Jan Kiszka, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Warner Losh, Cédric Le Goater, Joel Stanley, Alistair Francis, Alexander Bulekov On 17/10/25 14:03, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > 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 <jan.kiszka@siemens.com> > --- > CC: Warner Losh <imp@bsdimp.com> > CC: Cédric Le Goater <clg@kaod.org> > CC: Joel Stanley <joel@jms.id.au> > CC: Alistair Francis <alistair@alistair23.me> > CC: Alexander Bulekov <alxndr@bu.edu> > --- > hw/sd/sd.c | 65 +++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 45 insertions(+), 20 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index d7a496d77c..d1e1bb4f0e 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -2759,9 +2759,32 @@ 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); > + int64_t blk_size = -ENOMEDIUM; > int ret; > > switch (sd->spec_version) { > @@ -2774,32 +2797,34 @@ static void sd_realize(DeviceState *dev, Error **errp) > } > > if (sd->blk) { > - int64_t blk_size; > - > if (!blk_supports_write_perm(sd->blk)) { > error_setg(errp, "Cannot use read-only drive as SD card"); > 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); > - > + } > + if (blk_size >= 0) { > + blk_size -= sd->boot_part_size * 2; > + if (blk_size > SDSC_MAX_CAPACITY) { > + if (sd_is_emmc(sd) && blk_size % (1 << HWBLOCK_SHIFT) != 0) { I'll replace by: !QEMU_IS_ALIGNED(blk_size, 1 << HWBLOCK_SHIFT) > + 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)) { and: !QEMU_IS_ALIGNED(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"); > return; > } > Long due cleanup, thank you very much!!! Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC 2025-10-17 12:03 [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Jan Kiszka 2025-10-17 12:03 ` [PATCH v5 1/6] hw/sd/sdcard: Fix size check for backing block image Jan Kiszka @ 2025-10-17 12:03 ` Jan Kiszka 2025-10-27 11:55 ` Daniel P. Berrangé 2025-10-30 16:50 ` Jan Lübbe 2025-10-17 12:03 ` [PATCH v5 3/6] hw/sd/sdcard: Add basic support for RPMB partition Jan Kiszka ` (5 subsequent siblings) 7 siblings, 2 replies; 30+ messages in thread From: Jan Kiszka @ 2025-10-17 12:03 UTC (permalink / raw) To: qemu-devel Cc: Philippe Mathieu-Daudé, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier From: Jan Kiszka <jan.kiszka@siemens.com> Enable user-instantiation so that PCI-attached eMMCs can be created for virt machines, for QA purposes for the eMMC model itself and for complex firmware/OS integrations using the upcoming RPMB partition support. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/sd/sd.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index d1e1bb4f0e..305ea251cb 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -2928,8 +2928,6 @@ static void emmc_class_init(ObjectClass *klass, const void *data) dc->desc = "eMMC"; dc->realize = emmc_realize; device_class_set_props(dc, emmc_properties); - /* Reason: Soldered on board */ - dc->user_creatable = false; sc->proto = &sd_proto_emmc; -- 2.51.0 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC 2025-10-17 12:03 ` [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC Jan Kiszka @ 2025-10-27 11:55 ` Daniel P. Berrangé 2025-10-27 12:23 ` Philippe Mathieu-Daudé 2025-10-30 16:50 ` Jan Lübbe 1 sibling, 1 reply; 30+ messages in thread From: Daniel P. Berrangé @ 2025-10-27 11:55 UTC (permalink / raw) To: Jan Kiszka Cc: qemu-devel, Philippe Mathieu-Daudé, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier On Fri, Oct 17, 2025 at 02:03:54PM +0200, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Enable user-instantiation so that PCI-attached eMMCs can be created for > virt machines, for QA purposes for the eMMC model itself and for complex > firmware/OS integrations using the upcoming RPMB partition support. IIUC, the 'emmc' device wants an 'sd-bus' but this commit talks about it being PCI-attached ? Can you elaborate on / illustrate the usage example for an end user ? > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > hw/sd/sd.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index d1e1bb4f0e..305ea251cb 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -2928,8 +2928,6 @@ static void emmc_class_init(ObjectClass *klass, const void *data) > dc->desc = "eMMC"; > dc->realize = emmc_realize; > device_class_set_props(dc, emmc_properties); > - /* Reason: Soldered on board */ > - dc->user_creatable = false; > > sc->proto = &sd_proto_emmc; > > -- > 2.51.0 > > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC 2025-10-27 11:55 ` Daniel P. Berrangé @ 2025-10-27 12:23 ` Philippe Mathieu-Daudé 2025-10-27 12:35 ` Daniel P. Berrangé 2025-10-27 12:45 ` Jan Lübbe 0 siblings, 2 replies; 30+ messages in thread From: Philippe Mathieu-Daudé @ 2025-10-27 12:23 UTC (permalink / raw) To: Daniel P. Berrangé, Jan Kiszka Cc: qemu-devel, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier On 27/10/25 12:55, Daniel P. Berrangé wrote: > On Fri, Oct 17, 2025 at 02:03:54PM +0200, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> Enable user-instantiation so that PCI-attached eMMCs can be created for >> virt machines, for QA purposes for the eMMC model itself and for complex >> firmware/OS integrations using the upcoming RPMB partition support. > > IIUC, the 'emmc' device wants an 'sd-bus' but this commit talks about > it being PCI-attached ? Sigh, it should not, but it got introduced this way and we didn't have time / energy / good reason to rework the code, which currently just works. SD / MMC cards -> plugged over external SD bus embedded MMC cards -> no SD bus, directly mmio-mapped. > > Can you elaborate on / illustrate the usage example for an end user ? Saving time by testing virtual hardware, without having to implement a real model. Personally I'm not in favor of modelling unspecified devices (IOW not following a spec). But I can understand QEMU usefulness with fast HW prototyping. > >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> hw/sd/sd.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/hw/sd/sd.c b/hw/sd/sd.c >> index d1e1bb4f0e..305ea251cb 100644 >> --- a/hw/sd/sd.c >> +++ b/hw/sd/sd.c >> @@ -2928,8 +2928,6 @@ static void emmc_class_init(ObjectClass *klass, const void *data) >> dc->desc = "eMMC"; >> dc->realize = emmc_realize; >> device_class_set_props(dc, emmc_properties); >> - /* Reason: Soldered on board */ >> - dc->user_creatable = false; >> >> sc->proto = &sd_proto_emmc; >> >> -- >> 2.51.0 >> >> > > With regards, > Daniel ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC 2025-10-27 12:23 ` Philippe Mathieu-Daudé @ 2025-10-27 12:35 ` Daniel P. Berrangé 2025-10-27 12:44 ` Philippe Mathieu-Daudé 2025-10-27 12:45 ` Jan Lübbe 1 sibling, 1 reply; 30+ messages in thread From: Daniel P. Berrangé @ 2025-10-27 12:35 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Jan Kiszka, qemu-devel, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier On Mon, Oct 27, 2025 at 01:23:56PM +0100, Philippe Mathieu-Daudé wrote: > On 27/10/25 12:55, Daniel P. Berrangé wrote: > > On Fri, Oct 17, 2025 at 02:03:54PM +0200, Jan Kiszka wrote: > > > From: Jan Kiszka <jan.kiszka@siemens.com> > > > > > > Enable user-instantiation so that PCI-attached eMMCs can be created for > > > virt machines, for QA purposes for the eMMC model itself and for complex > > > firmware/OS integrations using the upcoming RPMB partition support. > > > > IIUC, the 'emmc' device wants an 'sd-bus' but this commit talks about > > it being PCI-attached ? > > Sigh, it should not, but it got introduced this way and we didn't > have time / energy / good reason to rework the code, which currently > just works. > > SD / MMC cards -> plugged over external SD bus > > embedded MMC cards -> no SD bus, directly mmio-mapped. > > > > > Can you elaborate on / illustrate the usage example for an end user ? > > Saving time by testing virtual hardware, without having to implement a > real model. Ok, more specifically, what are the suggested QEMU command line args to make use of this with PCI ? With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC 2025-10-27 12:35 ` Daniel P. Berrangé @ 2025-10-27 12:44 ` Philippe Mathieu-Daudé 2025-10-30 14:17 ` Jan Lübbe 0 siblings, 1 reply; 30+ messages in thread From: Philippe Mathieu-Daudé @ 2025-10-27 12:44 UTC (permalink / raw) To: Daniel P. Berrangé Cc: Jan Kiszka, qemu-devel, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier On 27/10/25 13:35, Daniel P. Berrangé wrote: > On Mon, Oct 27, 2025 at 01:23:56PM +0100, Philippe Mathieu-Daudé wrote: >> On 27/10/25 12:55, Daniel P. Berrangé wrote: >>> On Fri, Oct 17, 2025 at 02:03:54PM +0200, Jan Kiszka wrote: >>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>> >>>> Enable user-instantiation so that PCI-attached eMMCs can be created for >>>> virt machines, for QA purposes for the eMMC model itself and for complex >>>> firmware/OS integrations using the upcoming RPMB partition support. >>> >>> IIUC, the 'emmc' device wants an 'sd-bus' but this commit talks about >>> it being PCI-attached ? >> >> Sigh, it should not, but it got introduced this way and we didn't >> have time / energy / good reason to rework the code, which currently >> just works. >> >> SD / MMC cards -> plugged over external SD bus >> >> embedded MMC cards -> no SD bus, directly mmio-mapped. >> >>> >>> Can you elaborate on / illustrate the usage example for an end user ? >> >> Saving time by testing virtual hardware, without having to implement a >> real model. > > Ok, more specifically, what are the suggested QEMU command line > args to make use of this with PCI ? See patch #6 documentation: + -drive file=emmc.img,if=none,format=raw,id=emmc-img + -device sdhci-pci + -device emmc,drive=emmc-img,boot-partition-size=1048576,rpmb-partition-size=2097152 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC 2025-10-27 12:44 ` Philippe Mathieu-Daudé @ 2025-10-30 14:17 ` Jan Lübbe 0 siblings, 0 replies; 30+ messages in thread From: Jan Lübbe @ 2025-10-30 14:17 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Daniel P. Berrangé Cc: Jan Kiszka, qemu-devel, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jerome Forissier, Ahmad Fatoum On Mon, 2025-10-27 at 13:44 +0100, Philippe Mathieu-Daudé wrote: > On 27/10/25 13:35, Daniel P. Berrangé wrote: > > On Mon, Oct 27, 2025 at 01:23:56PM +0100, Philippe Mathieu-Daudé wrote: > > > On 27/10/25 12:55, Daniel P. Berrangé wrote: > > > > On Fri, Oct 17, 2025 at 02:03:54PM +0200, Jan Kiszka wrote: > > > > > From: Jan Kiszka <jan.kiszka@siemens.com> > > > > > > > > > > Enable user-instantiation so that PCI-attached eMMCs can be created for > > > > > virt machines, for QA purposes for the eMMC model itself and for complex > > > > > firmware/OS integrations using the upcoming RPMB partition support. > > > > > > > > IIUC, the 'emmc' device wants an 'sd-bus' but this commit talks about > > > > it being PCI-attached ? > > > > > > Sigh, it should not, but it got introduced this way and we didn't > > > have time / energy / good reason to rework the code, which currently > > > just works. > > > > > > SD / MMC cards -> plugged over external SD bus > > > > > > embedded MMC cards -> no SD bus, directly mmio-mapped. > > > > > > > > > > > Can you elaborate on / illustrate the usage example for an end user ? > > > > > > Saving time by testing virtual hardware, without having to implement a > > > real model. > > > > Ok, more specifically, what are the suggested QEMU command line > > args to make use of this with PCI ? > > See patch #6 documentation: > > + -drive file=emmc.img,if=none,format=raw,id=emmc-img > + -device sdhci-pci > + -device > emmc,drive=emmc-img,boot-partition-size=1048576,rpmb-partition-size=2097152 For example, our current use for an earlier variant of this can be seen here: https://github.com/rauc/rauc/blob/master/qemu-test#L79 if "$QEMU_BIN" -device help | grep -q 'name "emmc"'; then echo "found emmc support, enabling" truncate -s 64M qemu-test-emmc.img QEMU_ARGS="$QEMU_ARGS -drive if=none,id=emmc-drive,file=qemu-test-emmc.img,format=raw" QEMU_ARGS="$QEMU_ARGS -device sdhci-pci" QEMU_ARGS="$QEMU_ARGS -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576" fi We enable this only if our patched version of qemu [1] is found. For ARM bootloader (OP-TEE and barebox) development and regression testing, we'd use this as well (with the RPMB emulation). Avoiding the need for a custom QEMU build would help a lot. :) Best regards, Jan [1] https://github.com/qemu/qemu/compare/master...jluebbe:qemu:user-emmc#diff-e90f7f7336bf488567604b94aa0a3f5d272e392bca2e1e5c33c65f094c18466d -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC 2025-10-27 12:23 ` Philippe Mathieu-Daudé 2025-10-27 12:35 ` Daniel P. Berrangé @ 2025-10-27 12:45 ` Jan Lübbe 1 sibling, 0 replies; 30+ messages in thread From: Jan Lübbe @ 2025-10-27 12:45 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Daniel P. Berrangé, Jan Kiszka Cc: qemu-devel, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jerome Forissier On Mon, 2025-10-27 at 13:23 +0100, Philippe Mathieu-Daudé wrote: > On 27/10/25 12:55, Daniel P. Berrangé wrote: > > On Fri, Oct 17, 2025 at 02:03:54PM +0200, Jan Kiszka wrote: > > > From: Jan Kiszka <jan.kiszka@siemens.com> > > > > > > Enable user-instantiation so that PCI-attached eMMCs can be created for > > > virt machines, for QA purposes for the eMMC model itself and for complex > > > firmware/OS integrations using the upcoming RPMB partition support. > > > > IIUC, the 'emmc' device wants an 'sd-bus' but this commit talks about > > it being PCI-attached ? > > Sigh, it should not, but it got introduced this way and we didn't > have time / energy / good reason to rework the code, which currently > just works. > > SD / MMC cards -> plugged over external SD bus > > embedded MMC cards -> no SD bus, directly mmio-mapped. Hmm. This is not how I'd describe it and I've not seen directly MMIO mapped MMC cards. Both eMMCs and SD cards are separate/physical devices connected via an SD/MMC bus to a host controller. As both protocols are closely related, host controller usually implement both protocols and can detect which one to use at runtime. The host controller is connected to the CPU via a different bus, usually PCI(e) (e.g. in a notebook) or just MMIO mapped (e.g. in an ARM SoC). SDHCI is the most common interface spec for SD/MMC controlers. So, you need to create two device (eMMC + SDHCI) for this to work with a generic x86_64 VM: -drive if=none,id=emmc-drive,file=emmc.img,format=raw \ -device sdhci-pci \ -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \ If you emulate a machine/SoC which already has an integrated SD/MMC host controler, you'd leave out the sdhci-pci device. Regards, Jan -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC 2025-10-17 12:03 ` [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC Jan Kiszka 2025-10-27 11:55 ` Daniel P. Berrangé @ 2025-10-30 16:50 ` Jan Lübbe 2025-10-30 18:10 ` Jan Kiszka 1 sibling, 1 reply; 30+ messages in thread From: Jan Lübbe @ 2025-10-30 16:50 UTC (permalink / raw) To: Jan Kiszka, qemu-devel Cc: Philippe Mathieu-Daudé, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jerome Forissier On Fri, 2025-10-17 at 14:03 +0200, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Enable user-instantiation so that PCI-attached eMMCs can be created for > virt machines, for QA purposes for the eMMC model itself and for complex > firmware/OS integrations using the upcoming RPMB partition support. Perhaps reword to "Enable user-instantiation so that eMMCs can be created for PCI-attached SD/MMC host controllers (such as sdhci-pci) on virt machines, ..." to match the actual bus hierarchy? Jan > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > hw/sd/sd.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index d1e1bb4f0e..305ea251cb 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -2928,8 +2928,6 @@ static void emmc_class_init(ObjectClass *klass, const void *data) > dc->desc = "eMMC"; > dc->realize = emmc_realize; > device_class_set_props(dc, emmc_properties); > - /* Reason: Soldered on board */ > - dc->user_creatable = false; > > sc->proto = &sd_proto_emmc; > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC 2025-10-30 16:50 ` Jan Lübbe @ 2025-10-30 18:10 ` Jan Kiszka 0 siblings, 0 replies; 30+ messages in thread From: Jan Kiszka @ 2025-10-30 18:10 UTC (permalink / raw) To: Jan Lübbe, qemu-devel, Philippe Mathieu-Daudé Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jerome Forissier On 30.10.25 17:50, Jan Lübbe wrote: > On Fri, 2025-10-17 at 14:03 +0200, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> Enable user-instantiation so that PCI-attached eMMCs can be created for >> virt machines, for QA purposes for the eMMC model itself and for complex >> firmware/OS integrations using the upcoming RPMB partition support. > > Perhaps reword to "Enable user-instantiation so that eMMCs can be created for > PCI-attached SD/MMC host controllers (such as sdhci-pci) on virt machines, ..." > to match the actual bus hierarchy? > Sounds better to me as well. Can update this patch if desired, but I don't want to repost everything just for that now. Or Philippe just folds this in while merging if that is all. Thanks, Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v5 3/6] hw/sd/sdcard: Add basic support for RPMB partition 2025-10-17 12:03 [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Jan Kiszka 2025-10-17 12:03 ` [PATCH v5 1/6] hw/sd/sdcard: Fix size check for backing block image Jan Kiszka 2025-10-17 12:03 ` [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC Jan Kiszka @ 2025-10-17 12:03 ` Jan Kiszka 2025-11-03 15:08 ` Philippe Mathieu-Daudé 2025-10-17 12:03 ` [PATCH v5 4/6] hw/sd/sdcard: Handle RPMB MAC field Jan Kiszka ` (4 subsequent siblings) 7 siblings, 1 reply; 30+ messages in thread From: Jan Kiszka @ 2025-10-17 12:03 UTC (permalink / raw) To: qemu-devel Cc: Philippe Mathieu-Daudé, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier From: Jan Kiszka <jan.kiszka@siemens.com> The Replay Protected Memory Block (RPMB) is available since eMMC 4.4 which has been obsoleted by 4.41. Therefore lift the provided EXT_CSD_REV to 5 (4.41) and provide the basic logic to implement basic support for it. This allows to set the authentication key, read the write counter and authenticated perform data read and write requests. Those aren't actually authenticated yet, support for that will be added later. The RPMB image needs to be added to backing block images after potential boot partitions and before the user data. It's size is controlled by the rpmb-partition-size property. Also missing in this version (and actually not only for RPMB bits) is persistence of registers that are supposed to survive power cycles. Most prominent are the write counters or the authentication key. This feature can be added later, e.g. by append a state structure to the backing block image. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/sd/sd.c | 206 +++++++++++++++++++++++++++++++++++++++-- hw/sd/sdmmc-internal.h | 21 +++++ hw/sd/trace-events | 2 + 3 files changed, 221 insertions(+), 8 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 305ea251cb..918fe9f79f 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -117,6 +117,20 @@ typedef struct SDProto { } cmd[SDMMC_CMD_MAX], acmd[SDMMC_CMD_MAX]; } SDProto; +#define RPMB_KEY_MAC_LEN 32 + +typedef struct { + uint8_t stuff_bytes[196]; + uint8_t key_mac[RPMB_KEY_MAC_LEN]; + uint8_t data[256]; + uint8_t nonce[16]; + uint32_t write_counter; + uint16_t address; + uint16_t block_count; + uint16_t result; + uint16_t req_resp; +} RPMBDataFrame; + struct SDState { DeviceState parent_obj; @@ -140,6 +154,7 @@ struct SDState { uint8_t spec_version; uint64_t boot_part_size; + uint64_t rpmb_part_size; BlockBackend *blk; uint8_t boot_config; @@ -172,6 +187,10 @@ struct SDState { uint32_t data_offset; size_t data_size; uint8_t data[512]; + RPMBDataFrame rpmb_result; + uint32_t rpmb_write_counter; + uint8_t rpmb_key[32]; + uint8_t rpmb_key_set; QEMUTimer *ocr_power_timer; uint8_t dat_lines; bool cmd_line; @@ -506,7 +525,9 @@ static void emmc_set_ext_csd(SDState *sd, uint64_t size) sd->ext_csd[205] = 0x46; /* Min read perf for 4bit@26Mhz */ sd->ext_csd[EXT_CSD_CARD_TYPE] = 0b11; sd->ext_csd[EXT_CSD_STRUCTURE] = 2; - sd->ext_csd[EXT_CSD_REV] = 3; + sd->ext_csd[EXT_CSD_REV] = 5; + sd->ext_csd[EXT_CSD_RPMB_MULT] = sd->rpmb_part_size / (128 * KiB); + sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0b111; /* Mode segment (RW) */ sd->ext_csd[EXT_CSD_PART_CONFIG] = sd->boot_config; @@ -834,7 +855,8 @@ static uint32_t sd_blk_len(SDState *sd) /* * This requires a disk image that has two boot partitions inserted at the * beginning of it, followed by an RPMB partition. The size of the boot - * partitions is the "boot-partition-size" property. + * partitions is the "boot-partition-size" property, the one of the RPMB + * partition is 'rpmb-partition-size'. */ static uint32_t sd_part_offset(SDState *sd) { @@ -848,11 +870,13 @@ static uint32_t sd_part_offset(SDState *sd) & EXT_CSD_PART_CONFIG_ACC_MASK; switch (partition_access) { case EXT_CSD_PART_CONFIG_ACC_DEFAULT: - return sd->boot_part_size * 2; + return sd->boot_part_size * 2 + sd->rpmb_part_size; case EXT_CSD_PART_CONFIG_ACC_BOOT1: return 0; case EXT_CSD_PART_CONFIG_ACC_BOOT2: return sd->boot_part_size * 1; + case EXT_CSD_PART_CONFIG_ACC_RPMB: + return sd->boot_part_size * 2; default: g_assert_not_reached(); } @@ -891,7 +915,7 @@ static void sd_reset(DeviceState *dev) } size = sect << HWBLOCK_SHIFT; if (sd_is_emmc(sd)) { - size -= sd->boot_part_size * 2; + size -= sd->boot_part_size * 2 + sd->rpmb_part_size; } sect = sd_addr_to_wpnum(size) + 1; @@ -979,6 +1003,34 @@ static const VMStateDescription sd_ocr_vmstate = { }, }; +static bool vmstate_needed_for_rpmb(void *opaque) +{ + SDState *sd = opaque; + + return sd->rpmb_part_size > 0; +} + +static const VMStateDescription emmc_rpmb_vmstate = { + .name = "sd-card/ext_csd_modes-state", + .version_id = 1, + .minimum_version_id = 1, + .needed = vmstate_needed_for_rpmb, + .fields = (const VMStateField[]) { + VMSTATE_UINT8_ARRAY(rpmb_result.key_mac, SDState, RPMB_KEY_MAC_LEN), + VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, 256), + VMSTATE_UINT8_ARRAY(rpmb_result.nonce, SDState, 16), + VMSTATE_UINT32(rpmb_result.write_counter, SDState), + VMSTATE_UINT16(rpmb_result.address, SDState), + VMSTATE_UINT16(rpmb_result.block_count, SDState), + VMSTATE_UINT16(rpmb_result.result, SDState), + VMSTATE_UINT16(rpmb_result.req_resp, SDState), + VMSTATE_UINT32(rpmb_write_counter, SDState), + VMSTATE_UINT8_ARRAY(rpmb_key, SDState, 32), + VMSTATE_UINT8(rpmb_key_set, SDState), + VMSTATE_END_OF_LIST() + }, +}; + static bool vmstate_needed_for_emmc(void *opaque) { SDState *sd = opaque; @@ -1045,6 +1097,7 @@ static const VMStateDescription sd_vmstate = { .subsections = (const VMStateDescription * const []) { &sd_ocr_vmstate, &emmc_extcsd_vmstate, + &emmc_rpmb_vmstate, NULL }, }; @@ -1067,6 +1120,105 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) } } +static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) +{ + uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp); + uint16_t result = be16_to_cpu(sd->rpmb_result.result); + unsigned int curr_block = 0; + + if ((result & ~RPMB_RESULT_COUTER_EXPIRED) == RPMB_RESULT_OK && + resp == RPMB_RESP(RPMB_REQ_AUTH_DATA_READ)) { + curr_block = be16_to_cpu(sd->rpmb_result.address); + if (sd->rpmb_result.block_count == 0) { + sd->rpmb_result.block_count = cpu_to_be16(sd->multi_blk_cnt); + } else { + curr_block += be16_to_cpu(sd->rpmb_result.block_count) - + sd->multi_blk_cnt; + } + addr = curr_block * 256 + sd_part_offset(sd); + if (blk_pread(sd->blk, addr, 256, sd->rpmb_result.data, 0) < 0) { + fprintf(stderr, "sd_blk_read: read error on host side\n"); + memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data)); + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_READ_FAILURE | + (result & RPMB_RESULT_COUTER_EXPIRED)); + } + } + memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result)); + + trace_sdcard_rpmb_read_block(resp, curr_block, + be16_to_cpu(sd->rpmb_result.result)); +} + +static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) +{ + RPMBDataFrame *frame = (RPMBDataFrame *)sd->data; + uint16_t req = be16_to_cpu(frame->req_resp); + + if (req == RPMB_REQ_READ_RESULT) { + /* just return the current result register */ + goto exit; + } + memset(&sd->rpmb_result, 0, sizeof(sd->rpmb_result)); + memcpy(sd->rpmb_result.nonce, frame->nonce, sizeof(sd->rpmb_result.nonce)); + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_OK); + sd->rpmb_result.req_resp = cpu_to_be16(RPMB_RESP(req)); + + if (!sd->rpmb_key_set && req != RPMB_REQ_PROGRAM_AUTH_KEY) { + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_NO_AUTH_KEY); + goto exit; + } + + switch (req) { + case RPMB_REQ_PROGRAM_AUTH_KEY: + if (sd->rpmb_key_set) { + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); + break; + } + memcpy(sd->rpmb_key, frame->key_mac, sizeof(sd->rpmb_key)); + sd->rpmb_key_set = 1; + break; + case RPMB_REQ_READ_WRITE_COUNTER: + sd->rpmb_result.write_counter = cpu_to_be32(sd->rpmb_write_counter); + break; + case RPMB_REQ_AUTH_DATA_WRITE: + /* We only support single-block writes so far */ + if (sd->multi_blk_cnt != 1) { + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); + break; + } + if (sd->rpmb_write_counter == 0xffffffff) { + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); + break; + } + if (be32_to_cpu(frame->write_counter) != sd->rpmb_write_counter) { + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE); + break; + } + sd->rpmb_result.address = frame->address; + addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); + if (blk_pwrite(sd->blk, addr, 256, frame->data, 0) < 0) { + fprintf(stderr, "sd_blk_write: write error on host side\n"); + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); + } else { + sd->rpmb_write_counter++; + } + sd->rpmb_result.write_counter = cpu_to_be32(sd->rpmb_write_counter); + break; + case RPMB_REQ_AUTH_DATA_READ: + sd->rpmb_result.address = frame->address; + break; + default: + qemu_log_mask(LOG_UNIMP, "RPMB request %d not implemented\n", req); + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); + break; + } +exit: + if (sd->rpmb_write_counter == 0xffffffff) { + sd->rpmb_result.result |= cpu_to_be16(RPMB_RESULT_COUTER_EXPIRED); + } + trace_sdcard_rpmb_write_block(req, be16_to_cpu(sd->rpmb_result.result)); +} + static void sd_erase(SDState *sd) { uint64_t erase_start = sd->erase_start; @@ -1180,6 +1332,19 @@ static void emmc_function_switch(SDState *sd, uint32_t arg) break; } + if (index == EXT_CSD_PART_CONFIG) { + uint8_t part = b & EXT_CSD_PART_CONFIG_ACC_MASK; + + if (((part == EXT_CSD_PART_CONFIG_ACC_BOOT1 || + part == EXT_CSD_PART_CONFIG_ACC_BOOT2) && !sd->boot_part_size) || + (part == EXT_CSD_PART_CONFIG_ACC_RPMB && !sd->rpmb_part_size)) { + qemu_log_mask(LOG_GUEST_ERROR, + "MMC switching to illegal partition\n"); + sd->card_status |= R_CSR_SWITCH_ERROR_MASK; + return; + } + } + trace_sdcard_ext_csd_update(index, sd->ext_csd[index], b); sd->ext_csd[index] = b; } @@ -2378,6 +2543,7 @@ static bool sd_generic_read_byte(SDState *sd, uint8_t *value) static void sd_write_byte(SDState *sd, uint8_t value) { + unsigned int partition_access; int i; if (!sd->blk || !blk_is_inserted(sd->blk)) { @@ -2427,7 +2593,13 @@ static void sd_write_byte(SDState *sd, uint8_t value) if (sd->data_offset >= sd->blk_len) { /* TODO: Check CRC before committing */ sd->state = sd_programming_state; - sd_blk_write(sd, sd->data_start, sd->data_offset); + partition_access = sd->ext_csd[EXT_CSD_PART_CONFIG] + & EXT_CSD_PART_CONFIG_ACC_MASK; + if (partition_access == EXT_CSD_PART_CONFIG_ACC_RPMB) { + emmc_rpmb_blk_write(sd, sd->data_start, sd->data_offset); + } else { + sd_blk_write(sd, sd->data_start, sd->data_offset); + } sd->blk_written++; sd->data_start += sd->blk_len; sd->data_offset = 0; @@ -2510,6 +2682,7 @@ static uint8_t sd_read_byte(SDState *sd) { /* TODO: Append CRCs */ const uint8_t dummy_byte = 0x00; + unsigned int partition_access; uint8_t ret; uint32_t io_len; @@ -2553,7 +2726,13 @@ static uint8_t sd_read_byte(SDState *sd) sd->data_start, io_len)) { return dummy_byte; } - sd_blk_read(sd, sd->data_start, io_len); + partition_access = sd->ext_csd[EXT_CSD_PART_CONFIG] + & EXT_CSD_PART_CONFIG_ACC_MASK; + if (partition_access == EXT_CSD_PART_CONFIG_ACC_RPMB) { + emmc_rpmb_blk_read(sd, sd->data_start, io_len); + } else { + sd_blk_read(sd, sd->data_start, io_len); + } } ret = sd->data[sd->data_offset ++]; @@ -2805,7 +2984,7 @@ static void sd_realize(DeviceState *dev, Error **errp) blk_size = blk_getlength(sd->blk); } if (blk_size >= 0) { - blk_size -= sd->boot_part_size * 2; + blk_size -= sd->boot_part_size * 2 + sd->rpmb_part_size; if (blk_size > SDSC_MAX_CAPACITY) { if (sd_is_emmc(sd) && blk_size % (1 << HWBLOCK_SHIFT) != 0) { int64_t blk_size_aligned = @@ -2844,13 +3023,23 @@ static void sd_realize(DeviceState *dev, Error **errp) "The boot partition size must be multiples of 128K" "and not larger than 32640K.\n"); } + if (sd->rpmb_part_size % (128 * KiB) || + sd->rpmb_part_size > 128 * 128 * KiB) { + char *size_str = size_to_str(sd->boot_part_size); + + error_setg(errp, "Invalid RPMB partition size: %s", size_str); + g_free(size_str); + error_append_hint(errp, + "The RPMB partition size must be multiples of 128K" + "and not larger than 16384K.\n"); + } } static void emmc_realize(DeviceState *dev, Error **errp) { SDState *sd = SDMMC_COMMON(dev); - sd->spec_version = SD_PHY_SPECv3_01_VERS; /* Actually v4.3 */ + sd->spec_version = SD_PHY_SPECv3_01_VERS; /* Actually v4.5 */ sd_realize(dev, errp); } @@ -2867,6 +3056,7 @@ static const Property sd_properties[] = { static const Property emmc_properties[] = { DEFINE_PROP_UINT64("boot-partition-size", SDState, boot_part_size, 0), DEFINE_PROP_UINT8("boot-config", SDState, boot_config, 0x0), + DEFINE_PROP_UINT64("rpmb-partition-size", SDState, rpmb_part_size, 0), }; static void sdmmc_common_class_init(ObjectClass *klass, const void *data) diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h index ce6bc4e6ec..c4a9aa8edf 100644 --- a/hw/sd/sdmmc-internal.h +++ b/hw/sd/sdmmc-internal.h @@ -118,9 +118,30 @@ DECLARE_OBJ_CHECKERS(SDState, SDCardClass, SDMMC_COMMON, TYPE_SDMMC_COMMON) #define EXT_CSD_PART_CONFIG_ACC_DEFAULT (0x0) #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x1) #define EXT_CSD_PART_CONFIG_ACC_BOOT2 (0x2) +#define EXT_CSD_PART_CONFIG_ACC_RPMB (0x3) #define EXT_CSD_PART_CONFIG_EN_MASK (0x7 << 3) #define EXT_CSD_PART_CONFIG_EN_BOOT0 (0x1 << 3) #define EXT_CSD_PART_CONFIG_EN_USER (0x7 << 3) +#define RPMB_REQ_PROGRAM_AUTH_KEY (1) +#define RPMB_REQ_READ_WRITE_COUNTER (2) +#define RPMB_REQ_AUTH_DATA_WRITE (3) +#define RPMB_REQ_AUTH_DATA_READ (4) +#define RPMB_REQ_READ_RESULT (5) +#define RPMB_REQ_AUTH_CONFIG_WRITE (6) +#define RPMB_REQ_AUTH_CONFIG_READ (7) + +#define RPMB_RESP(__req__) ((__req__) << 8) + +#define RPMB_RESULT_OK (0) +#define RPMB_RESULT_GENERAL_FAILURE (1) +#define RPMB_RESULT_AUTH_FAILURE (2) +#define RPMB_RESULT_COUNTER_FAILURE (3) +#define RPMB_RESULT_ADDRESS_FAILURE (4) +#define RPMB_RESULT_WRITE_FAILURE (5) +#define RPMB_RESULT_READ_FAILURE (6) +#define RPMB_RESULT_NO_AUTH_KEY (7) +#define RPMB_RESULT_COUTER_EXPIRED (0x80) + #endif diff --git a/hw/sd/trace-events b/hw/sd/trace-events index 8d49840917..d30daa2143 100644 --- a/hw/sd/trace-events +++ b/hw/sd/trace-events @@ -59,6 +59,8 @@ sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t sdcard_set_voltage(uint16_t millivolts) "%u mV" sdcard_ext_csd_update(unsigned index, uint8_t oval, uint8_t nval) "index %u: 0x%02x -> 0x%02x" sdcard_switch(unsigned access, unsigned index, unsigned value, unsigned set) "SWITCH acc:%u idx:%u val:%u set:%u" +sdcard_rpmb_read_block(uint16_t resp, uint16_t read_addr, uint16_t result) "resp 0x%x read_addr 0x%x result 0x%x" +sdcard_rpmb_write_block(uint16_t req, uint16_t result) "req 0x%x result 0x%x" # pl181.c pl181_command_send(uint8_t cmd, uint32_t arg) "sending CMD%02d arg 0x%08" PRIx32 -- 2.51.0 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v5 3/6] hw/sd/sdcard: Add basic support for RPMB partition 2025-10-17 12:03 ` [PATCH v5 3/6] hw/sd/sdcard: Add basic support for RPMB partition Jan Kiszka @ 2025-11-03 15:08 ` Philippe Mathieu-Daudé 2025-11-04 6:34 ` Jan Kiszka 0 siblings, 1 reply; 30+ messages in thread From: Philippe Mathieu-Daudé @ 2025-11-03 15:08 UTC (permalink / raw) To: Jan Kiszka, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier Hi Jan, On 17/10/25 14:03, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > The Replay Protected Memory Block (RPMB) is available since eMMC 4.4 > which has been obsoleted by 4.41. Therefore lift the provided > EXT_CSD_REV to 5 (4.41) and provide the basic logic to implement basic > support for it. This allows to set the authentication key, read the > write counter and authenticated perform data read and write requests. > Those aren't actually authenticated yet, support for that will be added > later. > > The RPMB image needs to be added to backing block images after potential > boot partitions and before the user data. It's size is controlled by > the rpmb-partition-size property. > > Also missing in this version (and actually not only for RPMB bits) is > persistence of registers that are supposed to survive power cycles. Most > prominent are the write counters or the authentication key. This feature > can be added later, e.g. by append a state structure to the backing > block image. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > hw/sd/sd.c | 206 +++++++++++++++++++++++++++++++++++++++-- > hw/sd/sdmmc-internal.h | 21 +++++ > hw/sd/trace-events | 2 + > 3 files changed, 221 insertions(+), 8 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 305ea251cb..918fe9f79f 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -117,6 +117,20 @@ typedef struct SDProto { > } cmd[SDMMC_CMD_MAX], acmd[SDMMC_CMD_MAX]; > } SDProto; > #define RPMB_STUFF_LEN 196 > +#define RPMB_KEY_MAC_LEN 32 > + > +typedef QEMU_PACKED (better safe than sorry). > struct { > + uint8_t stuff_bytes[196]; > + uint8_t key_mac[RPMB_KEY_MAC_LEN]; > + uint8_t data[256]; > + uint8_t nonce[16]; > + uint32_t write_counter; > + uint16_t address; > + uint16_t block_count; > + uint16_t result; > + uint16_t req_resp; > +} RPMBDataFrame; > + > struct SDState { > DeviceState parent_obj; > > @@ -140,6 +154,7 @@ struct SDState { > > uint8_t spec_version; > uint64_t boot_part_size; > + uint64_t rpmb_part_size; > BlockBackend *blk; > uint8_t boot_config; > > @@ -172,6 +187,10 @@ struct SDState { > uint32_t data_offset; > size_t data_size; > uint8_t data[512]; > + RPMBDataFrame rpmb_result; > + uint32_t rpmb_write_counter; > + uint8_t rpmb_key[32]; > + uint8_t rpmb_key_set; Matter of style: struct { uint32_t write_counter; uint8_t key[RPMB_KEY_MAC_LEN]; uint8_t key_set; RPMBDataFrame result; } rpmb; > QEMUTimer *ocr_power_timer; > uint8_t dat_lines; > bool cmd_line; > @@ -506,7 +525,9 @@ static void emmc_set_ext_csd(SDState *sd, uint64_t size) > sd->ext_csd[205] = 0x46; /* Min read perf for 4bit@26Mhz */ > sd->ext_csd[EXT_CSD_CARD_TYPE] = 0b11; > sd->ext_csd[EXT_CSD_STRUCTURE] = 2; > - sd->ext_csd[EXT_CSD_REV] = 3; > + sd->ext_csd[EXT_CSD_REV] = 5; > + sd->ext_csd[EXT_CSD_RPMB_MULT] = sd->rpmb_part_size / (128 * KiB); > + sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0b111; > > /* Mode segment (RW) */ > sd->ext_csd[EXT_CSD_PART_CONFIG] = sd->boot_config; > @@ -834,7 +855,8 @@ static uint32_t sd_blk_len(SDState *sd) > /* > * This requires a disk image that has two boot partitions inserted at the > * beginning of it, followed by an RPMB partition. The size of the boot > - * partitions is the "boot-partition-size" property. > + * partitions is the "boot-partition-size" property, the one of the RPMB > + * partition is 'rpmb-partition-size'. > */ > static uint32_t sd_part_offset(SDState *sd) > { > @@ -848,11 +870,13 @@ static uint32_t sd_part_offset(SDState *sd) > & EXT_CSD_PART_CONFIG_ACC_MASK; > switch (partition_access) { > case EXT_CSD_PART_CONFIG_ACC_DEFAULT: > - return sd->boot_part_size * 2; > + return sd->boot_part_size * 2 + sd->rpmb_part_size; > case EXT_CSD_PART_CONFIG_ACC_BOOT1: > return 0; > case EXT_CSD_PART_CONFIG_ACC_BOOT2: > return sd->boot_part_size * 1; > + case EXT_CSD_PART_CONFIG_ACC_RPMB: > + return sd->boot_part_size * 2; > default: > g_assert_not_reached(); > } > @@ -891,7 +915,7 @@ static void sd_reset(DeviceState *dev) > } > size = sect << HWBLOCK_SHIFT; > if (sd_is_emmc(sd)) { > - size -= sd->boot_part_size * 2; > + size -= sd->boot_part_size * 2 + sd->rpmb_part_size; > } > > sect = sd_addr_to_wpnum(size) + 1; > @@ -979,6 +1003,34 @@ static const VMStateDescription sd_ocr_vmstate = { > }, > }; > > +static bool vmstate_needed_for_rpmb(void *opaque) > +{ > + SDState *sd = opaque; > + > + return sd->rpmb_part_size > 0; > +} > + > +static const VMStateDescription emmc_rpmb_vmstate = { > + .name = "sd-card/ext_csd_modes-state", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = vmstate_needed_for_rpmb, > + .fields = (const VMStateField[]) { > + VMSTATE_UINT8_ARRAY(rpmb_result.key_mac, SDState, RPMB_KEY_MAC_LEN), > + VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, 256), > + VMSTATE_UINT8_ARRAY(rpmb_result.nonce, SDState, 16), > + VMSTATE_UINT32(rpmb_result.write_counter, SDState), > + VMSTATE_UINT16(rpmb_result.address, SDState), > + VMSTATE_UINT16(rpmb_result.block_count, SDState), > + VMSTATE_UINT16(rpmb_result.result, SDState), > + VMSTATE_UINT16(rpmb_result.req_resp, SDState), > + VMSTATE_UINT32(rpmb_write_counter, SDState), > + VMSTATE_UINT8_ARRAY(rpmb_key, SDState, 32), > + VMSTATE_UINT8(rpmb_key_set, SDState), > + VMSTATE_END_OF_LIST() > + }, > +}; > + > static bool vmstate_needed_for_emmc(void *opaque) > { > SDState *sd = opaque; > @@ -1045,6 +1097,7 @@ static const VMStateDescription sd_vmstate = { > .subsections = (const VMStateDescription * const []) { > &sd_ocr_vmstate, > &emmc_extcsd_vmstate, > + &emmc_rpmb_vmstate, > NULL > }, > }; > @@ -1067,6 +1120,105 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) > } > } > > +static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) > +{ > + uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp); > + uint16_t result = be16_to_cpu(sd->rpmb_result.result); > + unsigned int curr_block = 0; > + > + if ((result & ~RPMB_RESULT_COUTER_EXPIRED) == RPMB_RESULT_OK && > + resp == RPMB_RESP(RPMB_REQ_AUTH_DATA_READ)) { > + curr_block = be16_to_cpu(sd->rpmb_result.address); > + if (sd->rpmb_result.block_count == 0) { > + sd->rpmb_result.block_count = cpu_to_be16(sd->multi_blk_cnt); > + } else { > + curr_block += be16_to_cpu(sd->rpmb_result.block_count) - > + sd->multi_blk_cnt; > + } > + addr = curr_block * 256 + sd_part_offset(sd); > + if (blk_pread(sd->blk, addr, 256, sd->rpmb_result.data, 0) < 0) { Would be nice to re-use sd_blk_read(), but I notice we want to read the frame data to then copy the whole message into sd->data. > + fprintf(stderr, "sd_blk_read: read error on host side\n"); Although a pre-existing pattern in this file, no new fprintf(stderr) please. Better use the Error* API, otherwise error_report(). error_report("sd_blk_read: read error on host side"); > + memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data)); > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_READ_FAILURE | > + (result & RPMB_RESULT_COUTER_EXPIRED)); > + } > + } > + memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result)); > + > + trace_sdcard_rpmb_read_block(resp, curr_block, > + be16_to_cpu(sd->rpmb_result.result)); > +} > + > +static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) > +{ > + RPMBDataFrame *frame = (RPMBDataFrame *)sd->data; > + uint16_t req = be16_to_cpu(frame->req_resp); > + > + if (req == RPMB_REQ_READ_RESULT) { > + /* just return the current result register */ > + goto exit; > + } > + memset(&sd->rpmb_result, 0, sizeof(sd->rpmb_result)); > + memcpy(sd->rpmb_result.nonce, frame->nonce, sizeof(sd->rpmb_result.nonce)); > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_OK); > + sd->rpmb_result.req_resp = cpu_to_be16(RPMB_RESP(req)); > + > + if (!sd->rpmb_key_set && req != RPMB_REQ_PROGRAM_AUTH_KEY) { > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_NO_AUTH_KEY); > + goto exit; > + } > + > + switch (req) { > + case RPMB_REQ_PROGRAM_AUTH_KEY: > + if (sd->rpmb_key_set) { > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); > + break; > + } > + memcpy(sd->rpmb_key, frame->key_mac, sizeof(sd->rpmb_key)); > + sd->rpmb_key_set = 1; > + break; > + case RPMB_REQ_READ_WRITE_COUNTER: > + sd->rpmb_result.write_counter = cpu_to_be32(sd->rpmb_write_counter); > + break; > + case RPMB_REQ_AUTH_DATA_WRITE: > + /* We only support single-block writes so far */ > + if (sd->multi_blk_cnt != 1) { > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); > + break; > + } > + if (sd->rpmb_write_counter == 0xffffffff) { > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); > + break; > + } > + if (be32_to_cpu(frame->write_counter) != sd->rpmb_write_counter) { > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE); > + break; > + } > + sd->rpmb_result.address = frame->address; > + addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); > + if (blk_pwrite(sd->blk, addr, 256, frame->data, 0) < 0) { > + fprintf(stderr, "sd_blk_write: write error on host side\n"); error_report("sd_blk_write: write error on host side"); > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); > + } else { > + sd->rpmb_write_counter++; > + } > + sd->rpmb_result.write_counter = cpu_to_be32(sd->rpmb_write_counter); > + break; > + case RPMB_REQ_AUTH_DATA_READ: > + sd->rpmb_result.address = frame->address; > + break; > + default: > + qemu_log_mask(LOG_UNIMP, "RPMB request %d not implemented\n", req); > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); > + break; > + } > +exit: > + if (sd->rpmb_write_counter == 0xffffffff) { > + sd->rpmb_result.result |= cpu_to_be16(RPMB_RESULT_COUTER_EXPIRED); > + } > + trace_sdcard_rpmb_write_block(req, be16_to_cpu(sd->rpmb_result.result)); > +} > + > static void sd_erase(SDState *sd) > { > uint64_t erase_start = sd->erase_start; > @@ -1180,6 +1332,19 @@ static void emmc_function_switch(SDState *sd, uint32_t arg) > break; > } > > + if (index == EXT_CSD_PART_CONFIG) { > + uint8_t part = b & EXT_CSD_PART_CONFIG_ACC_MASK; > + > + if (((part == EXT_CSD_PART_CONFIG_ACC_BOOT1 || > + part == EXT_CSD_PART_CONFIG_ACC_BOOT2) && !sd->boot_part_size) || > + (part == EXT_CSD_PART_CONFIG_ACC_RPMB && !sd->rpmb_part_size)) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "MMC switching to illegal partition\n"); > + sd->card_status |= R_CSR_SWITCH_ERROR_MASK; > + return; > + } > + } > + > trace_sdcard_ext_csd_update(index, sd->ext_csd[index], b); > sd->ext_csd[index] = b; > } > @@ -2378,6 +2543,7 @@ static bool sd_generic_read_byte(SDState *sd, uint8_t *value) > > static void sd_write_byte(SDState *sd, uint8_t value) > { > + unsigned int partition_access; > int i; > > if (!sd->blk || !blk_is_inserted(sd->blk)) { > @@ -2427,7 +2593,13 @@ static void sd_write_byte(SDState *sd, uint8_t value) > if (sd->data_offset >= sd->blk_len) { > /* TODO: Check CRC before committing */ > sd->state = sd_programming_state; > - sd_blk_write(sd, sd->data_start, sd->data_offset); > + partition_access = sd->ext_csd[EXT_CSD_PART_CONFIG] > + & EXT_CSD_PART_CONFIG_ACC_MASK; > + if (partition_access == EXT_CSD_PART_CONFIG_ACC_RPMB) { > + emmc_rpmb_blk_write(sd, sd->data_start, sd->data_offset); > + } else { > + sd_blk_write(sd, sd->data_start, sd->data_offset); > + } > sd->blk_written++; > sd->data_start += sd->blk_len; > sd->data_offset = 0; > @@ -2510,6 +2682,7 @@ static uint8_t sd_read_byte(SDState *sd) > { > /* TODO: Append CRCs */ > const uint8_t dummy_byte = 0x00; > + unsigned int partition_access; > uint8_t ret; > uint32_t io_len; > > @@ -2553,7 +2726,13 @@ static uint8_t sd_read_byte(SDState *sd) > sd->data_start, io_len)) { > return dummy_byte; > } > - sd_blk_read(sd, sd->data_start, io_len); > + partition_access = sd->ext_csd[EXT_CSD_PART_CONFIG] > + & EXT_CSD_PART_CONFIG_ACC_MASK; > + if (partition_access == EXT_CSD_PART_CONFIG_ACC_RPMB) { > + emmc_rpmb_blk_read(sd, sd->data_start, io_len); > + } else { > + sd_blk_read(sd, sd->data_start, io_len); > + } > } > ret = sd->data[sd->data_offset ++]; > > @@ -2805,7 +2984,7 @@ static void sd_realize(DeviceState *dev, Error **errp) > blk_size = blk_getlength(sd->blk); > } > if (blk_size >= 0) { > - blk_size -= sd->boot_part_size * 2; > + blk_size -= sd->boot_part_size * 2 + sd->rpmb_part_size; > if (blk_size > SDSC_MAX_CAPACITY) { > if (sd_is_emmc(sd) && blk_size % (1 << HWBLOCK_SHIFT) != 0) { > int64_t blk_size_aligned = > @@ -2844,13 +3023,23 @@ static void sd_realize(DeviceState *dev, Error **errp) > "The boot partition size must be multiples of 128K" > "and not larger than 32640K.\n"); > } > + if (sd->rpmb_part_size % (128 * KiB) || > + sd->rpmb_part_size > 128 * 128 * KiB) { > + char *size_str = size_to_str(sd->boot_part_size); > + > + error_setg(errp, "Invalid RPMB partition size: %s", size_str); > + g_free(size_str); > + error_append_hint(errp, > + "The RPMB partition size must be multiples of 128K" > + "and not larger than 16384K.\n"); > + } > } > > static void emmc_realize(DeviceState *dev, Error **errp) > { > SDState *sd = SDMMC_COMMON(dev); > > - sd->spec_version = SD_PHY_SPECv3_01_VERS; /* Actually v4.3 */ > + sd->spec_version = SD_PHY_SPECv3_01_VERS; /* Actually v4.5 */ > > sd_realize(dev, errp); > } > @@ -2867,6 +3056,7 @@ static const Property sd_properties[] = { > static const Property emmc_properties[] = { > DEFINE_PROP_UINT64("boot-partition-size", SDState, boot_part_size, 0), > DEFINE_PROP_UINT8("boot-config", SDState, boot_config, 0x0), > + DEFINE_PROP_UINT64("rpmb-partition-size", SDState, rpmb_part_size, 0), > }; > > static void sdmmc_common_class_init(ObjectClass *klass, const void *data) > diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h > index ce6bc4e6ec..c4a9aa8edf 100644 > --- a/hw/sd/sdmmc-internal.h > +++ b/hw/sd/sdmmc-internal.h > @@ -118,9 +118,30 @@ DECLARE_OBJ_CHECKERS(SDState, SDCardClass, SDMMC_COMMON, TYPE_SDMMC_COMMON) > #define EXT_CSD_PART_CONFIG_ACC_DEFAULT (0x0) > #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x1) > #define EXT_CSD_PART_CONFIG_ACC_BOOT2 (0x2) > +#define EXT_CSD_PART_CONFIG_ACC_RPMB (0x3) > > #define EXT_CSD_PART_CONFIG_EN_MASK (0x7 << 3) > #define EXT_CSD_PART_CONFIG_EN_BOOT0 (0x1 << 3) > #define EXT_CSD_PART_CONFIG_EN_USER (0x7 << 3) > > +#define RPMB_REQ_PROGRAM_AUTH_KEY (1) > +#define RPMB_REQ_READ_WRITE_COUNTER (2) > +#define RPMB_REQ_AUTH_DATA_WRITE (3) > +#define RPMB_REQ_AUTH_DATA_READ (4) > +#define RPMB_REQ_READ_RESULT (5) > +#define RPMB_REQ_AUTH_CONFIG_WRITE (6) > +#define RPMB_REQ_AUTH_CONFIG_READ (7) > + > +#define RPMB_RESP(__req__) ((__req__) << 8) > + > +#define RPMB_RESULT_OK (0) > +#define RPMB_RESULT_GENERAL_FAILURE (1) > +#define RPMB_RESULT_AUTH_FAILURE (2) > +#define RPMB_RESULT_COUNTER_FAILURE (3) > +#define RPMB_RESULT_ADDRESS_FAILURE (4) > +#define RPMB_RESULT_WRITE_FAILURE (5) > +#define RPMB_RESULT_READ_FAILURE (6) > +#define RPMB_RESULT_NO_AUTH_KEY (7) > +#define RPMB_RESULT_COUTER_EXPIRED (0x80) > + > #endif If you are OK, I'd like to squash: -- >8 --diff --git a/hw/sd/sd.c b/hw/sd/sd.c diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h index c4a9aa8edf6..c115f472efe 100644 --- a/hw/sd/sdmmc-internal.h +++ b/hw/sd/sdmmc-internal.h @@ -144,2 +144,3 @@ DECLARE_OBJ_CHECKERS(SDState, SDCardClass, SDMMC_COMMON, TYPE_SDMMC_COMMON) #define RPMB_RESULT_NO_AUTH_KEY (7) + #define RPMB_RESULT_COUTER_EXPIRED (0x80) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index f8883860fb1..ac8f6b94746 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -119,8 +119,10 @@ typedef struct SDProto { +#define RPMB_STUFF_LEN 196 #define RPMB_KEY_MAC_LEN 32 +#define RPMB_DATA_LEN 256 /* one RPMB block is half a sector */ -typedef struct { - uint8_t stuff_bytes[196]; +typedef struct QEMU_PACKED { + uint8_t stuff_bytes[RPMB_STUFF_LEN]; uint8_t key_mac[RPMB_KEY_MAC_LEN]; - uint8_t data[256]; + uint8_t data[RPMB_DATA_LEN]; uint8_t nonce[16]; @@ -133,2 +135,5 @@ typedef struct { +QEMU_BUILD_BUG_MSG(sizeof(RPMBDataFrame) != 512, + "invalid RPMBDataFrame size"); + struct SDState { @@ -191,3 +196,3 @@ struct SDState { uint32_t rpmb_write_counter; - uint8_t rpmb_key[32]; + uint8_t rpmb_key[RPMB_KEY_MAC_LEN]; uint8_t rpmb_key_set; @@ -1019,3 +1024,3 @@ static const VMStateDescription emmc_rpmb_vmstate = { VMSTATE_UINT8_ARRAY(rpmb_result.key_mac, SDState, RPMB_KEY_MAC_LEN), - VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, 256), + VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, RPMB_DATA_LEN), VMSTATE_UINT8_ARRAY(rpmb_result.nonce, SDState, 16), @@ -1137,5 +1142,6 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) } - addr = curr_block * 256 + sd_part_offset(sd); - if (blk_pread(sd->blk, addr, 256, sd->rpmb_result.data, 0) < 0) { - fprintf(stderr, "sd_blk_read: read error on host side\n"); + addr = curr_block * RPMB_DATA_LEN + sd_part_offset(sd); + if (blk_pread(sd->blk, addr, RPMB_DATA_LEN, + sd->rpmb_result.data, 0) < 0) { + error_report("sd_blk_read: read error on host side"); memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data)); @@ -1197,5 +1203,5 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) sd->rpmb_result.address = frame->address; - addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); - if (blk_pwrite(sd->blk, addr, 256, frame->data, 0) < 0) { - fprintf(stderr, "sd_blk_write: write error on host side\n"); + addr = be16_to_cpu(frame->address) * RPMB_DATA_LEN + sd_part_offset(sd); + if (blk_pwrite(sd->blk, addr, RPMB_DATA_LEN, frame->data, 0) < 0) { + error_report("sd_blk_write: write error on host side"); sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); @@ -3027,3 +3033,3 @@ static void sd_realize(DeviceState *dev, Error **errp) } - if (sd->rpmb_part_size % (128 * KiB) || + if (!QEMU_IS_ALIGNED(sd->rpmb_part_size, 128 * KiB) || sd->rpmb_part_size > 128 * 128 * KiB) { --- And on top, if you don't mind (but can do that later): -- >8 -- diff --git a/hw/sd/sd.c b/hw/sd/sd.c index ac8f6b94746..71f396cb4d6 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -195,4 +195,6 @@ struct SDState { - RPMBDataFrame rpmb_result; - uint32_t rpmb_write_counter; - uint8_t rpmb_key[RPMB_KEY_MAC_LEN]; - uint8_t rpmb_key_set; + struct { + uint32_t write_counter; + uint8_t key[RPMB_KEY_MAC_LEN]; + uint8_t key_set; + RPMBDataFrame result; + } rpmb; @@ -1024,11 +1026,11 @@ static const VMStateDescription emmc_rpmb_vmstate = { - VMSTATE_UINT8_ARRAY(rpmb_result.key_mac, SDState, RPMB_KEY_MAC_LEN), - VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, RPMB_DATA_LEN), - VMSTATE_UINT8_ARRAY(rpmb_result.nonce, SDState, 16), - VMSTATE_UINT32(rpmb_result.write_counter, SDState), - VMSTATE_UINT16(rpmb_result.address, SDState), - VMSTATE_UINT16(rpmb_result.block_count, SDState), - VMSTATE_UINT16(rpmb_result.result, SDState), - VMSTATE_UINT16(rpmb_result.req_resp, SDState), - VMSTATE_UINT32(rpmb_write_counter, SDState), - VMSTATE_UINT8_ARRAY(rpmb_key, SDState, 32), - VMSTATE_UINT8(rpmb_key_set, SDState), + VMSTATE_UINT8_ARRAY(rpmb.result.key_mac, SDState, RPMB_KEY_MAC_LEN), + VMSTATE_UINT8_ARRAY(rpmb.result.data, SDState, RPMB_DATA_LEN), + VMSTATE_UINT8_ARRAY(rpmb.result.nonce, SDState, 16), + VMSTATE_UINT32(rpmb.result.write_counter, SDState), + VMSTATE_UINT16(rpmb.result.address, SDState), + VMSTATE_UINT16(rpmb.result.block_count, SDState), + VMSTATE_UINT16(rpmb.result.result, SDState), + VMSTATE_UINT16(rpmb.result.req_resp, SDState), + VMSTATE_UINT32(rpmb.write_counter, SDState), + VMSTATE_UINT8_ARRAY(rpmb.key, SDState, 32), + VMSTATE_UINT8(rpmb.key_set, SDState), @@ -1130,2 +1132,2 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) - uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp); - uint16_t result = be16_to_cpu(sd->rpmb_result.result); + uint16_t resp = be16_to_cpu(sd->rpmb.result.req_resp); + uint16_t result = be16_to_cpu(sd->rpmb.result.result); @@ -1136,3 +1138,3 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) - curr_block = be16_to_cpu(sd->rpmb_result.address); - if (sd->rpmb_result.block_count == 0) { - sd->rpmb_result.block_count = cpu_to_be16(sd->multi_blk_cnt); + curr_block = be16_to_cpu(sd->rpmb.result.address); + if (sd->rpmb.result.block_count == 0) { + sd->rpmb.result.block_count = cpu_to_be16(sd->multi_blk_cnt); @@ -1140 +1142 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) - curr_block += be16_to_cpu(sd->rpmb_result.block_count) - + curr_block += be16_to_cpu(sd->rpmb.result.block_count) - @@ -1144,2 +1146 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) - if (blk_pread(sd->blk, addr, RPMB_DATA_LEN, - sd->rpmb_result.data, 0) < 0) { + if (blk_pread(sd->blk, addr, RPMB_DATA_LEN, sd->rpmb.result.data, 0) < 0) { @@ -1147,2 +1148,2 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) - memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data)); - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_READ_FAILURE | + memset(sd->rpmb.result.data, 0, sizeof(sd->rpmb.result.data)); + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_READ_FAILURE | @@ -1152 +1153 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) - memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result)); + memcpy(sd->data, &sd->rpmb.result, sizeof(sd->rpmb.result)); @@ -1155 +1156 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) - be16_to_cpu(sd->rpmb_result.result)); + be16_to_cpu(sd->rpmb.result.result)); @@ -1167,4 +1168,4 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - memset(&sd->rpmb_result, 0, sizeof(sd->rpmb_result)); - memcpy(sd->rpmb_result.nonce, frame->nonce, sizeof(sd->rpmb_result.nonce)); - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_OK); - sd->rpmb_result.req_resp = cpu_to_be16(RPMB_RESP(req)); + memset(&sd->rpmb.result, 0, sizeof(sd->rpmb.result)); + memcpy(sd->rpmb.result.nonce, frame->nonce, sizeof(sd->rpmb.result.nonce)); + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_OK); + sd->rpmb.result.req_resp = cpu_to_be16(RPMB_RESP(req)); @@ -1172,2 +1173,2 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - if (!sd->rpmb_key_set && req != RPMB_REQ_PROGRAM_AUTH_KEY) { - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_NO_AUTH_KEY); + if (!sd->rpmb.key_set && req != RPMB_REQ_PROGRAM_AUTH_KEY) { + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_NO_AUTH_KEY); @@ -1179,2 +1180,2 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - if (sd->rpmb_key_set) { - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); + if (sd->rpmb.key_set) { + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); @@ -1183,2 +1184,2 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - memcpy(sd->rpmb_key, frame->key_mac, sizeof(sd->rpmb_key)); - sd->rpmb_key_set = 1; + memcpy(sd->rpmb.key, frame->key_mac, sizeof(sd->rpmb.key)); + sd->rpmb.key_set = 1; @@ -1187 +1188 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - sd->rpmb_result.write_counter = cpu_to_be32(sd->rpmb_write_counter); + sd->rpmb.result.write_counter = cpu_to_be32(sd->rpmb.write_counter); @@ -1192 +1193 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); @@ -1195,2 +1196,2 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - if (sd->rpmb_write_counter == 0xffffffff) { - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); + if (sd->rpmb.write_counter == 0xffffffff) { + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); @@ -1199,2 +1200,2 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - if (be32_to_cpu(frame->write_counter) != sd->rpmb_write_counter) { - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE); + if (be32_to_cpu(frame->write_counter) != sd->rpmb.write_counter) { + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE); @@ -1203,3 +1204,3 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - sd->rpmb_result.address = frame->address; - addr = be16_to_cpu(frame->address) * RPMB_DATA_LEN + sd_part_offset(sd); - if (blk_pwrite(sd->blk, addr, RPMB_DATA_LEN, frame->data, 0) < 0) { + sd->rpmb.result.address = frame->address; + addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); + if (blk_pwrite(sd->blk, addr, 256, frame->data, 0) < 0) { @@ -1207 +1208 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); @@ -1209 +1210 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - sd->rpmb_write_counter++; + sd->rpmb.write_counter++; @@ -1211 +1212 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - sd->rpmb_result.write_counter = cpu_to_be32(sd->rpmb_write_counter); + sd->rpmb.result.write_counter = cpu_to_be32(sd->rpmb.write_counter); @@ -1214 +1215 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - sd->rpmb_result.address = frame->address; + sd->rpmb.result.address = frame->address; @@ -1218 +1219 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); @@ -1222,2 +1223,2 @@ exit: - if (sd->rpmb_write_counter == 0xffffffff) { - sd->rpmb_result.result |= cpu_to_be16(RPMB_RESULT_COUTER_EXPIRED); + if (sd->rpmb.write_counter == 0xffffffff) { + sd->rpmb.result.result |= cpu_to_be16(RPMB_RESULT_COUTER_EXPIRED); @@ -1225 +1226 @@ exit: - trace_sdcard_rpmb_write_block(req, be16_to_cpu(sd->rpmb_result.result)); + trace_sdcard_rpmb_write_block(req, be16_to_cpu(sd->rpmb.result.result)); --- Regards, Phil. ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v5 3/6] hw/sd/sdcard: Add basic support for RPMB partition 2025-11-03 15:08 ` Philippe Mathieu-Daudé @ 2025-11-04 6:34 ` Jan Kiszka 0 siblings, 0 replies; 30+ messages in thread From: Jan Kiszka @ 2025-11-04 6:34 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier On 03.11.25 16:08, Philippe Mathieu-Daudé wrote: > Hi Jan, > > On 17/10/25 14:03, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> The Replay Protected Memory Block (RPMB) is available since eMMC 4.4 >> which has been obsoleted by 4.41. Therefore lift the provided >> EXT_CSD_REV to 5 (4.41) and provide the basic logic to implement basic >> support for it. This allows to set the authentication key, read the >> write counter and authenticated perform data read and write requests. >> Those aren't actually authenticated yet, support for that will be added >> later. >> >> The RPMB image needs to be added to backing block images after potential >> boot partitions and before the user data. It's size is controlled by >> the rpmb-partition-size property. >> >> Also missing in this version (and actually not only for RPMB bits) is >> persistence of registers that are supposed to survive power cycles. Most >> prominent are the write counters or the authentication key. This feature >> can be added later, e.g. by append a state structure to the backing >> block image. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> hw/sd/sd.c | 206 +++++++++++++++++++++++++++++++++++++++-- >> hw/sd/sdmmc-internal.h | 21 +++++ >> hw/sd/trace-events | 2 + >> 3 files changed, 221 insertions(+), 8 deletions(-) >> >> diff --git a/hw/sd/sd.c b/hw/sd/sd.c >> index 305ea251cb..918fe9f79f 100644 >> --- a/hw/sd/sd.c >> +++ b/hw/sd/sd.c >> @@ -117,6 +117,20 @@ typedef struct SDProto { >> } cmd[SDMMC_CMD_MAX], acmd[SDMMC_CMD_MAX]; >> } SDProto; >> > > #define RPMB_STUFF_LEN 196 > >> +#define RPMB_KEY_MAC_LEN 32 >> + >> +typedef > > QEMU_PACKED (better safe than sorry). > >> struct { >> + uint8_t stuff_bytes[196]; >> + uint8_t key_mac[RPMB_KEY_MAC_LEN]; >> + uint8_t data[256]; >> + uint8_t nonce[16]; >> + uint32_t write_counter; >> + uint16_t address; >> + uint16_t block_count; >> + uint16_t result; >> + uint16_t req_resp; >> +} RPMBDataFrame; >> + >> struct SDState { >> DeviceState parent_obj; >> @@ -140,6 +154,7 @@ struct SDState { >> uint8_t spec_version; >> uint64_t boot_part_size; >> + uint64_t rpmb_part_size; >> BlockBackend *blk; >> uint8_t boot_config; >> @@ -172,6 +187,10 @@ struct SDState { >> uint32_t data_offset; >> size_t data_size; >> uint8_t data[512]; >> + RPMBDataFrame rpmb_result; >> + uint32_t rpmb_write_counter; >> + uint8_t rpmb_key[32]; >> + uint8_t rpmb_key_set; > > Matter of style: > > struct { > uint32_t write_counter; > uint8_t key[RPMB_KEY_MAC_LEN]; > uint8_t key_set; > RPMBDataFrame result; > } rpmb; > >> QEMUTimer *ocr_power_timer; >> uint8_t dat_lines; >> bool cmd_line; >> @@ -506,7 +525,9 @@ static void emmc_set_ext_csd(SDState *sd, uint64_t >> size) >> sd->ext_csd[205] = 0x46; /* Min read perf for 4bit@26Mhz */ >> sd->ext_csd[EXT_CSD_CARD_TYPE] = 0b11; >> sd->ext_csd[EXT_CSD_STRUCTURE] = 2; >> - sd->ext_csd[EXT_CSD_REV] = 3; >> + sd->ext_csd[EXT_CSD_REV] = 5; >> + sd->ext_csd[EXT_CSD_RPMB_MULT] = sd->rpmb_part_size / (128 * KiB); >> + sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0b111; >> /* Mode segment (RW) */ >> sd->ext_csd[EXT_CSD_PART_CONFIG] = sd->boot_config; >> @@ -834,7 +855,8 @@ static uint32_t sd_blk_len(SDState *sd) >> /* >> * This requires a disk image that has two boot partitions inserted >> at the >> * beginning of it, followed by an RPMB partition. The size of the boot >> - * partitions is the "boot-partition-size" property. >> + * partitions is the "boot-partition-size" property, the one of the RPMB >> + * partition is 'rpmb-partition-size'. >> */ >> static uint32_t sd_part_offset(SDState *sd) >> { >> @@ -848,11 +870,13 @@ static uint32_t sd_part_offset(SDState *sd) >> & EXT_CSD_PART_CONFIG_ACC_MASK; >> switch (partition_access) { >> case EXT_CSD_PART_CONFIG_ACC_DEFAULT: >> - return sd->boot_part_size * 2; >> + return sd->boot_part_size * 2 + sd->rpmb_part_size; >> case EXT_CSD_PART_CONFIG_ACC_BOOT1: >> return 0; >> case EXT_CSD_PART_CONFIG_ACC_BOOT2: >> return sd->boot_part_size * 1; >> + case EXT_CSD_PART_CONFIG_ACC_RPMB: >> + return sd->boot_part_size * 2; >> default: >> g_assert_not_reached(); >> } >> @@ -891,7 +915,7 @@ static void sd_reset(DeviceState *dev) >> } >> size = sect << HWBLOCK_SHIFT; >> if (sd_is_emmc(sd)) { >> - size -= sd->boot_part_size * 2; >> + size -= sd->boot_part_size * 2 + sd->rpmb_part_size; >> } >> sect = sd_addr_to_wpnum(size) + 1; >> @@ -979,6 +1003,34 @@ static const VMStateDescription sd_ocr_vmstate = { >> }, >> }; >> +static bool vmstate_needed_for_rpmb(void *opaque) >> +{ >> + SDState *sd = opaque; >> + >> + return sd->rpmb_part_size > 0; >> +} >> + >> +static const VMStateDescription emmc_rpmb_vmstate = { >> + .name = "sd-card/ext_csd_modes-state", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .needed = vmstate_needed_for_rpmb, >> + .fields = (const VMStateField[]) { >> + VMSTATE_UINT8_ARRAY(rpmb_result.key_mac, SDState, >> RPMB_KEY_MAC_LEN), >> + VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, 256), >> + VMSTATE_UINT8_ARRAY(rpmb_result.nonce, SDState, 16), >> + VMSTATE_UINT32(rpmb_result.write_counter, SDState), >> + VMSTATE_UINT16(rpmb_result.address, SDState), >> + VMSTATE_UINT16(rpmb_result.block_count, SDState), >> + VMSTATE_UINT16(rpmb_result.result, SDState), >> + VMSTATE_UINT16(rpmb_result.req_resp, SDState), >> + VMSTATE_UINT32(rpmb_write_counter, SDState), >> + VMSTATE_UINT8_ARRAY(rpmb_key, SDState, 32), >> + VMSTATE_UINT8(rpmb_key_set, SDState), >> + VMSTATE_END_OF_LIST() >> + }, >> +}; >> + >> static bool vmstate_needed_for_emmc(void *opaque) >> { >> SDState *sd = opaque; >> @@ -1045,6 +1097,7 @@ static const VMStateDescription sd_vmstate = { >> .subsections = (const VMStateDescription * const []) { >> &sd_ocr_vmstate, >> &emmc_extcsd_vmstate, >> + &emmc_rpmb_vmstate, >> NULL >> }, >> }; >> @@ -1067,6 +1120,105 @@ static void sd_blk_write(SDState *sd, uint64_t >> addr, uint32_t len) >> } >> } >> +static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t >> len) >> +{ >> + uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp); >> + uint16_t result = be16_to_cpu(sd->rpmb_result.result); >> + unsigned int curr_block = 0; >> + >> + if ((result & ~RPMB_RESULT_COUTER_EXPIRED) == RPMB_RESULT_OK && >> + resp == RPMB_RESP(RPMB_REQ_AUTH_DATA_READ)) { >> + curr_block = be16_to_cpu(sd->rpmb_result.address); >> + if (sd->rpmb_result.block_count == 0) { >> + sd->rpmb_result.block_count = cpu_to_be16(sd- >> >multi_blk_cnt); >> + } else { >> + curr_block += be16_to_cpu(sd->rpmb_result.block_count) - >> + sd->multi_blk_cnt; >> + } >> + addr = curr_block * 256 + sd_part_offset(sd); >> + if (blk_pread(sd->blk, addr, 256, sd->rpmb_result.data, 0) < >> 0) { > > Would be nice to re-use sd_blk_read(), but I notice we want to read the > frame data to then copy the whole message into sd->data. > >> + fprintf(stderr, "sd_blk_read: read error on host side\n"); > > Although a pre-existing pattern in this file, no new fprintf(stderr) > please. Better use the Error* API, otherwise error_report(). > > error_report("sd_blk_read: read error on host side"); > >> + memset(sd->rpmb_result.data, 0, sizeof(sd- >> >rpmb_result.data)); >> + sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_READ_FAILURE | >> + (result & RPMB_RESULT_COUTER_EXPIRED)); >> + } >> + } >> + memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result)); >> + >> + trace_sdcard_rpmb_read_block(resp, curr_block, >> + be16_to_cpu(sd->rpmb_result.result)); >> +} >> + >> +static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t >> len) >> +{ >> + RPMBDataFrame *frame = (RPMBDataFrame *)sd->data; >> + uint16_t req = be16_to_cpu(frame->req_resp); >> + >> + if (req == RPMB_REQ_READ_RESULT) { >> + /* just return the current result register */ >> + goto exit; >> + } >> + memset(&sd->rpmb_result, 0, sizeof(sd->rpmb_result)); >> + memcpy(sd->rpmb_result.nonce, frame->nonce, sizeof(sd- >> >rpmb_result.nonce)); >> + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_OK); >> + sd->rpmb_result.req_resp = cpu_to_be16(RPMB_RESP(req)); >> + >> + if (!sd->rpmb_key_set && req != RPMB_REQ_PROGRAM_AUTH_KEY) { >> + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_NO_AUTH_KEY); >> + goto exit; >> + } >> + >> + switch (req) { >> + case RPMB_REQ_PROGRAM_AUTH_KEY: >> + if (sd->rpmb_key_set) { >> + sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); >> + break; >> + } >> + memcpy(sd->rpmb_key, frame->key_mac, sizeof(sd->rpmb_key)); >> + sd->rpmb_key_set = 1; >> + break; >> + case RPMB_REQ_READ_WRITE_COUNTER: >> + sd->rpmb_result.write_counter = cpu_to_be32(sd- >> >rpmb_write_counter); >> + break; >> + case RPMB_REQ_AUTH_DATA_WRITE: >> + /* We only support single-block writes so far */ >> + if (sd->multi_blk_cnt != 1) { >> + sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); >> + break; >> + } >> + if (sd->rpmb_write_counter == 0xffffffff) { >> + sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); >> + break; >> + } >> + if (be32_to_cpu(frame->write_counter) != sd- >> >rpmb_write_counter) { >> + sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE); >> + break; >> + } >> + sd->rpmb_result.address = frame->address; >> + addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); >> + if (blk_pwrite(sd->blk, addr, 256, frame->data, 0) < 0) { >> + fprintf(stderr, "sd_blk_write: write error on host side\n"); > > error_report("sd_blk_write: write error on host side"); > >> + sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); >> + } else { >> + sd->rpmb_write_counter++; >> + } >> + sd->rpmb_result.write_counter = cpu_to_be32(sd- >> >rpmb_write_counter); >> + break; >> + case RPMB_REQ_AUTH_DATA_READ: >> + sd->rpmb_result.address = frame->address; >> + break; >> + default: >> + qemu_log_mask(LOG_UNIMP, "RPMB request %d not implemented\n", >> req); >> + sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); >> + break; >> + } >> +exit: >> + if (sd->rpmb_write_counter == 0xffffffff) { >> + sd->rpmb_result.result |= >> cpu_to_be16(RPMB_RESULT_COUTER_EXPIRED); >> + } >> + trace_sdcard_rpmb_write_block(req, be16_to_cpu(sd- >> >rpmb_result.result)); >> +} >> + >> static void sd_erase(SDState *sd) >> { >> uint64_t erase_start = sd->erase_start; >> @@ -1180,6 +1332,19 @@ static void emmc_function_switch(SDState *sd, >> uint32_t arg) >> break; >> } >> + if (index == EXT_CSD_PART_CONFIG) { >> + uint8_t part = b & EXT_CSD_PART_CONFIG_ACC_MASK; >> + >> + if (((part == EXT_CSD_PART_CONFIG_ACC_BOOT1 || >> + part == EXT_CSD_PART_CONFIG_ACC_BOOT2) && !sd- >> >boot_part_size) || >> + (part == EXT_CSD_PART_CONFIG_ACC_RPMB && !sd- >> >rpmb_part_size)) { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "MMC switching to illegal partition\n"); >> + sd->card_status |= R_CSR_SWITCH_ERROR_MASK; >> + return; >> + } >> + } >> + >> trace_sdcard_ext_csd_update(index, sd->ext_csd[index], b); >> sd->ext_csd[index] = b; >> } >> @@ -2378,6 +2543,7 @@ static bool sd_generic_read_byte(SDState *sd, >> uint8_t *value) >> static void sd_write_byte(SDState *sd, uint8_t value) >> { >> + unsigned int partition_access; >> int i; >> if (!sd->blk || !blk_is_inserted(sd->blk)) { >> @@ -2427,7 +2593,13 @@ static void sd_write_byte(SDState *sd, uint8_t >> value) >> if (sd->data_offset >= sd->blk_len) { >> /* TODO: Check CRC before committing */ >> sd->state = sd_programming_state; >> - sd_blk_write(sd, sd->data_start, sd->data_offset); >> + partition_access = sd->ext_csd[EXT_CSD_PART_CONFIG] >> + & EXT_CSD_PART_CONFIG_ACC_MASK; >> + if (partition_access == EXT_CSD_PART_CONFIG_ACC_RPMB) { >> + emmc_rpmb_blk_write(sd, sd->data_start, sd- >> >data_offset); >> + } else { >> + sd_blk_write(sd, sd->data_start, sd->data_offset); >> + } >> sd->blk_written++; >> sd->data_start += sd->blk_len; >> sd->data_offset = 0; >> @@ -2510,6 +2682,7 @@ static uint8_t sd_read_byte(SDState *sd) >> { >> /* TODO: Append CRCs */ >> const uint8_t dummy_byte = 0x00; >> + unsigned int partition_access; >> uint8_t ret; >> uint32_t io_len; >> @@ -2553,7 +2726,13 @@ static uint8_t sd_read_byte(SDState *sd) >> sd->data_start, io_len)) { >> return dummy_byte; >> } >> - sd_blk_read(sd, sd->data_start, io_len); >> + partition_access = sd->ext_csd[EXT_CSD_PART_CONFIG] >> + & EXT_CSD_PART_CONFIG_ACC_MASK; >> + if (partition_access == EXT_CSD_PART_CONFIG_ACC_RPMB) { >> + emmc_rpmb_blk_read(sd, sd->data_start, io_len); >> + } else { >> + sd_blk_read(sd, sd->data_start, io_len); >> + } >> } >> ret = sd->data[sd->data_offset ++]; >> @@ -2805,7 +2984,7 @@ static void sd_realize(DeviceState *dev, Error >> **errp) >> blk_size = blk_getlength(sd->blk); >> } >> if (blk_size >= 0) { >> - blk_size -= sd->boot_part_size * 2; >> + blk_size -= sd->boot_part_size * 2 + sd->rpmb_part_size; >> if (blk_size > SDSC_MAX_CAPACITY) { >> if (sd_is_emmc(sd) && blk_size % (1 << HWBLOCK_SHIFT) != >> 0) { >> int64_t blk_size_aligned = >> @@ -2844,13 +3023,23 @@ static void sd_realize(DeviceState *dev, Error >> **errp) >> "The boot partition size must be multiples >> of 128K" >> "and not larger than 32640K.\n"); >> } >> + if (sd->rpmb_part_size % (128 * KiB) || >> + sd->rpmb_part_size > 128 * 128 * KiB) { >> + char *size_str = size_to_str(sd->boot_part_size); >> + >> + error_setg(errp, "Invalid RPMB partition size: %s", size_str); >> + g_free(size_str); >> + error_append_hint(errp, >> + "The RPMB partition size must be multiples >> of 128K" >> + "and not larger than 16384K.\n"); >> + } >> } >> static void emmc_realize(DeviceState *dev, Error **errp) >> { >> SDState *sd = SDMMC_COMMON(dev); >> - sd->spec_version = SD_PHY_SPECv3_01_VERS; /* Actually v4.3 */ >> + sd->spec_version = SD_PHY_SPECv3_01_VERS; /* Actually v4.5 */ >> sd_realize(dev, errp); >> } >> @@ -2867,6 +3056,7 @@ static const Property sd_properties[] = { >> static const Property emmc_properties[] = { >> DEFINE_PROP_UINT64("boot-partition-size", SDState, >> boot_part_size, 0), >> DEFINE_PROP_UINT8("boot-config", SDState, boot_config, 0x0), >> + DEFINE_PROP_UINT64("rpmb-partition-size", SDState, >> rpmb_part_size, 0), >> }; >> static void sdmmc_common_class_init(ObjectClass *klass, const void >> *data) >> diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h >> index ce6bc4e6ec..c4a9aa8edf 100644 >> --- a/hw/sd/sdmmc-internal.h >> +++ b/hw/sd/sdmmc-internal.h >> @@ -118,9 +118,30 @@ DECLARE_OBJ_CHECKERS(SDState, SDCardClass, >> SDMMC_COMMON, TYPE_SDMMC_COMMON) >> #define EXT_CSD_PART_CONFIG_ACC_DEFAULT (0x0) >> #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x1) >> #define EXT_CSD_PART_CONFIG_ACC_BOOT2 (0x2) >> +#define EXT_CSD_PART_CONFIG_ACC_RPMB (0x3) >> #define EXT_CSD_PART_CONFIG_EN_MASK (0x7 << 3) >> #define EXT_CSD_PART_CONFIG_EN_BOOT0 (0x1 << 3) >> #define EXT_CSD_PART_CONFIG_EN_USER (0x7 << 3) >> +#define RPMB_REQ_PROGRAM_AUTH_KEY (1) >> +#define RPMB_REQ_READ_WRITE_COUNTER (2) >> +#define RPMB_REQ_AUTH_DATA_WRITE (3) >> +#define RPMB_REQ_AUTH_DATA_READ (4) >> +#define RPMB_REQ_READ_RESULT (5) >> +#define RPMB_REQ_AUTH_CONFIG_WRITE (6) >> +#define RPMB_REQ_AUTH_CONFIG_READ (7) >> + >> +#define RPMB_RESP(__req__) ((__req__) << 8) >> + >> +#define RPMB_RESULT_OK (0) >> +#define RPMB_RESULT_GENERAL_FAILURE (1) >> +#define RPMB_RESULT_AUTH_FAILURE (2) >> +#define RPMB_RESULT_COUNTER_FAILURE (3) >> +#define RPMB_RESULT_ADDRESS_FAILURE (4) >> +#define RPMB_RESULT_WRITE_FAILURE (5) >> +#define RPMB_RESULT_READ_FAILURE (6) >> +#define RPMB_RESULT_NO_AUTH_KEY (7) > >> +#define RPMB_RESULT_COUTER_EXPIRED (0x80) >> + >> #endif > > If you are OK, I'd like to squash: > > -- >8 --diff --git a/hw/sd/sd.c b/hw/sd/sd.c > diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h > index c4a9aa8edf6..c115f472efe 100644 > --- a/hw/sd/sdmmc-internal.h > +++ b/hw/sd/sdmmc-internal.h > @@ -144,2 +144,3 @@ DECLARE_OBJ_CHECKERS(SDState, SDCardClass, > SDMMC_COMMON, TYPE_SDMMC_COMMON) > #define RPMB_RESULT_NO_AUTH_KEY (7) > + > #define RPMB_RESULT_COUTER_EXPIRED (0x80) > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index f8883860fb1..ac8f6b94746 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -119,8 +119,10 @@ typedef struct SDProto { > > +#define RPMB_STUFF_LEN 196 > #define RPMB_KEY_MAC_LEN 32 > +#define RPMB_DATA_LEN 256 /* one RPMB block is half a sector */ > > -typedef struct { > - uint8_t stuff_bytes[196]; > +typedef struct QEMU_PACKED { > + uint8_t stuff_bytes[RPMB_STUFF_LEN]; > uint8_t key_mac[RPMB_KEY_MAC_LEN]; > - uint8_t data[256]; > + uint8_t data[RPMB_DATA_LEN]; > uint8_t nonce[16]; You left the nonce length without a constant now. > @@ -133,2 +135,5 @@ typedef struct { > > +QEMU_BUILD_BUG_MSG(sizeof(RPMBDataFrame) != 512, > + "invalid RPMBDataFrame size"); > + > struct SDState { > @@ -191,3 +196,3 @@ struct SDState { > uint32_t rpmb_write_counter; > - uint8_t rpmb_key[32]; > + uint8_t rpmb_key[RPMB_KEY_MAC_LEN]; > uint8_t rpmb_key_set; > @@ -1019,3 +1024,3 @@ static const VMStateDescription emmc_rpmb_vmstate = { > VMSTATE_UINT8_ARRAY(rpmb_result.key_mac, SDState, > RPMB_KEY_MAC_LEN), > - VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, 256), > + VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, RPMB_DATA_LEN), > VMSTATE_UINT8_ARRAY(rpmb_result.nonce, SDState, 16), > @@ -1137,5 +1142,6 @@ static void emmc_rpmb_blk_read(SDState *sd, > uint64_t addr, uint32_t len) > } > - addr = curr_block * 256 + sd_part_offset(sd); > - if (blk_pread(sd->blk, addr, 256, sd->rpmb_result.data, 0) < 0) { > - fprintf(stderr, "sd_blk_read: read error on host side\n"); > + addr = curr_block * RPMB_DATA_LEN + sd_part_offset(sd); > + if (blk_pread(sd->blk, addr, RPMB_DATA_LEN, > + sd->rpmb_result.data, 0) < 0) { > + error_report("sd_blk_read: read error on host side"); > memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data)); > @@ -1197,5 +1203,5 @@ static void emmc_rpmb_blk_write(SDState *sd, > uint64_t addr, uint32_t len) > sd->rpmb_result.address = frame->address; > - addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); > - if (blk_pwrite(sd->blk, addr, 256, frame->data, 0) < 0) { > - fprintf(stderr, "sd_blk_write: write error on host side\n"); > + addr = be16_to_cpu(frame->address) * RPMB_DATA_LEN + > sd_part_offset(sd); > + if (blk_pwrite(sd->blk, addr, RPMB_DATA_LEN, frame->data, 0) < > 0) { > + error_report("sd_blk_write: write error on host side"); > sd->rpmb_result.result = > cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); > @@ -3027,3 +3033,3 @@ static void sd_realize(DeviceState *dev, Error > **errp) > } > - if (sd->rpmb_part_size % (128 * KiB) || > + if (!QEMU_IS_ALIGNED(sd->rpmb_part_size, 128 * KiB) || > sd->rpmb_part_size > 128 * 128 * KiB) { > --- > > And on top, if you don't mind (but can do that later): > > -- >8 -- > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index ac8f6b94746..71f396cb4d6 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -195,4 +195,6 @@ struct SDState { > - RPMBDataFrame rpmb_result; > - uint32_t rpmb_write_counter; > - uint8_t rpmb_key[RPMB_KEY_MAC_LEN]; > - uint8_t rpmb_key_set; > + struct { > + uint32_t write_counter; > + uint8_t key[RPMB_KEY_MAC_LEN]; > + uint8_t key_set; > + RPMBDataFrame result; > + } rpmb; > @@ -1024,11 +1026,11 @@ static const VMStateDescription > emmc_rpmb_vmstate = { > - VMSTATE_UINT8_ARRAY(rpmb_result.key_mac, SDState, > RPMB_KEY_MAC_LEN), > - VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, RPMB_DATA_LEN), > - VMSTATE_UINT8_ARRAY(rpmb_result.nonce, SDState, 16), > - VMSTATE_UINT32(rpmb_result.write_counter, SDState), > - VMSTATE_UINT16(rpmb_result.address, SDState), > - VMSTATE_UINT16(rpmb_result.block_count, SDState), > - VMSTATE_UINT16(rpmb_result.result, SDState), > - VMSTATE_UINT16(rpmb_result.req_resp, SDState), > - VMSTATE_UINT32(rpmb_write_counter, SDState), > - VMSTATE_UINT8_ARRAY(rpmb_key, SDState, 32), > - VMSTATE_UINT8(rpmb_key_set, SDState), > + VMSTATE_UINT8_ARRAY(rpmb.result.key_mac, SDState, > RPMB_KEY_MAC_LEN), > + VMSTATE_UINT8_ARRAY(rpmb.result.data, SDState, RPMB_DATA_LEN), > + VMSTATE_UINT8_ARRAY(rpmb.result.nonce, SDState, 16), > + VMSTATE_UINT32(rpmb.result.write_counter, SDState), > + VMSTATE_UINT16(rpmb.result.address, SDState), > + VMSTATE_UINT16(rpmb.result.block_count, SDState), > + VMSTATE_UINT16(rpmb.result.result, SDState), > + VMSTATE_UINT16(rpmb.result.req_resp, SDState), > + VMSTATE_UINT32(rpmb.write_counter, SDState), > + VMSTATE_UINT8_ARRAY(rpmb.key, SDState, 32), > + VMSTATE_UINT8(rpmb.key_set, SDState), > @@ -1130,2 +1132,2 @@ static void emmc_rpmb_blk_read(SDState *sd, > uint64_t addr, uint32_t len) > - uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp); > - uint16_t result = be16_to_cpu(sd->rpmb_result.result); > + uint16_t resp = be16_to_cpu(sd->rpmb.result.req_resp); > + uint16_t result = be16_to_cpu(sd->rpmb.result.result); > @@ -1136,3 +1138,3 @@ static void emmc_rpmb_blk_read(SDState *sd, > uint64_t addr, uint32_t len) > - curr_block = be16_to_cpu(sd->rpmb_result.address); > - if (sd->rpmb_result.block_count == 0) { > - sd->rpmb_result.block_count = cpu_to_be16(sd->multi_blk_cnt); > + curr_block = be16_to_cpu(sd->rpmb.result.address); > + if (sd->rpmb.result.block_count == 0) { > + sd->rpmb.result.block_count = cpu_to_be16(sd->multi_blk_cnt); > @@ -1140 +1142 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t > addr, uint32_t len) > - curr_block += be16_to_cpu(sd->rpmb_result.block_count) - > + curr_block += be16_to_cpu(sd->rpmb.result.block_count) - > @@ -1144,2 +1146 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t > addr, uint32_t len) > - if (blk_pread(sd->blk, addr, RPMB_DATA_LEN, > - sd->rpmb_result.data, 0) < 0) { > + if (blk_pread(sd->blk, addr, RPMB_DATA_LEN, sd- >>rpmb.result.data, 0) < 0) { > @@ -1147,2 +1148,2 @@ static void emmc_rpmb_blk_read(SDState *sd, > uint64_t addr, uint32_t len) > - memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data)); > - sd->rpmb_result.result = > cpu_to_be16(RPMB_RESULT_READ_FAILURE | > + memset(sd->rpmb.result.data, 0, sizeof(sd->rpmb.result.data)); > + sd->rpmb.result.result = > cpu_to_be16(RPMB_RESULT_READ_FAILURE | > @@ -1152 +1153 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t > addr, uint32_t len) > - memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result)); > + memcpy(sd->data, &sd->rpmb.result, sizeof(sd->rpmb.result)); > @@ -1155 +1156 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t > addr, uint32_t len) > - be16_to_cpu(sd->rpmb_result.result)); > + be16_to_cpu(sd->rpmb.result.result)); > @@ -1167,4 +1168,4 @@ static void emmc_rpmb_blk_write(SDState *sd, > uint64_t addr, uint32_t len) > - memset(&sd->rpmb_result, 0, sizeof(sd->rpmb_result)); > - memcpy(sd->rpmb_result.nonce, frame->nonce, sizeof(sd- >>rpmb_result.nonce)); > - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_OK); > - sd->rpmb_result.req_resp = cpu_to_be16(RPMB_RESP(req)); > + memset(&sd->rpmb.result, 0, sizeof(sd->rpmb.result)); > + memcpy(sd->rpmb.result.nonce, frame->nonce, sizeof(sd- >>rpmb.result.nonce)); > + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_OK); > + sd->rpmb.result.req_resp = cpu_to_be16(RPMB_RESP(req)); > @@ -1172,2 +1173,2 @@ static void emmc_rpmb_blk_write(SDState *sd, > uint64_t addr, uint32_t len) > - if (!sd->rpmb_key_set && req != RPMB_REQ_PROGRAM_AUTH_KEY) { > - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_NO_AUTH_KEY); > + if (!sd->rpmb.key_set && req != RPMB_REQ_PROGRAM_AUTH_KEY) { > + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_NO_AUTH_KEY); > @@ -1179,2 +1180,2 @@ static void emmc_rpmb_blk_write(SDState *sd, > uint64_t addr, uint32_t len) > - if (sd->rpmb_key_set) { > - sd->rpmb_result.result = > cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); > + if (sd->rpmb.key_set) { > + sd->rpmb.result.result = > cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); > @@ -1183,2 +1184,2 @@ static void emmc_rpmb_blk_write(SDState *sd, > uint64_t addr, uint32_t len) > - memcpy(sd->rpmb_key, frame->key_mac, sizeof(sd->rpmb_key)); > - sd->rpmb_key_set = 1; > + memcpy(sd->rpmb.key, frame->key_mac, sizeof(sd->rpmb.key)); > + sd->rpmb.key_set = 1; > @@ -1187 +1188 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t > addr, uint32_t len) > - sd->rpmb_result.write_counter = cpu_to_be32(sd- >>rpmb_write_counter); > + sd->rpmb.result.write_counter = cpu_to_be32(sd- >>rpmb.write_counter); > @@ -1192 +1193 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t > addr, uint32_t len) > - sd->rpmb_result.result = > cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); > + sd->rpmb.result.result = > cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); > @@ -1195,2 +1196,2 @@ static void emmc_rpmb_blk_write(SDState *sd, > uint64_t addr, uint32_t len) > - if (sd->rpmb_write_counter == 0xffffffff) { > - sd->rpmb_result.result = > cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); > + if (sd->rpmb.write_counter == 0xffffffff) { > + sd->rpmb.result.result = > cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); > @@ -1199,2 +1200,2 @@ static void emmc_rpmb_blk_write(SDState *sd, > uint64_t addr, uint32_t len) > - if (be32_to_cpu(frame->write_counter) != sd->rpmb_write_counter) { > - sd->rpmb_result.result = > cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE); > + if (be32_to_cpu(frame->write_counter) != sd->rpmb.write_counter) { > + sd->rpmb.result.result = > cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE); > @@ -1203,3 +1204,3 @@ static void emmc_rpmb_blk_write(SDState *sd, > uint64_t addr, uint32_t len) > - sd->rpmb_result.address = frame->address; > - addr = be16_to_cpu(frame->address) * RPMB_DATA_LEN + > sd_part_offset(sd); > - if (blk_pwrite(sd->blk, addr, RPMB_DATA_LEN, frame->data, 0) < > 0) { > + sd->rpmb.result.address = frame->address; > + addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); > + if (blk_pwrite(sd->blk, addr, 256, frame->data, 0) < 0) { > @@ -1207 +1208 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t > addr, uint32_t len) > - sd->rpmb_result.result = > cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); > + sd->rpmb.result.result = > cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); > @@ -1209 +1210 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t > addr, uint32_t len) > - sd->rpmb_write_counter++; > + sd->rpmb.write_counter++; > @@ -1211 +1212 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t > addr, uint32_t len) > - sd->rpmb_result.write_counter = cpu_to_be32(sd- >>rpmb_write_counter); > + sd->rpmb.result.write_counter = cpu_to_be32(sd- >>rpmb.write_counter); > @@ -1214 +1215 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t > addr, uint32_t len) > - sd->rpmb_result.address = frame->address; > + sd->rpmb.result.address = frame->address; > @@ -1218 +1219 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t > addr, uint32_t len) > - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); > + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE); > @@ -1222,2 +1223,2 @@ exit: > - if (sd->rpmb_write_counter == 0xffffffff) { > - sd->rpmb_result.result |= cpu_to_be16(RPMB_RESULT_COUTER_EXPIRED); > + if (sd->rpmb.write_counter == 0xffffffff) { > + sd->rpmb.result.result |= cpu_to_be16(RPMB_RESULT_COUTER_EXPIRED); > @@ -1225 +1226 @@ exit: > - trace_sdcard_rpmb_write_block(req, be16_to_cpu(sd- >>rpmb_result.result)); > + trace_sdcard_rpmb_write_block(req, be16_to_cpu(sd- >>rpmb.result.result)); > --- > > Regards, > > Phil. Let me pick up and test those changes for v6. Thanks, Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v5 4/6] hw/sd/sdcard: Handle RPMB MAC field 2025-10-17 12:03 [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Jan Kiszka ` (2 preceding siblings ...) 2025-10-17 12:03 ` [PATCH v5 3/6] hw/sd/sdcard: Add basic support for RPMB partition Jan Kiszka @ 2025-10-17 12:03 ` Jan Kiszka 2025-11-03 15:18 ` Philippe Mathieu-Daudé 2025-10-17 12:03 ` [PATCH v5 5/6] scripts: Add helper script to generate eMMC block device images Jan Kiszka ` (3 subsequent siblings) 7 siblings, 1 reply; 30+ messages in thread From: Jan Kiszka @ 2025-10-17 12:03 UTC (permalink / raw) To: qemu-devel Cc: Philippe Mathieu-Daudé, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier From: Jan Kiszka <jan.kiszka@siemens.com> Implement correct setting of the MAC field when passing RPMB frames back to the guest. Also check the MAC on authenticated write requests. This depends on HMAC support for QCRYPTO_HASH_ALGO_SHA256 which is always available via glib - assert this, just to be safe. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/sd/sd.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 918fe9f79f..759e284ac0 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -51,6 +51,7 @@ #include "qemu/module.h" #include "sdmmc-internal.h" #include "trace.h" +#include "crypto/hmac.h" //#define DEBUG_SD 1 @@ -118,6 +119,7 @@ typedef struct SDProto { } SDProto; #define RPMB_KEY_MAC_LEN 32 +#define RPMB_HASH_LEN 284 typedef struct { uint8_t stuff_bytes[196]; @@ -1120,6 +1122,66 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) } } +static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame, + unsigned int num_blocks, uint8_t *mac) +{ + size_t mac_len = RPMB_KEY_MAC_LEN; + bool success = true; + Error *err = NULL; + QCryptoHmac *hmac; + uint64_t addr; + + hmac = qcrypto_hmac_new(QCRYPTO_HASH_ALGO_SHA256, sd->rpmb_key, + RPMB_KEY_MAC_LEN, &err); + if (!hmac) { + error_report_err(err); + return false; + } + + /* + * This implies a read request because we only support single-block write + * requests so far. + */ + if (num_blocks > 1) { + /* + * Unfortunately, the underlying crypto libraries do not allow us to + * migrate an active QCryptoHmac state. Therefore, we have to calculate + * the HMAC in one run. To avoid buffering a complete read sequence in + * SDState, reconstruct all frames except for the last one. + */ + char *buf = (char *)sd->data; + + memcpy(buf, frame->data, RPMB_HASH_LEN); + addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); + do { + if (blk_pread(sd->blk, addr, 256, buf, 0) < 0) { + fprintf(stderr, "sd_blk_read: read error on host side\n"); + success = false; + break; + } + if (qcrypto_hmac_bytes(hmac, buf, RPMB_HASH_LEN, NULL, NULL, + &err) < 0) { + error_report_err(err); + success = false; + break; + } + addr += 256; + } while (--num_blocks > 1); + } + + if (success && + qcrypto_hmac_bytes(hmac, (const char*)frame->data, RPMB_HASH_LEN, &mac, + &mac_len, &err) < 0) { + error_report_err(err); + success = false; + } + assert(!success || mac_len == RPMB_KEY_MAC_LEN); + + qcrypto_hmac_free(hmac); + + return success; +} + static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) { uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp); @@ -1142,6 +1204,17 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_READ_FAILURE | (result & RPMB_RESULT_COUTER_EXPIRED)); } + if (sd->multi_blk_cnt == 1 && + !rpmb_calc_hmac(sd, &sd->rpmb_result, + be16_to_cpu(sd->rpmb_result.block_count), + sd->rpmb_result.key_mac)) { + memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data)); + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE); + } + } else if (!rpmb_calc_hmac(sd, &sd->rpmb_result, 1, + sd->rpmb_result.key_mac)) { + memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data)); + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE); } memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result)); @@ -1153,6 +1226,7 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) { RPMBDataFrame *frame = (RPMBDataFrame *)sd->data; uint16_t req = be16_to_cpu(frame->req_resp); + uint8_t mac[RPMB_KEY_MAC_LEN]; if (req == RPMB_REQ_READ_RESULT) { /* just return the current result register */ @@ -1190,6 +1264,11 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); break; } + if (!rpmb_calc_hmac(sd, frame, 1, mac) || + memcmp(frame->key_mac, mac, RPMB_KEY_MAC_LEN) != 0) { + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE); + break; + } if (be32_to_cpu(frame->write_counter) != sd->rpmb_write_counter) { sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE); break; @@ -3115,6 +3194,8 @@ static void emmc_class_init(ObjectClass *klass, const void *data) DeviceClass *dc = DEVICE_CLASS(klass); SDCardClass *sc = SDMMC_COMMON_CLASS(klass); + assert(qcrypto_hmac_supports(QCRYPTO_HASH_ALGO_SHA256)); + dc->desc = "eMMC"; dc->realize = emmc_realize; device_class_set_props(dc, emmc_properties); -- 2.51.0 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v5 4/6] hw/sd/sdcard: Handle RPMB MAC field 2025-10-17 12:03 ` [PATCH v5 4/6] hw/sd/sdcard: Handle RPMB MAC field Jan Kiszka @ 2025-11-03 15:18 ` Philippe Mathieu-Daudé 2025-11-04 6:43 ` Jan Kiszka 0 siblings, 1 reply; 30+ messages in thread From: Philippe Mathieu-Daudé @ 2025-11-03 15:18 UTC (permalink / raw) To: Jan Kiszka, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Daniel P. Berrangé Hi Jan, On 17/10/25 14:03, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Implement correct setting of the MAC field when passing RPMB frames back > to the guest. Also check the MAC on authenticated write requests. > > This depends on HMAC support for QCRYPTO_HASH_ALGO_SHA256 which is > always available via glib - assert this, just to be safe. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > hw/sd/sd.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 81 insertions(+) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 918fe9f79f..759e284ac0 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -51,6 +51,7 @@ > #include "qemu/module.h" > #include "sdmmc-internal.h" > #include "trace.h" > +#include "crypto/hmac.h" > > //#define DEBUG_SD 1 > > @@ -118,6 +119,7 @@ typedef struct SDProto { > } SDProto; > > #define RPMB_KEY_MAC_LEN 32 > +#define RPMB_HASH_LEN 284 > > typedef struct { > uint8_t stuff_bytes[196]; > @@ -1120,6 +1122,66 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) > } > } > > +static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame, const frame. > + unsigned int num_blocks, uint8_t *mac) > +{ > + size_t mac_len = RPMB_KEY_MAC_LEN; > + bool success = true; > + Error *err = NULL; > + QCryptoHmac *hmac; Preferably: g_autoptr(QCryptoHmac) hmac = NULL; > + uint64_t addr; Maybe better named 'offset'. > + > + hmac = qcrypto_hmac_new(QCRYPTO_HASH_ALGO_SHA256, sd->rpmb_key, > + RPMB_KEY_MAC_LEN, &err); > + if (!hmac) { > + error_report_err(err); > + return false; > + } > + > + /* > + * This implies a read request because we only support single-block write > + * requests so far. > + */ > + if (num_blocks > 1) { > + /* > + * Unfortunately, the underlying crypto libraries do not allow us to > + * migrate an active QCryptoHmac state. Therefore, we have to calculate > + * the HMAC in one run. To avoid buffering a complete read sequence in > + * SDState, reconstruct all frames except for the last one. > + */ > + char *buf = (char *)sd->data; Why not plain 'void *'? Ah, qcrypto_hmac_bytes() takes a 'char *', odd... [Update, now about to be fixed: https://lore.kernel.org/qemu-devel/20251103133727.423041-4-berrange@redhat.com/] Better safe than sorry: assert(RPMB_HASH_LEN <= sizeof(sd->data)); > + memcpy(buf, frame->data, RPMB_HASH_LEN); Nitpicking, no need to fill the block we'll overwrite: memcpy(&buf[256], &frame->data[256], RPMB_HASH_LEN - 256); > + addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); Personally I prefer the ld/st API, and I'd rather see it used consistently within hw/sd/. So (matter of taste): block_index = ldw_be_p(&frame->address); block_offset = block_index * 256 + sd_part_offset(sd); > + do { > + if (blk_pread(sd->blk, addr, 256, buf, 0) < 0) { > + fprintf(stderr, "sd_blk_read: read error on host side\n"); Although a pre-existing pattern in this file, no new fprintf(stderr) please. Better use the Error* API, otherwise error_report(). > + success = false; > + break; > + } > + if (qcrypto_hmac_bytes(hmac, buf, RPMB_HASH_LEN, NULL, NULL,> + &err) < 0) { > + error_report_err(err); > + success = false; > + break; > + } > + addr += 256; > + } while (--num_blocks > 1); > + } > + > + if (success && > + qcrypto_hmac_bytes(hmac, (const char*)frame->data, RPMB_HASH_LEN, &mac, > + &mac_len, &err) < 0) { > + error_report_err(err); Ideally Error* should to be propagated to upper layers. Here SDCardClass::read_byte() and SDCardClass::write_byte() don't expect failure, so OK -- maybe they should ... --. > + success = false; > + } > + assert(!success || mac_len == RPMB_KEY_MAC_LEN); > + > + qcrypto_hmac_free(hmac); > + > + return success; > +} > + > static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) > { > uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp); > @@ -1142,6 +1204,17 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len) > sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_READ_FAILURE | > (result & RPMB_RESULT_COUTER_EXPIRED)); > } > + if (sd->multi_blk_cnt == 1 && > + !rpmb_calc_hmac(sd, &sd->rpmb_result, > + be16_to_cpu(sd->rpmb_result.block_count), > + sd->rpmb_result.key_mac)) { > + memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data)); > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE); > + } > + } else if (!rpmb_calc_hmac(sd, &sd->rpmb_result, 1, > + sd->rpmb_result.key_mac)) { > + memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data)); > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE); > } > memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result)); > > @@ -1153,6 +1226,7 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) > { > RPMBDataFrame *frame = (RPMBDataFrame *)sd->data; > uint16_t req = be16_to_cpu(frame->req_resp); > + uint8_t mac[RPMB_KEY_MAC_LEN]; > > if (req == RPMB_REQ_READ_RESULT) { > /* just return the current result register */ > @@ -1190,6 +1264,11 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len) > sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); > break; > } > + if (!rpmb_calc_hmac(sd, frame, 1, mac) || > + memcmp(frame->key_mac, mac, RPMB_KEY_MAC_LEN) != 0) { > + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE); > + break; > + } > if (be32_to_cpu(frame->write_counter) != sd->rpmb_write_counter) { > sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE); > break; > @@ -3115,6 +3194,8 @@ static void emmc_class_init(ObjectClass *klass, const void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > SDCardClass *sc = SDMMC_COMMON_CLASS(klass); > > + assert(qcrypto_hmac_supports(QCRYPTO_HASH_ALGO_SHA256)); > + > dc->desc = "eMMC"; > dc->realize = emmc_realize; > device_class_set_props(dc, emmc_properties); I'd like to squash on this patch: -- >8 -- diff --git a/hw/sd/sd.c b/hw/sd/sd.c index f2260796db8..6e4eeeda157 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1129,3 +1129,3 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) -static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame, +static bool rpmb_calc_hmac(SDState *sd, const RPMBDataFrame *frame, unsigned int num_blocks, uint8_t *mac) @@ -1133,5 +1133,5 @@ static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame, size_t mac_len = RPMB_KEY_MAC_LEN; + g_autoptr(QCryptoHmac) hmac = NULL; bool success = true; Error *err = NULL; - QCryptoHmac *hmac; uint64_t addr; @@ -1156,9 +1156,12 @@ static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame, */ - char *buf = (char *)sd->data; + void *buf = sd->data; - memcpy(buf, frame->data, RPMB_HASH_LEN); - addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); + assert(RPMB_HASH_LEN <= sizeof(sd->data)); + memcpy(&buf[RPMB_DATA_LEN], &frame->data[RPMB_DATA_LEN], + RPMB_HASH_LEN - RPMB_DATA_LEN); + + addr = be16_to_cpu(frame->address) * RPMB_DATA_LEN + sd_part_offset(sd); do { - if (blk_pread(sd->blk, addr, 256, buf, 0) < 0) { - fprintf(stderr, "sd_blk_read: read error on host side\n"); + if (blk_pread(sd->blk, addr, RPMB_DATA_LEN, buf, 0) < 0) { + error_report("sd_blk_read: read error on host side"); success = false; @@ -1172,3 +1175,3 @@ static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame, } - addr += 256; + addr += RPMB_DATA_LEN; } while (--num_blocks > 1); @@ -1177,3 +1180,3 @@ static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame, if (success && - qcrypto_hmac_bytes(hmac, (const char*)frame->data, RPMB_HASH_LEN, &mac, + qcrypto_hmac_bytes(hmac, frame->data, RPMB_HASH_LEN, &mac, &mac_len, &err) < 0) { @@ -1184,4 +1187,2 @@ static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame, - qcrypto_hmac_free(hmac); - return success; --- Regards, Phil. ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v5 4/6] hw/sd/sdcard: Handle RPMB MAC field 2025-11-03 15:18 ` Philippe Mathieu-Daudé @ 2025-11-04 6:43 ` Jan Kiszka 0 siblings, 0 replies; 30+ messages in thread From: Jan Kiszka @ 2025-11-04 6:43 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Daniel P. Berrangé On 03.11.25 16:18, Philippe Mathieu-Daudé wrote: > Hi Jan, > > On 17/10/25 14:03, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> Implement correct setting of the MAC field when passing RPMB frames back >> to the guest. Also check the MAC on authenticated write requests. >> >> This depends on HMAC support for QCRYPTO_HASH_ALGO_SHA256 which is >> always available via glib - assert this, just to be safe. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> hw/sd/sd.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 81 insertions(+) >> >> diff --git a/hw/sd/sd.c b/hw/sd/sd.c >> index 918fe9f79f..759e284ac0 100644 >> --- a/hw/sd/sd.c >> +++ b/hw/sd/sd.c >> @@ -51,6 +51,7 @@ >> #include "qemu/module.h" >> #include "sdmmc-internal.h" >> #include "trace.h" >> +#include "crypto/hmac.h" >> //#define DEBUG_SD 1 >> @@ -118,6 +119,7 @@ typedef struct SDProto { >> } SDProto; >> #define RPMB_KEY_MAC_LEN 32 >> +#define RPMB_HASH_LEN 284 >> typedef struct { >> uint8_t stuff_bytes[196]; >> @@ -1120,6 +1122,66 @@ static void sd_blk_write(SDState *sd, uint64_t >> addr, uint32_t len) >> } >> } >> +static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame, > > const frame. > >> + unsigned int num_blocks, uint8_t *mac) >> +{ >> + size_t mac_len = RPMB_KEY_MAC_LEN; >> + bool success = true; >> + Error *err = NULL; >> + QCryptoHmac *hmac; > > Preferably: > > g_autoptr(QCryptoHmac) hmac = NULL; > >> + uint64_t addr; > > Maybe better named 'offset'. > >> + >> + hmac = qcrypto_hmac_new(QCRYPTO_HASH_ALGO_SHA256, sd->rpmb_key, >> + RPMB_KEY_MAC_LEN, &err); >> + if (!hmac) { >> + error_report_err(err); >> + return false; >> + } >> + >> + /* >> + * This implies a read request because we only support single- >> block write >> + * requests so far. >> + */ >> + if (num_blocks > 1) { >> + /* >> + * Unfortunately, the underlying crypto libraries do not >> allow us to >> + * migrate an active QCryptoHmac state. Therefore, we have to >> calculate >> + * the HMAC in one run. To avoid buffering a complete read >> sequence in >> + * SDState, reconstruct all frames except for the last one. >> + */ >> + char *buf = (char *)sd->data; > Why not plain 'void *'? Ah, qcrypto_hmac_bytes() takes a 'char *', > odd... [Update, now about to be fixed: > https://lore.kernel.org/qemu-devel/20251103133727.423041-4- > berrange@redhat.com/] > > Better safe than sorry: > > assert(RPMB_HASH_LEN <= sizeof(sd->data)); > >> + memcpy(buf, frame->data, RPMB_HASH_LEN); > > Nitpicking, no need to fill the block we'll overwrite: > > memcpy(&buf[256], &frame->data[256], RPMB_HASH_LEN - 256); > >> + addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); > Personally I prefer the ld/st API, and I'd rather see it used > consistently within hw/sd/. So (matter of taste): > > block_index = ldw_be_p(&frame->address); > block_offset = block_index * 256 + sd_part_offset(sd); > >> + do { >> + if (blk_pread(sd->blk, addr, 256, buf, 0) < 0) { >> + fprintf(stderr, "sd_blk_read: read error on host >> side\n"); > > Although a pre-existing pattern in this file, no new fprintf(stderr) > please. Better use the Error* API, otherwise error_report(). > >> + success = false; >> + break; >> + } >> + if (qcrypto_hmac_bytes(hmac, buf, RPMB_HASH_LEN, NULL, >> NULL,> + &err) < 0) { >> + error_report_err(err); >> + success = false; >> + break; >> + } >> + addr += 256; >> + } while (--num_blocks > 1); >> + } >> + >> + if (success && >> + qcrypto_hmac_bytes(hmac, (const char*)frame->data, >> RPMB_HASH_LEN, &mac, >> + &mac_len, &err) < 0) { >> + error_report_err(err); > > Ideally Error* should to be propagated to upper layers. > > Here SDCardClass::read_byte() and SDCardClass::write_byte() > don't expect failure, so OK -- maybe they should ... --. > So, leave it like it is for now? >> + success = false; >> + } >> + assert(!success || mac_len == RPMB_KEY_MAC_LEN); >> + >> + qcrypto_hmac_free(hmac); >> + >> + return success; >> +} >> + >> static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t >> len) >> { >> uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp); >> @@ -1142,6 +1204,17 @@ static void emmc_rpmb_blk_read(SDState *sd, >> uint64_t addr, uint32_t len) >> sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_READ_FAILURE | >> (result & RPMB_RESULT_COUTER_EXPIRED)); >> } >> + if (sd->multi_blk_cnt == 1 && >> + !rpmb_calc_hmac(sd, &sd->rpmb_result, >> + be16_to_cpu(sd->rpmb_result.block_count), >> + sd->rpmb_result.key_mac)) { >> + memset(sd->rpmb_result.data, 0, sizeof(sd- >> >rpmb_result.data)); >> + sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_AUTH_FAILURE); >> + } >> + } else if (!rpmb_calc_hmac(sd, &sd->rpmb_result, 1, >> + sd->rpmb_result.key_mac)) { >> + memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data)); >> + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE); >> } >> memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result)); >> @@ -1153,6 +1226,7 @@ static void emmc_rpmb_blk_write(SDState *sd, >> uint64_t addr, uint32_t len) >> { >> RPMBDataFrame *frame = (RPMBDataFrame *)sd->data; >> uint16_t req = be16_to_cpu(frame->req_resp); >> + uint8_t mac[RPMB_KEY_MAC_LEN]; >> if (req == RPMB_REQ_READ_RESULT) { >> /* just return the current result register */ >> @@ -1190,6 +1264,11 @@ static void emmc_rpmb_blk_write(SDState *sd, >> uint64_t addr, uint32_t len) >> sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE); >> break; >> } >> + if (!rpmb_calc_hmac(sd, frame, 1, mac) || >> + memcmp(frame->key_mac, mac, RPMB_KEY_MAC_LEN) != 0) { >> + sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_AUTH_FAILURE); >> + break; >> + } >> if (be32_to_cpu(frame->write_counter) != sd- >> >rpmb_write_counter) { >> sd->rpmb_result.result = >> cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE); >> break; >> @@ -3115,6 +3194,8 @@ static void emmc_class_init(ObjectClass *klass, >> const void *data) >> DeviceClass *dc = DEVICE_CLASS(klass); >> SDCardClass *sc = SDMMC_COMMON_CLASS(klass); >> + assert(qcrypto_hmac_supports(QCRYPTO_HASH_ALGO_SHA256)); >> + >> dc->desc = "eMMC"; >> dc->realize = emmc_realize; >> device_class_set_props(dc, emmc_properties); > I'd like to squash on this patch: > -- >8 -- > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index f2260796db8..6e4eeeda157 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -1129,3 +1129,3 @@ static void sd_blk_write(SDState *sd, uint64_t > addr, uint32_t len) > > -static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame, > +static bool rpmb_calc_hmac(SDState *sd, const RPMBDataFrame *frame, > unsigned int num_blocks, uint8_t *mac) > @@ -1133,5 +1133,5 @@ static bool rpmb_calc_hmac(SDState *sd, > RPMBDataFrame *frame, > size_t mac_len = RPMB_KEY_MAC_LEN; > + g_autoptr(QCryptoHmac) hmac = NULL; > bool success = true; > Error *err = NULL; > - QCryptoHmac *hmac; > uint64_t addr; > @@ -1156,9 +1156,12 @@ static bool rpmb_calc_hmac(SDState *sd, > RPMBDataFrame *frame, > */ > - char *buf = (char *)sd->data; > + void *buf = sd->data; > > - memcpy(buf, frame->data, RPMB_HASH_LEN); > - addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd); > + assert(RPMB_HASH_LEN <= sizeof(sd->data)); > + memcpy(&buf[RPMB_DATA_LEN], &frame->data[RPMB_DATA_LEN], > + RPMB_HASH_LEN - RPMB_DATA_LEN); > + > + addr = be16_to_cpu(frame->address) * RPMB_DATA_LEN + > sd_part_offset(sd); > do { > - if (blk_pread(sd->blk, addr, 256, buf, 0) < 0) { > - fprintf(stderr, "sd_blk_read: read error on host side\n"); > + if (blk_pread(sd->blk, addr, RPMB_DATA_LEN, buf, 0) < 0) { > + error_report("sd_blk_read: read error on host side"); > success = false; > @@ -1172,3 +1175,3 @@ static bool rpmb_calc_hmac(SDState *sd, > RPMBDataFrame *frame, > } > - addr += 256; > + addr += RPMB_DATA_LEN; > } while (--num_blocks > 1); > @@ -1177,3 +1180,3 @@ static bool rpmb_calc_hmac(SDState *sd, > RPMBDataFrame *frame, > if (success && > - qcrypto_hmac_bytes(hmac, (const char*)frame->data, > RPMB_HASH_LEN, &mac, > + qcrypto_hmac_bytes(hmac, frame->data, RPMB_HASH_LEN, &mac, > &mac_len, &err) < 0) { > @@ -1184,4 +1187,2 @@ static bool rpmb_calc_hmac(SDState *sd, > RPMBDataFrame *frame, > > - qcrypto_hmac_free(hmac); > - > return success; > --- > > Regards, > > Phil. Same story: Will pick it up and test. Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v5 5/6] scripts: Add helper script to generate eMMC block device images 2025-10-17 12:03 [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Jan Kiszka ` (3 preceding siblings ...) 2025-10-17 12:03 ` [PATCH v5 4/6] hw/sd/sdcard: Handle RPMB MAC field Jan Kiszka @ 2025-10-17 12:03 ` Jan Kiszka 2025-11-03 13:12 ` Philippe Mathieu-Daudé 2025-10-17 12:03 ` [PATCH v5 6/6] docs: Add eMMC device model description Jan Kiszka ` (2 subsequent siblings) 7 siblings, 1 reply; 30+ messages in thread From: Jan Kiszka @ 2025-10-17 12:03 UTC (permalink / raw) To: qemu-devel Cc: Philippe Mathieu-Daudé, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier From: Jan Kiszka <jan.kiszka@siemens.com> As an eMMC block device image may consist of more than just the user data partition, provide a helper script that can compose the image from boot partitions, an RPMB partition and the user data image. The script also does the required size validation and/or rounding. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- scripts/mkemmc.sh | 218 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100755 scripts/mkemmc.sh diff --git a/scripts/mkemmc.sh b/scripts/mkemmc.sh new file mode 100755 index 0000000000..1a2b7a6193 --- /dev/null +++ b/scripts/mkemmc.sh @@ -0,0 +1,218 @@ +#!/bin/sh -e +# SPDX-License-Identifier: GPL-2.0-only +# +# Create eMMC block device image from boot, RPMB and user data images +# +# Copyright (c) Siemens, 2025 +# +# Authors: +# Jan Kiszka <jan.kiszka@siemens.com> +# +# This work is licensed under the terms of the GNU GPL version 2. +# See the COPYING file in the top-level directory. +# + +usage() { + echo "$0 [OPTIONS] USER_IMG[:SIZE] OUTPUT_IMG" + echo "" + echo "SIZE must be a power of 2 up to 2G and multiples of 512 byte from there on." + echo "If no SIZE is specified, the size of USER_ING will be used (rounded up)." + echo "" + echo "Supported options:" + echo " -b BOOT1_IMG[:SIZE] Add boot partitions. SIZE must be multiples of 128K. If" + echo " no SIZE is specified, the size of BOOT1_IMG will be" + echo " used (rounded up). BOOT1_IMG will be stored in boot" + echo " partition 1, and a boot partition 2 of the same size" + echo " will be created as empty (all zeros) unless -B is" + echo " specified as well." + echo " -B BOOT2_IMG Fill boot partition 2 with BOOT2_IMG. Must be combined" + echo " with -b which is also defining the partition size." + echo " -r RPMB_IMG[:SIZE] Add RPMB partition. SIZE must be multiples of 128K. If" + echo " no SIZE is specified, the size of RPMB_IMG will be" + echo " used (rounded up)." + echo " -h, --help This help" + echo "" + echo "All SIZE parameters support the units K, M, G. If SIZE is smaller than the" + echo "associated image, it will be truncated in the output image." + exit "$1" +} + +process_size() { + name=$1 + image_file=$2 + alignment=$3 + image_arg=$4 + if [ "${image_arg#*:}" = "$image_arg" ]; then + if ! size=$(stat -L -c %s "$image_file" 2>/dev/null); then + echo "Missing $name image '$image_file'." >&2 + exit 1 + fi + if [ "$alignment" = 128 ]; then + size=$(( (size + 128 * 1024 - 1) & ~(128 * 1024 - 1) )) + elif [ $size -gt $((2 * 1024 * 1024 * 1024)) ]; then + size=$(( (size + 511) & ~511 )) + elif [ $(( size & (size - 1) )) -gt 0 ]; then + n=0 + while [ "$size" -gt 0 ]; do + size=$((size >> 1)) + n=$((n + 1)) + done + size=$((1 << n)) + fi + else + value="${image_arg#*:}" + if [ "${value%K}" != "$value" ]; then + size=${value%K} + multiplier=1024 + elif [ "${value%M}" != "$value" ]; then + size=${value%M} + multiplier=$((1024 * 1024)) + elif [ "${value%G}" != "$value" ]; then + size=${value%G} + multiplier=$((1024 * 1024 * 1024)) + else + size=$value + multiplier=1 + fi + if [ "$size" -eq "$size" ] 2>/dev/null; then + size=$((size * multiplier)) + else + echo "Invalid value '$value' specified for $image_file image size." >&2 + exit 1 + fi + if [ "$alignment" = 128 ]; then + if [ $(( size & (128 * 1024 - 1) )) -ne 0 ]; then + echo "The $name image size must be multiples of 128K." >&2 + exit 1 + fi + elif [ $size -gt $((2 * 1024 * 1024 * 1024)) ]; then + if [ $(( size & 511)) -ne 0 ]; then + echo "The $name image size must be multiples of 512 (if >2G)." >&2 + exit 1 + fi + elif [ $(( size & (size - 1) )) -gt 0 ]; then + echo "The $name image size must be power of 2 (up to 2G)." >&2 + exit 1 + fi + fi + echo $size +} + +check_truncation() { + image_file=$1 + output_size=$2 + if [ "$image_file" = "/dev/zero" ]; then + return + fi + if ! actual_size=$(stat -L -c %s "$image_file" 2>/dev/null); then + echo "Missing image '$image_file'." >&2 + exit 1 + fi + if [ "$actual_size" -gt "$output_size" ]; then + echo "Warning: image '$image_file' will be truncated on output." + fi +} + +userimg= +outimg= +bootimg1= +bootimg2=/dev/zero +bootsz=0 +rpmbimg= +rpmbsz=0 + +while [ $# -gt 0 ]; do + case "$1" in + -b) + shift + [ $# -ge 1 ] || usage 1 + bootimg1=${1%%:*} + bootsz=$(process_size boot "$bootimg1" 128 "$1") + shift + ;; + -B) + shift + [ $# -ge 1 ] || usage 1 + bootimg2=$1 + shift + ;; + -r) + shift + [ $# -ge 1 ] || usage 1 + rpmbimg=${1%%:*} + rpmbsz=$(process_size RPMB "$rpmbimg" 128 "$1") + shift + ;; + -h|--help) + usage 0 + ;; + *) + if [ -z "$userimg" ]; then + userimg=${1%%:*} + usersz=$(process_size user "$userimg" U "$1") + elif [ -z "$outimg" ]; then + outimg=$1 + else + usage 1 + fi + shift + ;; + esac +done + +[ -n "$outimg" ] || usage 1 + +if [ "$bootsz" -gt $((32640 * 1024)) ]; then + echo "Boot image size is larger than 32640K." >&2 + exit 1 +fi +if [ "$rpmbsz" -gt $((16384 * 1024)) ]; then + echo "RPMB image size is larger than 16384K." >&2 + exit 1 +fi + +echo "Creating eMMC image" + +truncate "$outimg" -s 0 +pos=0 + +if [ "$bootsz" -gt 0 ]; then + echo " Boot partition 1 and 2: $((bootsz / 1024))K each" + blocks=$(( bootsz / (128 * 1024) )) + check_truncation "$bootimg1" "$bootsz" + dd if="$bootimg1" of="$outimg" conv=sparse bs=128K count=$blocks \ + status=none + check_truncation "$bootimg2" "$bootsz" + dd if="$bootimg2" of="$outimg" conv=sparse bs=128K count=$blocks \ + seek=$blocks status=none + pos=$((2 * bootsz)) +fi + +if [ "$rpmbsz" -gt 0 ]; then + echo " RPMB partition: $((rpmbsz / 1024))K" + blocks=$(( rpmbsz / (128 * 1024) )) + check_truncation "$rpmbimg" "$rpmbsz" + dd if="$rpmbimg" of="$outimg" conv=sparse bs=128K count=$blocks \ + seek=$(( pos / (128 * 1024) )) status=none + pos=$((pos + rpmbsz)) +fi + +if [ "$usersz" -lt 1024 ]; then + echo " User data: $usersz bytes" +elif [ "$usersz" -lt $((1024 * 1024)) ]; then + echo " User data: $(( (usersz + 1023) / 1024 ))K ($usersz)" +elif [ "$usersz" -lt $((1024 * 1024 * 1024)) ]; then + echo " User data: $(( (usersz + 1048575) / 1048576))M ($usersz)" +else + echo " User data: $(( (usersz + 1073741823) / 1073741824))G ($usersz)" +fi +check_truncation "$userimg" "$usersz" +dd if="$userimg" of="$outimg" conv=sparse bs=128K seek=$(( pos / (128 * 1024) )) \ + count=$(( (usersz + 128 * 1024 - 1) / (128 * 1024) )) status=none +pos=$((pos + usersz)) +truncate "$outimg" -s $pos + +echo "" +echo "Instantiate by appending to the qemu command line:" +echo " -drive file=$outimg,if=none,format=raw,id=emmc-img" +echo " -device emmc,boot-partition-size=$bootsz,rpmb-partition-size=$rpmbsz,drive=emmc-img" -- 2.51.0 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/6] scripts: Add helper script to generate eMMC block device images 2025-10-17 12:03 ` [PATCH v5 5/6] scripts: Add helper script to generate eMMC block device images Jan Kiszka @ 2025-11-03 13:12 ` Philippe Mathieu-Daudé 2025-11-04 6:30 ` Jan Kiszka 0 siblings, 1 reply; 30+ messages in thread From: Philippe Mathieu-Daudé @ 2025-11-03 13:12 UTC (permalink / raw) To: Jan Kiszka, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Eric Blake Hi Jan, On 17/10/25 14:03, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > As an eMMC block device image may consist of more than just the user > data partition, provide a helper script that can compose the image from > boot partitions, an RPMB partition and the user data image. The script > also does the required size validation and/or rounding. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > scripts/mkemmc.sh | 218 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 218 insertions(+) > create mode 100755 scripts/mkemmc.sh > > diff --git a/scripts/mkemmc.sh b/scripts/mkemmc.sh > new file mode 100755 > index 0000000000..1a2b7a6193 > --- /dev/null > +++ b/scripts/mkemmc.sh > @@ -0,0 +1,218 @@ > +#!/bin/sh -e > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# Create eMMC block device image from boot, RPMB and user data images > +# > +# Copyright (c) Siemens, 2025 > +# > +# Authors: > +# Jan Kiszka <jan.kiszka@siemens.com> > +# > +# This work is licensed under the terms of the GNU GPL version 2. > +# See the COPYING file in the top-level directory. > +# > + > +usage() { > + echo "$0 [OPTIONS] USER_IMG[:SIZE] OUTPUT_IMG" > + echo "" > + echo "SIZE must be a power of 2 up to 2G and multiples of 512 byte from there on." > + echo "If no SIZE is specified, the size of USER_ING will be used (rounded up)." > + echo "" > + echo "Supported options:" > + echo " -b BOOT1_IMG[:SIZE] Add boot partitions. SIZE must be multiples of 128K. If" > + echo " no SIZE is specified, the size of BOOT1_IMG will be" > + echo " used (rounded up). BOOT1_IMG will be stored in boot" > + echo " partition 1, and a boot partition 2 of the same size" > + echo " will be created as empty (all zeros) unless -B is" > + echo " specified as well." > + echo " -B BOOT2_IMG Fill boot partition 2 with BOOT2_IMG. Must be combined" > + echo " with -b which is also defining the partition size." > + echo " -r RPMB_IMG[:SIZE] Add RPMB partition. SIZE must be multiples of 128K. If" > + echo " no SIZE is specified, the size of RPMB_IMG will be" > + echo " used (rounded up)." > + echo " -h, --help This help" > + echo "" > + echo "All SIZE parameters support the units K, M, G. If SIZE is smaller than the" > + echo "associated image, it will be truncated in the output image." > + exit "$1" > +} > + > +process_size() { > + name=$1 > + image_file=$2 > + alignment=$3 > + image_arg=$4 > + if [ "${image_arg#*:}" = "$image_arg" ]; then > + if ! size=$(stat -L -c %s "$image_file" 2>/dev/null); then > + echo "Missing $name image '$image_file'." >&2 > + exit 1 > + fi > + if [ "$alignment" = 128 ]; then > + size=$(( (size + 128 * 1024 - 1) & ~(128 * 1024 - 1) )) > + elif [ $size -gt $((2 * 1024 * 1024 * 1024)) ]; then > + size=$(( (size + 511) & ~511 )) > + elif [ $(( size & (size - 1) )) -gt 0 ]; then > + n=0 > + while [ "$size" -gt 0 ]; do > + size=$((size >> 1)) > + n=$((n + 1)) > + done > + size=$((1 << n)) > + fi > + else > + value="${image_arg#*:}" > + if [ "${value%K}" != "$value" ]; then > + size=${value%K} > + multiplier=1024 > + elif [ "${value%M}" != "$value" ]; then > + size=${value%M} > + multiplier=$((1024 * 1024)) > + elif [ "${value%G}" != "$value" ]; then > + size=${value%G} > + multiplier=$((1024 * 1024 * 1024)) > + else > + size=$value > + multiplier=1 > + fi > + if [ "$size" -eq "$size" ] 2>/dev/null; then I don't get this check, should one be "$value"? > + size=$((size * multiplier)) > + else > + echo "Invalid value '$value' specified for $image_file image size." >&2 > + exit 1 > + fi > + if [ "$alignment" = 128 ]; then > + if [ $(( size & (128 * 1024 - 1) )) -ne 0 ]; then > + echo "The $name image size must be multiples of 128K." >&2 > + exit 1 > + fi > + elif [ $size -gt $((2 * 1024 * 1024 * 1024)) ]; then > + if [ $(( size & 511)) -ne 0 ]; then > + echo "The $name image size must be multiples of 512 (if >2G)." >&2 > + exit 1 > + fi > + elif [ $(( size & (size - 1) )) -gt 0 ]; then > + echo "The $name image size must be power of 2 (up to 2G)." >&2 > + exit 1 > + fi > + fi > + echo $size > +} > + > +check_truncation() { > + image_file=$1 > + output_size=$2 > + if [ "$image_file" = "/dev/zero" ]; then > + return > + fi > + if ! actual_size=$(stat -L -c %s "$image_file" 2>/dev/null); then > + echo "Missing image '$image_file'." >&2 > + exit 1 > + fi > + if [ "$actual_size" -gt "$output_size" ]; then > + echo "Warning: image '$image_file' will be truncated on output." > + fi > +} > + > +userimg= > +outimg= > +bootimg1= > +bootimg2=/dev/zero > +bootsz=0 > +rpmbimg= > +rpmbsz=0 > + > +while [ $# -gt 0 ]; do > + case "$1" in > + -b) > + shift > + [ $# -ge 1 ] || usage 1 > + bootimg1=${1%%:*} > + bootsz=$(process_size boot "$bootimg1" 128 "$1") > + shift > + ;; > + -B) > + shift > + [ $# -ge 1 ] || usage 1 > + bootimg2=$1 > + shift > + ;; > + -r) > + shift > + [ $# -ge 1 ] || usage 1 > + rpmbimg=${1%%:*} > + rpmbsz=$(process_size RPMB "$rpmbimg" 128 "$1") > + shift > + ;; > + -h|--help) > + usage 0 > + ;; > + *) > + if [ -z "$userimg" ]; then > + userimg=${1%%:*} > + usersz=$(process_size user "$userimg" U "$1") > + elif [ -z "$outimg" ]; then > + outimg=$1 > + else > + usage 1 > + fi > + shift > + ;; > + esac > +done > + > +[ -n "$outimg" ] || usage 1 > + > +if [ "$bootsz" -gt $((32640 * 1024)) ]; then Running on macOS: scripts/mkemmc.sh: line 165: [: : integer expression expected scripts/mkemmc.sh: line 169: [: : integer expression expected scripts/mkemmc.sh: line 179: [: : integer expression expected scripts/mkemmc.sh: line 191: [: : integer expression expected scripts/mkemmc.sh: line 200: [: : integer expression expected scripts/mkemmc.sh: line 202: [: : integer expression expected scripts/mkemmc.sh: line 204: [: : integer expression expected $ sh --version GNU bash, version 3.2.57(1)-release (arm64-apple-darwin24) When using dash: scripts/mkemmc.sh: 165: [: Illegal number: scripts/mkemmc.sh: 169: [: Illegal number: scripts/mkemmc.sh: 179: [: Illegal number: scripts/mkemmc.sh: 191: [: Illegal number: scripts/mkemmc.sh: 200: [: Illegal number: scripts/mkemmc.sh: 202: [: Illegal number: scripts/mkemmc.sh: 204: [: Illegal number: Should we replace s/[/[[/? > + echo "Boot image size is larger than 32640K." >&2 > + exit 1 > +fi > +if [ "$rpmbsz" -gt $((16384 * 1024)) ]; then > + echo "RPMB image size is larger than 16384K." >&2 > + exit 1 > +fi > + > +echo "Creating eMMC image" > + > +truncate "$outimg" -s 0 I'd replace here by: truncate -s 0 "$outimg" to avoid on macOS: usage: truncate [-c] -s [+|-|%|/]size[K|k|M|m|G|g|T|t] file ... truncate [-c] -r rfile file ... > +pos=0 > + > +if [ "$bootsz" -gt 0 ]; then > + echo " Boot partition 1 and 2: $((bootsz / 1024))K each" > + blocks=$(( bootsz / (128 * 1024) )) > + check_truncation "$bootimg1" "$bootsz" > + dd if="$bootimg1" of="$outimg" conv=sparse bs=128K count=$blocks \ > + status=none > + check_truncation "$bootimg2" "$bootsz" > + dd if="$bootimg2" of="$outimg" conv=sparse bs=128K count=$blocks \ > + seek=$blocks status=none > + pos=$((2 * bootsz)) > +fi > + > +if [ "$rpmbsz" -gt 0 ]; then > + echo " RPMB partition: $((rpmbsz / 1024))K" > + blocks=$(( rpmbsz / (128 * 1024) )) > + check_truncation "$rpmbimg" "$rpmbsz" > + dd if="$rpmbimg" of="$outimg" conv=sparse bs=128K count=$blocks \ > + seek=$(( pos / (128 * 1024) )) status=none > + pos=$((pos + rpmbsz)) > +fi > + > +if [ "$usersz" -lt 1024 ]; then > + echo " User data: $usersz bytes" > +elif [ "$usersz" -lt $((1024 * 1024)) ]; then > + echo " User data: $(( (usersz + 1023) / 1024 ))K ($usersz)" > +elif [ "$usersz" -lt $((1024 * 1024 * 1024)) ]; then > + echo " User data: $(( (usersz + 1048575) / 1048576))M ($usersz)" > +else > + echo " User data: $(( (usersz + 1073741823) / 1073741824))G ($usersz)" > +fi > +check_truncation "$userimg" "$usersz" > +dd if="$userimg" of="$outimg" conv=sparse bs=128K seek=$(( pos / (128 * 1024) )) \ > + count=$(( (usersz + 128 * 1024 - 1) / (128 * 1024) )) status=none > +pos=$((pos + usersz)) > +truncate "$outimg" -s $pos truncate -s $pos "$outimg" > + > +echo "" > +echo "Instantiate by appending to the qemu command line:" > +echo " -drive file=$outimg,if=none,format=raw,id=emmc-img" > +echo " -device emmc,boot-partition-size=$bootsz,rpmb-partition-size=$rpmbsz,drive=emmc-img" ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/6] scripts: Add helper script to generate eMMC block device images 2025-11-03 13:12 ` Philippe Mathieu-Daudé @ 2025-11-04 6:30 ` Jan Kiszka 2025-11-04 9:26 ` Jan Kiszka 0 siblings, 1 reply; 30+ messages in thread From: Jan Kiszka @ 2025-11-04 6:30 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Eric Blake On 03.11.25 14:12, Philippe Mathieu-Daudé wrote: > Hi Jan, > > On 17/10/25 14:03, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> As an eMMC block device image may consist of more than just the user >> data partition, provide a helper script that can compose the image from >> boot partitions, an RPMB partition and the user data image. The script >> also does the required size validation and/or rounding. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> scripts/mkemmc.sh | 218 ++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 218 insertions(+) >> create mode 100755 scripts/mkemmc.sh >> >> diff --git a/scripts/mkemmc.sh b/scripts/mkemmc.sh >> new file mode 100755 >> index 0000000000..1a2b7a6193 >> --- /dev/null >> +++ b/scripts/mkemmc.sh >> @@ -0,0 +1,218 @@ >> +#!/bin/sh -e >> +# SPDX-License-Identifier: GPL-2.0-only >> +# >> +# Create eMMC block device image from boot, RPMB and user data images >> +# >> +# Copyright (c) Siemens, 2025 >> +# >> +# Authors: >> +# Jan Kiszka <jan.kiszka@siemens.com> >> +# >> +# This work is licensed under the terms of the GNU GPL version 2. >> +# See the COPYING file in the top-level directory. >> +# >> + >> +usage() { >> + echo "$0 [OPTIONS] USER_IMG[:SIZE] OUTPUT_IMG" >> + echo "" >> + echo "SIZE must be a power of 2 up to 2G and multiples of 512 >> byte from there on." >> + echo "If no SIZE is specified, the size of USER_ING will be used >> (rounded up)." >> + echo "" >> + echo "Supported options:" >> + echo " -b BOOT1_IMG[:SIZE] Add boot partitions. SIZE must be >> multiples of 128K. If" >> + echo " no SIZE is specified, the size of >> BOOT1_IMG will be" >> + echo " used (rounded up). BOOT1_IMG will >> be stored in boot" >> + echo " partition 1, and a boot partition >> 2 of the same size" >> + echo " will be created as empty (all >> zeros) unless -B is" >> + echo " specified as well." >> + echo " -B BOOT2_IMG Fill boot partition 2 with >> BOOT2_IMG. Must be combined" >> + echo " with -b which is also defining >> the partition size." >> + echo " -r RPMB_IMG[:SIZE] Add RPMB partition. SIZE must be >> multiples of 128K. If" >> + echo " no SIZE is specified, the size of >> RPMB_IMG will be" >> + echo " used (rounded up)." >> + echo " -h, --help This help" >> + echo "" >> + echo "All SIZE parameters support the units K, M, G. If SIZE is >> smaller than the" >> + echo "associated image, it will be truncated in the output image." >> + exit "$1" >> +} >> + >> +process_size() { >> + name=$1 >> + image_file=$2 >> + alignment=$3 >> + image_arg=$4 >> + if [ "${image_arg#*:}" = "$image_arg" ]; then >> + if ! size=$(stat -L -c %s "$image_file" 2>/dev/null); then >> + echo "Missing $name image '$image_file'." >&2 >> + exit 1 >> + fi >> + if [ "$alignment" = 128 ]; then >> + size=$(( (size + 128 * 1024 - 1) & ~(128 * 1024 - 1) )) >> + elif [ $size -gt $((2 * 1024 * 1024 * 1024)) ]; then >> + size=$(( (size + 511) & ~511 )) >> + elif [ $(( size & (size - 1) )) -gt 0 ]; then >> + n=0 >> + while [ "$size" -gt 0 ]; do >> + size=$((size >> 1)) >> + n=$((n + 1)) >> + done >> + size=$((1 << n)) >> + fi >> + else >> + value="${image_arg#*:}" >> + if [ "${value%K}" != "$value" ]; then >> + size=${value%K} >> + multiplier=1024 >> + elif [ "${value%M}" != "$value" ]; then >> + size=${value%M} >> + multiplier=$((1024 * 1024)) >> + elif [ "${value%G}" != "$value" ]; then >> + size=${value%G} >> + multiplier=$((1024 * 1024 * 1024)) >> + else >> + size=$value >> + multiplier=1 >> + fi >> + if [ "$size" -eq "$size" ] 2>/dev/null; then > > I don't get this check, should one be "$value"? > Likely deserves a comment, I had to refresh my own memory as well: This checks if $size is a valid integer value. If we just run the multiplication below, we won't be able to react properly. >> + size=$((size * multiplier)) >> + else >> + echo "Invalid value '$value' specified for $image_file >> image size." >&2 >> + exit 1 >> + fi >> + if [ "$alignment" = 128 ]; then >> + if [ $(( size & (128 * 1024 - 1) )) -ne 0 ]; then >> + echo "The $name image size must be multiples of >> 128K." >&2 >> + exit 1 >> + fi >> + elif [ $size -gt $((2 * 1024 * 1024 * 1024)) ]; then >> + if [ $(( size & 511)) -ne 0 ]; then >> + echo "The $name image size must be multiples of 512 >> (if >2G)." >&2 >> + exit 1 >> + fi >> + elif [ $(( size & (size - 1) )) -gt 0 ]; then >> + echo "The $name image size must be power of 2 (up to >> 2G)." >&2 >> + exit 1 >> + fi >> + fi >> + echo $size >> +} >> + >> +check_truncation() { >> + image_file=$1 >> + output_size=$2 >> + if [ "$image_file" = "/dev/zero" ]; then >> + return >> + fi >> + if ! actual_size=$(stat -L -c %s "$image_file" 2>/dev/null); then >> + echo "Missing image '$image_file'." >&2 >> + exit 1 >> + fi >> + if [ "$actual_size" -gt "$output_size" ]; then >> + echo "Warning: image '$image_file' will be truncated on output." >> + fi >> +} >> + >> +userimg= >> +outimg= >> +bootimg1= >> +bootimg2=/dev/zero >> +bootsz=0 >> +rpmbimg= >> +rpmbsz=0 >> + >> +while [ $# -gt 0 ]; do >> + case "$1" in >> + -b) >> + shift >> + [ $# -ge 1 ] || usage 1 >> + bootimg1=${1%%:*} >> + bootsz=$(process_size boot "$bootimg1" 128 "$1") >> + shift >> + ;; >> + -B) >> + shift >> + [ $# -ge 1 ] || usage 1 >> + bootimg2=$1 >> + shift >> + ;; >> + -r) >> + shift >> + [ $# -ge 1 ] || usage 1 >> + rpmbimg=${1%%:*} >> + rpmbsz=$(process_size RPMB "$rpmbimg" 128 "$1") >> + shift >> + ;; >> + -h|--help) >> + usage 0 >> + ;; >> + *) >> + if [ -z "$userimg" ]; then >> + userimg=${1%%:*} >> + usersz=$(process_size user "$userimg" U "$1") >> + elif [ -z "$outimg" ]; then >> + outimg=$1 >> + else >> + usage 1 >> + fi >> + shift >> + ;; >> + esac >> +done >> + >> +[ -n "$outimg" ] || usage 1 >> + >> +if [ "$bootsz" -gt $((32640 * 1024)) ]; then > > Running on macOS: > > scripts/mkemmc.sh: line 165: [: : integer expression expected > scripts/mkemmc.sh: line 169: [: : integer expression expected > scripts/mkemmc.sh: line 179: [: : integer expression expected > scripts/mkemmc.sh: line 191: [: : integer expression expected > scripts/mkemmc.sh: line 200: [: : integer expression expected > scripts/mkemmc.sh: line 202: [: : integer expression expected > scripts/mkemmc.sh: line 204: [: : integer expression expected > > $ sh --version > GNU bash, version 3.2.57(1)-release (arm64-apple-darwin24) > > When using dash: > > scripts/mkemmc.sh: 165: [: Illegal number: > scripts/mkemmc.sh: 169: [: Illegal number: > scripts/mkemmc.sh: 179: [: Illegal number: > scripts/mkemmc.sh: 191: [: Illegal number: > scripts/mkemmc.sh: 200: [: Illegal number: > scripts/mkemmc.sh: 202: [: Illegal number: > scripts/mkemmc.sh: 204: [: Illegal number: > > Should we replace s/[/[[/? No, that would be invalid outside of bash. There must be a logical error. How did you invoke the script? What was the value of bootsz then? > >> + echo "Boot image size is larger than 32640K." >&2 >> + exit 1 >> +fi >> +if [ "$rpmbsz" -gt $((16384 * 1024)) ]; then >> + echo "RPMB image size is larger than 16384K." >&2 >> + exit 1 >> +fi >> + >> +echo "Creating eMMC image" >> + >> +truncate "$outimg" -s 0 > I'd replace here by: > truncate -s 0 "$outimg" > > to avoid on macOS: > > usage: truncate [-c] -s [+|-|%|/]size[K|k|M|m|G|g|T|t] file ... > truncate [-c] -r rfile file ... > Will do. >> +pos=0 >> + >> +if [ "$bootsz" -gt 0 ]; then >> + echo " Boot partition 1 and 2: $((bootsz / 1024))K each" >> + blocks=$(( bootsz / (128 * 1024) )) >> + check_truncation "$bootimg1" "$bootsz" >> + dd if="$bootimg1" of="$outimg" conv=sparse bs=128K count=$blocks \ >> + status=none >> + check_truncation "$bootimg2" "$bootsz" >> + dd if="$bootimg2" of="$outimg" conv=sparse bs=128K count=$blocks \ >> + seek=$blocks status=none >> + pos=$((2 * bootsz)) >> +fi >> + >> +if [ "$rpmbsz" -gt 0 ]; then >> + echo " RPMB partition: $((rpmbsz / 1024))K" >> + blocks=$(( rpmbsz / (128 * 1024) )) >> + check_truncation "$rpmbimg" "$rpmbsz" >> + dd if="$rpmbimg" of="$outimg" conv=sparse bs=128K count=$blocks \ >> + seek=$(( pos / (128 * 1024) )) status=none >> + pos=$((pos + rpmbsz)) >> +fi >> + >> +if [ "$usersz" -lt 1024 ]; then >> + echo " User data: $usersz bytes" >> +elif [ "$usersz" -lt $((1024 * 1024)) ]; then >> + echo " User data: $(( (usersz + 1023) / 1024 ))K >> ($usersz)" >> +elif [ "$usersz" -lt $((1024 * 1024 * 1024)) ]; then >> + echo " User data: $(( (usersz + 1048575) / >> 1048576))M ($usersz)" >> +else >> + echo " User data: $(( (usersz + 1073741823) / >> 1073741824))G ($usersz)" >> +fi >> +check_truncation "$userimg" "$usersz" >> +dd if="$userimg" of="$outimg" conv=sparse bs=128K seek=$(( pos / (128 >> * 1024) )) \ >> + count=$(( (usersz + 128 * 1024 - 1) / (128 * 1024) )) status=none >> +pos=$((pos + usersz)) >> +truncate "$outimg" -s $pos > > truncate -s $pos "$outimg" > >> + >> +echo "" >> +echo "Instantiate by appending to the qemu command line:" >> +echo " -drive file=$outimg,if=none,format=raw,id=emmc-img" >> +echo " -device emmc,boot-partition-size=$bootsz,rpmb-partition- >> size=$rpmbsz,drive=emmc-img" > Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/6] scripts: Add helper script to generate eMMC block device images 2025-11-04 6:30 ` Jan Kiszka @ 2025-11-04 9:26 ` Jan Kiszka 2025-11-04 12:33 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 30+ messages in thread From: Jan Kiszka @ 2025-11-04 9:26 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Eric Blake On 04.11.25 07:30, Jan Kiszka wrote: > On 03.11.25 14:12, Philippe Mathieu-Daudé wrote: >> Hi Jan, >> >> On 17/10/25 14:03, Jan Kiszka wrote: >>> From: Jan Kiszka <jan.kiszka@siemens.com> >>> >>> As an eMMC block device image may consist of more than just the user >>> data partition, provide a helper script that can compose the image from >>> boot partitions, an RPMB partition and the user data image. The script >>> also does the required size validation and/or rounding. >>> >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>> --- >>> scripts/mkemmc.sh | 218 ++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 218 insertions(+) >>> create mode 100755 scripts/mkemmc.sh >>> >>> diff --git a/scripts/mkemmc.sh b/scripts/mkemmc.sh >>> new file mode 100755 >>> index 0000000000..1a2b7a6193 >>> --- /dev/null >>> +++ b/scripts/mkemmc.sh >>> @@ -0,0 +1,218 @@ >>> +#!/bin/sh -e >>> +# SPDX-License-Identifier: GPL-2.0-only >>> +# >>> +# Create eMMC block device image from boot, RPMB and user data images >>> +# >>> +# Copyright (c) Siemens, 2025 >>> +# >>> +# Authors: >>> +# Jan Kiszka <jan.kiszka@siemens.com> >>> +# >>> +# This work is licensed under the terms of the GNU GPL version 2. >>> +# See the COPYING file in the top-level directory. >>> +# >>> + >>> +usage() { >>> + echo "$0 [OPTIONS] USER_IMG[:SIZE] OUTPUT_IMG" >>> + echo "" >>> + echo "SIZE must be a power of 2 up to 2G and multiples of 512 >>> byte from there on." >>> + echo "If no SIZE is specified, the size of USER_ING will be used >>> (rounded up)." >>> + echo "" >>> + echo "Supported options:" >>> + echo " -b BOOT1_IMG[:SIZE] Add boot partitions. SIZE must be >>> multiples of 128K. If" >>> + echo " no SIZE is specified, the size of >>> BOOT1_IMG will be" >>> + echo " used (rounded up). BOOT1_IMG will >>> be stored in boot" >>> + echo " partition 1, and a boot partition >>> 2 of the same size" >>> + echo " will be created as empty (all >>> zeros) unless -B is" >>> + echo " specified as well." >>> + echo " -B BOOT2_IMG Fill boot partition 2 with >>> BOOT2_IMG. Must be combined" >>> + echo " with -b which is also defining >>> the partition size." >>> + echo " -r RPMB_IMG[:SIZE] Add RPMB partition. SIZE must be >>> multiples of 128K. If" >>> + echo " no SIZE is specified, the size of >>> RPMB_IMG will be" >>> + echo " used (rounded up)." >>> + echo " -h, --help This help" >>> + echo "" >>> + echo "All SIZE parameters support the units K, M, G. If SIZE is >>> smaller than the" >>> + echo "associated image, it will be truncated in the output image." >>> + exit "$1" >>> +} >>> + >>> +process_size() { >>> + name=$1 >>> + image_file=$2 >>> + alignment=$3 >>> + image_arg=$4 >>> + if [ "${image_arg#*:}" = "$image_arg" ]; then >>> + if ! size=$(stat -L -c %s "$image_file" 2>/dev/null); then >>> + echo "Missing $name image '$image_file'." >&2 >>> + exit 1 >>> + fi >>> + if [ "$alignment" = 128 ]; then >>> + size=$(( (size + 128 * 1024 - 1) & ~(128 * 1024 - 1) )) >>> + elif [ $size -gt $((2 * 1024 * 1024 * 1024)) ]; then >>> + size=$(( (size + 511) & ~511 )) >>> + elif [ $(( size & (size - 1) )) -gt 0 ]; then >>> + n=0 >>> + while [ "$size" -gt 0 ]; do >>> + size=$((size >> 1)) >>> + n=$((n + 1)) >>> + done >>> + size=$((1 << n)) >>> + fi >>> + else >>> + value="${image_arg#*:}" >>> + if [ "${value%K}" != "$value" ]; then >>> + size=${value%K} >>> + multiplier=1024 >>> + elif [ "${value%M}" != "$value" ]; then >>> + size=${value%M} >>> + multiplier=$((1024 * 1024)) >>> + elif [ "${value%G}" != "$value" ]; then >>> + size=${value%G} >>> + multiplier=$((1024 * 1024 * 1024)) >>> + else >>> + size=$value >>> + multiplier=1 >>> + fi >>> + if [ "$size" -eq "$size" ] 2>/dev/null; then >> >> I don't get this check, should one be "$value"? >> > > Likely deserves a comment, I had to refresh my own memory as well: > This checks if $size is a valid integer value. If we just run the > multiplication below, we won't be able to react properly. > >>> + size=$((size * multiplier)) >>> + else >>> + echo "Invalid value '$value' specified for $image_file >>> image size." >&2 >>> + exit 1 >>> + fi >>> + if [ "$alignment" = 128 ]; then >>> + if [ $(( size & (128 * 1024 - 1) )) -ne 0 ]; then >>> + echo "The $name image size must be multiples of >>> 128K." >&2 >>> + exit 1 >>> + fi >>> + elif [ $size -gt $((2 * 1024 * 1024 * 1024)) ]; then >>> + if [ $(( size & 511)) -ne 0 ]; then >>> + echo "The $name image size must be multiples of 512 >>> (if >2G)." >&2 >>> + exit 1 >>> + fi >>> + elif [ $(( size & (size - 1) )) -gt 0 ]; then >>> + echo "The $name image size must be power of 2 (up to >>> 2G)." >&2 >>> + exit 1 >>> + fi >>> + fi >>> + echo $size >>> +} >>> + >>> +check_truncation() { >>> + image_file=$1 >>> + output_size=$2 >>> + if [ "$image_file" = "/dev/zero" ]; then >>> + return >>> + fi >>> + if ! actual_size=$(stat -L -c %s "$image_file" 2>/dev/null); then >>> + echo "Missing image '$image_file'." >&2 >>> + exit 1 >>> + fi >>> + if [ "$actual_size" -gt "$output_size" ]; then >>> + echo "Warning: image '$image_file' will be truncated on output." >>> + fi >>> +} >>> + >>> +userimg= >>> +outimg= >>> +bootimg1= >>> +bootimg2=/dev/zero >>> +bootsz=0 >>> +rpmbimg= >>> +rpmbsz=0 >>> + >>> +while [ $# -gt 0 ]; do >>> + case "$1" in >>> + -b) >>> + shift >>> + [ $# -ge 1 ] || usage 1 >>> + bootimg1=${1%%:*} >>> + bootsz=$(process_size boot "$bootimg1" 128 "$1") >>> + shift >>> + ;; >>> + -B) >>> + shift >>> + [ $# -ge 1 ] || usage 1 >>> + bootimg2=$1 >>> + shift >>> + ;; >>> + -r) >>> + shift >>> + [ $# -ge 1 ] || usage 1 >>> + rpmbimg=${1%%:*} >>> + rpmbsz=$(process_size RPMB "$rpmbimg" 128 "$1") >>> + shift >>> + ;; >>> + -h|--help) >>> + usage 0 >>> + ;; >>> + *) >>> + if [ -z "$userimg" ]; then >>> + userimg=${1%%:*} >>> + usersz=$(process_size user "$userimg" U "$1") >>> + elif [ -z "$outimg" ]; then >>> + outimg=$1 >>> + else >>> + usage 1 >>> + fi >>> + shift >>> + ;; >>> + esac >>> +done >>> + >>> +[ -n "$outimg" ] || usage 1 >>> + >>> +if [ "$bootsz" -gt $((32640 * 1024)) ]; then >> >> Running on macOS: >> >> scripts/mkemmc.sh: line 165: [: : integer expression expected >> scripts/mkemmc.sh: line 169: [: : integer expression expected >> scripts/mkemmc.sh: line 179: [: : integer expression expected >> scripts/mkemmc.sh: line 191: [: : integer expression expected >> scripts/mkemmc.sh: line 200: [: : integer expression expected >> scripts/mkemmc.sh: line 202: [: : integer expression expected >> scripts/mkemmc.sh: line 204: [: : integer expression expected >> >> $ sh --version >> GNU bash, version 3.2.57(1)-release (arm64-apple-darwin24) >> >> When using dash: >> >> scripts/mkemmc.sh: 165: [: Illegal number: >> scripts/mkemmc.sh: 169: [: Illegal number: >> scripts/mkemmc.sh: 179: [: Illegal number: >> scripts/mkemmc.sh: 191: [: Illegal number: >> scripts/mkemmc.sh: 200: [: Illegal number: >> scripts/mkemmc.sh: 202: [: Illegal number: >> scripts/mkemmc.sh: 204: [: Illegal number: >> >> Should we replace s/[/[[/? > > No, that would be invalid outside of bash. There must be a logical error. > > How did you invoke the script? What was the value of bootsz then? > I tried with dash from debian trixie, and there is no issue like yours visible. What problem could macOS have here? Will need your help, don't have anything mac-like around right now. Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/6] scripts: Add helper script to generate eMMC block device images 2025-11-04 9:26 ` Jan Kiszka @ 2025-11-04 12:33 ` Philippe Mathieu-Daudé 2025-11-04 12:40 ` Jan Kiszka 0 siblings, 1 reply; 30+ messages in thread From: Philippe Mathieu-Daudé @ 2025-11-04 12:33 UTC (permalink / raw) To: Jan Kiszka, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Eric Blake On 4/11/25 10:26, Jan Kiszka wrote: > On 04.11.25 07:30, Jan Kiszka wrote: >> On 03.11.25 14:12, Philippe Mathieu-Daudé wrote: >>> Hi Jan, >>> >>> On 17/10/25 14:03, Jan Kiszka wrote: >>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>> +if [ "$bootsz" -gt $((32640 * 1024)) ]; then >>> >>> Running on macOS: >>> >>> scripts/mkemmc.sh: line 165: [: : integer expression expected >>> scripts/mkemmc.sh: line 169: [: : integer expression expected >>> scripts/mkemmc.sh: line 179: [: : integer expression expected >>> scripts/mkemmc.sh: line 191: [: : integer expression expected >>> scripts/mkemmc.sh: line 200: [: : integer expression expected >>> scripts/mkemmc.sh: line 202: [: : integer expression expected >>> scripts/mkemmc.sh: line 204: [: : integer expression expected >>> >>> $ sh --version >>> GNU bash, version 3.2.57(1)-release (arm64-apple-darwin24) >>> >>> When using dash: >>> >>> scripts/mkemmc.sh: 165: [: Illegal number: >>> scripts/mkemmc.sh: 169: [: Illegal number: >>> scripts/mkemmc.sh: 179: [: Illegal number: >>> scripts/mkemmc.sh: 191: [: Illegal number: >>> scripts/mkemmc.sh: 200: [: Illegal number: >>> scripts/mkemmc.sh: 202: [: Illegal number: >>> scripts/mkemmc.sh: 204: [: Illegal number: >>> >>> Should we replace s/[/[[/? >> >> No, that would be invalid outside of bash. There must be a logical error. >> >> How did you invoke the script? What was the value of bootsz then? >> > > I tried with dash from debian trixie, and there is no issue like yours > visible. > > What problem could macOS have here? Will need your help, don't have > anything mac-like around right now. Don't worry, we can merge v6 code part, and add the script / doc during the freeze period. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/6] scripts: Add helper script to generate eMMC block device images 2025-11-04 12:33 ` Philippe Mathieu-Daudé @ 2025-11-04 12:40 ` Jan Kiszka 2025-11-04 14:14 ` Jan Kiszka 0 siblings, 1 reply; 30+ messages in thread From: Jan Kiszka @ 2025-11-04 12:40 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Eric Blake On 04.11.25 13:33, Philippe Mathieu-Daudé wrote: > On 4/11/25 10:26, Jan Kiszka wrote: >> On 04.11.25 07:30, Jan Kiszka wrote: >>> On 03.11.25 14:12, Philippe Mathieu-Daudé wrote: >>>> Hi Jan, >>>> >>>> On 17/10/25 14:03, Jan Kiszka wrote: >>>>> From: Jan Kiszka <jan.kiszka@siemens.com> > > >>>>> +if [ "$bootsz" -gt $((32640 * 1024)) ]; then >>>> >>>> Running on macOS: >>>> >>>> scripts/mkemmc.sh: line 165: [: : integer expression expected >>>> scripts/mkemmc.sh: line 169: [: : integer expression expected >>>> scripts/mkemmc.sh: line 179: [: : integer expression expected >>>> scripts/mkemmc.sh: line 191: [: : integer expression expected >>>> scripts/mkemmc.sh: line 200: [: : integer expression expected >>>> scripts/mkemmc.sh: line 202: [: : integer expression expected >>>> scripts/mkemmc.sh: line 204: [: : integer expression expected >>>> >>>> $ sh --version >>>> GNU bash, version 3.2.57(1)-release (arm64-apple-darwin24) >>>> >>>> When using dash: >>>> >>>> scripts/mkemmc.sh: 165: [: Illegal number: >>>> scripts/mkemmc.sh: 169: [: Illegal number: >>>> scripts/mkemmc.sh: 179: [: Illegal number: >>>> scripts/mkemmc.sh: 191: [: Illegal number: >>>> scripts/mkemmc.sh: 200: [: Illegal number: >>>> scripts/mkemmc.sh: 202: [: Illegal number: >>>> scripts/mkemmc.sh: 204: [: Illegal number: >>>> >>>> Should we replace s/[/[[/? >>> >>> No, that would be invalid outside of bash. There must be a logical >>> error. >>> >>> How did you invoke the script? What was the value of bootsz then? >>> >> >> I tried with dash from debian trixie, and there is no issue like yours >> visible. >> >> What problem could macOS have here? Will need your help, don't have >> anything mac-like around right now. > > Don't worry, we can merge v6 code part, and add the script / doc > during the freeze period. I will send out v6 then. It passed local testing, and I addressed some further details. A colleague with a Mac offered to have look tomorrow on this. It is always a pain there regarding the subtle differences... Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/6] scripts: Add helper script to generate eMMC block device images 2025-11-04 12:40 ` Jan Kiszka @ 2025-11-04 14:14 ` Jan Kiszka 0 siblings, 0 replies; 30+ messages in thread From: Jan Kiszka @ 2025-11-04 14:14 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Eric Blake On 04.11.25 13:40, Jan Kiszka wrote: > On 04.11.25 13:33, Philippe Mathieu-Daudé wrote: >> On 4/11/25 10:26, Jan Kiszka wrote: >>> On 04.11.25 07:30, Jan Kiszka wrote: >>>> On 03.11.25 14:12, Philippe Mathieu-Daudé wrote: >>>>> Hi Jan, >>>>> >>>>> On 17/10/25 14:03, Jan Kiszka wrote: >>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> >>>>>> +if [ "$bootsz" -gt $((32640 * 1024)) ]; then >>>>> >>>>> Running on macOS: >>>>> >>>>> scripts/mkemmc.sh: line 165: [: : integer expression expected >>>>> scripts/mkemmc.sh: line 169: [: : integer expression expected >>>>> scripts/mkemmc.sh: line 179: [: : integer expression expected >>>>> scripts/mkemmc.sh: line 191: [: : integer expression expected >>>>> scripts/mkemmc.sh: line 200: [: : integer expression expected >>>>> scripts/mkemmc.sh: line 202: [: : integer expression expected >>>>> scripts/mkemmc.sh: line 204: [: : integer expression expected >>>>> >>>>> $ sh --version >>>>> GNU bash, version 3.2.57(1)-release (arm64-apple-darwin24) >>>>> >>>>> When using dash: >>>>> >>>>> scripts/mkemmc.sh: 165: [: Illegal number: >>>>> scripts/mkemmc.sh: 169: [: Illegal number: >>>>> scripts/mkemmc.sh: 179: [: Illegal number: >>>>> scripts/mkemmc.sh: 191: [: Illegal number: >>>>> scripts/mkemmc.sh: 200: [: Illegal number: >>>>> scripts/mkemmc.sh: 202: [: Illegal number: >>>>> scripts/mkemmc.sh: 204: [: Illegal number: >>>>> >>>>> Should we replace s/[/[[/? >>>> >>>> No, that would be invalid outside of bash. There must be a logical >>>> error. >>>> >>>> How did you invoke the script? What was the value of bootsz then? >>>> >>> >>> I tried with dash from debian trixie, and there is no issue like yours >>> visible. >>> >>> What problem could macOS have here? Will need your help, don't have >>> anything mac-like around right now. >> >> Don't worry, we can merge v6 code part, and add the script / doc >> during the freeze period. > > I will send out v6 then. It passed local testing, and I addressed some > further details. > > A colleague with a Mac offered to have look tomorrow on this. It is > always a pain there regarding the subtle differences... > Linux: stat -c format mac (BSD): stat -f format Seems like "wc -c < file" could be a portable alternative to "stat -L -c %s file". Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v5 6/6] docs: Add eMMC device model description 2025-10-17 12:03 [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Jan Kiszka ` (4 preceding siblings ...) 2025-10-17 12:03 ` [PATCH v5 5/6] scripts: Add helper script to generate eMMC block device images Jan Kiszka @ 2025-10-17 12:03 ` Jan Kiszka 2025-11-03 13:15 ` Philippe Mathieu-Daudé 2025-10-20 7:28 ` [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Cédric Le Goater 2025-10-27 11:15 ` Jan Kiszka 7 siblings, 1 reply; 30+ messages in thread From: Jan Kiszka @ 2025-10-17 12:03 UTC (permalink / raw) To: qemu-devel Cc: Philippe Mathieu-Daudé, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier From: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> --- docs/system/device-emulation.rst | 1 + docs/system/devices/emmc.rst | 53 ++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 docs/system/devices/emmc.rst diff --git a/docs/system/device-emulation.rst b/docs/system/device-emulation.rst index 911381643f..36429b1d17 100644 --- a/docs/system/device-emulation.rst +++ b/docs/system/device-emulation.rst @@ -101,3 +101,4 @@ Emulated Devices devices/canokey.rst devices/usb-u2f.rst devices/igb.rst + devices/emmc.rst diff --git a/docs/system/devices/emmc.rst b/docs/system/devices/emmc.rst new file mode 100644 index 0000000000..7e15b62270 --- /dev/null +++ b/docs/system/devices/emmc.rst @@ -0,0 +1,53 @@ +============== +eMMC Emulation +============== + +Besides SD card emulation, QEMU also offers an eMMC model as found on many +embedded boards. An eMMC, just like an SD card, is connected to the machine +via an SDHCI controller. + +Create eMMC Images +================== + +A recent eMMC consists of 4 partitions: 2 boot partitions, 1 Replay protected +Memory Block (RPMB), and the user data area. QEMU expects backing images for +the eMMC to contain those partitions concatenated in exactly that order. +However, the boot partitions as well as the RPMB might be absent if their sizes +are configured to zero. + +The eMMC specification defines alignment constraints for the partitions. The +two boot partitions must be of the same size. Furthermore, boot and RPMB +partitions must be multiples of 128 KB with a maximum of 32640 KB for each +boot partition and 16384K for the RPMB partition. + +The alignment constrain of the user data area depends on its size. Up to 2 +GByte, the size must be a power of 2. From 2 GByte onward, the size has to be +multiples of 512 byte. + +QEMU is enforcing those alignment rules before instantiating the device. +Therefore, the provided image has to strictly follow them as well. The helper +script ``scripts/mkemmc.sh`` can be used to create compliant images, with or +without pre-filled partitions. E.g., to create an eMMC image from a firmware +image and an OS image with an empty 2 MByte RPMB, use the following command: + +.. code-block:: console + + scripts/mkemmc.sh -b firmware.img -r /dev/zero:2MB os.img emmc.img + +This will take care of rounding up the partition sizes to the next valid value +and will leave the RPMB and the second boot partition empty (zeroed). + +Adding eMMC Devices +=================== + +An eMMC is either automatically created by a machine model (e.g. Aspeed boards) +or can be user-created when using a PCI-attached SDHCI controller. To +instantiate the eMMC image from the example above in a machine without other +SDHCI controllers while assuming that the firmware needs a boot partitions of +1 MB, use the following options: + +.. code-block:: console + + -drive file=emmc.img,if=none,format=raw,id=emmc-img + -device sdhci-pci + -device emmc,drive=emmc-img,boot-partition-size=1048576,rpmb-partition-size=2097152 -- 2.51.0 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v5 6/6] docs: Add eMMC device model description 2025-10-17 12:03 ` [PATCH v5 6/6] docs: Add eMMC device model description Jan Kiszka @ 2025-11-03 13:15 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 30+ messages in thread From: Philippe Mathieu-Daudé @ 2025-11-03 13:15 UTC (permalink / raw) To: Jan Kiszka, qemu-devel Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier On 17/10/25 14:03, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > --- > docs/system/device-emulation.rst | 1 + > docs/system/devices/emmc.rst | 53 ++++++++++++++++++++++++++++++++ > 2 files changed, 54 insertions(+) > create mode 100644 docs/system/devices/emmc.rst Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model 2025-10-17 12:03 [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Jan Kiszka ` (5 preceding siblings ...) 2025-10-17 12:03 ` [PATCH v5 6/6] docs: Add eMMC device model description Jan Kiszka @ 2025-10-20 7:28 ` Cédric Le Goater 2025-10-27 11:15 ` Jan Kiszka 7 siblings, 0 replies; 30+ messages in thread From: Cédric Le Goater @ 2025-10-20 7:28 UTC (permalink / raw) To: Jan Kiszka, qemu-devel Cc: Philippe Mathieu-Daudé, Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Alexander Bulekov, Alistair Francis, Joel Stanley, Warner Losh On 10/17/25 14:03, Jan Kiszka wrote: > Changes in v5 [1]: > - fix regression of patch 1 with unplugged SD cards > - address review comments on documentation > > Changes in v4: > - add truncation warning to mkemmc.sh > - fix typos in doc and mkemmc.sh > > Changes in v3: > - rebased, dropping merged patches > - rework image alignment rules to match hardware > - improve/fix mkemmc script > - add emmc documentation > > Changes in v2: > - handle write counter expiry > - assert() availability of QCRYPTO_HASH_ALGO_SHA256 > - add missing SPDX-License-Identifier > > This closes an old gap in system integration testing for the very > complex ARM firmware stacks by adding fairly advanced Replay Protected > Memory Block (RPMB) emulation to the eMMC device model. Key programming > and message authentication are working, so is the write counter. Known > users are happy with the result. What is missing, but not only for RPMB- > related registers, is state persistence across QEMU restarts. This is OK > at this stage for most test scenarios, though, and could still be added > later on. > > What can already be done with it is demonstrated in the WIP branch of > isar-cip-core at [2]: TF-A + OP-TEE + StandaloneMM TA + fTPM TA, used by > U-Boot and Linux for UEFI variable storage and TPM scenarios. If you > want to try: build qemu-arm64 target for trixie with 6.12-cip *head* > kernel, enable secure boot and disk encryption, then run > > $ QEMU_PATH=/path/to/qemu-build/ ./start-qemu.sh > > Deploy snakeoil keys into PK, KEK and db after first boot to enable > secure booting: > > root@demo:~# cert-to-efi-sig-list PkKek-1-snakeoil.pem PK.esl > root@demo:~# sign-efi-sig-list -k PkKek-1-snakeoil.key -c PkKek-1-snakeoil.pem PK PK.esl PK.auth > root@demo:~# efi-updatevar -f PK.auth db > root@demo:~# efi-updatevar -f PK.auth KEK > root@demo:~# efi-updatevar -f PK.auth PK > > Note that emulation is a bit slow in general, and specifically the > partition encryption on first boot is taking 20 min. - we should > probably reduce its size or understand if there is still something to > optimize. > > Jan > > [1] https://github.com/siemens/qemu/commits/queues/emmc/ > [2] https://gitlab.com/cip-project/cip-core/isar-cip-core/-/commits/wip/qemu-rpmb > > CC: Alexander Bulekov <alxndr@bu.edu> > CC: Alistair Francis <alistair@alistair23.me> > CC: Cédric Le Goater <clg@kaod.org> > CC: Joel Stanley <joel@jms.id.au> > CC: Warner Losh <imp@bsdimp.com> > > Jan Kiszka (6): > hw/sd/sdcard: Fix size check for backing block image > hw/sd/sdcard: Allow user-instantiated eMMC > hw/sd/sdcard: Add basic support for RPMB partition > hw/sd/sdcard: Handle RPMB MAC field > scripts: Add helper script to generate eMMC block device images > docs: Add eMMC device model description > > docs/system/device-emulation.rst | 1 + > docs/system/devices/emmc.rst | 53 +++++ > hw/sd/sd.c | 352 ++++++++++++++++++++++++++++--- > hw/sd/sdmmc-internal.h | 21 ++ > hw/sd/trace-events | 2 + > scripts/mkemmc.sh | 218 +++++++++++++++++++ > 6 files changed, 618 insertions(+), 29 deletions(-) > create mode 100644 docs/system/devices/emmc.rst > create mode 100755 scripts/mkemmc.sh > I checked the series on the aspeed tree : Tested-by: Cédric Le Goater <clg@redhat.com> Thanks, C. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model 2025-10-17 12:03 [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Jan Kiszka ` (6 preceding siblings ...) 2025-10-20 7:28 ` [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Cédric Le Goater @ 2025-10-27 11:15 ` Jan Kiszka 7 siblings, 0 replies; 30+ messages in thread From: Jan Kiszka @ 2025-10-27 11:15 UTC (permalink / raw) To: qemu-devel, Philippe Mathieu-Daudé Cc: Bin Meng, qemu-block, Ilias Apalodimas, Alex Bennée, Jan Lübbe, Jerome Forissier, Alexander Bulekov, Alistair Francis, Cédric Le Goater, Joel Stanley, Warner Losh On 17.10.25 14:03, Jan Kiszka wrote: > Changes in v5 [1]: > - fix regression of patch 1 with unplugged SD cards > - address review comments on documentation > > Changes in v4: > - add truncation warning to mkemmc.sh > - fix typos in doc and mkemmc.sh > > Changes in v3: > - rebased, dropping merged patches > - rework image alignment rules to match hardware > - improve/fix mkemmc script > - add emmc documentation > > Changes in v2: > - handle write counter expiry > - assert() availability of QCRYPTO_HASH_ALGO_SHA256 > - add missing SPDX-License-Identifier > > This closes an old gap in system integration testing for the very > complex ARM firmware stacks by adding fairly advanced Replay Protected > Memory Block (RPMB) emulation to the eMMC device model. Key programming > and message authentication are working, so is the write counter. Known > users are happy with the result. What is missing, but not only for RPMB- > related registers, is state persistence across QEMU restarts. This is OK > at this stage for most test scenarios, though, and could still be added > later on. > > What can already be done with it is demonstrated in the WIP branch of > isar-cip-core at [2]: TF-A + OP-TEE + StandaloneMM TA + fTPM TA, used by > U-Boot and Linux for UEFI variable storage and TPM scenarios. If you > want to try: build qemu-arm64 target for trixie with 6.12-cip *head* > kernel, enable secure boot and disk encryption, then run > > $ QEMU_PATH=/path/to/qemu-build/ ./start-qemu.sh > > Deploy snakeoil keys into PK, KEK and db after first boot to enable > secure booting: > > root@demo:~# cert-to-efi-sig-list PkKek-1-snakeoil.pem PK.esl > root@demo:~# sign-efi-sig-list -k PkKek-1-snakeoil.key -c PkKek-1-snakeoil.pem PK PK.esl PK.auth > root@demo:~# efi-updatevar -f PK.auth db > root@demo:~# efi-updatevar -f PK.auth KEK > root@demo:~# efi-updatevar -f PK.auth PK > > Note that emulation is a bit slow in general, and specifically the > partition encryption on first boot is taking 20 min. - we should > probably reduce its size or understand if there is still something to > optimize. > > Jan > > [1] https://github.com/siemens/qemu/commits/queues/emmc/ > [2] https://gitlab.com/cip-project/cip-core/isar-cip-core/-/commits/wip/qemu-rpmb > > CC: Alexander Bulekov <alxndr@bu.edu> > CC: Alistair Francis <alistair@alistair23.me> > CC: Cédric Le Goater <clg@kaod.org> > CC: Joel Stanley <joel@jms.id.au> > CC: Warner Losh <imp@bsdimp.com> > > Jan Kiszka (6): > hw/sd/sdcard: Fix size check for backing block image > hw/sd/sdcard: Allow user-instantiated eMMC > hw/sd/sdcard: Add basic support for RPMB partition > hw/sd/sdcard: Handle RPMB MAC field > scripts: Add helper script to generate eMMC block device images > docs: Add eMMC device model description > > docs/system/device-emulation.rst | 1 + > docs/system/devices/emmc.rst | 53 +++++ > hw/sd/sd.c | 352 ++++++++++++++++++++++++++++--- > hw/sd/sdmmc-internal.h | 21 ++ > hw/sd/trace-events | 2 + > scripts/mkemmc.sh | 218 +++++++++++++++++++ > 6 files changed, 618 insertions(+), 29 deletions(-) > create mode 100644 docs/system/devices/emmc.rst > create mode 100755 scripts/mkemmc.sh > Gentle ping: Would be great to get it into the next QEMU release. Please let me know if something is still missing for that. Thanks, Jan -- Siemens AG, Foundational Technologies Linux Expert Center ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2025-11-04 14:15 UTC | newest] Thread overview: 30+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-17 12:03 [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Jan Kiszka 2025-10-17 12:03 ` [PATCH v5 1/6] hw/sd/sdcard: Fix size check for backing block image Jan Kiszka 2025-10-20 7:29 ` Cédric Le Goater 2025-10-30 22:10 ` Philippe Mathieu-Daudé 2025-10-17 12:03 ` [PATCH v5 2/6] hw/sd/sdcard: Allow user-instantiated eMMC Jan Kiszka 2025-10-27 11:55 ` Daniel P. Berrangé 2025-10-27 12:23 ` Philippe Mathieu-Daudé 2025-10-27 12:35 ` Daniel P. Berrangé 2025-10-27 12:44 ` Philippe Mathieu-Daudé 2025-10-30 14:17 ` Jan Lübbe 2025-10-27 12:45 ` Jan Lübbe 2025-10-30 16:50 ` Jan Lübbe 2025-10-30 18:10 ` Jan Kiszka 2025-10-17 12:03 ` [PATCH v5 3/6] hw/sd/sdcard: Add basic support for RPMB partition Jan Kiszka 2025-11-03 15:08 ` Philippe Mathieu-Daudé 2025-11-04 6:34 ` Jan Kiszka 2025-10-17 12:03 ` [PATCH v5 4/6] hw/sd/sdcard: Handle RPMB MAC field Jan Kiszka 2025-11-03 15:18 ` Philippe Mathieu-Daudé 2025-11-04 6:43 ` Jan Kiszka 2025-10-17 12:03 ` [PATCH v5 5/6] scripts: Add helper script to generate eMMC block device images Jan Kiszka 2025-11-03 13:12 ` Philippe Mathieu-Daudé 2025-11-04 6:30 ` Jan Kiszka 2025-11-04 9:26 ` Jan Kiszka 2025-11-04 12:33 ` Philippe Mathieu-Daudé 2025-11-04 12:40 ` Jan Kiszka 2025-11-04 14:14 ` Jan Kiszka 2025-10-17 12:03 ` [PATCH v5 6/6] docs: Add eMMC device model description Jan Kiszka 2025-11-03 13:15 ` Philippe Mathieu-Daudé 2025-10-20 7:28 ` [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model Cédric Le Goater 2025-10-27 11:15 ` Jan Kiszka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).