Re: [PATCH 9/9] ARM: Rockchip: RK3568: implement failsafe barebox update

[Michael Riesch <michael.riesch@wolfvision.net>]

Hello Sascha,

On 10/11/21 10:03 AM, Sascha Hauer wrote:
> The RK3568 ROM searches for valid boot images at different positions
> on SD/eMMC cards. This can be used to implement a failsafe barebox
> update which is immune against power failures.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/boards/rockchip-rk3568-evb/board.c |   4 +-
>  arch/arm/dts/rk3568-evb1-v10.dts            |  10 --
>  arch/arm/mach-rockchip/Makefile             |   1 +
>  arch/arm/mach-rockchip/bbu.c                | 117 ++++++++++++++++++++
>  arch/arm/mach-rockchip/include/mach/bbu.h   |   9 +-
>  5 files changed, 126 insertions(+), 15 deletions(-)
>  create mode 100644 arch/arm/mach-rockchip/bbu.c
> 
> diff --git a/arch/arm/boards/rockchip-rk3568-evb/board.c b/arch/arm/boards/rockchip-rk3568-evb/board.c
> index 2d472d1331..09385bea29 100644
> --- a/arch/arm/boards/rockchip-rk3568-evb/board.c
> +++ b/arch/arm/boards/rockchip-rk3568-evb/board.c
> @@ -28,8 +28,8 @@ static int rk3568_evb_probe(struct device_d *dev)
>  	else
>  		of_device_enable_path("/chosen/environment-emmc");
>  
> -	rk3568_bbu_mmc_register("emmc", BBU_HANDLER_FLAG_DEFAULT, "/dev/emmc.barebox");
> -	rk3568_bbu_mmc_register("sd", 0, "/dev/sd.barebox");
> +	rk3568_bbu_mmc_register("emmc", BBU_HANDLER_FLAG_DEFAULT, "/dev/emmc");
> +	rk3568_bbu_mmc_register("sd", 0, "/dev/sd");
>  
>  	return 0;
>  }
> diff --git a/arch/arm/dts/rk3568-evb1-v10.dts b/arch/arm/dts/rk3568-evb1-v10.dts
> index ebfd45ada8..6f1eebc619 100644
> --- a/arch/arm/dts/rk3568-evb1-v10.dts
> +++ b/arch/arm/dts/rk3568-evb1-v10.dts
> @@ -475,11 +475,6 @@
>  		#address-cells = <2>;
>  		#size-cells = <2>;
>  
> -		partition@8000 {
> -			label = "barebox";
> -			reg = <0x0 0x8000 0x0 0x400000>;
> -		};
> -
>  		environment_emmc: partition@408000 {
>  			label = "barebox-environment";
>  			reg = <0x0 0x408000 0x0 0x8000>;
> @@ -506,11 +501,6 @@
>  		#address-cells = <2>;
>  		#size-cells = <2>;
>  
> -		partition@8000 {
> -			label = "barebox";
> -			reg = <0x0 0x8000 0x0 0x400000>;
> -		};
> -
>  		environment_sd: partition@408000 {
>  			label = "barebox-environment";
>  			reg = <0x0 0x408000 0x0 0x8000>;
> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
> index ebaa3a5450..66bcdba2eb 100644
> --- a/arch/arm/mach-rockchip/Makefile
> +++ b/arch/arm/mach-rockchip/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_ARCH_RK3188) += rk3188.o
>  obj-$(CONFIG_ARCH_RK3288) += rk3288.o
>  obj-pbl-$(CONFIG_ARCH_RK3568) += rk3568.o
>  obj-$(CONFIG_ARCH_RK3568) += bootm.o
> +obj-$(CONFIG_BAREBOX_UPDATE) += bbu.o
> diff --git a/arch/arm/mach-rockchip/bbu.c b/arch/arm/mach-rockchip/bbu.c
> new file mode 100644
> index 0000000000..6bfba95133
> --- /dev/null
> +++ b/arch/arm/mach-rockchip/bbu.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include <common.h>
> +#include <malloc.h>
> +#include <bbu.h>
> +#include <filetype.h>
> +#include <errno.h>
> +#include <fs.h>
> +#include <fcntl.h>
> +#include <linux/sizes.h>
> +#include <linux/stat.h>
> +#include <ioctl.h>
> +#include <environment.h>
> +#include <mach/bbu.h>
> +#include <libfile.h>
> +
> +/* The MaskROM looks for images on these four locations: */
> +#define IMG_OFFSET_0	(0 * SZ_1K + SZ_32K)
> +#define IMG_OFFSET_1	(512 * SZ_1K + SZ_32K)
> +#define IMG_OFFSET_2	(1024 * SZ_1K + SZ_32K)
> +#define IMG_OFFSET_3	(1536 * SZ_1K + SZ_32K)
> +#define IMG_OFFSET_4	(2048 * SZ_1K + SZ_32K)

That's five locations, isn't it?

> +
> +/*
> + * The strategy here is:
> + * The MaskROM iterates over the above four locations until it finds a valid

Five?

Best regards,
Michael

> + * boot image. The images are protected with sha sums, so any change to an
> + * image on disk is invalidating it. We first check if we have enough space to
> + * write two copies of barebox. To make it simple we only use IMG_OFFSET_0 and
> + * IMG_OFFSET_4 which leaves the maximum size for a single image. When there's
> + * not enough free space on the beginning of the disk we only write a single
> + * image. When we have enough space for two images we first write the second one
> + * (leaving the first one intact). Afterwards we write the first one which
> + * leaves the previously written second image as a fallback in case writing the
> + * first one gets interrupted.
> + * This mechanism assumes that the first image is valid which isn't the case
> + * when the previous update has been interrupted. Two power failures in a row
> + * can leave the board unbootable. To prevent that we would have to be able
> + * to detect valid images to determine the order in which we write the two
> + * images.
> + */
> +static int rk3568_bbu_mmc_handler(struct bbu_handler *handler,
> +				  struct bbu_data *data)
> +{
> +	enum filetype filetype;
> +	int ret, fd;
> +	loff_t space;
> +
> +	filetype = file_detect_type(data->image, data->len);
> +	if (filetype != filetype_rockchip_rkns_image) {
> +		if (!bbu_force(data, "incorrect image type. Expected: %s, got %s",
> +				file_type_to_string(filetype_rockchip_rkns_image),
> +				file_type_to_string(filetype)))
> +			return -EINVAL;
> +	}
> +
> +	device_detect_by_name(devpath_to_name(data->devicefile));
> +
> +	ret = bbu_confirm(data);
> +	if (ret)
> +		return ret;
> +
> +	space = cdev_unallocated_start(devpath_to_name(data->devicefile));
> +
> +	if (space < IMG_OFFSET_0 + data->len) {
> +		pr_err("Unallocated space on %s is too small for one image\n", data->devicefile);
> +		return -ENOSPC;
> +	}
> +
> +	fd = open(data->devicefile, O_WRONLY);
> +	if (fd < 0)
> +		return fd;
> +        
> +	if (space >= IMG_OFFSET_4 + data->len) {
> +		pr_info("Unallocated space is enough for two copies, doing failsafe update\n");
> +
> +		ret = pwrite_full(fd, data->image, data->len, IMG_OFFSET_4);
> +		if (ret < 0) {
> +			pr_err("writing to %s failed with %s\n", data->devicefile,
> +				strerror(-ret));
> +			goto err_close;
> +		}
> +	}
> +
> +	ret = pwrite_full(fd, data->image, data->len, IMG_OFFSET_0);
> +	if (ret < 0) {
> +		pr_err("writing to %s failed with %s\n", data->devicefile,
> +			strerror(-ret));
> +		goto err_close;
> +	}
> +
> +	ret = 0;
> +
> +err_close:
> +	close(fd);
> +
> +	return ret;
> +}
> +
> +int rk3568_bbu_mmc_register(const char *name, unsigned long flags,
> +                const char *devicefile)
> +{
> +	struct bbu_handler *handler;
> +	int ret;
> +
> +	handler = xzalloc(sizeof(*handler));
> +
> +	handler->flags = flags;
> +	handler->devicefile = devicefile;
> +	handler->name = name;
> +	handler->handler = rk3568_bbu_mmc_handler;
> +
> +	ret = bbu_register_handler(handler);
> +	if (ret)
> +		free(handler);
> +
> +	return ret;
> +}
> diff --git a/arch/arm/mach-rockchip/include/mach/bbu.h b/arch/arm/mach-rockchip/include/mach/bbu.h
> index e61e0615e2..7fb08a0a9e 100644
> --- a/arch/arm/mach-rockchip/include/mach/bbu.h
> +++ b/arch/arm/mach-rockchip/include/mach/bbu.h
> @@ -3,12 +3,15 @@
>  
>  #include <bbu.h>
>  
> +#ifdef CONFIG_BAREBOX_UPDATE
> +int rk3568_bbu_mmc_register(const char *name, unsigned long flags,
> +                const char *devicefile);
> +#else
>  static inline int rk3568_bbu_mmc_register(const char *name, unsigned long flags,
>                  const char *devicefile)
>  {
> -	return bbu_register_std_file_update(name, flags,
> -                devicefile, filetype_rockchip_rkns_image);
> -
> +	return -ENOSYS;
>  }
> +#endif
>  
>  # endif /* __MACH_ROCKCHIP_BBU_H */
> 

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox