[PATCH 06/16] Add new fail-safe mode flag
60 views
Skip to first unread message
Andreas J. Reichel
May 25, 2018, 8:05:41 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
Add a flag that marks an environment as a fail-safe environment.
Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
include/envdata.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/envdata.h b/include/envdata.h
index d1e5980..d2fa6bf 100644
--- a/include/envdata.h
+++ b/include/envdata.h
@@ -21,6 +21,7 @@
#define CONFIG_PARTITION_MAXCOUNT 64
#define ENV_STATUS_IN_PROGRESS 1
+#define ENV_STATUS_FAILSAFE 128
#define USTATE_OK 0
#define USTATE_INSTALLED 1
--
2.17.0
Add a flag that marks an environment as a fail-safe environment.
Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
include/envdata.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/envdata.h b/include/envdata.h
index d1e5980..d2fa6bf 100644
--- a/include/envdata.h
+++ b/include/envdata.h
@@ -21,6 +21,7 @@
#define CONFIG_PARTITION_MAXCOUNT 64
#define ENV_STATUS_IN_PROGRESS 1
+#define ENV_STATUS_FAILSAFE 128
#define USTATE_OK 0
#define USTATE_INSTALLED 1
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:41 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
To compare device paths it is necessary to be able to split of
sub-paths from the right hand side. Implement a simple function
for that purpose.
include/utils.h | 1 +
utils.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/include/utils.h b/include/utils.h
index 285e45e..06721eb 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -29,6 +29,7 @@ typedef struct _VOLUME_DESC {
typedef enum { DOSFSLABEL, CUSTOMLABEL, NOLABEL } LABELMODE;
+CHAR16 *StrRStrip(CHAR16 *input, CHAR16 delim);
uint32_t calc_crc32(void *data, int32_t size);
void __noreturn error_exit(CHAR16 *message, EFI_STATUS status);
VOID sleep(int32_t sec);
diff --git a/utils.c b/utils.c
index 33e3a90..4f44a37 100644
--- a/utils.c
+++ b/utils.c
@@ -16,6 +16,29 @@
#include <bootguard.h>
#include <utils.h>
+CHAR16 *StrRStrip(CHAR16 *input, CHAR16 delim)
+{
+ CHAR16 *dst;
+ UINTN len;
+
+ len = StrLen(input);
+ dst = mmalloc((len + 1) * 2);
+ if (!dst) {
+ return NULL;
+ }
+
+ StrCpy(dst, input);
+
+ for (UINTN i = len; i > 0; i--)
+ {
+ if (dst[i * 2] == delim) {
+ dst[i * 2] = L'\0';
+ break;
+ }
+ }
+ return dst;
+}
+
uint32_t calc_crc32(void *data, int32_t size)
{
uint32_t crc;
--
2.17.0
To compare device paths it is necessary to be able to split of
sub-paths from the right hand side. Implement a simple function
for that purpose.
include/utils.h | 1 +
utils.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/include/utils.h b/include/utils.h
index 285e45e..06721eb 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -29,6 +29,7 @@ typedef struct _VOLUME_DESC {
typedef enum { DOSFSLABEL, CUSTOMLABEL, NOLABEL } LABELMODE;
+CHAR16 *StrRStrip(CHAR16 *input, CHAR16 delim);
uint32_t calc_crc32(void *data, int32_t size);
void __noreturn error_exit(CHAR16 *message, EFI_STATUS status);
VOID sleep(int32_t sec);
diff --git a/utils.c b/utils.c
index 33e3a90..4f44a37 100644
--- a/utils.c
+++ b/utils.c
@@ -16,6 +16,29 @@
#include <bootguard.h>
#include <utils.h>
+CHAR16 *StrRStrip(CHAR16 *input, CHAR16 delim)
+{
+ CHAR16 *dst;
+ UINTN len;
+
+ len = StrLen(input);
+ dst = mmalloc((len + 1) * 2);
+ if (!dst) {
+ return NULL;
+ }
+
+ StrCpy(dst, input);
+
+ for (UINTN i = len; i > 0; i--)
+ {
+ if (dst[i * 2] == delim) {
+ dst[i * 2] = L'\0';
+ break;
+ }
+ }
+ return dst;
+}
+
uint32_t calc_crc32(void *data, int32_t size)
{
uint32_t crc;
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:41 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
During environment enumeration, notify if it is on the boot device
utils.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/utils.c b/utils.c
index 0477cbc..c36f428 100644
--- a/utils.c
+++ b/utils.c
@@ -198,6 +198,9 @@ EFI_STATUS get_volumes(VOLUME_DESC **volumes, UINTN *count)
Print(L"Volume %d: %s, LABEL=%s, CLABEL=%s\n", rootCount,
DevicePathToStr(devpath), (*volumes)[rootCount].fslabel,
(*volumes)[rootCount].fscustomlabel);
+ if (IsOnBootDevice(devpath)) {
+ Print(L"(On the boot device)\n");
+ }
rootCount++;
}
*count = rootCount;
--
2.17.0
During environment enumeration, notify if it is on the boot device
utils.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/utils.c b/utils.c
index 0477cbc..c36f428 100644
--- a/utils.c
+++ b/utils.c
@@ -198,6 +198,9 @@ EFI_STATUS get_volumes(VOLUME_DESC **volumes, UINTN *count)
Print(L"Volume %d: %s, LABEL=%s, CLABEL=%s\n", rootCount,
DevicePathToStr(devpath), (*volumes)[rootCount].fslabel,
(*volumes)[rootCount].fscustomlabel);
+ if (IsOnBootDevice(devpath)) {
+ Print(L"(On the boot device)\n");
+ }
rootCount++;
}
*count = rootCount;
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:41 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
Sometimes it is of advantage to have time to read the output
before the OS is booted. Implement a simple sleep function.
include/utils.h | 1 +
utils.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/include/utils.h b/include/utils.h
index a73c554..285e45e 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -31,6 +31,7 @@ typedef enum { DOSFSLABEL, CUSTOMLABEL, NOLABEL } LABELMODE;
uint32_t calc_crc32(void *data, int32_t size);
void __noreturn error_exit(CHAR16 *message, EFI_STATUS status);
+VOID sleep(int32_t sec);
VOID *mmalloc(UINTN bytes);
EFI_STATUS mfree(VOID *p);
CHAR16 *get_volume_label(EFI_FILE_HANDLE fh);
diff --git a/utils.c b/utils.c
index 9d3eb4d..33e3a90 100644
--- a/utils.c
+++ b/utils.c
@@ -32,6 +32,11 @@ void __noreturn error_exit(CHAR16 *message, EFI_STATUS status)
unreachable();
}
+VOID sleep(int32_t sec)
+{
+ uefi_call_wrapper(BS->Stall, 1, sec * 1000 * 1000);
+}
+
VOID *mmalloc(UINTN bytes)
{
EFI_STATUS status;
--
2.17.0
Sometimes it is of advantage to have time to read the output
before the OS is booted. Implement a simple sleep function.
include/utils.h | 1 +
utils.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/include/utils.h b/include/utils.h
index a73c554..285e45e 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -31,6 +31,7 @@ typedef enum { DOSFSLABEL, CUSTOMLABEL, NOLABEL } LABELMODE;
uint32_t calc_crc32(void *data, int32_t size);
void __noreturn error_exit(CHAR16 *message, EFI_STATUS status);
VOID *mmalloc(UINTN bytes);
EFI_STATUS mfree(VOID *p);
CHAR16 *get_volume_label(EFI_FILE_HANDLE fh);
diff --git a/utils.c b/utils.c
index 9d3eb4d..33e3a90 100644
--- a/utils.c
+++ b/utils.c
@@ -32,6 +32,11 @@ void __noreturn error_exit(CHAR16 *message, EFI_STATUS status)
unreachable();
}
+VOID sleep(int32_t sec)
+{
+ uefi_call_wrapper(BS->Stall, 1, sec * 1000 * 1000);
+}
+
VOID *mmalloc(UINTN bytes)
{
EFI_STATUS status;
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:41 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
Code was wrong in trying to boot if additional config partitions
are added to the system. This can result in unwanted behavior.
If less are detected, try to boot, but if more are detected, stop.
env/fatvars.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/env/fatvars.c b/env/fatvars.c
index 4b87f55..bbff777 100644
--- a/env/fatvars.c
+++ b/env/fatvars.c
@@ -105,13 +105,18 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
return BG_CONFIG_ERROR;
}
- if (numHandles != ENV_NUM_CONFIG_PARTS) {
- Print(L"Warning, unexpected number of config partitions: found "
+ if (numHandles > ENV_NUM_CONFIG_PARTS) {
+ Print(L"Error, too many config partitions found. Aborting.\n");
+ mfree(roots);
+ return BG_CONFIG_ERROR;
+ }
+
+ if (numHandles < ENV_NUM_CONFIG_PARTS) {
+ Print(L"Warning, too few config partitions: found: "
L"%d, but expected %d.\n",
numHandles, ENV_NUM_CONFIG_PARTS);
/* Don't treat this as error because we may still be able to
- * find a
- * valid config */
+ * find a valid config */
result = BG_CONFIG_PARTIALLY_CORRUPTED;
}
--
2.17.0
Code was wrong in trying to boot if additional config partitions
are added to the system. This can result in unwanted behavior.
If less are detected, try to boot, but if more are detected, stop.
env/fatvars.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/env/fatvars.c b/env/fatvars.c
index 4b87f55..bbff777 100644
--- a/env/fatvars.c
+++ b/env/fatvars.c
@@ -105,13 +105,18 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
return BG_CONFIG_ERROR;
}
- if (numHandles != ENV_NUM_CONFIG_PARTS) {
- Print(L"Warning, unexpected number of config partitions: found "
+ if (numHandles > ENV_NUM_CONFIG_PARTS) {
+ Print(L"Error, too many config partitions found. Aborting.\n");
+ mfree(roots);
+ return BG_CONFIG_ERROR;
+ }
+
+ if (numHandles < ENV_NUM_CONFIG_PARTS) {
+ Print(L"Warning, too few config partitions: found: "
L"%d, but expected %d.\n",
numHandles, ENV_NUM_CONFIG_PARTS);
/* Don't treat this as error because we may still be able to
- * find a
- * valid config */
+ * find a valid config */
result = BG_CONFIG_PARTIALLY_CORRUPTED;
}
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:41 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
This patch series implements the new fail-safe mode.
As discussed on the mailing list, if fail-safe mode is enabled, only
environments on the boot-device are used. If not, all environments
found are used.
Fail-safe mode is enabled by setting the corresponding flag in ANY
environment on the boot medium.
This series does the following:
* Add some support functions
* Rename in_progress to env_status and reimplement in_progress
from uint8_t to a single bit within this new env_status
register.
* Add new failsafe flag into the env_status register.
* Fix some odd return values in bgenv_set / get, which
depended on the wrong value given
* Improve some error handling code
* Make travis-ci work again
* Update documentation
Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
Andreas Reichel (16):
utils: Add sleep function for debugging purpose
Add StrRStrip function for device path manipulation
Add check if environment is on boot device
Print information if environment is on boot dev
Change implementation of in_progress to flag
Add new fail-safe mode flag
Correct config logic to NOT boot if too many partitions found
New functions for config file open/close/read
Implement fail-safe mode as config filter
Simplify config partition management
Fix return values of bg_setenv with negative values
Add getter/setter for ENV_STATUS_FAILSAFE flag
bg_setenv: Add option S to set fail-safe mode
bg_printenv: Display env_status_flags
travis-ci: Update .travis.yml to run apt-get update
Update documentation
.travis.yml | 1 +
bootguard.c | 1 +
docs/FAILSAFE.md | 10 +++
docs/UPDATE.md | 6 +-
env/env_api.c | 8 +--
env/env_api_fat.c | 46 ++++++++++++--
env/fatvars.c | 116 +++++++++++++++++++---------------
env/syspart.c | 90 +++++++++++++++++++++++---
include/env_api.h | 1 +
include/envdata.h | 5 +-
include/syspart.h | 7 +-
include/utils.h | 3 +
main.c | 6 ++
tools/bg_setenv.c | 20 +++++-
tools/tests/test_ebgenv_api.c | 6 +-
utils.c | 44 +++++++++++++
16 files changed, 289 insertions(+), 81 deletions(-)
create mode 100644 docs/FAILSAFE.md
--
2.17.0
This patch series implements the new fail-safe mode.
As discussed on the mailing list, if fail-safe mode is enabled, only
environments on the boot-device are used. If not, all environments
found are used.
Fail-safe mode is enabled by setting the corresponding flag in ANY
environment on the boot medium.
This series does the following:
* Add some support functions
* Rename in_progress to env_status and reimplement in_progress
from uint8_t to a single bit within this new env_status
register.
* Add new failsafe flag into the env_status register.
* Fix some odd return values in bgenv_set / get, which
depended on the wrong value given
* Improve some error handling code
* Make travis-ci work again
* Update documentation
Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
Andreas Reichel (16):
utils: Add sleep function for debugging purpose
Add StrRStrip function for device path manipulation
Add check if environment is on boot device
Print information if environment is on boot dev
Change implementation of in_progress to flag
Add new fail-safe mode flag
Correct config logic to NOT boot if too many partitions found
New functions for config file open/close/read
Implement fail-safe mode as config filter
Simplify config partition management
Fix return values of bg_setenv with negative values
Add getter/setter for ENV_STATUS_FAILSAFE flag
bg_setenv: Add option S to set fail-safe mode
bg_printenv: Display env_status_flags
travis-ci: Update .travis.yml to run apt-get update
Update documentation
.travis.yml | 1 +
bootguard.c | 1 +
docs/FAILSAFE.md | 10 +++
docs/UPDATE.md | 6 +-
env/env_api.c | 8 +--
env/env_api_fat.c | 46 ++++++++++++--
env/fatvars.c | 116 +++++++++++++++++++---------------
env/syspart.c | 90 +++++++++++++++++++++++---
include/env_api.h | 1 +
include/envdata.h | 5 +-
include/syspart.h | 7 +-
include/utils.h | 3 +
main.c | 6 ++
tools/bg_setenv.c | 20 +++++-
tools/tests/test_ebgenv_api.c | 6 +-
utils.c | 44 +++++++++++++
16 files changed, 289 insertions(+), 81 deletions(-)
create mode 100644 docs/FAILSAFE.md
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:41 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
Get the boot device out of the loaded image's handle and add a
function to compare every environment's device path with this boot
device.
bootguard.c | 1 +
include/utils.h | 1 +
main.c | 6 ++++++
utils.c | 13 +++++++++++++
4 files changed, 21 insertions(+)
diff --git a/bootguard.c b/bootguard.c
index 0aa95b4..9eea350 100644
--- a/bootguard.c
+++ b/bootguard.c
@@ -18,3 +18,4 @@ EFI_HANDLE this_image;
VOLUME_DESC *volumes = NULL;
UINTN volume_count = 128;
+CHAR16 *boot_device_path;
diff --git a/include/utils.h b/include/utils.h
index 06721eb..3c97bfa 100644
--- a/include/utils.h
+++ b/include/utils.h
CHAR16 *StrRStrip(CHAR16 *input, CHAR16 delim);
index 3789319..f1cbf3c 100644
--- a/main.c
+++ b/main.c
@@ -24,6 +24,7 @@
extern const unsigned long init_array_start[];
extern const unsigned long init_array_end[];
+extern CHAR16 *boot_device_path;
static EFI_STATUS probe_watchdog(EFI_LOADED_IMAGE *loaded_image,
EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,
@@ -123,6 +124,11 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)
status);
}
+ boot_device_path = DevicePathToStr(DevicePathFromHandle(
+ loaded_image->DeviceHandle));
+ boot_device_path = StrRStrip(boot_device_path, L'/');
+ Print(L"Boot device: %s\n", boot_device_path);
+
status = get_volumes(&volumes, &volume_count);
if (EFI_ERROR(status)) {
error_exit(L"Could not get volumes installed on system.\n",
diff --git a/utils.c b/utils.c
index 4f44a37..0477cbc 100644
--- a/utils.c
+++ b/utils.c
@@ -16,6 +16,19 @@
#include <bootguard.h>
#include <utils.h>
+BOOLEAN IsOnBootDevice(EFI_DEVICE_PATH *dp)
+{
+ extern CHAR16 *boot_device_path;
+ CHAR16 *device_path;
+
+ device_path = DevicePathToStr(dp);
+ device_path = StrRStrip(device_path, L'/');
+ if (StrCmp(device_path, boot_device_path) == 0) {
+ return TRUE;
+ }
+ return FALSE;
+}
+
CHAR16 *StrRStrip(CHAR16 *input, CHAR16 delim)
{
CHAR16 *dst;
--
2.17.0
Get the boot device out of the loaded image's handle and add a
function to compare every environment's device path with this boot
device.
bootguard.c | 1 +
include/utils.h | 1 +
main.c | 6 ++++++
utils.c | 13 +++++++++++++
4 files changed, 21 insertions(+)
diff --git a/bootguard.c b/bootguard.c
index 0aa95b4..9eea350 100644
--- a/bootguard.c
+++ b/bootguard.c
@@ -18,3 +18,4 @@ EFI_HANDLE this_image;
VOLUME_DESC *volumes = NULL;
UINTN volume_count = 128;
+CHAR16 *boot_device_path;
diff --git a/include/utils.h b/include/utils.h
index 06721eb..3c97bfa 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -29,6 +29,7 @@ typedef struct _VOLUME_DESC {
typedef enum { DOSFSLABEL, CUSTOMLABEL, NOLABEL } LABELMODE;
+BOOLEAN IsOnBootDevice(EFI_DEVICE_PATH *dp);
CHAR16 *StrRStrip(CHAR16 *input, CHAR16 delim);
uint32_t calc_crc32(void *data, int32_t size);
void __noreturn error_exit(CHAR16 *message, EFI_STATUS status);
diff --git a/main.c b/main.c
void __noreturn error_exit(CHAR16 *message, EFI_STATUS status);
index 3789319..f1cbf3c 100644
--- a/main.c
+++ b/main.c
@@ -24,6 +24,7 @@
extern const unsigned long init_array_start[];
extern const unsigned long init_array_end[];
+extern CHAR16 *boot_device_path;
static EFI_STATUS probe_watchdog(EFI_LOADED_IMAGE *loaded_image,
EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,
@@ -123,6 +124,11 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)
status);
}
+ boot_device_path = DevicePathToStr(DevicePathFromHandle(
+ loaded_image->DeviceHandle));
+ boot_device_path = StrRStrip(boot_device_path, L'/');
+ Print(L"Boot device: %s\n", boot_device_path);
+
status = get_volumes(&volumes, &volume_count);
if (EFI_ERROR(status)) {
error_exit(L"Could not get volumes installed on system.\n",
diff --git a/utils.c b/utils.c
index 4f44a37..0477cbc 100644
--- a/utils.c
+++ b/utils.c
@@ -16,6 +16,19 @@
#include <bootguard.h>
#include <utils.h>
+BOOLEAN IsOnBootDevice(EFI_DEVICE_PATH *dp)
+{
+ extern CHAR16 *boot_device_path;
+ CHAR16 *device_path;
+
+ device_path = DevicePathToStr(dp);
+ device_path = StrRStrip(device_path, L'/');
+ if (StrCmp(device_path, boot_device_path) == 0) {
+ return TRUE;
+ }
+ return FALSE;
+}
+
CHAR16 *StrRStrip(CHAR16 *input, CHAR16 delim)
{
CHAR16 *dst;
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:41 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
For future implementation of a fail-safe mechanism an additional flag is
needed. Since 'in_progress' only needs one bit, the meaning of the
internal 'in_progress' variable is changed to a flags register and
'in_progress' is represented by the flag setting the LSB.
This way 7 additional flags can be implemented without the need for
additional variables.
env/env_api.c | 8 ++++----
env/env_api_fat.c | 16 +++++++++++++---
env/fatvars.c | 2 +-
include/envdata.h | 4 +++-
tools/bg_setenv.c | 5 ++++-
tools/tests/test_ebgenv_api.c | 6 ++++--
6 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/env/env_api.c b/env/env_api.c
index d6f1795..6cf6449 100644
--- a/env/env_api.c
+++ b/env/env_api.c
@@ -74,7 +74,7 @@ int ebg_env_create_new(ebgenv_t *e)
BG_ENVDATA *latest_data = ((BGENV *)latest_env)->data;
- if (latest_data->in_progress != 1) {
+ if (!(latest_data->status_flags & ENV_STATUS_IN_PROGRESS)) {
e->bgenv = (void *)bgenv_create_new();
if (!e->bgenv) {
bgenv_close(latest_env);
@@ -82,10 +82,10 @@ int ebg_env_create_new(ebgenv_t *e)
}
BG_ENVDATA *new_data = ((BGENV *)e->bgenv)->data;
uint32_t new_rev = new_data->revision;
- uint8_t new_in_progress = new_data->in_progress;
+ uint8_t new_status_flags = new_data->status_flags;
memcpy(new_data, latest_env->data, sizeof(BG_ENVDATA));
new_data->revision = new_rev;
- new_data->in_progress = new_in_progress;
+ new_data->status_flags = new_status_flags;
bgenv_close(latest_env);
} else {
e->bgenv = latest_env;
@@ -289,7 +289,7 @@ int ebg_env_finalize_update(ebgenv_t *e)
pgci = &tmp;
}
- ((BGENV *)e->bgenv)->data->in_progress = 0;
+ ((BGENV *)e->bgenv)->data->status_flags &= ~ENV_STATUS_IN_PROGRESS;
((BGENV *)e->bgenv)->data->ustate = USTATE_INSTALLED;
return 0;
}
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 378b20e..c51eec5 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -319,7 +319,8 @@ int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
USERVAR_TYPE_UINT8);
case EBGENV_IN_PROGRESS:
return bgenv_get_uint(buffer, type, data,
- env->data->in_progress,
+ env->data->status_flags &
+ ENV_STATUS_IN_PROGRESS,
USERVAR_TYPE_UINT8);
default:
if (!data) {
@@ -399,7 +400,16 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
if (val < 0) {
return val;
}
- env->data->in_progress = val;
+ switch(val) {
+ case 1:
+ env->data->status_flags |= ENV_STATUS_IN_PROGRESS;
+ break;
+ case 0:
+ env->data->status_flags &= ~ENV_STATUS_IN_PROGRESS;
+ break;
+ default:
+ return -EINVAL;
+ }
break;
default:
return -EINVAL;
@@ -432,7 +442,7 @@ BGENV *bgenv_create_new(void)
memset(env_new->data, 0, sizeof(BG_ENVDATA));
/* update revision field and testing mode */
env_new->data->revision = new_rev;
- env_new->data->in_progress = 1;
+ env_new->data->status_flags = ENV_STATUS_IN_PROGRESS;
/* set default watchdog timeout */
env_new->data->watchdog_timeout_sec = 30;
diff --git a/env/fatvars.c b/env/fatvars.c
index 0645af8..4b87f55 100644
--- a/env/fatvars.c
+++ b/env/fatvars.c
@@ -195,7 +195,7 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
/* Test if this environment is currently 'in_progress'. If yes,
* do not boot from it, instead ignore it */
- if (env[latest_idx].in_progress == 1) {
+ if (env[latest_idx].status_flags & ENV_STATUS_IN_PROGRESS) {
current_partition = pre_latest_idx;
} else if (env[latest_idx].ustate == USTATE_TESTING) {
/* If it has already been booted, this indicates a failed
diff --git a/include/envdata.h b/include/envdata.h
index 25bd6c5..d1e5980 100644
--- a/include/envdata.h
+++ b/include/envdata.h
@@ -20,6 +20,8 @@
#define CONFIG_PARTITION_MAXCOUNT 64
+#define ENV_STATUS_IN_PROGRESS 1
+
@@ -36,7 +38,7 @@
struct _BG_ENVDATA {
uint16_t kernelfile[ENV_STRING_LENGTH];
uint16_t kernelparams[ENV_STRING_LENGTH];
- uint8_t in_progress;
+ uint8_t status_flags;
uint8_t ustate;
uint16_t watchdog_timeout_sec;
uint32_t revision;
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index 5d8e9b8..cae268d 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -475,9 +475,12 @@ static void dump_uservars(uint8_t *udata)
static void dump_env(BG_ENVDATA *env)
{
char buffer[ENV_STRING_LENGTH];
+ bool in_progress;
+
+ in_progress = env->status_flags & ENV_STATUS_IN_PROGRESS;
fprintf(stdout, "Values:\n");
fprintf(stdout,
- "in_progress: %s\n",env->in_progress ? "yes" : "no");
+ "in_progress: %s\n", in_progress ? "yes" : "no");
fprintf(stdout, "revision: %u\n", env->revision);
fprintf(stdout,
"kernel: %s\n", str16to8(buffer, env->kernelfile));
diff --git a/tools/tests/test_ebgenv_api.c b/tools/tests/test_ebgenv_api.c
index f7a3410..54d2441 100644
--- a/tools/tests/test_ebgenv_api.c
+++ b/tools/tests/test_ebgenv_api.c
@@ -130,7 +130,8 @@ START_TEST(ebgenv_api_ebg_env_create_new)
ck_assert(((BGENV *)e.bgenv)->data == &envdata[0]);
- ck_assert_int_eq(((BGENV *)e.bgenv)->data->in_progress, 1);
+ ck_assert((((BGENV *)e.bgenv)->data->status_flags &
+ ENV_STATUS_IN_PROGRESS) != 0);
ck_assert_int_eq(
((BGENV *)e.bgenv)->data->revision, ENV_NUM_CONFIG_PARTS+1);
@@ -157,7 +158,8 @@ START_TEST(ebgenv_api_ebg_env_create_new)
ck_assert_int_eq(ret, 0);
ck_assert_int_eq(((BGENV *)e.bgenv)->data->ustate, USTATE_INSTALLED);
- ck_assert_int_eq(((BGENV *)e.bgenv)->data->in_progress, 0);
+ ck_assert((((BGENV *)e.bgenv)->data->status_flags &
+ ENV_STATUS_IN_PROGRESS) == 0);
}
END_TEST
--
2.17.0
For future implementation of a fail-safe mechanism an additional flag is
needed. Since 'in_progress' only needs one bit, the meaning of the
internal 'in_progress' variable is changed to a flags register and
'in_progress' is represented by the flag setting the LSB.
This way 7 additional flags can be implemented without the need for
additional variables.
env/env_api.c | 8 ++++----
env/env_api_fat.c | 16 +++++++++++++---
env/fatvars.c | 2 +-
include/envdata.h | 4 +++-
tools/bg_setenv.c | 5 ++++-
tools/tests/test_ebgenv_api.c | 6 ++++--
6 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/env/env_api.c b/env/env_api.c
index d6f1795..6cf6449 100644
--- a/env/env_api.c
+++ b/env/env_api.c
@@ -74,7 +74,7 @@ int ebg_env_create_new(ebgenv_t *e)
BG_ENVDATA *latest_data = ((BGENV *)latest_env)->data;
- if (latest_data->in_progress != 1) {
+ if (!(latest_data->status_flags & ENV_STATUS_IN_PROGRESS)) {
e->bgenv = (void *)bgenv_create_new();
if (!e->bgenv) {
bgenv_close(latest_env);
@@ -82,10 +82,10 @@ int ebg_env_create_new(ebgenv_t *e)
}
BG_ENVDATA *new_data = ((BGENV *)e->bgenv)->data;
uint32_t new_rev = new_data->revision;
- uint8_t new_in_progress = new_data->in_progress;
+ uint8_t new_status_flags = new_data->status_flags;
memcpy(new_data, latest_env->data, sizeof(BG_ENVDATA));
new_data->revision = new_rev;
- new_data->in_progress = new_in_progress;
+ new_data->status_flags = new_status_flags;
bgenv_close(latest_env);
} else {
e->bgenv = latest_env;
@@ -289,7 +289,7 @@ int ebg_env_finalize_update(ebgenv_t *e)
pgci = &tmp;
}
- ((BGENV *)e->bgenv)->data->in_progress = 0;
+ ((BGENV *)e->bgenv)->data->status_flags &= ~ENV_STATUS_IN_PROGRESS;
((BGENV *)e->bgenv)->data->ustate = USTATE_INSTALLED;
return 0;
}
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 378b20e..c51eec5 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -319,7 +319,8 @@ int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
USERVAR_TYPE_UINT8);
case EBGENV_IN_PROGRESS:
return bgenv_get_uint(buffer, type, data,
- env->data->in_progress,
+ env->data->status_flags &
+ ENV_STATUS_IN_PROGRESS,
USERVAR_TYPE_UINT8);
default:
if (!data) {
@@ -399,7 +400,16 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
if (val < 0) {
return val;
}
- env->data->in_progress = val;
+ switch(val) {
+ case 1:
+ env->data->status_flags |= ENV_STATUS_IN_PROGRESS;
+ break;
+ case 0:
+ env->data->status_flags &= ~ENV_STATUS_IN_PROGRESS;
+ break;
+ default:
+ return -EINVAL;
+ }
break;
default:
return -EINVAL;
@@ -432,7 +442,7 @@ BGENV *bgenv_create_new(void)
memset(env_new->data, 0, sizeof(BG_ENVDATA));
/* update revision field and testing mode */
env_new->data->revision = new_rev;
- env_new->data->in_progress = 1;
+ env_new->data->status_flags = ENV_STATUS_IN_PROGRESS;
/* set default watchdog timeout */
env_new->data->watchdog_timeout_sec = 30;
diff --git a/env/fatvars.c b/env/fatvars.c
index 0645af8..4b87f55 100644
--- a/env/fatvars.c
+++ b/env/fatvars.c
@@ -195,7 +195,7 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
/* Test if this environment is currently 'in_progress'. If yes,
* do not boot from it, instead ignore it */
- if (env[latest_idx].in_progress == 1) {
+ if (env[latest_idx].status_flags & ENV_STATUS_IN_PROGRESS) {
current_partition = pre_latest_idx;
} else if (env[latest_idx].ustate == USTATE_TESTING) {
/* If it has already been booted, this indicates a failed
diff --git a/include/envdata.h b/include/envdata.h
index 25bd6c5..d1e5980 100644
--- a/include/envdata.h
+++ b/include/envdata.h
@@ -20,6 +20,8 @@
#define CONFIG_PARTITION_MAXCOUNT 64
+#define ENV_STATUS_IN_PROGRESS 1
+
#define USTATE_OK 0
#define USTATE_INSTALLED 1
#define USTATE_TESTING 2
#define USTATE_INSTALLED 1
@@ -36,7 +38,7 @@
struct _BG_ENVDATA {
uint16_t kernelfile[ENV_STRING_LENGTH];
uint16_t kernelparams[ENV_STRING_LENGTH];
- uint8_t in_progress;
+ uint8_t status_flags;
uint8_t ustate;
uint16_t watchdog_timeout_sec;
uint32_t revision;
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index 5d8e9b8..cae268d 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -475,9 +475,12 @@ static void dump_uservars(uint8_t *udata)
static void dump_env(BG_ENVDATA *env)
{
char buffer[ENV_STRING_LENGTH];
+ bool in_progress;
+
+ in_progress = env->status_flags & ENV_STATUS_IN_PROGRESS;
fprintf(stdout, "Values:\n");
fprintf(stdout,
- "in_progress: %s\n",env->in_progress ? "yes" : "no");
+ "in_progress: %s\n", in_progress ? "yes" : "no");
fprintf(stdout, "revision: %u\n", env->revision);
fprintf(stdout,
"kernel: %s\n", str16to8(buffer, env->kernelfile));
diff --git a/tools/tests/test_ebgenv_api.c b/tools/tests/test_ebgenv_api.c
index f7a3410..54d2441 100644
--- a/tools/tests/test_ebgenv_api.c
+++ b/tools/tests/test_ebgenv_api.c
@@ -130,7 +130,8 @@ START_TEST(ebgenv_api_ebg_env_create_new)
ck_assert(((BGENV *)e.bgenv)->data == &envdata[0]);
- ck_assert_int_eq(((BGENV *)e.bgenv)->data->in_progress, 1);
+ ck_assert((((BGENV *)e.bgenv)->data->status_flags &
+ ENV_STATUS_IN_PROGRESS) != 0);
ck_assert_int_eq(
((BGENV *)e.bgenv)->data->revision, ENV_NUM_CONFIG_PARTS+1);
@@ -157,7 +158,8 @@ START_TEST(ebgenv_api_ebg_env_create_new)
ck_assert_int_eq(ret, 0);
ck_assert_int_eq(((BGENV *)e.bgenv)->data->ustate, USTATE_INSTALLED);
- ck_assert_int_eq(((BGENV *)e.bgenv)->data->in_progress, 0);
+ ck_assert((((BGENV *)e.bgenv)->data->status_flags &
+ ENV_STATUS_IN_PROGRESS) == 0);
}
END_TEST
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:41 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
If a variable like watchdog is set with a negative argument,
bg_setenv returns this negative argument instead of -EINVAL.
Fix this to -EINVAL.
env/env_api_fat.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index c51eec5..2f71094 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -371,7 +371,7 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
case EBGENV_REVISION:
val = bgenv_convert_to_long(value);
if (val < 0) {
- return val;
+ return -EINVAL;
}
env->data->revision = val;
break;
@@ -384,21 +384,21 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
case EBGENV_WATCHDOG_TIMEOUT_SEC:
val = bgenv_convert_to_long(value);
if (val < 0) {
- return val;
+ return -EINVAL;
}
env->data->watchdog_timeout_sec = val;
break;
case EBGENV_USTATE:
val = bgenv_convert_to_long(value);
if (val < 0) {
- return val;
+ return -EINVAL;
}
env->data->ustate = val;
break;
case EBGENV_IN_PROGRESS:
val = bgenv_convert_to_long(value);
if (val < 0) {
- return val;
+ return -EINVAL;
}
switch(val) {
case 1:
--
2.17.0
If a variable like watchdog is set with a negative argument,
bg_setenv returns this negative argument instead of -EINVAL.
Fix this to -EINVAL.
env/env_api_fat.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index c51eec5..2f71094 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -371,7 +371,7 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
case EBGENV_REVISION:
val = bgenv_convert_to_long(value);
if (val < 0) {
- return val;
+ return -EINVAL;
}
env->data->revision = val;
break;
@@ -384,21 +384,21 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
case EBGENV_WATCHDOG_TIMEOUT_SEC:
val = bgenv_convert_to_long(value);
if (val < 0) {
- return val;
+ return -EINVAL;
}
env->data->watchdog_timeout_sec = val;
break;
case EBGENV_USTATE:
val = bgenv_convert_to_long(value);
if (val < 0) {
- return val;
+ return -EINVAL;
}
env->data->ustate = val;
break;
case EBGENV_IN_PROGRESS:
val = bgenv_convert_to_long(value);
if (val < 0) {
- return val;
+ return -EINVAL;
}
switch(val) {
case 1:
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:41 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
If a config partition on the boot device has the fail-safe flag
set, only use environments on the boot device.
env/fatvars.c | 49 ++++++++++++++++++++++++++++++++++---
env/syspart.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++-
include/syspart.h | 7 +++++-
3 files changed, 112 insertions(+), 6 deletions(-)
diff --git a/env/fatvars.c b/env/fatvars.c
index ab1b5c4..cefff0c 100644
--- a/env/fatvars.c
+++ b/env/fatvars.c
@@ -25,6 +25,7 @@ BG_STATUS save_current_config(void)
EFI_STATUS efistatus;
UINTN numHandles = CONFIG_PARTITION_MAXCOUNT;
EFI_FILE_HANDLE *roots;
+ EFI_DEVICE_PATH **root_devices;
roots = (EFI_FILE_HANDLE *)mmalloc(sizeof(EFI_FILE_HANDLE) *
CONFIG_PARTITION_MAXCOUNT);
@@ -34,8 +35,25 @@ BG_STATUS save_current_config(void)
return BG_CONFIG_ERROR;
}
- if (EFI_ERROR(enumerate_cfg_parts(roots, &numHandles))) {
- Print(L"Error, could not enumerate config partitions.");
+ root_devices = (EFI_DEVICE_PATH **)mmalloc(sizeof(EFI_DEVICE_PATH *) *
+ CONFIG_PARTITION_MAXCOUNT);
+ if (!root_devices) {
+ Print(L"Error, could not allocate memory for config partition "
+ L"device paths.\n");
+ Print(L"Error, could not enumerate config partitions.\n");
+ mfree(root_devices);
+ Print(L"Error, could not filter config partitions.\n");
+ mfree(root_devices);
mfree(roots);
return BG_CONFIG_ERROR;
}
@@ -46,6 +64,7 @@ BG_STATUS save_current_config(void)
numHandles, ENV_NUM_CONFIG_PARTS);
/* In case of saving, this must be treated as error, to not
* overwrite another partition's config file. */
+ mfree(root_devices);
mfree(roots);
return BG_CONFIG_ERROR;
}
@@ -57,6 +76,7 @@ BG_STATUS save_current_config(void)
Print(L"Error, could not open environment file on system "
L"partition %d: %r\n",
current_partition, efistatus);
+ mfree(root_devices);
mfree(roots);
return BG_CONFIG_ERROR;
}
@@ -77,6 +97,7 @@ BG_STATUS save_current_config(void)
Print(L"Error, could not close environment config file.\n");
result = BG_CONFIG_ERROR;
}
+ mfree(root_devices);
mfree(roots);
return result;
}
@@ -86,6 +107,7 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
BG_STATUS result = BG_SUCCESS;
UINTN numHandles = CONFIG_PARTITION_MAXCOUNT;
EFI_FILE_HANDLE *roots;
+ EFI_DEVICE_PATH **root_devices;
UINTN i;
int env_invalid[ENV_NUM_CONFIG_PARTS] = {0};
@@ -97,14 +119,32 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
return BG_CONFIG_ERROR;
}
- if (EFI_ERROR(enumerate_cfg_parts(roots, &numHandles))) {
- Print(L"Error, could not enumerate config partitions.");
+ root_devices = (EFI_DEVICE_PATH **)mmalloc(sizeof(EFI_DEVICE_PATH *) *
+ CONFIG_PARTITION_MAXCOUNT);
+ if (!root_devices) {
+ Print(L"Error, could not allocate memory for config partition "
+ L"device paths.\n");
+ Print(L"Error, could not enumerate config partitions.\n");
+ mfree(root_devices);
+ Print(L"Error, could not filter config partitions.\n");
+ mfree(root_devices);
mfree(roots);
return BG_CONFIG_ERROR;
}
if (numHandles > ENV_NUM_CONFIG_PARTS) {
mfree(roots);
return BG_CONFIG_ERROR;
}
@@ -223,6 +263,7 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
Print(L" kernel: %s\n", bglp->payload_path);
Print(L" args: %s\n", bglp->payload_options);
Print(L" timeout: %d seconds\n", bglp->timeout);
+ mfree(root_devices);
mfree(roots);
return result;
}
diff --git a/env/syspart.c b/env/syspart.c
index aa848d0..86628a0 100644
--- a/env/syspart.c
+++ b/env/syspart.c
@@ -37,7 +37,9 @@ EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer)
return uefi_call_wrapper(fh->Read, 3, fh, &readlen, buffer);
}
-EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *numHandles)
+EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots,
+ EFI_DEVICE_PATH **root_devices,
+ UINTN *numHandles)
{
EFI_STATUS status;
UINTN rootCount = 0;
@@ -57,6 +59,7 @@ EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *numHandles)
if (status == EFI_SUCCESS) {
Print(L"Config file found on volume %d.\n", index);
roots[rootCount] = volumes[index].root;
+ root_devices[rootCount] = volumes[index].devpath;
rootCount++;
status = close_cfg_file(volumes[index].root, fh);
if (EFI_ERROR(status)) {
@@ -70,3 +73,60 @@ EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *numHandles)
Print(L"%d config partitions detected.\n", rootCount);
return EFI_SUCCESS;
}
+
+EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
+ EFI_DEVICE_PATH **root_devices, UINTN *numHandles)
+{
+ BOOLEAN only_envs_on_bootdevice = FALSE;
+
+ Print(L"Config filter: \n");
+ for (UINTN index = 0; index < *numHandles; index++) {
+ EFI_FILE_HANDLE fh = NULL;
+ EFI_STATUS status;
+ BG_ENVDATA env;
+
+ status = open_cfg_file(roots[index], &fh, EFI_FILE_MODE_READ);
+ if (EFI_ERROR(status)) {
+ return status;
+ }
+
+ status = read_cfg_file(fh, &env);
+ if (EFI_ERROR(status)) {
+ return status;
+ }
+
+ if (IsOnBootDevice(root_devices[index]) &&
+ (env.status_flags & ENV_STATUS_FAILSAFE)) {
+ only_envs_on_bootdevice = TRUE;
+ };
+
+ status = close_cfg_file(roots[index], fh);
+ if (EFI_ERROR(status)) {
+ return status;
+ }
+ }
+
+ if (!only_envs_on_bootdevice) {
+ // nothing to do
+ return EFI_SUCCESS;
+ }
+
+ Print(L"Fail-Safe Mode enabled.\n");
+ UINTN index = 0;
+ do {
+ if (!IsOnBootDevice(root_devices[index])) {
+ for (UINTN j = index; j < *numHandles-1; j++) {
+ roots[j] = roots[j+1];
+ root_devices[j] = root_devices[j+1];
+ }
+ mfree(roots[*numHandles-1]);
+ mfree(root_devices[*numHandles-1]);
+ (*numHandles)--;
+ Print(L"Filtered Config #%d\n", index);
+ }
+ index++;
+ } while(index < *numHandles);
+
+ Print(L"Remaining Config Partitions: %d\n", *numHandles);
+ return EFI_SUCCESS;
+}
diff --git a/include/syspart.h b/include/syspart.h
index 27caa93..269fa25 100644
--- a/include/syspart.h
+++ b/include/syspart.h
@@ -25,6 +25,11 @@ EFI_STATUS open_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE *fh,
UINT64 mode);
EFI_STATUS close_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE fh);
EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer);
-EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *maxHandles);
+EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots,
+ EFI_DEVICE_PATH **root_devices,
+ UINTN *maxHandles);
+EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
+ EFI_DEVICE_PATH **root_devices,
+ UINTN *maxHandles);
#endif // __H_SYSPART__
--
2.17.0
If a config partition on the boot device has the fail-safe flag
set, only use environments on the boot device.
env/fatvars.c | 49 ++++++++++++++++++++++++++++++++++---
env/syspart.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++-
include/syspart.h | 7 +++++-
3 files changed, 112 insertions(+), 6 deletions(-)
diff --git a/env/fatvars.c b/env/fatvars.c
index ab1b5c4..cefff0c 100644
--- a/env/fatvars.c
+++ b/env/fatvars.c
@@ -25,6 +25,7 @@ BG_STATUS save_current_config(void)
EFI_STATUS efistatus;
UINTN numHandles = CONFIG_PARTITION_MAXCOUNT;
EFI_FILE_HANDLE *roots;
+ EFI_DEVICE_PATH **root_devices;
roots = (EFI_FILE_HANDLE *)mmalloc(sizeof(EFI_FILE_HANDLE) *
CONFIG_PARTITION_MAXCOUNT);
@@ -34,8 +35,25 @@ BG_STATUS save_current_config(void)
return BG_CONFIG_ERROR;
}
- if (EFI_ERROR(enumerate_cfg_parts(roots, &numHandles))) {
- Print(L"Error, could not enumerate config partitions.");
+ root_devices = (EFI_DEVICE_PATH **)mmalloc(sizeof(EFI_DEVICE_PATH *) *
+ CONFIG_PARTITION_MAXCOUNT);
+ if (!root_devices) {
+ Print(L"Error, could not allocate memory for config partition "
+ L"device paths.\n");
+ mfree(roots);
+ return BG_CONFIG_ERROR;
+ }
+
+ if (EFI_ERROR(enumerate_cfg_parts(roots, root_devices, &numHandles))) {
+ return BG_CONFIG_ERROR;
+ }
+
+ Print(L"Error, could not enumerate config partitions.\n");
+ mfree(root_devices);
+ mfree(roots);
+ return BG_CONFIG_ERROR;
+ }
+
+ if (EFI_ERROR(filter_cfg_parts(roots, root_devices, &numHandles))) {
+ return BG_CONFIG_ERROR;
+ }
+
+ Print(L"Error, could not filter config partitions.\n");
+ mfree(root_devices);
mfree(roots);
return BG_CONFIG_ERROR;
}
@@ -46,6 +64,7 @@ BG_STATUS save_current_config(void)
numHandles, ENV_NUM_CONFIG_PARTS);
/* In case of saving, this must be treated as error, to not
* overwrite another partition's config file. */
+ mfree(root_devices);
mfree(roots);
return BG_CONFIG_ERROR;
}
@@ -57,6 +76,7 @@ BG_STATUS save_current_config(void)
Print(L"Error, could not open environment file on system "
L"partition %d: %r\n",
current_partition, efistatus);
+ mfree(root_devices);
mfree(roots);
return BG_CONFIG_ERROR;
}
@@ -77,6 +97,7 @@ BG_STATUS save_current_config(void)
Print(L"Error, could not close environment config file.\n");
result = BG_CONFIG_ERROR;
}
+ mfree(root_devices);
mfree(roots);
return result;
}
@@ -86,6 +107,7 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
BG_STATUS result = BG_SUCCESS;
UINTN numHandles = CONFIG_PARTITION_MAXCOUNT;
EFI_FILE_HANDLE *roots;
+ EFI_DEVICE_PATH **root_devices;
UINTN i;
int env_invalid[ENV_NUM_CONFIG_PARTS] = {0};
@@ -97,14 +119,32 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
return BG_CONFIG_ERROR;
}
- if (EFI_ERROR(enumerate_cfg_parts(roots, &numHandles))) {
- Print(L"Error, could not enumerate config partitions.");
+ root_devices = (EFI_DEVICE_PATH **)mmalloc(sizeof(EFI_DEVICE_PATH *) *
+ CONFIG_PARTITION_MAXCOUNT);
+ if (!root_devices) {
+ Print(L"Error, could not allocate memory for config partition "
+ L"device paths.\n");
+ mfree(roots);
+ return BG_CONFIG_ERROR;
+ }
+
+ if (EFI_ERROR(enumerate_cfg_parts(roots, root_devices, &numHandles))) {
+ return BG_CONFIG_ERROR;
+ }
+
+ Print(L"Error, could not enumerate config partitions.\n");
+ mfree(root_devices);
+ mfree(roots);
+ return BG_CONFIG_ERROR;
+ }
+
+ if (EFI_ERROR(filter_cfg_parts(roots, root_devices, &numHandles))) {
+ return BG_CONFIG_ERROR;
+ }
+
+ Print(L"Error, could not filter config partitions.\n");
+ mfree(root_devices);
mfree(roots);
return BG_CONFIG_ERROR;
}
if (numHandles > ENV_NUM_CONFIG_PARTS) {
Print(L"Error, too many config partitions found. Aborting.\n");
+ mfree(root_devices);
mfree(roots);
return BG_CONFIG_ERROR;
}
@@ -223,6 +263,7 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
Print(L" kernel: %s\n", bglp->payload_path);
Print(L" args: %s\n", bglp->payload_options);
Print(L" timeout: %d seconds\n", bglp->timeout);
+ mfree(root_devices);
mfree(roots);
return result;
}
diff --git a/env/syspart.c b/env/syspart.c
index aa848d0..86628a0 100644
--- a/env/syspart.c
+++ b/env/syspart.c
@@ -37,7 +37,9 @@ EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer)
return uefi_call_wrapper(fh->Read, 3, fh, &readlen, buffer);
}
-EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *numHandles)
+EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots,
+ EFI_DEVICE_PATH **root_devices,
+ UINTN *numHandles)
{
EFI_STATUS status;
UINTN rootCount = 0;
@@ -57,6 +59,7 @@ EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *numHandles)
if (status == EFI_SUCCESS) {
Print(L"Config file found on volume %d.\n", index);
roots[rootCount] = volumes[index].root;
+ root_devices[rootCount] = volumes[index].devpath;
rootCount++;
status = close_cfg_file(volumes[index].root, fh);
if (EFI_ERROR(status)) {
@@ -70,3 +73,60 @@ EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *numHandles)
Print(L"%d config partitions detected.\n", rootCount);
return EFI_SUCCESS;
}
+
+EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
+ EFI_DEVICE_PATH **root_devices, UINTN *numHandles)
+{
+ BOOLEAN only_envs_on_bootdevice = FALSE;
+
+ Print(L"Config filter: \n");
+ for (UINTN index = 0; index < *numHandles; index++) {
+ EFI_FILE_HANDLE fh = NULL;
+ EFI_STATUS status;
+ BG_ENVDATA env;
+
+ status = open_cfg_file(roots[index], &fh, EFI_FILE_MODE_READ);
+ if (EFI_ERROR(status)) {
+ return status;
+ }
+
+ status = read_cfg_file(fh, &env);
+ if (EFI_ERROR(status)) {
+ return status;
+ }
+
+ if (IsOnBootDevice(root_devices[index]) &&
+ (env.status_flags & ENV_STATUS_FAILSAFE)) {
+ only_envs_on_bootdevice = TRUE;
+ };
+
+ status = close_cfg_file(roots[index], fh);
+ if (EFI_ERROR(status)) {
+ return status;
+ }
+ }
+
+ if (!only_envs_on_bootdevice) {
+ // nothing to do
+ return EFI_SUCCESS;
+ }
+
+ Print(L"Fail-Safe Mode enabled.\n");
+ UINTN index = 0;
+ do {
+ if (!IsOnBootDevice(root_devices[index])) {
+ for (UINTN j = index; j < *numHandles-1; j++) {
+ roots[j] = roots[j+1];
+ root_devices[j] = root_devices[j+1];
+ }
+ mfree(roots[*numHandles-1]);
+ mfree(root_devices[*numHandles-1]);
+ (*numHandles)--;
+ Print(L"Filtered Config #%d\n", index);
+ }
+ index++;
+ } while(index < *numHandles);
+
+ Print(L"Remaining Config Partitions: %d\n", *numHandles);
+ return EFI_SUCCESS;
+}
diff --git a/include/syspart.h b/include/syspart.h
index 27caa93..269fa25 100644
--- a/include/syspart.h
+++ b/include/syspart.h
@@ -25,6 +25,11 @@ EFI_STATUS open_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE *fh,
UINT64 mode);
EFI_STATUS close_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE fh);
EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer);
-EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *maxHandles);
+EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots,
+ EFI_DEVICE_PATH **root_devices,
+ UINTN *maxHandles);
+EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
+ EFI_DEVICE_PATH **root_devices,
+ UINTN *maxHandles);
#endif // __H_SYSPART__
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:41 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
For additional config file checks like fail-safe mode, define one
common open/read and close function to simplify code.
env/fatvars.c | 21 +++++++--------------
env/syspart.c | 29 +++++++++++++++++++++++------
include/syspart.h | 4 ++++
3 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/env/fatvars.c b/env/fatvars.c
index bbff777..ab1b5c4 100644
--- a/env/fatvars.c
+++ b/env/fatvars.c
@@ -51,10 +51,8 @@ BG_STATUS save_current_config(void)
}
EFI_FILE_HANDLE fh = NULL;
- efistatus = uefi_call_wrapper(
- roots[current_partition]->Open, 5, roots[current_partition], &fh,
- ENV_FILE_NAME, EFI_FILE_MODE_WRITE | EFI_FILE_MODE_READ,
- EFI_FILE_ARCHIVE | EFI_FILE_HIDDEN | EFI_FILE_SYSTEM);
+ efistatus = open_cfg_file(roots[current_partition], &fh,
+ EFI_FILE_MODE_WRITE | EFI_FILE_MODE_READ);
if (EFI_ERROR(efistatus)) {
result = BG_CONFIG_ERROR;
}
- if (EFI_ERROR(uefi_call_wrapper(fh->Close, 1, fh))) {
+ if (EFI_ERROR(close_cfg_file(roots[current_partition], fh))) {
/* Load all config data */
for (i = 0; i < numHandles; i++) {
EFI_FILE_HANDLE fh = NULL;
- if (EFI_ERROR(uefi_call_wrapper(
- roots[i]->Open, 5, roots[i], &fh, ENV_FILE_NAME,
- EFI_FILE_MODE_READ,
- EFI_FILE_READ_ONLY | EFI_FILE_ARCHIVE |
- EFI_FILE_HIDDEN | EFI_FILE_SYSTEM))) {
+ if (EFI_ERROR(open_cfg_file(roots[i], &fh,
+ EFI_FILE_MODE_READ))) {
Print(L"Warning, could not open environment file on "
L"config partition %d\n",
i);
@@ -135,14 +130,12 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
continue;
}
- UINTN readlen = sizeof(BG_ENVDATA);
- if (EFI_ERROR(uefi_call_wrapper(fh->Read, 3, fh, &readlen,
- (VOID *)&env[i]))) {
+ if (EFI_ERROR(read_cfg_file(fh, (VOID *)&env[i]))) {
Print(L"Error reading environment from config "
L"partition %d.\n",
i);
env_invalid[i] = 1;
- if (EFI_ERROR(uefi_call_wrapper(fh->Close, 1, fh))) {
+ if (EFI_ERROR(close_cfg_file(roots[i], fh))) {
}
diff --git a/env/syspart.c b/env/syspart.c
index 7bdf595..aa848d0 100644
--- a/env/syspart.c
+++ b/env/syspart.c
@@ -14,9 +14,29 @@
#include <syspart.h>
#include <utils.h>
+#include <envdata.h>
#define MAX_INFO_SIZE 1024
+EFI_STATUS open_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE *fh,
+ UINT64 mode)
+{
+ return uefi_call_wrapper(root->Open, 5, root, fh, ENV_FILE_NAME, mode,
+ EFI_FILE_ARCHIVE | EFI_FILE_HIDDEN |
+ EFI_FILE_SYSTEM);
+}
+
+EFI_STATUS close_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE fh)
+{
+ return uefi_call_wrapper(root->Close, 1, fh);
+}
+
+EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer)
+{
+ UINTN readlen = sizeof(BG_ENVDATA);
+ return uefi_call_wrapper(fh->Read, 3, fh, &readlen, buffer);
+}
+
EFI_STATUS status;
@@ -32,16 +52,13 @@ EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *numHandles)
if (!volumes[index].root) {
continue;
}
- status = uefi_call_wrapper(
- volumes[index].root->Open, 5, volumes[index].root, &fh,
- ENV_FILE_NAME, EFI_FILE_MODE_WRITE | EFI_FILE_MODE_READ,
- EFI_FILE_ARCHIVE | EFI_FILE_HIDDEN | EFI_FILE_SYSTEM);
+ status = open_cfg_file(volumes[index].root, &fh,
+ EFI_FILE_MODE_READ);
- status = uefi_call_wrapper(volumes[index].root->Close,
- 1, fh);
+ status = close_cfg_file(volumes[index].root, fh);
if (EFI_ERROR(status)) {
Print(L"Could not close config file on "
L"partition %d.\n",
diff --git a/include/syspart.h b/include/syspart.h
index 2b70ed5..27caa93 100644
--- a/include/syspart.h
+++ b/include/syspart.h
@@ -21,6 +21,10 @@
#include <efipciio.h>
#include "bootguard.h"
+EFI_STATUS open_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE *fh,
+ UINT64 mode);
+EFI_STATUS close_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE fh);
+EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer);
EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *maxHandles);
For additional config file checks like fail-safe mode, define one
common open/read and close function to simplify code.
env/fatvars.c | 21 +++++++--------------
env/syspart.c | 29 +++++++++++++++++++++++------
include/syspart.h | 4 ++++
3 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/env/fatvars.c b/env/fatvars.c
index bbff777..ab1b5c4 100644
--- a/env/fatvars.c
+++ b/env/fatvars.c
@@ -51,10 +51,8 @@ BG_STATUS save_current_config(void)
}
EFI_FILE_HANDLE fh = NULL;
- efistatus = uefi_call_wrapper(
- roots[current_partition]->Open, 5, roots[current_partition], &fh,
- ENV_FILE_NAME, EFI_FILE_MODE_WRITE | EFI_FILE_MODE_READ,
- EFI_FILE_ARCHIVE | EFI_FILE_HIDDEN | EFI_FILE_SYSTEM);
+ efistatus = open_cfg_file(roots[current_partition], &fh,
+ EFI_FILE_MODE_WRITE | EFI_FILE_MODE_READ);
if (EFI_ERROR(efistatus)) {
Print(L"Error, could not open environment file on system "
L"partition %d: %r\n",
@@ -75,7 +73,7 @@ BG_STATUS save_current_config(void)
L"partition %d: %r\n",
result = BG_CONFIG_ERROR;
}
- if (EFI_ERROR(uefi_call_wrapper(fh->Close, 1, fh))) {
+ if (EFI_ERROR(close_cfg_file(roots[current_partition], fh))) {
Print(L"Error, could not close environment config file.\n");
result = BG_CONFIG_ERROR;
}
@@ -123,11 +121,8 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
result = BG_CONFIG_ERROR;
}
/* Load all config data */
for (i = 0; i < numHandles; i++) {
EFI_FILE_HANDLE fh = NULL;
- if (EFI_ERROR(uefi_call_wrapper(
- roots[i]->Open, 5, roots[i], &fh, ENV_FILE_NAME,
- EFI_FILE_MODE_READ,
- EFI_FILE_READ_ONLY | EFI_FILE_ARCHIVE |
- EFI_FILE_HIDDEN | EFI_FILE_SYSTEM))) {
+ if (EFI_ERROR(open_cfg_file(roots[i], &fh,
+ EFI_FILE_MODE_READ))) {
Print(L"Warning, could not open environment file on "
L"config partition %d\n",
i);
@@ -135,14 +130,12 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
continue;
}
- UINTN readlen = sizeof(BG_ENVDATA);
- if (EFI_ERROR(uefi_call_wrapper(fh->Read, 3, fh, &readlen,
- (VOID *)&env[i]))) {
+ if (EFI_ERROR(read_cfg_file(fh, (VOID *)&env[i]))) {
Print(L"Error reading environment from config "
L"partition %d.\n",
i);
env_invalid[i] = 1;
- if (EFI_ERROR(uefi_call_wrapper(fh->Close, 1, fh))) {
+ if (EFI_ERROR(close_cfg_file(roots[i], fh))) {
Print(L"Error, could not close environment "
L"config file.\n");
}
diff --git a/env/syspart.c b/env/syspart.c
index 7bdf595..aa848d0 100644
--- a/env/syspart.c
+++ b/env/syspart.c
@@ -14,9 +14,29 @@
#include <syspart.h>
#include <utils.h>
+#include <envdata.h>
#define MAX_INFO_SIZE 1024
+EFI_STATUS open_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE *fh,
+ UINT64 mode)
+{
+ return uefi_call_wrapper(root->Open, 5, root, fh, ENV_FILE_NAME, mode,
+ EFI_FILE_ARCHIVE | EFI_FILE_HIDDEN |
+ EFI_FILE_SYSTEM);
+}
+
+EFI_STATUS close_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE fh)
+{
+ return uefi_call_wrapper(root->Close, 1, fh);
+}
+
+EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer)
+{
+ UINTN readlen = sizeof(BG_ENVDATA);
+ return uefi_call_wrapper(fh->Read, 3, fh, &readlen, buffer);
+}
+
EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *numHandles)
{
EFI_STATUS status;
@@ -32,16 +52,13 @@ EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *numHandles)
if (!volumes[index].root) {
continue;
}
- status = uefi_call_wrapper(
- volumes[index].root->Open, 5, volumes[index].root, &fh,
- ENV_FILE_NAME, EFI_FILE_MODE_WRITE | EFI_FILE_MODE_READ,
- EFI_FILE_ARCHIVE | EFI_FILE_HIDDEN | EFI_FILE_SYSTEM);
+ status = open_cfg_file(volumes[index].root, &fh,
+ EFI_FILE_MODE_READ);
if (status == EFI_SUCCESS) {
Print(L"Config file found on volume %d.\n", index);
roots[rootCount] = volumes[index].root;
rootCount++;
Print(L"Config file found on volume %d.\n", index);
roots[rootCount] = volumes[index].root;
- status = uefi_call_wrapper(volumes[index].root->Close,
- 1, fh);
+ status = close_cfg_file(volumes[index].root, fh);
if (EFI_ERROR(status)) {
Print(L"Could not close config file on "
L"partition %d.\n",
diff --git a/include/syspart.h b/include/syspart.h
index 2b70ed5..27caa93 100644
--- a/include/syspart.h
+++ b/include/syspart.h
@@ -21,6 +21,10 @@
#include <efipciio.h>
#include "bootguard.h"
+EFI_STATUS open_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE *fh,
+ UINT64 mode);
+EFI_STATUS close_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE fh);
+EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer);
EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots, UINTN *maxHandles);
Andreas J. Reichel
May 25, 2018, 8:05:42 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
Add new enum member for ENV_STATUS_FAILSAFE flag to use bg_setenv
and bg_getenv for it.
env/env_api_fat.c | 24 ++++++++++++++++++++++++
include/env_api.h | 1 +
2 files changed, 25 insertions(+)
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 2f71094..6f759f0 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -43,6 +43,10 @@ EBGENVKEY bgenv_str2enum(char *key)
if (strncmp(key, "in_progress", strlen("in_progress") + 1) == 0) {
return EBGENV_IN_PROGRESS;
}
+ if (strncmp(key, "env_status_failsafe",
+ strlen("env_status_failsafe") + 1) == 0) {
+ return EBGENV_FAILSAFE;
+ }
return EBGENV_UNKNOWN;
}
@@ -322,6 +326,11 @@ int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
env->data->status_flags &
ENV_STATUS_IN_PROGRESS,
USERVAR_TYPE_UINT8);
+ case EBGENV_FAILSAFE:
+ return bgenv_get_uint(buffer, type, data,
+ USERVAR_TYPE_UINT8);
default:
if (!data) {
return 0;
@@ -411,6 +420,21 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
return -EINVAL;
}
break;
+ case EBGENV_FAILSAFE:
+ val = bgenv_convert_to_long(value);
+ if (val < 0) {
+ return val;
+ }
index 45bf4b7..04516bb 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -54,6 +54,7 @@ typedef enum {
EBGENV_REVISION,
EBGENV_USTATE,
EBGENV_IN_PROGRESS,
+ EBGENV_FAILSAFE,
EBGENV_UNKNOWN
} EBGENVKEY;
--
2.17.0
Add new enum member for ENV_STATUS_FAILSAFE flag to use bg_setenv
and bg_getenv for it.
env/env_api_fat.c | 24 ++++++++++++++++++++++++
include/env_api.h | 1 +
2 files changed, 25 insertions(+)
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 2f71094..6f759f0 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -43,6 +43,10 @@ EBGENVKEY bgenv_str2enum(char *key)
if (strncmp(key, "in_progress", strlen("in_progress") + 1) == 0) {
return EBGENV_IN_PROGRESS;
}
+ if (strncmp(key, "env_status_failsafe",
+ strlen("env_status_failsafe") + 1) == 0) {
+ return EBGENV_FAILSAFE;
+ }
return EBGENV_UNKNOWN;
}
@@ -322,6 +326,11 @@ int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
env->data->status_flags &
ENV_STATUS_IN_PROGRESS,
USERVAR_TYPE_UINT8);
+ case EBGENV_FAILSAFE:
+ return bgenv_get_uint(buffer, type, data,
+ env->data->status_flags &
+ ENV_STATUS_FAILSAFE,
+ USERVAR_TYPE_UINT8);
default:
if (!data) {
return 0;
@@ -411,6 +420,21 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
return -EINVAL;
}
break;
+ case EBGENV_FAILSAFE:
+ val = bgenv_convert_to_long(value);
+ if (val < 0) {
+ return val;
+ }
+ switch(val) {
+ case 1:
+ env->data->status_flags |= ENV_STATUS_FAILSAFE;
+ case 1:
+ break;
+ case 0:
+ env->data->status_flags &= ~ENV_STATUS_FAILSAFE;
+ case 0:
+ default:
+ return -EINVAL;
+ }
+ break;
default:
return -EINVAL;
}
diff --git a/include/env_api.h b/include/env_api.h
+ return -EINVAL;
+ }
+ break;
default:
return -EINVAL;
}
index 45bf4b7..04516bb 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -54,6 +54,7 @@ typedef enum {
EBGENV_REVISION,
EBGENV_USTATE,
EBGENV_IN_PROGRESS,
+ EBGENV_FAILSAFE,
EBGENV_UNKNOWN
} EBGENVKEY;
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:42 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
Display flags set in env_status_flags.
tools/bg_setenv.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index 0847de1..8abd015 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -483,8 +483,10 @@ static void dump_env(BG_ENVDATA *env)
{
char buffer[ENV_STRING_LENGTH];
bool in_progress;
+ bool failsafe;
in_progress = env->status_flags & ENV_STATUS_IN_PROGRESS;
+ failsafe = env->status_flags & ENV_STATUS_FAILSAFE;
"watchdog timeout: %u seconds\n", env->watchdog_timeout_sec);
fprintf(stdout, "ustate: %u (%s)\n", (uint8_t)env->ustate,
ustate2str(env->ustate));
- fprintf(stdout, "\n");
+ fprintf(stdout, "flags: ");
+ if (failsafe) {
+ fprintf(stdout, "FAILSAFE ");
+ }
+ fprintf(stdout, "\n\n");
fprintf(stdout, "user variables:\n");
dump_uservars(env->userdata);
fprintf(stdout, "\n\n");
--
2.17.0
Display flags set in env_status_flags.
tools/bg_setenv.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index 0847de1..8abd015 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -483,8 +483,10 @@ static void dump_env(BG_ENVDATA *env)
{
char buffer[ENV_STRING_LENGTH];
bool in_progress;
+ bool failsafe;
in_progress = env->status_flags & ENV_STATUS_IN_PROGRESS;
fprintf(stdout, "Values:\n");
fprintf(stdout,
fprintf(stdout,
"in_progress: %s\n", in_progress ? "yes" : "no");
@@ -497,7 +499,11 @@ static void dump_env(BG_ENVDATA *env)
"watchdog timeout: %u seconds\n", env->watchdog_timeout_sec);
fprintf(stdout, "ustate: %u (%s)\n", (uint8_t)env->ustate,
ustate2str(env->ustate));
- fprintf(stdout, "\n");
+ fprintf(stdout, "flags: ");
+ if (failsafe) {
+ fprintf(stdout, "FAILSAFE ");
+ }
+ fprintf(stdout, "\n\n");
fprintf(stdout, "user variables:\n");
dump_uservars(env->userdata);
fprintf(stdout, "\n\n");
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:42 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
Prevent 404 during apt-get install with apt-get update in before_install
.travis.yml | 1 +
index 8098fe2..9cf215f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -25,6 +25,7 @@ compiler:
sudo: required
before_install:
+ - sudo apt-get update
- if [ $TRAVIS_BRANCH = coverity_scan ] && [ ${TRAVIS_JOB_NUMBER##*.} != 1 ]; then exit 0; fi
- echo -n | openssl s_client -connect scan.coverity.com:444 | sed -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' | sudo tee -a /etc/ssl/certs/ca-certificates.crt
--
2.17.0
Prevent 404 during apt-get install with apt-get update in before_install
.travis.yml | 1 +
1 file changed, 1 insertion(+)
diff --git a/.travis.yml b/.travis.yml
index 8098fe2..9cf215f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -25,6 +25,7 @@ compiler:
sudo: required
before_install:
+ - sudo apt-get update
- if [ $TRAVIS_BRANCH = coverity_scan ] && [ ${TRAVIS_JOB_NUMBER##*.} != 1 ]; then exit 0; fi
- echo -n | openssl s_client -connect scan.coverity.com:444 | sed -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' | sudo tee -a /etc/ssl/certs/ca-certificates.crt
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:42 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
* Update UPDATE.md
* Add new FAILSAFE.md to explain fail-safe mode
docs/FAILSAFE.md | 10 ++++++++++
docs/UPDATE.md | 6 +++---
2 files changed, 13 insertions(+), 3 deletions(-)
create mode 100644 docs/FAILSAFE.md
diff --git a/docs/FAILSAFE.md b/docs/FAILSAFE.md
new file mode 100644
index 0000000..d3d0a66
--- /dev/null
+++ b/docs/FAILSAFE.md
@@ -0,0 +1,10 @@
+# Fail-Safe Mode #
+
+If more than the expected number of environments is detected during boot, the
+system stops booting. This can be a problem if the user wants to boot the
+system with a memory stick to update a broken installation.
+
+In order to allow external boot devices with other environment configurations,
+the FAILSAFE flag was introduced. If any environment on the boot device has the
+ENV_STATUS_FAILSAVE bit set in the `status_flags`, the boot loader will filter
+out all found environments which are NOT on the boot device.
diff --git a/docs/UPDATE.md b/docs/UPDATE.md
index 19c2f39..9f00fd7 100644
--- a/docs/UPDATE.md
+++ b/docs/UPDATE.md
@@ -11,7 +11,7 @@ The structure of the environment data is as follows:
The fields have the following meaning:
* `kernelfile`: Path to the kernel image, utf-16 encoded
* `kernelparams`: Arguments to the kernel, utf-16 encoded
-* `padding`: Padding byte to stay compatible with the offsets of the previous
- version.
+* `status_flags`: Register to store special environment flags. Currently only
+ ENV_STATUS_IN_PROGRESS and ENV_STATUS_FAILSAFE are defined.
* `ustate`: Update status (`0` OK, `1` INSTALLED, `2` TESTING, `3`: FAILED)
* `watchdog_timeout_sec`: Number of seconds, the watchdog times out after
* `revision`: The revision number explained above
--
2.17.0
* Update UPDATE.md
* Add new FAILSAFE.md to explain fail-safe mode
docs/FAILSAFE.md | 10 ++++++++++
docs/UPDATE.md | 6 +++---
2 files changed, 13 insertions(+), 3 deletions(-)
create mode 100644 docs/FAILSAFE.md
diff --git a/docs/FAILSAFE.md b/docs/FAILSAFE.md
new file mode 100644
index 0000000..d3d0a66
--- /dev/null
+++ b/docs/FAILSAFE.md
@@ -0,0 +1,10 @@
+# Fail-Safe Mode #
+
+If more than the expected number of environments is detected during boot, the
+system stops booting. This can be a problem if the user wants to boot the
+system with a memory stick to update a broken installation.
+
+In order to allow external boot devices with other environment configurations,
+the FAILSAFE flag was introduced. If any environment on the boot device has the
+ENV_STATUS_FAILSAVE bit set in the `status_flags`, the boot loader will filter
+out all found environments which are NOT on the boot device.
diff --git a/docs/UPDATE.md b/docs/UPDATE.md
index 19c2f39..9f00fd7 100644
--- a/docs/UPDATE.md
+++ b/docs/UPDATE.md
@@ -11,7 +11,7 @@ The structure of the environment data is as follows:
struct _BG_ENVDATA {
uint16_t kernelfile[ENV_STRING_LENGTH];
uint16_t kernelparams[ENV_STRING_LENGTH];
- uint8_t padding;
uint16_t kernelfile[ENV_STRING_LENGTH];
uint16_t kernelparams[ENV_STRING_LENGTH];
+ uint8_t status_flags;
uint8_t ustate;
uint16_t watchdog_timeout_sec;
uint32_t revision;
@@ -23,8 +23,8 @@ struct _BG_ENVDATA {
uint8_t ustate;
uint16_t watchdog_timeout_sec;
uint32_t revision;
The fields have the following meaning:
* `kernelfile`: Path to the kernel image, utf-16 encoded
* `kernelparams`: Arguments to the kernel, utf-16 encoded
-* `padding`: Padding byte to stay compatible with the offsets of the previous
- version.
+* `status_flags`: Register to store special environment flags. Currently only
+ ENV_STATUS_IN_PROGRESS and ENV_STATUS_FAILSAFE are defined.
* `ustate`: Update status (`0` OK, `1` INSTALLED, `2` TESTING, `3`: FAILED)
* `watchdog_timeout_sec`: Number of seconds, the watchdog times out after
* `revision`: The revision number explained above
--
2.17.0
Andreas J. Reichel
May 25, 2018, 8:05:42 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
* volumes[] already contains all needed data, so instead of
allocating two additional arrays (roots, root_devpaths),
just create a mapping config_volumes[] and use
volumes[config_volumes[]]
* remove CONFIG_PARTITION_MAXCOUNT, since the maximum number of
config partitions is always the number of volumes present
* simplify error-handling in config loading and writing
env/fatvars.c | 121 ++++++++++++++++++----------------------------
env/syspart.c | 27 +++++------
include/envdata.h | 2 -
include/syspart.h | 8 +--
4 files changed, 60 insertions(+), 98 deletions(-)
diff --git a/env/fatvars.c b/env/fatvars.c
index cefff0c..556b836 100644
--- a/env/fatvars.c
+++ b/env/fatvars.c
@@ -21,41 +21,27 @@ static BG_ENVDATA env[ENV_NUM_CONFIG_PARTS];
BG_STATUS save_current_config(void)
{
- BG_STATUS result = BG_SUCCESS;
+ BG_STATUS result = BG_CONFIG_ERROR;
EFI_STATUS efistatus;
- UINTN numHandles = CONFIG_PARTITION_MAXCOUNT;
- EFI_FILE_HANDLE *roots;
- EFI_DEVICE_PATH **root_devices;
+ UINTN numHandles = volume_count;
+ UINTN *config_volumes;
- roots = (EFI_FILE_HANDLE *)mmalloc(sizeof(EFI_FILE_HANDLE) *
- CONFIG_PARTITION_MAXCOUNT);
- if (!roots) {
+ config_volumes = (UINTN *)mmalloc(sizeof(EFI_FILE_HANDLE) *
+ volume_count);
+ if (!config_volumes) {
- return BG_CONFIG_ERROR;
+ L"mapping.\n");
+ return result;
}
- root_devices = (EFI_DEVICE_PATH **)mmalloc(sizeof(EFI_DEVICE_PATH *) *
- CONFIG_PARTITION_MAXCOUNT);
- if (!root_devices) {
- Print(L"Error, could not allocate memory for config partition "
- L"device paths.\n");
- mfree(roots);
- return BG_CONFIG_ERROR;
- }
-
- if (EFI_ERROR(enumerate_cfg_parts(roots, root_devices, &numHandles))) {
+ if (EFI_ERROR(enumerate_cfg_parts(config_volumes, &numHandles))) {
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto scc_cleanup;
}
- if (EFI_ERROR(filter_cfg_parts(roots, root_devices, &numHandles))) {
+ if (EFI_ERROR(filter_cfg_parts(config_volumes, &numHandles))) {
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto scc_cleanup;
}
if (numHandles != ENV_NUM_CONFIG_PARTS) {
@@ -64,21 +50,18 @@ BG_STATUS save_current_config(void)
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto scc_cleanup;
}
+ VOLUME_DESC *v = &volumes[config_volumes[current_partition]];
EFI_FILE_HANDLE fh = NULL;
- efistatus = open_cfg_file(roots[current_partition], &fh,
- EFI_FILE_MODE_WRITE | EFI_FILE_MODE_READ);
+ efistatus = open_cfg_file(v->root, &fh, EFI_FILE_MODE_WRITE |
+ EFI_FILE_MODE_READ);
- mfree(root_devices);
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto scc_cleanup;
}
UINTN writelen = sizeof(BG_ENVDATA);
@@ -90,63 +73,50 @@ BG_STATUS save_current_config(void)
(VOID *)&env[current_partition]);
if (EFI_ERROR(efistatus)) {
Print(L"Error writing environment to file: %r\n", efistatus);
- result = BG_CONFIG_ERROR;
+ (VOID) close_cfg_file(v->root, fh);
+ goto scc_cleanup;
}
- if (EFI_ERROR(close_cfg_file(roots[current_partition], fh))) {
+ if (EFI_ERROR(close_cfg_file(v->root, fh))) {
+ goto scc_cleanup;
}
- mfree(root_devices);
- mfree(roots);
+
+ result = BG_SUCCESS;
+scc_cleanup:
+ mfree(config_volumes);
return result;
}
BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
{
- BG_STATUS result = BG_SUCCESS;
- UINTN numHandles = CONFIG_PARTITION_MAXCOUNT;
- EFI_FILE_HANDLE *roots;
- EFI_DEVICE_PATH **root_devices;
+ BG_STATUS result = BG_CONFIG_ERROR;
+ UINTN numHandles = volume_count;
+ UINTN *config_volumes;
- CONFIG_PARTITION_MAXCOUNT);
- if (!roots) {
+ config_volumes = (UINTN *)mmalloc(sizeof(EFI_FILE_HANDLE) *
+ volume_count);
+ if (!config_volumes) {
- return BG_CONFIG_ERROR;
+ L"mapping.\n");
+ return result;
}
- root_devices = (EFI_DEVICE_PATH **)mmalloc(sizeof(EFI_DEVICE_PATH *) *
- CONFIG_PARTITION_MAXCOUNT);
- if (!root_devices) {
- Print(L"Error, could not allocate memory for config partition "
- L"device paths.\n");
- mfree(roots);
- return BG_CONFIG_ERROR;
- }
-
- if (EFI_ERROR(enumerate_cfg_parts(roots, root_devices, &numHandles))) {
+ if (EFI_ERROR(enumerate_cfg_parts(config_volumes, &numHandles))) {
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto lc_cleanup;
}
- if (EFI_ERROR(filter_cfg_parts(roots, root_devices, &numHandles))) {
+ if (EFI_ERROR(filter_cfg_parts(config_volumes, &numHandles))) {
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto lc_cleanup;
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto lc_cleanup;
}
if (numHandles < ENV_NUM_CONFIG_PARTS) {
@@ -161,8 +131,9 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
- EFI_FILE_MODE_READ))) {
+ VOLUME_DESC *v = &volumes[config_volumes[i]];
+ if (EFI_ERROR(open_cfg_file(v->root, &fh,
+ if (EFI_ERROR(close_cfg_file(v->root, fh))) {
result = BG_CONFIG_PARTIALLY_CORRUPTED;
/* Don't abort, so we may still be able to boot a
@@ -263,8 +234,10 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
- mfree(roots);
+
+ result = BG_SUCCESS;
+lc_cleanup:
+ mfree(config_volumes);
return result;
}
diff --git a/env/syspart.c b/env/syspart.c
index 86628a0..48bb9a2 100644
--- a/env/syspart.c
+++ b/env/syspart.c
@@ -37,14 +37,12 @@ EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer)
- EFI_DEVICE_PATH **root_devices,
- UINTN *numHandles)
+EFI_STATUS enumerate_cfg_parts(UINTN *config_volumes, UINTN *numHandles)
+ if (!config_volumes || !numHandles) {
Print(L"Invalid parameter in system partition enumeration.\n");
return EFI_INVALID_PARAMETER;
}
@@ -58,8 +56,7 @@ EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots,
- root_devices[rootCount] = volumes[index].devpath;
+ config_volumes[rootCount] = index;
rootCount++;
return EFI_SUCCESS;
}
-EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
- EFI_DEVICE_PATH **root_devices, UINTN *numHandles)
+EFI_STATUS filter_cfg_parts(UINTN *config_volumes, UINTN *numHandles)
{
BOOLEAN only_envs_on_bootdevice = FALSE;
@@ -84,8 +80,9 @@ EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
EFI_FILE_HANDLE fh = NULL;
EFI_STATUS status;
BG_ENVDATA env;
+ VOLUME_DESC *v = &volumes[config_volumes[index]];
- status = open_cfg_file(roots[index], &fh, EFI_FILE_MODE_READ);
+ status = open_cfg_file(v->root, &fh, EFI_FILE_MODE_READ);
if (EFI_ERROR(status)) {
return status;
}
@@ -95,12 +92,12 @@ EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
return status;
}
- if (IsOnBootDevice(root_devices[index]) &&
+ if (IsOnBootDevice(v->devpath) &&
(env.status_flags & ENV_STATUS_FAILSAFE)) {
only_envs_on_bootdevice = TRUE;
};
- status = close_cfg_file(roots[index], fh);
+ status = close_cfg_file(v->root, fh);
if (EFI_ERROR(status)) {
return status;
}
@@ -114,13 +111,11 @@ EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
do {
- if (!IsOnBootDevice(root_devices[index])) {
+ VOLUME_DESC *v = &volumes[config_volumes[index]];
+ if (!IsOnBootDevice(v->devpath)) {
- root_devices[j] = root_devices[j+1];
+ config_volumes[j] = config_volumes[j+1];
}
- mfree(roots[*numHandles-1]);
- mfree(root_devices[*numHandles-1]);
(*numHandles)--;
index d2fa6bf..46dd718 100644
--- a/include/envdata.h
+++ b/include/envdata.h
@@ -18,8 +18,6 @@
#define FAT_ENV_FILENAME "BGENV.DAT"
#define ENV_STRING_LENGTH 255
-#define CONFIG_PARTITION_MAXCOUNT 64
-
#define ENV_STATUS_IN_PROGRESS 1
#define ENV_STATUS_FAILSAFE 128
diff --git a/include/syspart.h b/include/syspart.h
index 269fa25..18b9a23 100644
--- a/include/syspart.h
+++ b/include/syspart.h
@@ -25,11 +25,7 @@ EFI_STATUS open_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE *fh,
- UINTN *maxHandles);
-EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
- EFI_DEVICE_PATH **root_devices,
- UINTN *maxHandles);
+EFI_STATUS enumerate_cfg_parts(UINTN *config_volumes, UINTN *maxHandles);
+EFI_STATUS filter_cfg_parts(UINTN *config_volumes, UINTN *maxHandles);
* volumes[] already contains all needed data, so instead of
allocating two additional arrays (roots, root_devpaths),
just create a mapping config_volumes[] and use
volumes[config_volumes[]]
* remove CONFIG_PARTITION_MAXCOUNT, since the maximum number of
config partitions is always the number of volumes present
* simplify error-handling in config loading and writing
env/fatvars.c | 121 ++++++++++++++++++----------------------------
env/syspart.c | 27 +++++------
include/envdata.h | 2 -
include/syspart.h | 8 +--
4 files changed, 60 insertions(+), 98 deletions(-)
diff --git a/env/fatvars.c b/env/fatvars.c
index cefff0c..556b836 100644
--- a/env/fatvars.c
+++ b/env/fatvars.c
@@ -21,41 +21,27 @@ static BG_ENVDATA env[ENV_NUM_CONFIG_PARTS];
BG_STATUS save_current_config(void)
{
- BG_STATUS result = BG_SUCCESS;
+ BG_STATUS result = BG_CONFIG_ERROR;
EFI_STATUS efistatus;
- UINTN numHandles = CONFIG_PARTITION_MAXCOUNT;
- EFI_FILE_HANDLE *roots;
- EFI_DEVICE_PATH **root_devices;
+ UINTN numHandles = volume_count;
+ UINTN *config_volumes;
- roots = (EFI_FILE_HANDLE *)mmalloc(sizeof(EFI_FILE_HANDLE) *
- CONFIG_PARTITION_MAXCOUNT);
- if (!roots) {
+ config_volumes = (UINTN *)mmalloc(sizeof(EFI_FILE_HANDLE) *
+ volume_count);
+ if (!config_volumes) {
Print(L"Error, could not allocate memory for config partition "
- L"handles.\n");
- return BG_CONFIG_ERROR;
+ L"mapping.\n");
+ return result;
}
- root_devices = (EFI_DEVICE_PATH **)mmalloc(sizeof(EFI_DEVICE_PATH *) *
- CONFIG_PARTITION_MAXCOUNT);
- if (!root_devices) {
- Print(L"Error, could not allocate memory for config partition "
- L"device paths.\n");
- mfree(roots);
- return BG_CONFIG_ERROR;
- }
-
- if (EFI_ERROR(enumerate_cfg_parts(roots, root_devices, &numHandles))) {
+ if (EFI_ERROR(enumerate_cfg_parts(config_volumes, &numHandles))) {
Print(L"Error, could not enumerate config partitions.\n");
- mfree(root_devices);
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto scc_cleanup;
}
- if (EFI_ERROR(filter_cfg_parts(roots, root_devices, &numHandles))) {
+ if (EFI_ERROR(filter_cfg_parts(config_volumes, &numHandles))) {
Print(L"Error, could not filter config partitions.\n");
- mfree(root_devices);
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto scc_cleanup;
}
if (numHandles != ENV_NUM_CONFIG_PARTS) {
@@ -64,21 +50,18 @@ BG_STATUS save_current_config(void)
numHandles, ENV_NUM_CONFIG_PARTS);
/* In case of saving, this must be treated as error, to not
* overwrite another partition's config file. */
- mfree(root_devices);
/* In case of saving, this must be treated as error, to not
* overwrite another partition's config file. */
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto scc_cleanup;
}
+ VOLUME_DESC *v = &volumes[config_volumes[current_partition]];
EFI_FILE_HANDLE fh = NULL;
- efistatus = open_cfg_file(roots[current_partition], &fh,
- EFI_FILE_MODE_WRITE | EFI_FILE_MODE_READ);
+ efistatus = open_cfg_file(v->root, &fh, EFI_FILE_MODE_WRITE |
+ EFI_FILE_MODE_READ);
if (EFI_ERROR(efistatus)) {
Print(L"Error, could not open environment file on system "
L"partition %d: %r\n",
current_partition, efistatus);
Print(L"Error, could not open environment file on system "
L"partition %d: %r\n",
- mfree(root_devices);
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto scc_cleanup;
}
UINTN writelen = sizeof(BG_ENVDATA);
@@ -90,63 +73,50 @@ BG_STATUS save_current_config(void)
(VOID *)&env[current_partition]);
if (EFI_ERROR(efistatus)) {
Print(L"Error writing environment to file: %r\n", efistatus);
- result = BG_CONFIG_ERROR;
+ (VOID) close_cfg_file(v->root, fh);
+ goto scc_cleanup;
}
- if (EFI_ERROR(close_cfg_file(roots[current_partition], fh))) {
+ if (EFI_ERROR(close_cfg_file(v->root, fh))) {
Print(L"Error, could not close environment config file.\n");
- result = BG_CONFIG_ERROR;
+ goto scc_cleanup;
}
- mfree(root_devices);
- mfree(roots);
+
+ result = BG_SUCCESS;
+scc_cleanup:
+ mfree(config_volumes);
return result;
}
BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
{
- BG_STATUS result = BG_SUCCESS;
- UINTN numHandles = CONFIG_PARTITION_MAXCOUNT;
- EFI_FILE_HANDLE *roots;
- EFI_DEVICE_PATH **root_devices;
+ BG_STATUS result = BG_CONFIG_ERROR;
+ UINTN numHandles = volume_count;
+ UINTN *config_volumes;
UINTN i;
int env_invalid[ENV_NUM_CONFIG_PARTS] = {0};
- roots = (EFI_FILE_HANDLE *)mmalloc(sizeof(EFI_FILE_HANDLE) *
int env_invalid[ENV_NUM_CONFIG_PARTS] = {0};
- CONFIG_PARTITION_MAXCOUNT);
- if (!roots) {
+ config_volumes = (UINTN *)mmalloc(sizeof(EFI_FILE_HANDLE) *
+ volume_count);
+ if (!config_volumes) {
Print(L"Error, could not allocate memory for config partition "
- L"handles.\n");
- return BG_CONFIG_ERROR;
+ L"mapping.\n");
+ return result;
}
- root_devices = (EFI_DEVICE_PATH **)mmalloc(sizeof(EFI_DEVICE_PATH *) *
- CONFIG_PARTITION_MAXCOUNT);
- if (!root_devices) {
- Print(L"Error, could not allocate memory for config partition "
- L"device paths.\n");
- mfree(roots);
- return BG_CONFIG_ERROR;
- }
-
- if (EFI_ERROR(enumerate_cfg_parts(roots, root_devices, &numHandles))) {
+ if (EFI_ERROR(enumerate_cfg_parts(config_volumes, &numHandles))) {
Print(L"Error, could not enumerate config partitions.\n");
- mfree(root_devices);
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto lc_cleanup;
}
- if (EFI_ERROR(filter_cfg_parts(roots, root_devices, &numHandles))) {
+ if (EFI_ERROR(filter_cfg_parts(config_volumes, &numHandles))) {
Print(L"Error, could not filter config partitions.\n");
- mfree(root_devices);
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto lc_cleanup;
}
if (numHandles > ENV_NUM_CONFIG_PARTS) {
Print(L"Error, too many config partitions found. Aborting.\n");
- mfree(root_devices);
if (numHandles > ENV_NUM_CONFIG_PARTS) {
Print(L"Error, too many config partitions found. Aborting.\n");
- mfree(roots);
- return BG_CONFIG_ERROR;
+ goto lc_cleanup;
}
if (numHandles < ENV_NUM_CONFIG_PARTS) {
@@ -161,8 +131,9 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
/* Load all config data */
for (i = 0; i < numHandles; i++) {
EFI_FILE_HANDLE fh = NULL;
- if (EFI_ERROR(open_cfg_file(roots[i], &fh,
for (i = 0; i < numHandles; i++) {
EFI_FILE_HANDLE fh = NULL;
- EFI_FILE_MODE_READ))) {
+ VOLUME_DESC *v = &volumes[config_volumes[i]];
+ if (EFI_ERROR(open_cfg_file(v->root, &fh,
+ EFI_FILE_MODE_READ))) {
Print(L"Warning, could not open environment file on "
L"config partition %d\n",
i);
@@ -175,7 +146,7 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
Print(L"Warning, could not open environment file on "
L"config partition %d\n",
i);
L"partition %d.\n",
i);
env_invalid[i] = 1;
- if (EFI_ERROR(close_cfg_file(roots[i], fh))) {
i);
env_invalid[i] = 1;
+ if (EFI_ERROR(close_cfg_file(v->root, fh))) {
Print(L"Error, could not close environment "
L"config file.\n");
}
@@ -198,7 +169,7 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
L"config file.\n");
}
result = BG_CONFIG_PARTIALLY_CORRUPTED;
}
- if (EFI_ERROR(uefi_call_wrapper(fh->Close, 1, fh))) {
+ if (EFI_ERROR(close_cfg_file(v->root, fh))) {
- if (EFI_ERROR(uefi_call_wrapper(fh->Close, 1, fh))) {
Print(L"Error, could not close environment config "
L"file.\n");
/* Don't abort, so we may still be able to boot a
@@ -263,8 +234,10 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
Print(L" kernel: %s\n", bglp->payload_path);
Print(L" args: %s\n", bglp->payload_options);
Print(L" timeout: %d seconds\n", bglp->timeout);
- mfree(root_devices);
Print(L" args: %s\n", bglp->payload_options);
Print(L" timeout: %d seconds\n", bglp->timeout);
- mfree(roots);
+
+ result = BG_SUCCESS;
+lc_cleanup:
+ mfree(config_volumes);
return result;
}
diff --git a/env/syspart.c b/env/syspart.c
index 86628a0..48bb9a2 100644
--- a/env/syspart.c
+++ b/env/syspart.c
@@ -37,14 +37,12 @@ EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer)
return uefi_call_wrapper(fh->Read, 3, fh, &readlen, buffer);
}
-EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots,
}
- EFI_DEVICE_PATH **root_devices,
- UINTN *numHandles)
+EFI_STATUS enumerate_cfg_parts(UINTN *config_volumes, UINTN *numHandles)
{
EFI_STATUS status;
UINTN rootCount = 0;
- if (!roots || !numHandles) {
EFI_STATUS status;
UINTN rootCount = 0;
+ if (!config_volumes || !numHandles) {
Print(L"Invalid parameter in system partition enumeration.\n");
return EFI_INVALID_PARAMETER;
}
@@ -58,8 +56,7 @@ EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots,
EFI_FILE_MODE_READ);
if (status == EFI_SUCCESS) {
Print(L"Config file found on volume %d.\n", index);
- roots[rootCount] = volumes[index].root;
if (status == EFI_SUCCESS) {
Print(L"Config file found on volume %d.\n", index);
- root_devices[rootCount] = volumes[index].devpath;
+ config_volumes[rootCount] = index;
rootCount++;
status = close_cfg_file(volumes[index].root, fh);
if (EFI_ERROR(status)) {
@@ -74,8 +71,7 @@ EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots,
if (EFI_ERROR(status)) {
return EFI_SUCCESS;
}
-EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
- EFI_DEVICE_PATH **root_devices, UINTN *numHandles)
+EFI_STATUS filter_cfg_parts(UINTN *config_volumes, UINTN *numHandles)
{
BOOLEAN only_envs_on_bootdevice = FALSE;
@@ -84,8 +80,9 @@ EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
EFI_FILE_HANDLE fh = NULL;
EFI_STATUS status;
BG_ENVDATA env;
+ VOLUME_DESC *v = &volumes[config_volumes[index]];
- status = open_cfg_file(roots[index], &fh, EFI_FILE_MODE_READ);
+ status = open_cfg_file(v->root, &fh, EFI_FILE_MODE_READ);
if (EFI_ERROR(status)) {
return status;
}
@@ -95,12 +92,12 @@ EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
return status;
}
- if (IsOnBootDevice(root_devices[index]) &&
+ if (IsOnBootDevice(v->devpath) &&
(env.status_flags & ENV_STATUS_FAILSAFE)) {
only_envs_on_bootdevice = TRUE;
};
- status = close_cfg_file(roots[index], fh);
+ status = close_cfg_file(v->root, fh);
if (EFI_ERROR(status)) {
return status;
}
@@ -114,13 +111,11 @@ EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
Print(L"Fail-Safe Mode enabled.\n");
UINTN index = 0;
do {
- if (!IsOnBootDevice(root_devices[index])) {
+ VOLUME_DESC *v = &volumes[config_volumes[index]];
+ if (!IsOnBootDevice(v->devpath)) {
for (UINTN j = index; j < *numHandles-1; j++) {
- roots[j] = roots[j+1];
- root_devices[j] = root_devices[j+1];
+ config_volumes[j] = config_volumes[j+1];
}
- mfree(roots[*numHandles-1]);
- mfree(root_devices[*numHandles-1]);
(*numHandles)--;
Print(L"Filtered Config #%d\n", index);
}
diff --git a/include/envdata.h b/include/envdata.h
}
index d2fa6bf..46dd718 100644
--- a/include/envdata.h
+++ b/include/envdata.h
@@ -18,8 +18,6 @@
#define FAT_ENV_FILENAME "BGENV.DAT"
#define ENV_STRING_LENGTH 255
-#define CONFIG_PARTITION_MAXCOUNT 64
-
#define ENV_STATUS_IN_PROGRESS 1
#define ENV_STATUS_FAILSAFE 128
diff --git a/include/syspart.h b/include/syspart.h
index 269fa25..18b9a23 100644
--- a/include/syspart.h
+++ b/include/syspart.h
@@ -25,11 +25,7 @@ EFI_STATUS open_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE *fh,
UINT64 mode);
EFI_STATUS close_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE fh);
EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer);
-EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots,
- EFI_DEVICE_PATH **root_devices,
EFI_STATUS close_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE fh);
EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer);
-EFI_STATUS enumerate_cfg_parts(EFI_FILE_HANDLE *roots,
- UINTN *maxHandles);
-EFI_STATUS filter_cfg_parts(EFI_FILE_HANDLE *roots,
- EFI_DEVICE_PATH **root_devices,
- UINTN *maxHandles);
+EFI_STATUS enumerate_cfg_parts(UINTN *config_volumes, UINTN *maxHandles);
+EFI_STATUS filter_cfg_parts(UINTN *config_volumes, UINTN *maxHandles);
Andreas J. Reichel
May 25, 2018, 8:05:42 AM5/25/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
New option S sets or resets the fail-safe mode flag in the new
status flag register.
tools/bg_setenv.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index cae268d..0847de1 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -45,6 +45,7 @@ static struct argp_option options_setenv[] = {
"simulate a running update "
"process."},
{"version", 'V', 0, 0, "Print version"},
+ {"failsafe", 'S', "FAILSAFE", 0, "Set Fail-Safe Flag"},
{0}};
static struct argp_option options_printenv[] = {
@@ -393,6 +394,12 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
* argp_usage with non-zero return code */
argp_usage(state);
break;
+ case 'S':
+ VERBOSE(stdout, "Enabeling fail-safe mode.\n");
+ e = journal_add_action(ENV_TASK_SET,
+ "env_status_failsafe", 0,
+ (uint8_t *)arg, strlen(arg) + 1);
+ break;
default:
return ARGP_ERR_UNKNOWN;
}
--
2.17.0
New option S sets or resets the fail-safe mode flag in the new
status flag register.
tools/bg_setenv.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index cae268d..0847de1 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -45,6 +45,7 @@ static struct argp_option options_setenv[] = {
"simulate a running update "
"process."},
{"version", 'V', 0, 0, "Print version"},
+ {"failsafe", 'S', "FAILSAFE", 0, "Set Fail-Safe Flag"},
{0}};
static struct argp_option options_printenv[] = {
@@ -393,6 +394,12 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
* argp_usage with non-zero return code */
argp_usage(state);
break;
+ case 'S':
+ VERBOSE(stdout, "Enabeling fail-safe mode.\n");
+ e = journal_add_action(ENV_TASK_SET,
+ "env_status_failsafe", 0,
+ (uint8_t *)arg, strlen(arg) + 1);
+ break;
default:
return ARGP_ERR_UNKNOWN;
}
--
2.17.0
Claudius Heine
May 28, 2018, 7:05:59 AM5/28/18
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com, jan.k...@siemens.com
Hi Andreas,
here we go again ;)
Why not as macro? (I guess because of the includes?)
Anyway, you could also put a 'inline' there.
Claudius
> VOID *mmalloc(UINTN bytes)
> {
> EFI_STATUS status;
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
here we go again ;)
Anyway, you could also put a 'inline' there.
Claudius
> VOID *mmalloc(UINTN bytes)
> {
> EFI_STATUS status;
>
--
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
Claudius Heine
May 28, 2018, 7:21:17 AM5/28/18
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com, jan.k...@siemens.com
Hi,
On 2018-05-25 14:00, [ext] Andreas J. Reichel wrote:
Magic numbers. Use 'sizeof'.
> + if (!dst) {
> + return NULL;
> + }
> +
> + StrCpy(dst, input);
> +
> + for (UINTN i = len; i > 0; i--)
> + {
> + if (dst[i * 2] == delim) {
> + dst[i * 2] = L'\0';
> + break;
> + }
> + }
Here I get this itching in my coding fingers. I would try do copying and
striping in one loop.
Claudius
> + return dst;
> +}
> +
> uint32_t calc_crc32(void *data, int32_t size)
> {
> uint32_t crc;
>
--
On 2018-05-25 14:00, [ext] Andreas J. Reichel wrote:
> + if (!dst) {
> + return NULL;
> + }
> +
> + StrCpy(dst, input);
> +
> + for (UINTN i = len; i > 0; i--)
> + {
> + if (dst[i * 2] == delim) {
> + dst[i * 2] = L'\0';
> + break;
> + }
> + }
striping in one loop.
Claudius
> + return dst;
> +}
> +
> uint32_t calc_crc32(void *data, int32_t size)
> {
> uint32_t crc;
>
--
Andreas Reichel
May 28, 2018, 7:33:12 AM5/28/18
to Claudius Heine, efibootg...@googlegroups.com, jan.k...@siemens.com
On Mon, May 28, 2018 at 01:05:58PM +0200, Claudius Heine wrote:
> Hi Andreas,
>
> here we go again ;)
>
> On 2018-05-25 14:00, [ext] Andreas J. Reichel wrote:
> > From: Andreas Reichel <andreas.r...@siemens.com>
> >
> Hi Andreas,
>
> here we go again ;)
>
> On 2018-05-25 14:00, [ext] Andreas J. Reichel wrote:
> > From: Andreas Reichel <andreas.r...@siemens.com>
> >
> > +++ b/utils.c
> > @@ -32,6 +32,11 @@ void __noreturn error_exit(CHAR16 *message, EFI_STATUS status)
> > unreachable();
> > }
> > +VOID sleep(int32_t sec)
> > +{
> > + uefi_call_wrapper(BS->Stall, 1, sec * 1000 * 1000);
> > +}
> > +
>
> Why not as macro? (I guess because of the includes?)
Because I wrote it as a function.
> > @@ -32,6 +32,11 @@ void __noreturn error_exit(CHAR16 *message, EFI_STATUS status)
> > unreachable();
> > }
> > +VOID sleep(int32_t sec)
> > +{
> > + uefi_call_wrapper(BS->Stall, 1, sec * 1000 * 1000);
> > +}
> > +
>
> Why not as macro? (I guess because of the includes?)
Technically you gave no reason.
> Anyway, you could also put a 'inline' there.
Extern inline would be possible but that would produce the same code
inside the scope of each translation unit which is rather... hm ugly?
On the other hand... the compiler may decide to not inline the function.
> Claudius
>
>
> > VOID *mmalloc(UINTN bytes)
> > {
> > EFI_STATUS status;
> >
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant
Andreas...@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082
Claudius Heine
May 28, 2018, 7:38:35 AM5/28/18
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com, jan.k...@siemens.com
Hi,
On 2018-05-25 14:00, [ext] Andreas J. Reichel wrote:
You are allocating memory in StrRStrip and are not freeing it.
On 2018-05-25 14:00, [ext] Andreas J. Reichel wrote:
Claudius Heine
May 28, 2018, 7:54:30 AM5/28/18
to Andreas Reichel, efibootg...@googlegroups.com, jan.k...@siemens.com
Hi Andreas,
On 2018-05-28 13:28, Andreas Reichel wrote:
> On Mon, May 28, 2018 at 01:05:58PM +0200, Claudius Heine wrote:
>> Hi Andreas,
>>
>> here we go again ;)
>>
>> On 2018-05-25 14:00, [ext] Andreas J. Reichel wrote:
>>> From: Andreas Reichel <andreas.r...@siemens.com>
>>>
>>> +++ b/utils.c
>>> @@ -32,6 +32,11 @@ void __noreturn error_exit(CHAR16 *message, EFI_STATUS status)
>>> unreachable();
>>> }
>>> +VOID sleep(int32_t sec)
>>> +{
>>> + uefi_call_wrapper(BS->Stall, 1, sec * 1000 * 1000);
>>> +}
>>> +
>>
>> Why not as macro? (I guess because of the includes?)
> Because I wrote it as a function.
> Technically you gave no reason.
What you wanted to have is some kind of parameter binding to an existing
function and shortening the name.
I do such things normally with macros or inline functions. That is the
pattern I am used to. Argument is: better performance and memory usage.
That might not be that much if you have just some small cases. And with
a sleep that takes whole seconds as a parameter and only used for
debugging not important. But it might get more if used constantly or on
performance critical paths and can be avoided very simply. So its good
to get used to programming this way generally.
On 2018-05-28 13:28, Andreas Reichel wrote:
> On Mon, May 28, 2018 at 01:05:58PM +0200, Claudius Heine wrote:
>> Hi Andreas,
>>
>> here we go again ;)
>>
>> On 2018-05-25 14:00, [ext] Andreas J. Reichel wrote:
>>> From: Andreas Reichel <andreas.r...@siemens.com>
>>>
>>> +++ b/utils.c
>>> @@ -32,6 +32,11 @@ void __noreturn error_exit(CHAR16 *message, EFI_STATUS status)
>>> unreachable();
>>> }
>>> +VOID sleep(int32_t sec)
>>> +{
>>> + uefi_call_wrapper(BS->Stall, 1, sec * 1000 * 1000);
>>> +}
>>> +
>>
>> Why not as macro? (I guess because of the includes?)
> Because I wrote it as a function.
> Technically you gave no reason.
function and shortening the name.
I do such things normally with macros or inline functions. That is the
pattern I am used to. Argument is: better performance and memory usage.
That might not be that much if you have just some small cases. And with
a sleep that takes whole seconds as a parameter and only used for
debugging not important. But it might get more if used constantly or on
performance critical paths and can be avoided very simply. So its good
to get used to programming this way generally.
Claudius Heine
May 28, 2018, 8:52:17 AM5/28/18
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com, jan.k...@siemens.com
merge with patch 9 (where you use it)
On 2018-05-25 14:00, [ext] Andreas J. Reichel wrote:
On 2018-05-25 14:00, [ext] Andreas J. Reichel wrote:
Claudius Heine
May 28, 2018, 9:10:06 AM5/28/18
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com, jan.k...@siemens.com
all other functions look the same.
> +
> +EFI_STATUS close_cfg_file(EFI_FILE_HANDLE root, EFI_FILE_HANDLE fh)
> +{
> + return uefi_call_wrapper(root->Close, 1, fh);
> +}
> +
> +EFI_STATUS read_cfg_file(EFI_FILE_HANDLE fh, VOID *buffer)
> +{
> + UINTN readlen = sizeof(BG_ENVDATA);
> + return uefi_call_wrapper(fh->Read, 3, fh, &readlen, buffer);
> +}
Claudius Heine
May 28, 2018, 9:17:38 AM5/28/18
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com, jan.k...@siemens.com
On 2018-05-25 14:01, [ext] Andreas J. Reichel wrote:
does mfrees and return BG_CONFIG_ERROR becomes handy.
and returns.
Claudius Heine
May 28, 2018, 9:46:05 AM5/28/18
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com, jan.k...@siemens.com
On 2018-05-25 14:01, [ext] Andreas J. Reichel wrote:
code in one patch and then change it again in one of the next ones.
If you want to fix this, then take a look at 'git add -p'.
You could also split the implementation of the fail-safe from the
integration and maybe merge that into this patch if that is easier.
Or just merge 9 and 10.
Andreas Reichel
May 28, 2018, 10:24:33 AM5/28/18
to Claudius Heine, efibootg...@googlegroups.com, jan.k...@siemens.com
On Mon, May 28, 2018 at 03:46:03PM +0200, Claudius Heine wrote:
>
>
> On 2018-05-25 14:01, [ext] Andreas J. Reichel wrote:
> > From: Andreas Reichel <andreas.r...@siemens.com>
> >
> > * volumes[] already contains all needed data, so instead of
> > allocating two additional arrays (roots, root_devpaths),
> > just create a mapping config_volumes[] and use
> > volumes[config_volumes[]]
> > * remove CONFIG_PARTITION_MAXCOUNT, since the maximum number of
> > config partitions is always the number of volumes present
> > * simplify error-handling in config loading and writing
> >
> > Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
> > ---
> > env/fatvars.c | 121 ++++++++++++++++++----------------------------
> > env/syspart.c | 27 +++++------
> > include/envdata.h | 2 -
> > include/syspart.h | 8 +--
> > 4 files changed, 60 insertions(+), 98 deletions(-)
> >
> > diff --git a/env/fatvars.c b/env/fatvars.c
> > index cefff0c..556b836 100644
>
>
> On 2018-05-25 14:01, [ext] Andreas J. Reichel wrote:
> > From: Andreas Reichel <andreas.r...@siemens.com>
> >
> > * volumes[] already contains all needed data, so instead of
> > allocating two additional arrays (roots, root_devpaths),
> > just create a mapping config_volumes[] and use
> > volumes[config_volumes[]]
> > * remove CONFIG_PARTITION_MAXCOUNT, since the maximum number of
> > config partitions is always the number of volumes present
> > * simplify error-handling in config loading and writing
> >
> > Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
> > ---
> > env/fatvars.c | 121 ++++++++++++++++++----------------------------
> > env/syspart.c | 27 +++++------
> > include/envdata.h | 2 -
> > include/syspart.h | 8 +--
> > 4 files changed, 60 insertions(+), 98 deletions(-)
> >
> > diff --git a/env/fatvars.c b/env/fatvars.c
> > index cefff0c..556b836 100644
> > - mfree(root_devices);
> > - mfree(roots);
> > - return BG_CONFIG_ERROR;
> > + goto scc_cleanup;
>
> Yep that is the kind of thing I don't like in patchsets :) Don't add code in
> one patch and then change it again in one of the next ones.
>
> If you want to fix this, then take a look at 'git add -p'.
>
> You could also split the implementation of the fail-safe from the
> integration and maybe merge that into this patch if that is easier.
>
> Or just merge 9 and 10.
>
As our long discussion just pointed out, merging 9 and 10 seems to be
> > - mfree(roots);
> > - return BG_CONFIG_ERROR;
> > + goto scc_cleanup;
>
> Yep that is the kind of thing I don't like in patchsets :) Don't add code in
> one patch and then change it again in one of the next ones.
>
> If you want to fix this, then take a look at 'git add -p'.
>
> You could also split the implementation of the fail-safe from the
> integration and maybe merge that into this patch if that is easier.
>
> Or just merge 9 and 10.
>
the best solution.
> >
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
Andreas Reichel
May 28, 2018, 10:28:07 AM5/28/18
to Claudius Heine, efibootg...@googlegroups.com, jan.k...@siemens.com
> > + if (!dst) {
> > + return NULL;
> > + }
> > +
> > + StrCpy(dst, input);
> > +
> > + for (UINTN i = len; i > 0; i--)
> > + {
> > + if (dst[i * 2] == delim) {
> > + dst[i * 2] = L'\0';
> > + break;
> > + }
> > + }
>
> Here I get this itching in my coding fingers. I would try do copying and
> striping in one loop.
>
about what a 'RStrip' function should do. Changing the name to make
clear that this is a function of special intent for device paths is
better.
> Claudius
>
> > + return dst;
> > +}
> > +
> > uint32_t calc_crc32(void *data, int32_t size)
> > {
> > uint32_t crc;
> >
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
Claudius Heine
May 28, 2018, 10:28:14 AM5/28/18
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com, jan.k...@siemens.com
On 2018-05-25 14:01, [ext] Andreas J. Reichel wrote:
> From: Andreas Reichel <andreas.r...@siemens.com>
>
> If a variable like watchdog is set with a negative argument,
> bg_setenv returns this negative argument instead of -EINVAL.
> Fix this to -EINVAL.
This patch could have been posted outside of this patchset.
> From: Andreas Reichel <andreas.r...@siemens.com>
>
> If a variable like watchdog is set with a negative argument,
> bg_setenv returns this negative argument instead of -EINVAL.
> Fix this to -EINVAL.
Claudius Heine
May 28, 2018, 10:40:36 AM5/28/18
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com, jan.k...@siemens.com
On 2018-05-25 14:01, [ext] Andreas J. Reichel wrote:
> default:
> if (!data) {
> return 0;
> @@ -411,6 +420,21 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
> return -EINVAL;
> }
> break;
> + case EBGENV_FAILSAFE:
> + val = bgenv_convert_to_long(value);
> + if (val < 0) {
> + return val;
> + }
> + switch(val) {
> + case 1:
> + env->data->status_flags |= ENV_STATUS_FAILSAFE;
> + break;
> + case 0:
> + env->data->status_flags &= ~ENV_STATUS_FAILSAFE;
> + default:
> + return -EINVAL;
> + }
> + break;
> default:
> return -EINVAL;
> }
> diff --git a/include/env_api.h b/include/env_api.h
> index 45bf4b7..04516bb 100644
> --- a/include/env_api.h
> +++ b/include/env_api.h
> @@ -54,6 +54,7 @@ typedef enum {
> EBGENV_REVISION,
> EBGENV_USTATE,
> EBGENV_IN_PROGRESS,
> + EBGENV_FAILSAFE,
> EBGENV_UNKNOWN
> } EBGENVKEY;
>
>
--
Claudius Heine
May 28, 2018, 10:58:25 AM5/28/18
to [ext] Andreas J. Reichel, efibootg...@googlegroups.com, jan.k...@siemens.com
EBGENV_FAILSAFE != ENV_STATUS_FAILSAFE
Andreas Reichel
May 29, 2018, 7:09:53 AM5/29/18
to efibootg...@googlegroups.com, jan.k...@siemens.com
These patch series will be split and the new two series start with
version 1 again.
version 1 again.
Andreas J. Reichel
May 29, 2018, 7:17:05 AM5/29/18
to efibootg...@googlegroups.com, jan.k...@siemens.com, Andreas Reichel
From: Andreas Reichel <andreas.r...@siemens.com>
Prevent 404 during apt-get install with apt-get update in before_install
.travis.yml | 1 +
Prevent 404 during apt-get install with apt-get update in before_install
1 file changed, 1 insertion(+)
Reply all
Reply to author
Forward
0 new messages