[PATCH 1/2] handler: eMMC internal register

[Stefano Babic]

This adds support for managing hardware functions in eMMC device. First
version of the handler supports toggeling of the hardware boot
partitions.

Signed-off-by: Stefano Babic <stefan...@swupdate.org>
---
corelib/Makefile | 3 +-
corelib/emmc.h | 140 ++++++++++++++++++++++++++++++++++++
corelib/emmc_utils.c | 110 ++++++++++++++++++++++++++++
handlers/Config.in | 11 +++
handlers/Makefile | 1 +
handlers/emmc_csd_handler.c | 67 +++++++++++++++++
include/util.h | 4 ++
7 files changed, 335 insertions(+), 1 deletion(-)
create mode 100644 corelib/emmc.h
create mode 100644 corelib/emmc_utils.c
create mode 100644 handlers/emmc_csd_handler.c

diff --git a/corelib/Makefile b/corelib/Makefile
index 7e706d87..5917e379 100644
--- a/corelib/Makefile
+++ b/corelib/Makefile
@@ -2,7 +2,8 @@
#
# SPDX-License-Identifier: GPL-2.0-only

-lib-y += multipart_parser.o \
+lib-y += emmc_utils.o \
+ multipart_parser.o \
parsing_library_libjson.o \
server_utils.o
lib-$(CONFIG_DOWNLOAD) += downloader.o
diff --git a/corelib/emmc.h b/corelib/emmc.h
new file mode 100644
index 00000000..35f8a304
--- /dev/null
+++ b/corelib/emmc.h
@@ -0,0 +1,140 @@
+/*
+ * (C) Copyright 2024
+ * Stefano Babic, stefan...@swupdate.org
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ */
+
+#pragma once
+
+/* From kernel linux/mmc/mmc.h */
+#define MMC_SEND_EXT_CSD 8 /* adtc R1 */
+#define MMC_SWITCH 6 /* ac [31:0] See below R1b */
+#define MMC_SEND_EXT_CSD 8 /* adtc R1 */
+#define MMC_SEND_STATUS 13 /* ac [31:16] RCA R1 */
+#define MMC_SWITCH_MODE_WRITE_BYTE 0x03 /* Set target to value */
+
+/* From kernel linux/mmc/core.h */
+#define MMC_RSP_NONE 0 /* no response */
+#define MMC_RSP_PRESENT (1 << 0)
+#define MMC_RSP_136 (1 << 1) /* 136 bit response */
+#define MMC_RSP_CRC (1 << 2) /* expect valid crc */
+#define MMC_RSP_BUSY (1 << 3) /* card may send busy */
+#define MMC_RSP_OPCODE (1 << 4) /* response contains opcode */
+
+#define MMC_CMD_AC (0 << 5)
+#define MMC_CMD_ADTC (1 << 5)
+#define MMC_CMD_BC (2 << 5)
+
+#define MMC_RSP_SPI_S1 (1 << 7) /* one status byte */
+#define MMC_RSP_SPI_BUSY (1 << 10) /* card may send busy */
+
+#define MMC_RSP_SPI_R1 (MMC_RSP_SPI_S1)
+#define MMC_RSP_SPI_R1B (MMC_RSP_SPI_S1|MMC_RSP_SPI_BUSY)
+
+#define MMC_RSP_R1 (MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE)
+#define MMC_RSP_R1B (MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE|MMC_RSP_BUSY)
+
+/*
+ * EXT_CSD fields
+ */
+#define EXT_CSD_S_CMD_SET 504
+#define EXT_CSD_HPI_FEATURE 503
+#define EXT_CSD_BKOPS_SUPPORT 502 /* RO */
+#define EXT_CSD_SUPPORTED_MODES 493 /* RO */
+#define EXT_CSD_FFU_FEATURES 492 /* RO */
+#define EXT_CSD_FFU_ARG_3 490 /* RO */
+#define EXT_CSD_FFU_ARG_2 489 /* RO */
+#define EXT_CSD_FFU_ARG_1 488 /* RO */
+#define EXT_CSD_FFU_ARG_0 487 /* RO */
+#define EXT_CSD_CMDQ_DEPTH 307 /* RO */
+#define EXT_CSD_CMDQ_SUPPORT 308 /* RO */
+#define EXT_CSD_NUM_OF_FW_SEC_PROG_3 305 /* RO */
+#define EXT_CSD_NUM_OF_FW_SEC_PROG_2 304 /* RO */
+#define EXT_CSD_NUM_OF_FW_SEC_PROG_1 303 /* RO */
+#define EXT_CSD_NUM_OF_FW_SEC_PROG_0 302 /* RO */
+#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B 269 /* RO */
+#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A 268 /* RO */
+#define EXT_CSD_PRE_EOL_INFO 267 /* RO */
+#define EXT_CSD_FIRMWARE_VERSION 254 /* RO */
+#define EXT_CSD_CACHE_SIZE_3 252
+#define EXT_CSD_CACHE_SIZE_2 251
+#define EXT_CSD_CACHE_SIZE_1 250
+#define EXT_CSD_CACHE_SIZE_0 249
+#define EXT_CSD_SEC_FEATURE_SUPPORT 231
+#define EXT_CSD_BOOT_INFO 228 /* R/W */
+#define EXT_CSD_BOOT_MULT 226 /* RO */
+#define EXT_CSD_HC_ERASE_GRP_SIZE 224
+#define EXT_CSD_HC_WP_GRP_SIZE 221
+#define EXT_CSD_SEC_COUNT_3 215
+#define EXT_CSD_SEC_COUNT_2 214
+#define EXT_CSD_SEC_COUNT_1 213
+#define EXT_CSD_SEC_COUNT_0 212
+#define EXT_CSD_PART_SWITCH_TIME 199
+#define EXT_CSD_REV 192
+#define EXT_CSD_BOOT_CFG 179
+#define EXT_CSD_PART_CONFIG 179
+#define EXT_CSD_BOOT_BUS_CONDITIONS 177
+#define EXT_CSD_ERASE_GROUP_DEF 175
+#define EXT_CSD_BOOT_WP_STATUS 174
+#define EXT_CSD_BOOT_WP 173
+#define EXT_CSD_USER_WP 171
+#define EXT_CSD_FW_CONFIG 169 /* R/W */
+#define EXT_CSD_WR_REL_SET 167
+#define EXT_CSD_WR_REL_PARAM 166
+#define EXT_CSD_SANITIZE_START 165
+#define EXT_CSD_BKOPS_EN 163 /* R/W */
+#define EXT_CSD_RST_N_FUNCTION 162 /* R/W */
+#define EXT_CSD_PARTITIONING_SUPPORT 160 /* RO */
+#define EXT_CSD_MAX_ENH_SIZE_MULT_2 159
+#define EXT_CSD_MAX_ENH_SIZE_MULT_1 158
+#define EXT_CSD_MAX_ENH_SIZE_MULT_0 157
+#define EXT_CSD_PARTITIONS_ATTRIBUTE 156 /* R/W */
+#define EXT_CSD_PARTITION_SETTING_COMPLETED 155 /* R/W */
+#define EXT_CSD_GP_SIZE_MULT_4_2 154
+#define EXT_CSD_GP_SIZE_MULT_4_1 153
+#define EXT_CSD_GP_SIZE_MULT_4_0 152
+#define EXT_CSD_GP_SIZE_MULT_3_2 151
+#define EXT_CSD_GP_SIZE_MULT_3_1 150
+#define EXT_CSD_GP_SIZE_MULT_3_0 149
+#define EXT_CSD_GP_SIZE_MULT_2_2 148
+#define EXT_CSD_GP_SIZE_MULT_2_1 147
+#define EXT_CSD_GP_SIZE_MULT_2_0 146
+#define EXT_CSD_GP_SIZE_MULT_1_2 145
+#define EXT_CSD_GP_SIZE_MULT_1_1 144
+#define EXT_CSD_GP_SIZE_MULT_1_0 143
+#define EXT_CSD_ENH_SIZE_MULT_2 142
+#define EXT_CSD_ENH_SIZE_MULT_1 141
+#define EXT_CSD_ENH_SIZE_MULT_0 140
+#define EXT_CSD_ENH_START_ADDR_3 139
+#define EXT_CSD_ENH_START_ADDR_2 138
+#define EXT_CSD_ENH_START_ADDR_1 137
+#define EXT_CSD_ENH_START_ADDR_0 136
+#define EXT_CSD_NATIVE_SECTOR_SIZE 63 /* R */
+#define EXT_CSD_USE_NATIVE_SECTOR 62 /* R/W */
+#define EXT_CSD_DATA_SECTOR_SIZE 61 /* R */
+#define EXT_CSD_EXT_PARTITIONS_ATTRIBUTE_1 53
+#define EXT_CSD_EXT_PARTITIONS_ATTRIBUTE_0 52
+#define EXT_CSD_CACHE_CTRL 33
+#define EXT_CSD_MODE_CONFIG 30
+#define EXT_CSD_MODE_OPERATION_CODES 29 /* W */
+#define EXT_CSD_FFU_STATUS 26 /* R */
+#define EXT_CSD_SECURE_REMOVAL_TYPE 16 /* R/W */
+#define EXT_CSD_CMDQ_MODE_EN 15 /* R/W */
+
+
+/*
+ * EXT_CSD field definitions
+ */
+#define EXT_CSD_CONFIG_SECRM_TYPE (0x30)
+#define EXT_CSD_SUPPORTED_SECRM_TYPE (0x0f)
+#define EXT_CSD_FFU_INSTALL (0x01)
+#define EXT_CSD_FFU_MODE (0x01)
+#define EXT_CSD_NORMAL_MODE (0x00)
+#define EXT_CSD_FFU (1<<0)
+#define EXT_CSD_UPDATE_DISABLE (1<<0)
+#define EXT_CSD_HPI_SUPP (1<<0)
+#define EXT_CSD_HPI_IMPL (1<<1)
+#define EXT_CSD_CMD_SET_NORMAL (1<<0)
+
+
diff --git a/corelib/emmc_utils.c b/corelib/emmc_utils.c
new file mode 100644
index 00000000..f12c7867
--- /dev/null
+++ b/corelib/emmc_utils.c
@@ -0,0 +1,110 @@
+/*
+ * (C) Copyright 2024
+ * Stefano Babic, stefan...@swupdate.org
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ */
+#include <stdio.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <errno.h>
+#include <linux/version.h>
+#include <sys/ioctl.h>
+#include <linux/major.h>
+#include <linux/mmc/ioctl.h>
+#include "emmc.h"
+#include "util.h"
+
+/*
+ * Code taken from mmc-utils, mmc_cmds.c
+ */
+static int emmc_read_extcsd(int fd, __u8 *ext_csd)
+{
+ int ret = 0;
+ struct mmc_ioc_cmd idata;
+ memset(&idata, 0, sizeof(idata));
+ memset(ext_csd, 0, sizeof(__u8) * 512);
+ idata.write_flag = 0;
+ idata.opcode = MMC_SEND_EXT_CSD;
+ idata.arg = 0;
+ idata.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
+ idata.blksz = 512;
+ idata.blocks = 1;
+ mmc_ioc_cmd_set_data(idata, ext_csd);
+
+ ret = ioctl(fd, MMC_IOC_CMD, &idata);
+ if (ret)
+ ERROR("eMMC ioctl return error %d", ret);
+
+ return ret;
+}
+
+static void fill_switch_cmd(struct mmc_ioc_cmd *cmd, __u8 index, __u8 value)
+{
+ cmd->opcode = MMC_SWITCH;
+ cmd->write_flag = 1;
+ cmd->arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) | (index << 16) |
+ (value << 8) | EXT_CSD_CMD_SET_NORMAL;
+ cmd->flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
+}
+
+static int emmc_write_extcsd_value(int fd, __u8 index, __u8 value, unsigned int timeout_ms)
+{
+ int ret = 0;
+ struct mmc_ioc_cmd idata = {};
+
+ fill_switch_cmd(&idata, index, value);
+
+ /* Kernel will set cmd_timeout_ms if 0 is set */
+ idata.cmd_timeout_ms = timeout_ms;
+
+ ret = ioctl(fd, MMC_IOC_CMD, &idata);
+ if (ret)
+ ERROR("eMMC ioctl return error %d", ret);
+
+ return ret;
+} /* end of imported code */
+
+int emmc_get_active_bootpart(int fd)
+{
+ int ret;
+ uint8_t extcsd[512];
+ int active;
+
+ ret = emmc_read_extcsd(fd, extcsd);
+
+ if (ret)
+ return -1;
+
+ /*
+ * Return partition number starting from 0
+ * This corresponds to mmcblkXboot0 and mmcblkXboot1
+ */
+ active = ((extcsd[EXT_CSD_PART_CONFIG] & 0x38) >> 3) - 1;
+
+ return active;
+}
+
+int emmc_write_bootpart(int fd, int bootpart)
+{
+ uint8_t value;
+ int ret;
+ uint8_t extcsd[512];
+
+ /*
+ * Do not clear BOOT_ACK
+ */
+ ret = emmc_read_extcsd(fd, extcsd);
+ value = extcsd[EXT_CSD_PART_CONFIG] & (1 << 6);
+
+ bootpart = ((bootpart + 1) & 0x3) << 3;
+ value |= bootpart;
+
+ ret = emmc_write_extcsd_value(fd, EXT_CSD_PART_CONFIG, value, 0);
+
+ return ret;
+}
diff --git a/handlers/Config.in b/handlers/Config.in
index b01d033d..ca473646 100644
--- a/handlers/Config.in
+++ b/handlers/Config.in
@@ -140,6 +140,17 @@ config EMBEDDED_LUA_HANDLER_SOURCE
Path to the Lua handler source code file to be
embedded into the SWUpdate binary.

+
+config EMMC_HANDLER
+ bool "eMMC handler"
+ default n
+ help
+ This handler allows to switch the boot partitions via
+ the eMMC internal CSD register. One common use case is to
+ upgrade the bootloader, using the two hardware partitions
+ with a dual-copy concept. This guarantees that the upgrade
+ is power-cut safe.
+
config RAW
bool "raw"
default n
diff --git a/handlers/Makefile b/handlers/Makefile
index 24fd487c..12d4aeb8 100644
--- a/handlers/Makefile
+++ b/handlers/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_BTRFS_FILESYSTEM) += btrfs_handler.o
obj-$(CONFIG_COPY) += copy_handler.o
obj-$(CONFIG_CFI) += flash_handler.o
obj-$(CONFIG_DELTA) += delta_handler.o delta_downloader.o zchunk_range.o
+obj-$(CONFIG_EMMC_HANDLER) += emmc_csd_handler.o
obj-$(CONFIG_DISKFORMAT_HANDLER) += diskformat_handler.o
obj-$(CONFIG_DISKPART) += diskpart_handler.o
obj-$(CONFIG_UNIQUEUUID) += uniqueuuid_handler.o
diff --git a/handlers/emmc_csd_handler.c b/handlers/emmc_csd_handler.c
new file mode 100644
index 00000000..f71dd1a1
--- /dev/null
+++ b/handlers/emmc_csd_handler.c
@@ -0,0 +1,67 @@
+/*
+ * (C) Copyright 2024
+ * Stefano Babic, stefan...@swupdate.org
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ */
+/*
+ * This handler manages the CSD register accoriding to eMMC
+ * specifications. Base for this handler are the mmcutils,
+ * see:
+ * https://git.kernel.org/pub/scm/utils/mmc/mmc-utils.git
+ */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <errno.h>
+#include <linux/version.h>
+#include <sys/ioctl.h>
+#include <linux/mmc/ioctl.h>
+#include "swupdate_image.h"
+#include "handler.h"
+#include "util.h"
+
+void emmc_csd_handler(void);
+
+static int emmc_boot_toggle(struct img_type *img, void *data)
+{
+ int active, ret;
+ struct script_handler_data *script_data = data;
+ if (script_data->scriptfn == PREINSTALL)
+ return 0;
+
+ /* Open the device (partition) */
+ int fdin = open(img->device, O_RDONLY);
+ if (fdin < 0) {
+ ERROR("Failed to open %s: %s", img->device, strerror(errno));
+ return -ENODEV;
+ }
+
+ active = emmc_get_active_bootpart(fdin);
+ if (active < 0) {
+ ERROR("Current HW boot partition cannot be retrieved");
+ close(fdin);
+ return -1;
+ }
+
+ active = (active == 0) ? 1 : 0;
+
+ ret = emmc_write_bootpart(fdin, active);
+
+ if (ret)
+ ERROR("Failure writing CSD register");
+
+ close(fdin);
+ return ret;
+}
+
+__attribute__((constructor))
+void emmc_csd_handler(void)
+{
+ register_handler("emmc_boot_toggle", emmc_boot_toggle,
+ SCRIPT_HANDLER | NO_DATA_HANDLER, NULL);
+}
diff --git a/include/util.h b/include/util.h
index 490014e7..e1350633 100644
--- a/include/util.h
+++ b/include/util.h
@@ -272,3 +272,7 @@ int swupdate_umount(const char *dir);

/* Date / Time utilities */
char *swupdate_time_iso8601(struct timeval *tv);
+
+/* eMMC functions */
+int emmc_write_bootpart(int fd, int bootpart);
+int emmc_get_active_bootpart(int fd);
--
2.34.1

[Michael Glembotzki]

Hi Stefano,

can you also add documentation, on how to use it?

How do you prevent the bootloader from being reinstalled with every update (if bootloader is present)? I think a version check should be possible. You would have to read the current version from boot0/boot1 and compare it with the provided version from sw-description. We cannot rely on the content of /etc/sw-versions with a dual copy strategy, because bootloader slot 0/1 can diverge with firmware slot a/b.
E.g. on a failed update if for example, only the bootloader has been updated but the rest has not yet been updated.

Best, Michael

[Stefano Babic]

Hi Michael,

On 16.02.24 14:34, Michael Glembotzki wrote:
> Hi Stefano,
>
> can you also add documentation, on how to use it?
>

The handler just toggle the hw partitions via CSD register, as described
in eMMC spec. That means if your board can boot from the HW partitions
(/dev/mmcblxXboot[0|1]), a power-cut safe update can be implemented by
toggling the device. This feature is present on eMMC.

The handler is not responsible to install the bootloader - this is
already supported via one of the handlers.

> How do you prevent the bootloader from being reinstalled with every
> update (if bootloader is present)?

It has nothing to do with the handler. Update of the bootloader is
supported since a long time, and where the bootloader is installed
depends on your SOC. It can be on flash (then the "flash" handler), on
mmcblkbootX (then the "raw" handler), or something different.

> I think a version check should be
> possible.

This is also already implemented, please check versioning inside SWUpdat
and how to use it by filling /etc/sw-version. Your use case means that
the image with the bootloader should have the "name", "version" and
"installed-if-different" attribute.

> You would have to read the current version from boot0/boot1
> and compare it with the provided version from sw-description.

....and this is supported since a verylong time....

It is out of scope here. The handler is not an image installer, it
registers as script.

> We cannot
> rely on the content of /etc/sw-versions with a dual copy strategy,
> because bootloader slot 0/1 can diverge with firmware slot a/b.

This is just because it seems you are using a static version of the
file. This is never correct.

The correct way is to fill /etc/sw-versions (or put somewhere else, see
related CONFIG) during the boot to be sure that the versions are consistent.

> E.g. on a failed update if for example, only the bootloader has been
> updated but the rest has not yet been updated.

In many cases, where the bootloader is a single point of failure, it is.
The handler here is to provide a power-cut way to update the bootloader.
But it is just half of the story. The handler runs as postinstall
script, that is after all images / files were already installed. The use
case is something like:

images: (
{
filename = "new-bootloader-spl.bin";
name = "U-BOOT";
version = "2024.01";
installed-if-different = true;
device = <...standby mmmcblkbootx>;
type = "raw";
offset = <requested by some SOCs>;
}

.....many other artifacts

scripts: (
{
type = "emmc_boot_toggle";
device = "/dev/mmcblk1X;
}
)

Best regards,
Stefano Babic

> --
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/swupdate/c59bd5b9-d02e-48c3-b2c9-66e9af62740cn%40googlegroups.com <https://groups.google.com/d/msgid/swupdate/c59bd5b9-d02e-48c3-b2c9-66e9af62740cn%40googlegroups.com?utm_medium=email&utm_source=footer>.

[Michael]

Hi Stefano,

Am Fr., 16. Feb. 2024 um 17:11 Uhr schrieb Stefano Babic
<stefan...@swupdate.org>:

>
> Hi Michael,
>
> On 16.02.24 14:34, Michael Glembotzki wrote:
> > Hi Stefano,
> >
> > can you also add documentation, on how to use it?
> >
>
> The handler just toggle the hw partitions via CSD register, as described
> in eMMC spec. That means if your board can boot from the HW partitions
> (/dev/mmcblxXboot[0|1]), a power-cut safe update can be implemented by
> toggling the device. This feature is present on eMMC.
>
> The handler is not responsible to install the bootloader - this is
> already supported via one of the handlers.
>
> > How do you prevent the bootloader from being reinstalled with every
> > update (if bootloader is present)?
>
> It has nothing to do with the handler. Update of the bootloader is
> supported since a long time, and where the bootloader is installed
> depends on your SOC. It can be on flash (then the "flash" handler), on
> mmcblkbootX (then the "raw" handler), or something different.
>
> > I think a version check should be
> > possible.
>
> This is also already implemented, please check versioning inside SWUpdat
> and how to use it by filling /etc/sw-version. Your use case means that
> the image with the bootloader should have the "name", "version" and
> "installed-if-different" attribute.

Thanks for the explanation.

> > You would have to read the current version from boot0/boot1
> > and compare it with the provided version from sw-description.
>
> ....and this is supported since a verylong time....
>
> It is out of scope here. The handler is not an image installer, it
> registers as script.
>
> > We cannot
> > rely on the content of /etc/sw-versions with a dual copy strategy,
> > because bootloader slot 0/1 can diverge with firmware slot a/b.
>
> This is just because it seems you are using a static version of the
> file. This is never correct.

ok

> The correct way is to fill /etc/sw-versions (or put somewhere else, see
> related CONFIG) during the boot to be sure that the versions are consistent.

ok

> > E.g. on a failed update if for example, only the bootloader has been
> > updated but the rest has not yet been updated.
>
> In many cases, where the bootloader is a single point of failure, it is.
> The handler here is to provide a power-cut way to update the bootloader.
> But it is just half of the story. The handler runs as postinstall
> script, that is after all images / files were already installed. The use
> case is something like:
>
> images: (
> {
> filename = "new-bootloader-spl.bin";
> name = "U-BOOT";
> version = "2024.01";
> installed-if-different = true;
> device = <...standby mmmcblkbootx>;
> type = "raw";
> offset = <requested by some SOCs>;
> }
>
> .....many other artifacts
>
> scripts: (
> {
> type = "emmc_boot_toggle";
> device = "/dev/mmcblk1X;
> }
> )

Best,
Michael

[Michael Glembotzki]

Hi Stefano,

I tested the patch. It does what it's supposed to do. The boot partitions are toggled.
Only the trace message is a bit misleading. "Internal script handler emmc_boot_toggle is called." would be clearer.

[TRACE] : SWUPDATE running : [extract_scripts] : No script provided for script of type emmc_boot_toggle

> + active = emmc_get_active_bootpart(fdin);
> + if (active < 0) {
> + ERROR("Current HW boot partition cannot be retrieved");
> + close(fdin);
> + return -1;
> + }

Unlikely, but would be cleaner if "active" is checked for 0 or 1. Except -1, "active" could also be 6, if BOOT_PARTITION_ENABLE is 0x7.

Bit[5:3] : BOOT_PARTITION_ENABLE (R/W/E)
User selects boot data that will be sent to master
0x0 : Device not boot enabled (default)
0x1 : Boot partition 1 enabled for boot
0x2 : Boot partition 2 enabled for boot
0x3–0x6 : Reserved
0x7 : User area enabled for boot 


Best regards,
Michael

[Stefano Babic]

Hi Michael,

On 22.02.24 20:37, Michael Glembotzki wrote:
> Hi Stefano,
>

> I tested the patch. It does what it's supposed to do. The boot
> partitions are toggled.
> Only the trace message is a bit misleading. "Internal script handler
> emmc_boot_toggle is called." would be clearer.
>
> [TRACE] : SWUPDATE running : [extract_scripts] : No script provided for
> script of type emmc_boot_toggle

But the goal of this trace is just to indicate that there is no script
inside the SWU. It is printed before a "script" is executed.

It is part of checks in code:

https://github.com/sbabic/swupdate/blob/master/core/installer.c#L108

And this just says that no script code is part of the SWU - but not yet
that it could be executed.

I could change the trace, but which can be a suitable text for it ?

>
> > + active = emmc_get_active_bootpart(fdin);
> > + if (active < 0) {
> > + ERROR("Current HW boot partition cannot be retrieved");
> > + close(fdin);
> > + return -1;
> > + }
> Unlikely, but would be cleaner if "active" is checked for 0 or 1. Except
> -1, "active" could also be 6, if BOOT_PARTITION_ENABLE is 0x7.

Yes, that the reason the value of the register is not interpreted, and
masking can be done by the user, and 0x7 should be verified as well.
What do you mind ?

Best regards,
Stefano

> --
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/swupdate/5b3ff963-0392-4e05-b07b-32988403976bn%40googlegroups.com <https://groups.google.com/d/msgid/swupdate/5b3ff963-0392-4e05-b07b-32988403976bn%40googlegroups.com?utm_medium=email&utm_source=footer>.

[Michael]

Hi Stefano,

Am Fr., 23. Feb. 2024 um 16:16 Uhr schrieb Stefano Babic
<stefan...@swupdate.org>:
>
> Hi Michael,
>

> On 22.02.24 20:37, Michael Glembotzki wrote:
> > Hi Stefano,
> >
> > I tested the patch. It does what it's supposed to do. The boot
> > partitions are toggled.
> > Only the trace message is a bit misleading. "Internal script handler
> > emmc_boot_toggle is called." would be clearer.
> >
> > [TRACE] : SWUPDATE running : [extract_scripts] : No script provided for
> > script of type emmc_boot_toggle
>
> But the goal of this trace is just to indicate that there is no script
> inside the SWU. It is printed before a "script" is executed.
>
> It is part of checks in code:
>
> https://github.com/sbabic/swupdate/blob/master/core/installer.c#L108
>
> And this just says that no script code is part of the SWU - but not yet
> that it could be executed.
>
> I could change the trace, but which can be a suitable text for it ?

Let's keep this trace and add another in the function emmc_boot_toggle to
indicate from/to which slot the emmc boot partition is toggled.

> >
> > > + active = emmc_get_active_bootpart(fdin);
> > > + if (active < 0) {
> > > + ERROR("Current HW boot partition cannot be retrieved");
> > > + close(fdin);
> > > + return -1;
> > > + }
> > Unlikely, but would be cleaner if "active" is checked for 0 or 1. Except
> > -1, "active" could also be 6, if BOOT_PARTITION_ENABLE is 0x7.
>
> Yes, that the reason the value of the register is not interpreted, and
> masking can be done by the user, and 0x7 should be verified as well.

I meant the call in emmc_boot_toggle.

> What do you mind ?

active = (active == 0) ? 1 : 0;

ret = emmc_write_bootpart(fdin, active);

If BOOT_PARTITION_ENABLE was set to 0x7, emmc_get_active_bootpart
returns a 6 and next it toggles to boot partition 0. I would prevent that case,
also if it is rare.

Best regards,
Michael

[Stefano Babic]

This adds support for managing hardware functions in eMMC device. First
version of the handler supports toggeling of the hardware boot
partitions.

Signed-off-by: Stefano Babic <stefan...@swupdate.org>
---

Changes since V1:
- check for returned active partition to exclude user Boot Area
- Add trace to log which becomes the next boot device

corelib/Makefile | 3 +-
corelib/emmc.h | 140 ++++++++++++++++++++++++++++++++++++
corelib/emmc_utils.c | 110 ++++++++++++++++++++++++++++
handlers/Config.in | 11 +++
handlers/Makefile | 1 +

handlers/emmc_csd_handler.c | 75 +++++++++++++++++++
include/util.h | 4 ++
7 files changed, 343 insertions(+), 1 deletion(-)

index 00000000..27bd0257
--- /dev/null
+++ b/handlers/emmc_csd_handler.c
@@ -0,0 +1,75 @@

+/*
+ * (C) Copyright 2024
+ * Stefano Babic, stefan...@swupdate.org
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ */
+/*
+ * This handler manages the CSD register accoriding to eMMC
+ * specifications. Base for this handler are the mmcutils,
+ * see:
+ * https://git.kernel.org/pub/scm/utils/mmc/mmc-utils.git

+ active = emmc_get_active_bootpart(fdin);
+ if (active < 0) {
+ ERROR("Current HW boot partition cannot be retrieved");
+ close(fdin);
+ return -1;
+ }

+
+ /*
+ * If User Partition is activated, does nothing
+ * and report this to the user.
+ */
+ if (active > 1) {
+ WARN("Boot device set to User area, no changes !");
+ ret = 0;
+ } else {
+ active = (active == 0) ? 1 : 0;
+ TRACE("Setting Boot to HW Partition %d", active);

[Michael Glembotzki]

Hi Stefano,

thanks for the changes. I tested again. Code does what it's supposed to, great!

Best regards,

Michael

[Michael Glembotzki]

Hi Stefano,

I think I need to ask again about the exact use case for the emmc_boot_toggle handler.

Our use case with dual update strategy for the bootloader is as follows:

1. Check if bootloader version is greater
  - As I understood, that shall be done with dynamic filled /etc/swversions on every boot (probably with an init script ?)
  - install-if-higher must be set in sw-description

2. Update bootloader
  - Use a symlink for the device e.g. /dev/swu_bootloader and set the symlink for /dev/mmcblk0boot0 (or boot1) in another shellscript handler
  - Update the bootloader with the raw image handler as always

3. Toggle emmc register for boot0 / boot1 "only" if bootloader has been updated
  - Right now, it seems (please correct me if I am wrong) the boot partition is always toggled if the handler is present and also
  - on postfailure

4. Overwrite old bootloader with the new one
  - Could be done directly after the toggle, because the bootloader has no rollback (It is only power cut safe.)


How can you archive the conditional synchronization between bootloader update and emmc_boot_toggle?
We have currently written our own lua script handler that carries out all these steps one after the other. I just hoped that our handler could be replaced by build-in machanism.

I think it would also be helpful for others if the documentation area for emmc_boot_toggle was expanded. In particular, how it can and cannot be used for bootloader dual copy.

Best regards,
Michael