[PATCH v2 0/3] state: add support for pre-mainline state format

[PATCH v2 0/3] state: add support for pre-mainline state format

[Uwe Kleine-König]

Hello,

when the state framework was developed the underlying raw format was
simpler (only one checksum and no length) and no meta data for direct
buckets.

To be able to keep the binary representation with the current state
framework this patch series introduces the old "raw" format as
"rawcompat" and teaches the direct bucket backend to skip writing meta
data in some cases.

This is v2 of the series, compared to v1 this also updates the
documentation.

Best regards
Uwe

Uwe Kleine-König (3):
  Documentation: state: adapt description to reality
  state: new backend format "rawcompat"
  state: storage: direct: skip writing meta data if this makes data fit

 .../devicetree/bindings/barebox/barebox,state.rst  |  25 ++--
 common/state/Makefile                              |   1 +
 common/state/backend_bucket_direct.c               |  23 +++-
 common/state/backend_format_rawcompat.c            | 145 +++++++++++++++++++++
 common/state/state.c                               |   3 +
 common/state/state.h                               |   2 +
 6 files changed, 183 insertions(+), 16 deletions(-)
 create mode 100644 common/state/backend_format_rawcompat.c

-- 
2.11.0


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

[PATCH v2 1/3] Documentation: state: adapt description to reality

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../devicetree/bindings/barebox/barebox,state.rst      | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/barebox/barebox,state.rst b/Documentation/devicetree/bindings/barebox/barebox,state.rst
index 40d7bc2e6fdf..ed17e3e75883 100644
--- a/Documentation/devicetree/bindings/barebox/barebox,state.rst
+++ b/Documentation/devicetree/bindings/barebox/barebox,state.rst
@@ -127,14 +127,16 @@ Variable Types
 Backends
 --------
 
-Currently two backends exist. The raw backend is a very compact format
-consisting of a magic value for identification, the raw values and a
-CRC. Two copies are maintained for making sure that during update the
-storage device still contains a valid state. The dtb backend stores
-the state as a devicetree binary blob. This is exactly the original
-devicetree description of the state itself, but additionally contains
-the actual values of the variables. Unlike the raw state backend the
-dtb state backend can describe itself.
+Currently two backends exist. The ``raw`` backend is a very compact format
+consisting of a magic value for identification, a crc-protected header giving
+the actual length and then the raw values and another CRC. To make sure that
+during update the storage device always contains a valid state, the data is
+replicated on the medium.
+
+The ``dtb`` backend stores the state as a devicetree binary blob.  This is
+exactly the original devicetree description of the state itself, but
+additionally contains the actual values of the variables. Unlike the ``raw``
+state backend the ``dtb`` state backend can describe itself.
 
 HMAC
 ----
-- 
2.11.0


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

[PATCH v2 2/3] state: new backend format "rawcompat"

[Uwe Kleine-König]

This is the format that was initially used during development of the
state framework. Some early adopters still stick to this one.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../devicetree/bindings/barebox/barebox,state.rst  |   9 +-
 common/state/Makefile                              |   1 +
 common/state/backend_format_rawcompat.c            | 145 +++++++++++++++++++++
 common/state/state.c                               |   3 +
 common/state/state.h                               |   2 +
 5 files changed, 158 insertions(+), 2 deletions(-)
 create mode 100644 common/state/backend_format_rawcompat.c

diff --git a/Documentation/devicetree/bindings/barebox/barebox,state.rst b/Documentation/devicetree/bindings/barebox/barebox,state.rst
index ed17e3e75883..c13a001d797e 100644
--- a/Documentation/devicetree/bindings/barebox/barebox,state.rst
+++ b/Documentation/devicetree/bindings/barebox/barebox,state.rst
@@ -31,7 +31,7 @@ Required properties:
 * ``magic``: A 32bit number used as a magic to identify the state
 * ``backend``: contains a phandle to the device/partition which holds the
   actual state data.
-* ``backend-type``: should be ``raw`` or ``dtb``.
+* ``backend-type``: should be ``raw``, ``dtb`` or ``rawcompat`` (not recommended).
 * additionally a state node must have an alias in the /aliases/ node pointing
   to it.
 
@@ -127,7 +127,7 @@ Variable Types
 Backends
 --------
 
-Currently two backends exist. The ``raw`` backend is a very compact format
+Currently three backends exist. The ``raw`` backend is a very compact format
 consisting of a magic value for identification, a crc-protected header giving
 the actual length and then the raw values and another CRC. To make sure that
 during update the storage device always contains a valid state, the data is
@@ -138,6 +138,11 @@ exactly the original devicetree description of the state itself, but
 additionally contains the actual values of the variables. Unlike the ``raw``
 state backend the ``dtb`` state backend can describe itself.
 
+There is a third backend called ``rawcompat`` which implements the binary
+format of the early days of the state framework. It is not recommended for new
+usage and only exists to make barebox work on machines that have to stick to
+the old format for compatibility reasons.
+
 HMAC
 ----
 
diff --git a/common/state/Makefile b/common/state/Makefile
index fcf9add52cbe..e4357e9c209a 100644
--- a/common/state/Makefile
+++ b/common/state/Makefile
@@ -2,6 +2,7 @@ obj-y += state.o
 obj-y += state_variables.o
 obj-y += backend_format_dtb.o
 obj-y += backend_format_raw.o
+obj-y += backend_format_rawcompat.o
 obj-y += backend_storage.o
 obj-y += backend_bucket_direct.o
 obj-$(CONFIG_MTD) += backend_bucket_circular.o
diff --git a/common/state/backend_format_rawcompat.c b/common/state/backend_format_rawcompat.c
new file mode 100644
index 000000000000..ffc9b4b0554a
--- /dev/null
+++ b/common/state/backend_format_rawcompat.c
@@ -0,0 +1,145 @@
+/*
+ * Copyright (C) 2017 Pengutronix, Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <common.h>
+#include <linux/kernel.h>
+#include <malloc.h>
+#include <crc.h>
+#include <asm/unaligned.h>
+
+#include "state.h"
+
+struct state_backend_format_rawcompat {
+	struct state_backend_format format;
+
+	/* For outputs */
+	struct device_d *dev;
+};
+
+static inline struct state_backend_format_rawcompat *get_format_rawcompat(
+		struct state_backend_format *format)
+{
+	return container_of(format, struct state_backend_format_rawcompat, format);
+}
+
+static int backend_format_rawcompat_verify(struct state_backend_format *format,
+					   uint32_t magic, const void *buf,
+					   ssize_t *lenp, enum state_flags flags)
+{
+	uint32_t crc;
+	uint32_t tmp;
+	struct state_backend_format_rawcompat *backend = get_format_rawcompat(format);
+
+	tmp = get_unaligned((uint32_t *)buf);
+	if (magic && magic != tmp) {
+		dev_err(backend->dev,
+			"Error, found invalid magic 0x%08x, should be 0x%08x\n",
+			tmp, magic);
+		return -EINVAL;
+	}
+
+	tmp = get_unaligned((uint32_t *)(buf + *lenp - sizeof(uint32_t)));
+	crc = crc32(0, buf, *lenp - sizeof(uint32_t));
+	if (crc != tmp) {
+		dev_err(backend->dev,
+			"Error, invalid header crc in rawcompat format, calculated 0x%08x, found 0x%08x\n",
+			crc, tmp);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int backend_format_rawcompat_unpack(struct state_backend_format *format,
+					   struct state *state, const void *buf,
+					   ssize_t len)
+{
+	struct state_variable *sv;
+	const void *data;
+	struct state_backend_format_rawcompat *backend = get_format_rawcompat(format);
+
+	data = buf + sizeof(uint32_t);
+
+	list_for_each_entry(sv, &state->variables, list) {
+		if (sv->start + sv->size > len - 8) {
+			dev_err(backend->dev, "State variable ends behind valid data, %s\n",
+				sv->name);
+			continue;
+		}
+		memcpy(sv->raw, data + sv->start, sv->size);
+	}
+
+	return 0;
+}
+
+static int backend_format_rawcompat_pack(struct state_backend_format *format,
+					 struct state *state, void **buf_out,
+					 ssize_t *len_out)
+{
+	struct state_backend_format_rawcompat *backend = get_format_rawcompat(format);
+	void *buf, *data;
+	struct state_variable *sv;
+	uint32_t crc;
+
+	*len_out = 0;
+	list_for_each_entry(sv, &state->variables, list) {
+		if (*len_out < sv->start + sv->size)
+			*len_out = sv->start + sv->size;
+	}
+
+	/* for header and crc */
+	*len_out += 2 * sizeof(uint32_t);
+
+	buf = xzalloc(*len_out);
+
+	put_unaligned_le32(state->magic, buf);
+
+	data = buf + sizeof(uint32_t);
+
+	list_for_each_entry(sv, &state->variables, list) {
+		memcpy(data + sv->start, sv->raw, sv->size);
+	}
+
+	crc = crc32(0, buf, *len_out - sizeof(uint32_t));
+	put_unaligned_le32(crc, buf + *len_out - sizeof(uint32_t));
+
+	*buf_out = buf;
+
+	return 0;
+}
+
+static void backend_format_rawcompat_free(struct state_backend_format *format)
+{
+	struct state_backend_format_rawcompat *backend = get_format_rawcompat(format);
+
+	free(backend);
+}
+
+int backend_format_rawcompat_create(struct state_backend_format **format,
+				    struct device_node *node, struct device_d *dev)
+{
+	struct state_backend_format_rawcompat *rawcompat;
+
+	rawcompat = xzalloc(sizeof(*rawcompat));
+
+	rawcompat->dev = dev;
+
+	rawcompat->format.pack = backend_format_rawcompat_pack;
+	rawcompat->format.unpack = backend_format_rawcompat_unpack;
+	rawcompat->format.verify = backend_format_rawcompat_verify;
+	rawcompat->format.free = backend_format_rawcompat_free;
+	rawcompat->format.name = "rawcompat";
+	*format = &rawcompat->format;
+
+	return 0;
+}
diff --git a/common/state/state.c b/common/state/state.c
index 121ba0c6d34a..c383c529e973 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -124,6 +124,9 @@ static int state_format_init(struct state *state, const char *backend_format,
 	if (!backend_format || !strcmp(backend_format, "raw")) {
 		ret = backend_format_raw_create(&state->format, node,
 						state_name, &state->dev);
+	} else if (!strcmp(backend_format, "rawcompat")) {
+		ret = backend_format_rawcompat_create(&state->format, node,
+						      &state->dev);
 	} else if (!strcmp(backend_format, "dtb")) {
 		ret = backend_format_dtb_create(&state->format, &state->dev);
 	} else {
diff --git a/common/state/state.h b/common/state/state.h
index 81aaec23b62b..587bc11e4501 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -188,6 +188,8 @@ struct device_node *state_to_node(struct state *state,
 int backend_format_raw_create(struct state_backend_format **format,
 			      struct device_node *node, const char *secret_name,
 			      struct device_d *dev);
+int backend_format_rawcompat_create(struct state_backend_format **format,
+				    struct device_node *node, struct device_d *dev);
 int backend_format_dtb_create(struct state_backend_format **format,
 			      struct device_d *dev);
 int state_storage_init(struct state *state, const char *path,
-- 
2.11.0


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

[PATCH v2 3/3] state: storage: direct: skip writing meta data if this makes data fit

[Uwe Kleine-König]

When a direct bucket is read it handles missing meta data just fine for
compatibility reasons. This allows to skip writing meta data if that
makes the data small enough to fit in the bucket to be written.

The motivation for this change is that the storage format in
pre-mainline state didn't use meta data and some machines still have to
stick to this format. For these the stride size exactly fits the data
size such that there is no place for additional meta data and so the
data is written in the old format.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 common/state/backend_bucket_direct.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/common/state/backend_bucket_direct.c b/common/state/backend_bucket_direct.c
index 4465ed0e4115..8bf9a945643c 100644
--- a/common/state/backend_bucket_direct.c
+++ b/common/state/backend_bucket_direct.c
@@ -102,8 +102,14 @@ static int state_backend_bucket_direct_write(struct state_backend_storage_bucket
 	    get_bucket_direct(bucket);
 	int ret;
 	struct state_backend_storage_bucket_direct_meta meta;
+	int write_meta = 1;
 
-	if (len > direct->max_size - sizeof(meta))
+	if (len > direct->max_size - sizeof(meta)) {
+		dev_info(direct->dev, "skip writing metadata because of size constraints\n");
+		write_meta = 0;
+	}
+
+	if (len > direct->max_size)
 		return -E2BIG;
 
 	ret = lseek(direct->fd, direct->offset, SEEK_SET);
@@ -112,12 +118,15 @@ static int state_backend_bucket_direct_write(struct state_backend_storage_bucket
 		return ret;
 	}
 
-	meta.magic = direct_magic;
-	meta.written_length = len;
-	ret = write_full(direct->fd, &meta, sizeof(meta));
-	if (ret < 0) {
-		dev_err(direct->dev, "Failed to write metadata to file, %d\n", ret);
-		return ret;
+	if (write_meta) {
+		meta.magic = direct_magic;
+		meta.written_length = len;
+		ret = write_full(direct->fd, &meta, sizeof(meta));
+		if (ret < 0) {
+			dev_err(direct->dev,
+				"Failed to write metadata to file, %d\n", ret);
+			return ret;
+		}
 	}
 
 	ret = write_full(direct->fd, buf, len);
-- 
2.11.0


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

Re: [PATCH v2 1/3] Documentation: state: adapt description to reality

[Sascha Hauer]

On Thu, Sep 28, 2017 at 11:03:27AM +0200, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  .../devicetree/bindings/barebox/barebox,state.rst      | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)

This one does not apply. Please rebase to master.

Sascha

> 
> diff --git a/Documentation/devicetree/bindings/barebox/barebox,state.rst b/Documentation/devicetree/bindings/barebox/barebox,state.rst
> index 40d7bc2e6fdf..ed17e3e75883 100644
> --- a/Documentation/devicetree/bindings/barebox/barebox,state.rst
> +++ b/Documentation/devicetree/bindings/barebox/barebox,state.rst
> @@ -127,14 +127,16 @@ Variable Types
>  Backends
>  --------
>  
> -Currently two backends exist. The raw backend is a very compact format
> -consisting of a magic value for identification, the raw values and a
> -CRC. Two copies are maintained for making sure that during update the
> -storage device still contains a valid state. The dtb backend stores
> -the state as a devicetree binary blob. This is exactly the original
> -devicetree description of the state itself, but additionally contains
> -the actual values of the variables. Unlike the raw state backend the
> -dtb state backend can describe itself.
> +Currently two backends exist. The ``raw`` backend is a very compact format
> +consisting of a magic value for identification, a crc-protected header giving
> +the actual length and then the raw values and another CRC. To make sure that
> +during update the storage device always contains a valid state, the data is
> +replicated on the medium.
> +
> +The ``dtb`` backend stores the state as a devicetree binary blob.  This is
> +exactly the original devicetree description of the state itself, but
> +additionally contains the actual values of the variables. Unlike the ``raw``
> +state backend the ``dtb`` state backend can describe itself.
>  
>  HMAC
>  ----
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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