Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Boot nor fallback #1244

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Boot nor fallback #1244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca7ecef
Added nor fallback handler.
esden Aug 25, 2023
0cbb615
Added support for flashcp write last parameter.
esden Aug 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions src/install.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,8 +905,20 @@ static gboolean pre_install_check_slot_mount_status(RaucSlot *slot, RaucImage *m
return FALSE;
}

static gboolean check_device(gchar* device, GError **error)
{
if (!g_file_test(device, G_FILE_TEST_EXISTS)) {
g_set_error(error, R_INSTALL_ERROR, R_INSTALL_ERROR_NODST,
"Destination device '%s' not found", device);
return FALSE;
}
return TRUE;
}

static gboolean pre_install_checks(gchar* bundledir, GPtrArray *install_plans, GHashTable *target_group, GError **error)
{
gchar** devices = NULL;

for (guint i = 0; i < install_plans->len; i++) {
const RImageInstallPlan *plan = g_ptr_array_index(install_plans, i);

Expand Down Expand Up @@ -934,10 +946,19 @@ static gboolean pre_install_checks(gchar* bundledir, GPtrArray *install_plans, G
}

skip_filename_checks:
if (!g_file_test(plan->target_slot->device, G_FILE_TEST_EXISTS)) {
g_set_error(error, R_INSTALL_ERROR, R_INSTALL_ERROR_NODST,
"Destination device '%s' not found", plan->target_slot->device);
return FALSE;
if (g_strcmp0(plan->target_slot->type, "boot-nor-fallback") == 0) {
devices = g_strsplit(plan->target_slot->device, ":", -1);
Copy link
Member

@jluebbe jluebbe Sep 20, 2023 •
edited by ejoerns
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This g_strsplit should not be at multiple places. Instead, the handling for multiple devices should be explicit in the RaucSlot struct and should be split during parsing in config_file.c. For consistency, it should use g_key_file_get_string_list, which splits at ;. If only one device is given, it should behave as before, storing it in the gchar *device. Otherwise, a new gchar **devices would be used and device would be NULL.

This way, it's explicit if one or multiple devices are configured. Then, we'd need a consistency check at the end of load_config() which ensures that only boot-nor-fallback passes two devices. All others would need only one.

for (int i = 0; i < g_strv_length(devices); i++) {
if (!check_device(devices[i], error)) {
g_strfreev(devices);
return FALSE;
}
}
g_strfreev(devices);
} else {
if (!check_device(plan->target_slot->device, error)) {
return FALSE;
}
}

if (!pre_install_check_slot_mount_status(plan->target_slot, plan->image, error)) {
Expand Down
1 change: 1 addition & 0 deletions src/slot.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ RaucSlotType supported_slot_types[] = {
{"boot-gpt-switch", FALSE},
{"vfat", TRUE},
{"boot-raw-fallback", FALSE},
{"boot-nor-fallback", FALSE},
{}
};

Expand Down
107 changes: 105 additions & 2 deletions src/update_handler.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1188,14 +1188,19 @@ static gboolean vfat_format_slot(RaucSlot *dest_slot, GError **error)
return res;
}

static gboolean nor_write_slot(const gchar *image, const gchar *device, GError **error)
static gboolean nor_write_slot(const gchar *image, const gchar *device, const unsigned long long wr_last, GError **error)
{
g_autoptr(GSubprocess) sproc = NULL;
GError *ierror = NULL;
gboolean res = FALSE;
g_autoptr(GPtrArray) args = g_ptr_array_new_full(5, g_free);

g_ptr_array_add(args, g_strdup("flashcp"));
if (wr_last > 0) {
g_ptr_array_add(args, g_strdup("-l"));
g_ptr_array_add(args, g_strdup_printf("%llu", wr_last));
g_ptr_array_add(args, g_strdup("-v")); // We should not need verbose except for debugging
}
g_ptr_array_add(args, g_strdup(image));
g_ptr_array_add(args, g_strdup(device));
g_ptr_array_add(args, NULL);
Expand Down Expand Up @@ -2001,7 +2006,7 @@ static gboolean img_to_nor_handler(RaucImage *image, RaucSlot *dest_slot, const

/* write */
g_message("writing slot device %s", dest_slot->device);
res = nor_write_slot(image->filename, dest_slot->device, &ierror);
res = nor_write_slot(image->filename, dest_slot->device, 0, &ierror);
if (!res) {
g_propagate_error(error, ierror);
goto out;
Expand Down Expand Up @@ -2527,6 +2532,102 @@ static gboolean check_if_area_is_clear(const gchar *device, guint64 start, gsize
return res;
}

static gboolean img_to_boot_nor_fallback_handler(RaucImage *image, RaucSlot *dest_slot, const gchar *hook_name, GError **error)
{
GError *ierror = NULL;
gboolean res = FALSE;
gchar** devices = NULL;
gboolean primary_clear;
char *part_names[2] = {
"primary",
"fallback"
};
int first_part_desc_index = 1;
const gsize header_size = 512; /* TODO: this value should be settable through the system config file. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this needs to be configurable.

My suggestion would be a header-size field in the system config and a guint64 header_size in RaucSlot, similar to how region_size works.


g_return_val_if_fail(image, FALSE);
g_return_val_if_fail(dest_slot, FALSE);
g_return_val_if_fail(error == NULL || *error == NULL, FALSE);

/* run slot pre install hook if enabled */
if (hook_name && image->hooks.pre_install) {
res = run_slot_hook(hook_name, R_SLOT_HOOK_PRE_INSTALL, image, dest_slot, &ierror);
if (!res) {
g_propagate_error(error, ierror);
goto out;
}
}

devices = g_strsplit(dest_slot->device, ":", -1);
if (g_strv_length(devices) != 2) {
g_set_error(error, R_UPDATE_ERROR, R_UPDATE_ERROR_FAILED,
"Slot device string does not contain two comma separated devices. Aborting!");
goto out;
}

g_message("Executing nor fallback handler on slot %s with primary device %s and fallback device %s",
dest_slot->name, devices[0], devices[1]);

/* If the primary partition is not fully programmed, it most likely means that the fallback
* partition was used to boot and is therefore valid. To avoid ending up with two broken partitions,
* upgrade the primary partition first.
*
* We are only checking the header area, as it is the last thing written.
*/
res = check_if_area_is_clear(devices[0], 0, header_size, &primary_clear, &ierror);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compared to block devices, NOR flash doesn't have atomic writes for 512 byte blocks. That results in some additional cases that need to be handled.

  • Power loss while writing the primary header (after successfully writing the secondary image)
  • Primary header is incomplete, but not clear
  • Secondary image is used for booting
  • Second update attempt erases secondary image, as primary header is not clear ⇒ temporarily no valid image

This means we'd need a better check to detect if the header is incomplete/corrupt. For simple cases, it might be enough if the last byte of the header region is != 0.

  • Power loss while writing the primary header (after successfully writing the secondary image)
  • Primary header is incomplete, but seems valid to the ROM code
  • Boot fails

This depends on the SoC: For headers with a proper checksum, this shouldn't be a problem. If not it probably needs SoC specific research on how it actually behaves for partially written headers.

An alternative solution would be to keep some state in the RAUC data partition, to be able to detect the different cases.

If these corner-cases are acceptable in your use case, I'd also be OK with only the current approach of checking if the header is clear. In that case, there needs to be a big warning in the documentation, though.

if (!res) {
g_set_error(error, R_UPDATE_ERROR, R_UPDATE_ERROR_FAILED,
"Failed to check area at %"G_GUINT64_FORMAT " on %s",
dest_slot->region_start, dest_slot->device);
goto out;
}

if (primary_clear) {
g_message("Updating the primary partition first as it is empty, so we assume we booted from fallback.");
first_part_desc_index++;
}

for (gint i = 0; i < 2; i++) {
gint part_index = (first_part_desc_index + i) % 2;

g_message("Updating %s partition on %s", part_names[part_index], devices[part_index]);

/* erase */
g_message("Erasing slot device %s", devices[part_index]);
res = flash_format_slot(devices[part_index], &ierror);
if (!res) {
g_propagate_error(error, ierror);
goto out;
}

/* TODO: This write should write the first sector last,
* unfortunately flashcp does not support offsets.
*/
/* write */
g_message("Writing slot device %s", devices[part_index]);
res = nor_write_slot(image->filename, devices[part_index], header_size, &ierror);
if (!res) {
g_propagate_error(error, ierror);
goto out;
}
}

/* run slot post install hook if enabled */
if (hook_name && image->hooks.post_install) {
res = run_slot_hook(hook_name, R_SLOT_HOOK_POST_INSTALL, image, dest_slot, &ierror);
if (!res) {
g_propagate_error(error, ierror);
goto out;
}
}

out:
if (devices != NULL) {
g_strfreev(devices);
}
return res;
}

static gboolean img_to_boot_raw_fallback_handler(RaucImage *image, RaucSlot *dest_slot, const gchar *hook_name, GError **error)
{
GError *ierror = NULL;
Expand Down Expand Up @@ -2754,6 +2855,8 @@ RaucUpdatePair updatepairs[] = {
{"*", "boot-gpt-switch", NULL},
{"*.img", "boot-raw-fallback", img_to_boot_raw_fallback_handler},
{"*", "boot-raw-fallback", NULL},
{"*.img", "boot-nor-fallback", img_to_boot_nor_fallback_handler},
{"*", "boot-nor-fallback", NULL},
{"*.img.caibx", "*", img_to_raw_handler}, /* fallback */
{"*.img", "*", img_to_raw_handler}, /* fallback */
{0}
Expand Down
Loading