diff mbox series

[2/6] fwu: v2: try reading both copies of metadata

Message ID 20240830114057.891069-3-sughosh.ganu@linaro.org
State New
Headers show
Series Miscellaneous FWU fixes | expand

Commit Message

Sughosh Ganu Aug. 30, 2024, 11:40 a.m. UTC
In the version 2 of the FWU metadata, the metadata is broken into two
parts, a top-level structure, which provides information on the total
size of the structure among other things. Try reading the primary
partition first, and if that fails, try reading the secondary
partition. This will help in the scenario where the primary metadata
partition has been corrupted, but the secondary partition is intact.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
 lib/fwu_updates/fwu_v2.c | 77 +++++++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 36 deletions(-)

Comments

Michal Simek Sept. 3, 2024, 5:07 p.m. UTC | #1
On 8/30/24 13:40, Sughosh Ganu wrote:
> In the version 2 of the FWU metadata, the metadata is broken into two
> parts, a top-level structure, which provides information on the total
> size of the structure among other things. Try reading the primary
> partition first, and if that fails, try reading the secondary
> partition. This will help in the scenario where the primary metadata
> partition has been corrupted, but the secondary partition is intact.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>   lib/fwu_updates/fwu_v2.c | 77 +++++++++++++++++++++-------------------
>   1 file changed, 41 insertions(+), 36 deletions(-)
> 
> diff --git a/lib/fwu_updates/fwu_v2.c b/lib/fwu_updates/fwu_v2.c
> index d0d8a25929..69306282aa 100644
> --- a/lib/fwu_updates/fwu_v2.c
> +++ b/lib/fwu_updates/fwu_v2.c
> @@ -10,6 +10,12 @@
>   #include <linux/types.h>
>   
>   #define FWU_MDATA_VERSION	0x2U
> +#define FWU_IMG_DESC_OFFSET	0x20U
> +
> +static struct fwu_mdata g_mdata;
> +
> +#define PRI_PART		0x1U
> +#define SEC_PART		0x2U
>   
>   static inline struct fwu_fw_store_desc *fwu_get_fw_desc(struct fwu_mdata *mdata)
>   {
> @@ -109,6 +115,31 @@ static int fwu_trial_state_start(uint update_index)
>   	return 0;
>   }
>   
> +static bool fwu_get_mdata_mandatory(uint part)
> +{
> +	int ret = 0;
> +	struct udevice *fwu_dev = fwu_get_dev();
> +
> +	memset(&g_mdata, 0, sizeof(struct fwu_mdata));
> +
> +	ret = fwu_read_mdata(fwu_dev, &g_mdata,
> +			     part == PRI_PART ? true : false,
> +			     sizeof(struct fwu_mdata));
> +	if (ret)
> +		return false;
> +
> +	if (g_mdata.version != FWU_MDATA_VERSION)
> +		return false;

Any reason to remove error logs which were here?

Thanks,
Michal
Sughosh Ganu Sept. 4, 2024, 7:18 a.m. UTC | #2
On Tue, 3 Sept 2024 at 22:37, Michal Simek <michal.simek@amd.com> wrote:
>
>
>
> On 8/30/24 13:40, Sughosh Ganu wrote:
> > In the version 2 of the FWU metadata, the metadata is broken into two
> > parts, a top-level structure, which provides information on the total
> > size of the structure among other things. Try reading the primary
> > partition first, and if that fails, try reading the secondary
> > partition. This will help in the scenario where the primary metadata
> > partition has been corrupted, but the secondary partition is intact.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> >   lib/fwu_updates/fwu_v2.c | 77 +++++++++++++++++++++-------------------
> >   1 file changed, 41 insertions(+), 36 deletions(-)
> >
> > diff --git a/lib/fwu_updates/fwu_v2.c b/lib/fwu_updates/fwu_v2.c
> > index d0d8a25929..69306282aa 100644
> > --- a/lib/fwu_updates/fwu_v2.c
> > +++ b/lib/fwu_updates/fwu_v2.c
> > @@ -10,6 +10,12 @@
> >   #include <linux/types.h>
> >
> >   #define FWU_MDATA_VERSION   0x2U
> > +#define FWU_IMG_DESC_OFFSET  0x20U
> > +
> > +static struct fwu_mdata g_mdata;
> > +
> > +#define PRI_PART             0x1U
> > +#define SEC_PART             0x2U
> >
> >   static inline struct fwu_fw_store_desc *fwu_get_fw_desc(struct fwu_mdata *mdata)
> >   {
> > @@ -109,6 +115,31 @@ static int fwu_trial_state_start(uint update_index)
> >       return 0;
> >   }
> >
> > +static bool fwu_get_mdata_mandatory(uint part)
> > +{
> > +     int ret = 0;
> > +     struct udevice *fwu_dev = fwu_get_dev();
> > +
> > +     memset(&g_mdata, 0, sizeof(struct fwu_mdata));
> > +
> > +     ret = fwu_read_mdata(fwu_dev, &g_mdata,
> > +                          part == PRI_PART ? true : false,
> > +                          sizeof(struct fwu_mdata));
> > +     if (ret)
> > +             return false;
> > +
> > +     if (g_mdata.version != FWU_MDATA_VERSION)
> > +             return false;
>
> Any reason to remove error logs which were here?

They just got dropped during the code refactor. Will put them back. Thanks.

-sughosh
Michal Simek Sept. 4, 2024, 7:59 a.m. UTC | #3
On 8/30/24 13:40, Sughosh Ganu wrote:
> In the version 2 of the FWU metadata, the metadata is broken into two
> parts, a top-level structure, which provides information on the total
> size of the structure among other things. Try reading the primary
> partition first, and if that fails, try reading the secondary
> partition. This will help in the scenario where the primary metadata
> partition has been corrupted, but the secondary partition is intact.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>   lib/fwu_updates/fwu_v2.c | 77 +++++++++++++++++++++-------------------
>   1 file changed, 41 insertions(+), 36 deletions(-)
> 
> diff --git a/lib/fwu_updates/fwu_v2.c b/lib/fwu_updates/fwu_v2.c
> index d0d8a25929..69306282aa 100644
> --- a/lib/fwu_updates/fwu_v2.c
> +++ b/lib/fwu_updates/fwu_v2.c
> @@ -10,6 +10,12 @@
>   #include <linux/types.h>
>   
>   #define FWU_MDATA_VERSION	0x2U
> +#define FWU_IMG_DESC_OFFSET	0x20U

And this is value which is sizeof(struct fwu_mdata) right?


> +
> +static struct fwu_mdata g_mdata;
> +
> +#define PRI_PART		0x1U
> +#define SEC_PART		0x2U

You already have them in include/fwu.h

  88 enum {
  89         PRIMARY_PART = 1,
  90         SECONDARY_PART,
  91         BOTH_PARTS,
  92 };
  93


M
diff mbox series

Patch

diff --git a/lib/fwu_updates/fwu_v2.c b/lib/fwu_updates/fwu_v2.c
index d0d8a25929..69306282aa 100644
--- a/lib/fwu_updates/fwu_v2.c
+++ b/lib/fwu_updates/fwu_v2.c
@@ -10,6 +10,12 @@ 
 #include <linux/types.h>
 
 #define FWU_MDATA_VERSION	0x2U
+#define FWU_IMG_DESC_OFFSET	0x20U
+
+static struct fwu_mdata g_mdata;
+
+#define PRI_PART		0x1U
+#define SEC_PART		0x2U
 
 static inline struct fwu_fw_store_desc *fwu_get_fw_desc(struct fwu_mdata *mdata)
 {
@@ -109,6 +115,31 @@  static int fwu_trial_state_start(uint update_index)
 	return 0;
 }
 
+static bool fwu_get_mdata_mandatory(uint part)
+{
+	int ret = 0;
+	struct udevice *fwu_dev = fwu_get_dev();
+
+	memset(&g_mdata, 0, sizeof(struct fwu_mdata));
+
+	ret = fwu_read_mdata(fwu_dev, &g_mdata,
+			     part == PRI_PART ? true : false,
+			     sizeof(struct fwu_mdata));
+	if (ret)
+		return false;
+
+	if (g_mdata.version != FWU_MDATA_VERSION)
+		return false;
+
+	if (!g_mdata.desc_offset)
+		return false;
+
+	if (g_mdata.desc_offset != FWU_IMG_DESC_OFFSET)
+		return false;
+
+	return true;
+}
+
 /**
  * fwu_populate_mdata_image_info() - Populate the image information
  * of the metadata
@@ -169,24 +200,14 @@  int fwu_state_machine_updates(bool trial_state, uint32_t update_index)
  */
 int fwu_get_mdata_size(uint32_t *mdata_size)
 {
-	int ret = 0;
-	struct fwu_mdata mdata = { 0 };
 	struct fwu_data *data = fwu_get_data();
-	struct udevice *fwu_dev = fwu_get_dev();
 
 	if (data->metadata_size) {
 		*mdata_size = data->metadata_size;
 		return 0;
 	}
 
-	ret = fwu_read_mdata(fwu_dev, &mdata, 1,
-			     sizeof(struct fwu_mdata));
-	if (ret) {
-		log_err("FWU metadata read failed\n");
-		return ret;
-	}
-
-	*mdata_size = mdata.metadata_size;
+	*mdata_size = g_mdata.metadata_size;
 	if (!*mdata_size)
 		return -EINVAL;
 
@@ -206,39 +227,23 @@  int fwu_get_mdata_size(uint32_t *mdata_size)
 int fwu_init(void)
 {
 	int ret;
-	struct fwu_mdata mdata = { 0 };
-	struct udevice *fwu_dev = fwu_get_dev();
 
 	/*
 	 * First we read only the top level structure
 	 * and get the size of the complete structure.
+	 * Try reading the first partition first, if
+	 * that does not work, try the secondary
+	 * partition. The idea is, if one of the
+	 * partitions is corrupted, it should be restored
+	 * from the intact partition.
 	 */
-	ret = fwu_read_mdata(fwu_dev, &mdata, 1,
-			     sizeof(struct fwu_mdata));
-	if (ret) {
+	if (!fwu_get_mdata_mandatory(PRI_PART) &&
+	    !fwu_get_mdata_mandatory(SEC_PART)) {
 		log_err("FWU metadata read failed\n");
-		return ret;
-	}
-
-	if (mdata.version != FWU_MDATA_VERSION) {
-		log_err("FWU metadata version %u. Expected value of %u\n",
-			mdata.version, FWU_MDATA_VERSION);
-		return -EINVAL;
-	}
-
-	if (!mdata.desc_offset) {
-		log_err("No image information provided with the Metadata. ");
-		log_err("Image information expected in the metadata\n");
-		return -EINVAL;
-	}
-
-	if (mdata.desc_offset != 0x20) {
-		log_err("Descriptor Offset(0x%x) in the FWU Metadata not equal to 0x20\n",
-			mdata.desc_offset);
-		return -EINVAL;
+		return -1;
 	}
 
-	ret = fwu_mdata_copies_allocate(mdata.metadata_size);
+	ret = fwu_mdata_copies_allocate(g_mdata.metadata_size);
 	if (ret)
 		return ret;