ACK: [PATCH] efi: enable module loading to load legacy or new efi driver

Alex Hung alex.hung at canonical.com
Thu Jul 14 07:24:53 UTC 2016


On 2016-07-14 12:46 AM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> The name of the efi module and the device is going to change once
> the driver lands upstream in the kernel.  So make the module loading
> more flexible to cater for legacy and the new upstream'd kernel driver.
>
> Also abstract out the efi test device driver name and provide a thin
> open/close device abstraction layer so we can keep the device name
> out of the tests.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>   src/lib/include/fwts_efi_module.h        |   2 +
>   src/lib/src/fwts_efi_module.c            | 202 ++++++++++++++++++++++++-------
>   src/uefi/uefirtauthvar/uefirtauthvar.c   |   6 +-
>   src/uefi/uefirtmisc/uefirtmisc.c         |   6 +-
>   src/uefi/uefirttime/uefirttime.c         |   6 +-
>   src/uefi/uefirtvariable/uefirtvariable.c |   6 +-
>   src/uefi/uefivarinfo/uefivarinfo.c       |   6 +-
>   7 files changed, 176 insertions(+), 58 deletions(-)
>
> diff --git a/src/lib/include/fwts_efi_module.h b/src/lib/include/fwts_efi_module.h
> index ec8eceb..73c4ba6 100644
> --- a/src/lib/include/fwts_efi_module.h
> +++ b/src/lib/include/fwts_efi_module.h
> @@ -22,5 +22,7 @@
>
>   int fwts_lib_efi_runtime_load_module(fwts_framework *fw);
>   int fwts_lib_efi_runtime_unload_module(fwts_framework *fw);
> +int fwts_lib_efi_runtime_open(void);
> +int fwts_lib_efi_runtime_close(int fd);
>
>   #endif
> diff --git a/src/lib/src/fwts_efi_module.c b/src/lib/src/fwts_efi_module.c
> index b3e0bdb..f95ee63 100644
> --- a/src/lib/src/fwts_efi_module.c
> +++ b/src/lib/src/fwts_efi_module.c
> @@ -21,90 +21,206 @@
>   #include <string.h>
>   #include <sys/types.h>
>   #include <sys/stat.h>
> +#include <fcntl.h>
>   #include <unistd.h>
>
>   #include "fwts_pipeio.h"
>
> -static bool module_already_loaded = false;
> +static char *efi_dev_name = NULL;
> +static char *module_name = NULL;
>
> -static int check_module_loaded(void)
> +/*
> + *  check_module_loaded()
> + *	check if a given module is loaded
> + */
> +static int check_module_loaded(
> +	fwts_framework *fw,
> +	char *module,
> +	bool *loaded)
>   {
>   	FILE *fp;
>
> -	module_already_loaded = false;
> +	*loaded = false;
> +
>   	if ((fp = fopen("/proc/modules", "r")) != NULL) {
>   		char buffer[1024];
>
>   		while (fgets(buffer, sizeof(buffer), fp) != NULL) {
> -			if (strstr(buffer, "efi_runtime") != NULL) {
> -				module_already_loaded = true;
> +			if (strstr(buffer, module)) {
> +				*loaded = true;
>   				break;
>   			}
>   		}
>   		fclose(fp);
>   		return FWTS_OK;
>   	}
> +	fwts_log_error(fw, "Could not open /proc/modules to check if efi module '%s' is loaded.", module);
> +
>   	return FWTS_ERROR;
>   }
>
> -int fwts_lib_efi_runtime_load_module(fwts_framework *fw)
> +/*
> + *  check_module_loaded_no_dev()
> + *	sanity check - we don't have a device so we definitely should
> + *	not have the module loaded either
> + */
> +static int check_module_loaded_no_dev(
> +	fwts_framework *fw,
> +	char *module)
>   {
> -	struct stat statbuf;
> +	bool loaded;
>
> -	if (check_module_loaded() != FWTS_OK) {
> -		fwts_log_error(fw, "Could not open /proc/modules for checking module loaded.");
> +	if (check_module_loaded(fw, module, &loaded) != FWTS_OK)
> +		return FWTS_ERROR;
> +	if (loaded) {
> +		fwts_log_error(fw, "Module '%s' is already loaded, but device not available.", module);
>   		return FWTS_ERROR;
>   	}
> +	return FWTS_OK;
> +}
>
> -	if (!module_already_loaded) {
> -		int status;
> -
> -		if (fwts_exec("modprobe efi_runtime", &status) != FWTS_OK) {
> -			fwts_log_error(fw, "Load efi_runtime module error.");
> -			return FWTS_ERROR;
> -		} else {
> -			(void)check_module_loaded();
> -			if (!module_already_loaded) {
> -				fwts_log_error(fw, "Could not load efi_runtime module.");
> -				return FWTS_ERROR;
> -			}
> -		}
> -	}
> +/*
> + *  check_device()
> + *	check if the device exists and is a char dev
> + */
> +static int check_device(char *devname)
> +{
> +	struct stat statbuf;
>
> -	if (stat("/dev/efi_runtime", &statbuf)) {
> -		fwts_log_error(fw, "Loaded efi_runtime module but /dev/efi_runtime is not present.");
> +	if (stat(devname, &statbuf))
>   		return FWTS_ERROR;
> +
> +	if (S_ISCHR(statbuf.st_mode)) {
> +		efi_dev_name = devname;
> +		return FWTS_OK;
>   	}
> +	return FWTS_ERROR;
> +}
> +
> +/*
> + *  load_module()
> + *	load the module and check if the device appears
> + */
> +static int load_module(
> +	fwts_framework *fw,
> +	char *module,
> +	char *devname)
> +{
> +	int status;
> +	char cmd[80];
> +	bool loaded;
> +
> +	snprintf(cmd, sizeof(cmd), "modprobe %s", module);
>
> -	if (!S_ISCHR(statbuf.st_mode)) {
> -		fwts_log_error(fw, "Loaded efi_runtime module but /dev/efi_runtime is not a char device.");
> +	if (fwts_exec(cmd, &status) != FWTS_OK)
> +		return FWTS_ERROR;
> +
> +	if (check_module_loaded(fw, module, &loaded) != FWTS_OK)
>   		return FWTS_ERROR;
> -	}
> +
> +	if (!loaded)
> +		return FWTS_ERROR;
> +
> +	if (check_device(devname) != FWTS_OK)
> +		return FWTS_ERROR;
> +
> +	module_name = module;
>
>   	return FWTS_OK;
>   }
>
> +/*
> + *  fwts_lib_efi_runtime_load_module()
> + *	load the runtime module, for historical reasons
> + *	we have two names for the module and the device
> + *	it creates
> + */
> +int fwts_lib_efi_runtime_load_module(fwts_framework *fw)
> +{
> +	efi_dev_name = NULL;
> +	module_name = NULL;
> +
> +	/* Check if dev is already available */
> +	if (check_device("/dev/efi_test") == FWTS_OK)
> +		return FWTS_OK;
> +	if (check_device("/dev/efi_runtime") == FWTS_OK)
> +		return FWTS_OK;
> +
> +	/* Since the devices can't be found, the module should be not loaded */
> +	if (check_module_loaded_no_dev(fw, "efi_test") != FWTS_OK)
> +		return FWTS_ERROR;
> +	if (check_module_loaded_no_dev(fw, "efi_runtime") != FWTS_OK)
> +		return FWTS_ERROR;
> +
> +	/* Now try to load the module */
> +
> +	if (load_module(fw, "efi_test", "/dev/efi_test") == FWTS_OK)
> +		return FWTS_OK;
> +	if (load_module(fw, "efi_runtime", "/dev/efi_runtime") == FWTS_OK)
> +		return FWTS_OK;
> +
> +	fwts_log_error(fw, "Failed to load efi test module.");
> +	return FWTS_ERROR;
> +}
>
> +/*
> + *  fwts_lib_efi_runtime_unload_module()
> + *	unload the runtile module
> + */
>   int fwts_lib_efi_runtime_unload_module(fwts_framework *fw)
>   {
> -	if (check_module_loaded() != FWTS_OK) {
> -		fwts_log_error(fw, "Could not open /proc/modules for checking module loaded.");
> +	bool loaded;
> +	int status;
> +	char cmd[80], *tmp_name = module_name;
> +
> +	efi_dev_name = NULL;
> +
> +	/* No module, not much to do */
> +	if (!module_name)
> +		return FWTS_OK;
> +
> +	module_name = NULL;
> +
> +	/* If it is not loaded, no need to unload it */
> +	if (check_module_loaded(fw, tmp_name, &loaded) != FWTS_OK)
> +		return FWTS_ERROR;
> +	if (!loaded)
> +		return FWTS_OK;
> +
> +	snprintf(cmd, sizeof(cmd), "modprobe -r %s", tmp_name);
> +	if (fwts_exec(cmd, &status) != FWTS_OK) {
> +		fwts_log_error(fw, "Failed to unload module '%s'.", tmp_name);
>   		return FWTS_ERROR;
>   	}
> -	if (module_already_loaded) {
> -		int status;
> -
> -		if (fwts_exec("modprobe -r efi_runtime", &status) != FWTS_OK) {
> -			fwts_log_error(fw, "Unload efi_runtime module error.");
> -			return FWTS_ERROR;
> -		} else {
> -			(void)check_module_loaded();
> -			if (module_already_loaded) {
> -				fwts_log_error(fw, "Could not unload efi_runtime module.");
> -				return FWTS_ERROR;
> -			}
> -		}
> +
> +	/* Module should not be loaded at this point */
> +	if (check_module_loaded(fw, tmp_name, &loaded) != FWTS_OK)
> +		return FWTS_ERROR;
> +	if (loaded) {
> +		fwts_log_error(fw, "Failed to unload module '%s'.", tmp_name);
> +		return FWTS_ERROR;
>   	}
>
>   	return FWTS_OK;
>   }
> +
> +/*
> + *  fwts_lib_efi_runtime_open()
> + *	open the device
> + */
> +int fwts_lib_efi_runtime_open(void)
> +{
> +	if (!efi_dev_name)
> +		return -1;
> +
> +	return open(efi_dev_name, O_WRONLY | O_RDWR);
> +}
> +
> +/*
> + *  fwts_lib_efi_runtime_close()
> + *	close the device
> + */
> +int fwts_lib_efi_runtime_close(int fd)
> +{
> +	return close(fd);
> +}
> diff --git a/src/uefi/uefirtauthvar/uefirtauthvar.c b/src/uefi/uefirtauthvar/uefirtauthvar.c
> index ed23a8e..cdfd7aa 100644
> --- a/src/uefi/uefirtauthvar/uefirtauthvar.c
> +++ b/src/uefi/uefirtauthvar/uefirtauthvar.c
> @@ -123,9 +123,9 @@ static int uefirtauthvar_init(fwts_framework *fw)
>   		return FWTS_ABORTED;
>   	}
>
> -	fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR);
> +	fd = fwts_lib_efi_runtime_open();
>   	if (fd == -1) {
> -		fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted.");
> +		fwts_log_info(fw, "Cannot open EFI test driver. Aborted.");
>   		return FWTS_ABORTED;
>   	}
>
> @@ -136,7 +136,7 @@ static int uefirtauthvar_init(fwts_framework *fw)
>
>   static int uefirtauthvar_deinit(fwts_framework *fw)
>   {
> -	close(fd);
> +	fwts_lib_efi_runtime_close(fd);
>   	fwts_lib_efi_runtime_unload_module(fw);
>
>   	return FWTS_OK;
> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
> index 347b2b1..d06e2a1 100644
> --- a/src/uefi/uefirtmisc/uefirtmisc.c
> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
> @@ -55,9 +55,9 @@ static int uefirtmisc_init(fwts_framework *fw)
>   		return FWTS_ABORTED;
>   	}
>
> -	fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR);
> +	fd = fwts_lib_efi_runtime_open();
>   	if (fd == -1) {
> -		fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted.");
> +		fwts_log_info(fw, "Cannot open EFI test driver. Aborted.");
>   		return FWTS_ABORTED;
>   	}
>
> @@ -68,7 +68,7 @@ static int uefirtmisc_deinit(fwts_framework *fw)
>   {
>   	FWTS_UNUSED(fw);
>
> -	close(fd);
> +	fwts_lib_efi_runtime_close(fd);
>   	fwts_lib_efi_runtime_unload_module(fw);
>
>   	return FWTS_OK;
> diff --git a/src/uefi/uefirttime/uefirttime.c b/src/uefi/uefirttime/uefirttime.c
> index f79e2da..55e9d19 100644
> --- a/src/uefi/uefirttime/uefirttime.c
> +++ b/src/uefi/uefirttime/uefirttime.c
> @@ -179,9 +179,9 @@ static int uefirttime_init(fwts_framework *fw)
>   		return FWTS_ABORTED;
>   	}
>
> -	fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR);
> +	fd = fwts_lib_efi_runtime_open();
>   	if (fd == -1) {
> -		fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted.");
> +		fwts_log_info(fw, "Cannot open EFI test driver. Aborted.");
>   		return FWTS_ABORTED;
>   	}
>
> @@ -192,7 +192,7 @@ static int uefirttime_deinit(fwts_framework *fw)
>   {
>   	FWTS_UNUSED(fw);
>
> -	close(fd);
> +	fwts_lib_efi_runtime_close(fd);
>   	fwts_lib_efi_runtime_unload_module(fw);
>
>   	return FWTS_OK;
> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
> index f60dbad..2adc064 100644
> --- a/src/uefi/uefirtvariable/uefirtvariable.c
> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
> @@ -101,9 +101,9 @@ static int uefirtvariable_init(fwts_framework *fw)
>   		return FWTS_ABORTED;
>   	}
>
> -	fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR);
> +	fd = fwts_lib_efi_runtime_open();
>   	if (fd == -1) {
> -		fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted.");
> +		fwts_log_info(fw, "Cannot open EFI test driver. Aborted.");
>   		return FWTS_ABORTED;
>   	}
>
> @@ -114,7 +114,7 @@ static int uefirtvariable_init(fwts_framework *fw)
>
>   static int uefirtvariable_deinit(fwts_framework *fw)
>   {
> -	close(fd);
> +	fwts_lib_efi_runtime_close(fd);
>   	fwts_lib_efi_runtime_unload_module(fw);
>
>   	return FWTS_OK;
> diff --git a/src/uefi/uefivarinfo/uefivarinfo.c b/src/uefi/uefivarinfo/uefivarinfo.c
> index 22afe53..24798a9 100644
> --- a/src/uefi/uefivarinfo/uefivarinfo.c
> +++ b/src/uefi/uefivarinfo/uefivarinfo.c
> @@ -46,9 +46,9 @@ static int uefivarinfo_init(fwts_framework *fw)
>   		return FWTS_ABORTED;
>   	}
>
> -	fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR);
> +	fd = fwts_lib_efi_runtime_open();
>   	if (fd == -1) {
> -		fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted.");
> +		fwts_log_info(fw, "Cannot open EFI test driver. Aborted.");
>   		return FWTS_ABORTED;
>   	}
>
> @@ -57,7 +57,7 @@ static int uefivarinfo_init(fwts_framework *fw)
>
>   static int uefivarinfo_deinit(fwts_framework *fw)
>   {
> -	close(fd);
> +	fwts_lib_efi_runtime_close(fd);
>   	fwts_lib_efi_runtime_unload_module(fw);
>
>   	return FWTS_OK;
>

Acked-by: Alex Hung <alex.hung at canonical.com>



More information about the fwts-devel mailing list