[v2] efi_runtime: add efi_runtime kernel driver module into fwts

efi_runtime kernel driver provides the runtime UEFI interfaces for fwts
to test the UEFI runtime service implementiation.
Current capabilities:
* provide the RT service interfaces:
   * GetVariable
   * SetVariable
   * GetTime
   * SetTime
   * GetWakeupTime
   * SetWakeupTime
   * GetNextVariableName

Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
---
 efi_runtime/Makefile      |    6 +
 efi_runtime/efi_runtime.c |  310 +++++++++++++++++++++++++++++++++++++++++++++
 efi_runtime/efi_runtime.h |  112 ++++++++++++++++
 3 files changed, 428 insertions(+)
 create mode 100644 efi_runtime/Makefile
 create mode 100644 efi_runtime/efi_runtime.c
 create mode 100644 efi_runtime/efi_runtime.h

On 20/09/12 16:03, Ivan Hu wrote:
> efi_runtime kernel driver provides the runtime UEFI interfaces for fwts
> to test the UEFI runtime service implementiation.
> Current capabilities:
> * provide the RT service interfaces:
>     * GetVariable
>     * SetVariable
>     * GetTime
>     * SetTime
>     * GetWakeupTime
>     * SetWakeupTime
>     * GetNextVariableName
>
> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
> ---
>   efi_runtime/Makefile      |    6 +
>   efi_runtime/efi_runtime.c |  310 +++++++++++++++++++++++++++++++++++++++++++++
>   efi_runtime/efi_runtime.h |  112 ++++++++++++++++
>   3 files changed, 428 insertions(+)
>   create mode 100644 efi_runtime/Makefile
>   create mode 100644 efi_runtime/efi_runtime.c
>   create mode 100644 efi_runtime/efi_runtime.h
>
> diff --git a/efi_runtime/Makefile b/efi_runtime/Makefile
> new file mode 100644
> index 0000000..8ed7dea
> --- /dev/null
> +++ b/efi_runtime/Makefile
> @@ -0,0 +1,6 @@
> +obj-m += efi_runtime.o
> +all:
> +	make -C /lib/modules/`uname -r`/build M=`pwd` modules
> +
> +clean:
> +	make -C /lib/modules/`uname -r`/build M=`pwd` clean
> diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c
> new file mode 100644
> index 0000000..901af42
> --- /dev/null
> +++ b/efi_runtime/efi_runtime.c
> @@ -0,0 +1,310 @@
> +/*
> + * EFI Runtime driver
> + *
> + * Copyright(C) 2012 Canonical Ltd.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/proc_fs.h>
> +#include <linux/efi.h>
> +
> +#include <linux/uaccess.h>
> +
> +#include "efi_runtime.h"
> +
> +#define EFI_FWTS_EFI_VERSION	"0.1"
> +
> +MODULE_AUTHOR("Ivan Hu");
> +MODULE_DESCRIPTION("EFI Runtime Driver");
> +MODULE_LICENSE("GPL");
> +
> +static void convert_from_efi_time(efi_time_t *eft, EFI_TIME *time)
> +{
> +	memset(time, 0, sizeof(EFI_TIME));
> +	time->Year = eft->year;
> +	time->Month = eft->month;
> +	time->Day  = eft->day;
> +	time->Hour = eft->hour;
> +	time->Minute = eft->minute;
> +	time->Second  = eft->second;
> +	time->Pad1 = eft->pad1;
> +	time->Nanosecond = eft->nanosecond;
> +	time->TimeZone = eft->timezone;
> +	time->Daylight = eft->daylight;
> +	time->Pad2 = eft->pad2;
> +}
> +
> +static void convert_to_efi_time(efi_time_t *eft, EFI_TIME *time)
> +{
> +	memset(eft, 0, sizeof(eft));
> +	eft->year = time->Year;
> +	eft->month = time->Month;
> +	eft->day = time->Day;
> +	eft->hour = time->Hour;
> +	eft->minute = time->Minute;
> +	eft->second = time->Second;
> +	eft->pad1 = time->Pad1;
> +	eft->nanosecond = time->Nanosecond;
> +	eft->timezone = time->TimeZone;
> +	eft->daylight = time->Daylight;
> +	eft->pad2 = time->Pad2;
> +}
> +
> +static void convert_from_guid(efi_guid_t *vendor, EFI_GUID *vendor_guid)
> +{
> +	int i;
> +	for (i = 0; i < 16; i++) {
> +		if (i < 4)
> +			vendor->b[i] = (vendor_guid->Data1 >> (8*i)) & 0xff;
> +		else if (i < 6)
> +			vendor->b[i] = (vendor_guid->Data2 >> (8*(i-4))) & 0xff;
> +		else if (i < 8)
> +			vendor->b[i] = (vendor_guid->Data3 >> (8*(i-6))) & 0xff;
> +		else
> +			vendor->b[i] = (vendor_guid->Data4[i-8]);
> +	}
> +}
> +
> +static void convert_to_guid(efi_guid_t *vendor, EFI_GUID *vendor_guid)
> +{
> +	int i;
> +	vendor_guid->Data1 = vendor->b[0] + (vendor->b[1] << 8) +
> +				(vendor->b[2] << 16) + (vendor->b[3] << 24);
> +	vendor_guid->Data2 = vendor->b[4] + (vendor->b[5] << 8);
> +	vendor_guid->Data3 = vendor->b[6] + (vendor->b[7] << 8);
> +	for (i = 0; i < 8; i++)
> +		vendor_guid->Data4[i] = vendor->b[i+8];
> +}
> +
> +static long efi_runtime_ioctl(struct file *file, unsigned int cmd,
> +							unsigned long arg)
> +{
> +	efi_status_t status;
> +	struct efi_getvariable __user *pgetvariable;
> +	struct efi_setvariable __user *psetvariable;
> +
> +	efi_guid_t vendor;
> +	EFI_GUID vendor_guid;
> +	unsigned long datasize;
> +	uint32_t attr;
> +
> +	efi_time_t eft;
> +	efi_time_cap_t cap;
> +	struct efi_gettime __user *pgettime;
> +	struct efi_settime __user *psettime;
> +
> +	unsigned char enabled, pending;
> +	EFI_TIME efi_time;
> +	struct efi_getwakeuptime __user *pgetwakeuptime;
> +	struct efi_setwakeuptime __user *psetwakeuptime;
> +
> +	struct efi_getnextvariablename __user *pgetnextvariablename;
> +	unsigned long name_size;
> +
> +	switch (cmd) {
> +	case EFI_RUNTIME_GET_VARIABLE:
> +		pgetvariable = (struct efi_getvariable __user *)arg;
> +
> +		if (get_user(datasize, pgetvariable->DataSize) ||
> +			copy_from_user(&vendor_guid, pgetvariable->VendorGuid,
> +							sizeof(EFI_GUID)))
> +			return -EFAULT;
> +
> +		convert_from_guid(&vendor, &vendor_guid);
> +		status = efi.get_variable(pgetvariable->VariableName, &vendor,
> +					&attr, &datasize, pgetvariable->Data);
> +
> +		if (status == EFI_SUCCESS) {
> +			if (put_user(attr, pgetvariable->Attributes) ||
> +				put_user(datasize, pgetvariable->DataSize))
> +				return -EFAULT;
> +			return status;
> +		} else {
> +			printk(KERN_ERR "efi_runtime: can't get variable\n");
> +			return status;
> +		}
> +
> +	case EFI_RUNTIME_SET_VARIABLE:
> +		psetvariable = (struct efi_setvariable __user *)arg;
> +		if (get_user(datasize, &psetvariable->DataSize) ||
> +			get_user(attr, &psetvariable->Attributes) ||
> +			copy_from_user(&vendor_guid, psetvariable->VendorGuid,
> +							sizeof(EFI_GUID)))
> +			return -EFAULT;
> +
> +		convert_from_guid(&vendor, &vendor_guid);
> +		status = efi.set_variable(psetvariable->VariableName, &vendor,
> +					attr, datasize, psetvariable->Data);
> +		return status;
> +
> +	case EFI_RUNTIME_GET_TIME:
> +		status = efi.get_time(&eft, &cap);
> +		if (status != EFI_SUCCESS) {
> +			printk(KERN_ERR "efitime: can't read time\n");
> +			return status;
> +		}
> +
> +		pgettime = (struct efi_gettime __user *)arg;
> +		if (put_user(cap.resolution,
> +					&pgettime->Capabilities->Resolution) ||
> +					put_user(cap.accuracy,
> +					&pgettime->Capabilities->Accuracy) ||
> +					put_user(cap.sets_to_zero,
> +					&pgettime->Capabilities->SetsToZero))
> +			return -EFAULT;
> +		return copy_to_user(pgettime->Time, &eft,
> +				sizeof(EFI_TIME)) ? -EFAULT : status;
> +
> +	case EFI_RUNTIME_SET_TIME:
> +
> +		psettime = (struct efi_settime __user *)arg;
> +		if (copy_from_user(&efi_time, psettime->Time,
> +						sizeof(EFI_TIME)))
> +			return -EFAULT;
> +		convert_to_efi_time(&eft, &efi_time);
> +		status = efi.set_time(&eft);
> +		return status == EFI_SUCCESS ? status : -EINVAL;
> +
> +	case EFI_RUNTIME_GET_WAKETIME:
> +
> +		status = efi.get_wakeup_time((efi_bool_t *)&enabled,
> +						(efi_bool_t *)&pending, &eft);
> +
> +		if (status != EFI_SUCCESS)
> +			return status;
> +
> +		pgetwakeuptime = (struct efi_getwakeuptime __user *)arg;
> +
> +		if (put_user(enabled, pgetwakeuptime->Enabled) ||
> +				put_user(pending, pgetwakeuptime->Pending))
> +			return -EFAULT;
> +
> +		convert_from_efi_time(&eft, &efi_time);
> +
> +		return copy_to_user(pgetwakeuptime->Time, &efi_time,
> +				sizeof(EFI_TIME)) ? -EFAULT : status;
> +
> +	case EFI_RUNTIME_SET_WAKETIME:
> +
> +		psetwakeuptime = (struct efi_setwakeuptime __user *)arg;
> +
> +		if (get_user(enabled, &psetwakeuptime->Enabled) ||
> +					copy_from_user(&efi_time,
> +					psetwakeuptime->Time,
> +					sizeof(EFI_TIME)))
> +			return -EFAULT;
> +
> +		convert_to_efi_time(&eft, &efi_time);
> +
> +		status = efi.set_wakeup_time(enabled, &eft);
> +
> +		return status == EFI_SUCCESS ? status : -EINVAL;
> +
> +	case EFI_RUNTIME_GET_NEXTVARIABLENAME:
> +
> +		pgetnextvariablename = (struct efi_getnextvariablename
> +								__user *)arg;
> +
> +		if (get_user(name_size, pgetnextvariablename->VariableNameSize)
> +				|| copy_from_user(&vendor_guid,
> +					pgetnextvariablename->VendorGuid,
> +					sizeof(EFI_GUID)))
> +			return -EFAULT;
> +		if (name_size > 1024)
> +			return -EFAULT;
> +
> +		convert_from_guid(&vendor, &vendor_guid);
> +
> +		status = efi.get_next_variable(&name_size,
> +					pgetnextvariablename->VariableName,
> +								&vendor);
> +
> +		if (status != EFI_SUCCESS)
> +			return status;
> +		convert_to_guid(&vendor, &vendor_guid);
> +
> +		if (put_user(name_size, pgetnextvariablename->VariableNameSize))
> +			return -EFAULT;
> +
> +		if (copy_to_user(pgetnextvariablename->VendorGuid,
> +						&vendor_guid, sizeof(EFI_GUID)))
> +			return -EFAULT;
> +		return status;
> +	}
> +
> +	return -ENOTTY;
> +}


OK, so efi_runtime_ioctl() returns a signed long, but the code is 
actually returning status which is an efi_status which is an unsigned 
long.  What happens if the efi status has the top bit set? .. this would 
return a -ve value which looks like an error.  I think we need to see if 
this makes sense or not - what is the range of return values from 
underling EFI calls?


> +
> +static int efi_runtime_open(struct inode *inode, struct file *file)
> +{
> +	/*
> +	 * nothing special to do here
> +	 * We do accept multiple open files at the same time as we
> +	 * synchronize on the per call operation.
> +	 */
> +	return 0;
> +}
> +
> +static int efi_runtime_close(struct inode *inode, struct file *file)
> +{
> +	return 0;
> +}
> +
> +/*
> + *	The various file operations we support.
> + */
> +static const struct file_operations efi_runtime_fops = {
> +	.owner		= THIS_MODULE,
> +	.unlocked_ioctl	= efi_runtime_ioctl,
> +	.open		= efi_runtime_open,
> +	.release	= efi_runtime_close,
> +	.llseek		= no_llseek,
> +};
> +
> +static struct miscdevice efi_runtime_dev = {
> +	MISC_DYNAMIC_MINOR,
> +	"efi_runtime",
> +	&efi_runtime_fops
> +};
> +
> +static int __init efi_runtime_init(void)
> +{
> +	int ret;
> +
> +	printk(KERN_INFO "EFI_RUNTIME Driver v%s\n", EFI_FWTS_EFI_VERSION);
> +
> +	ret = misc_register(&efi_runtime_dev);
> +	if (ret) {
> +		printk(KERN_ERR "efi_runtime: can't misc_register on minor=%d\n",
> +				MISC_DYNAMIC_MINOR);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void __exit efi_runtime_exit(void)
> +{
> +	printk(KERN_INFO "EFI_RUNTIME Driver Exit.\n");
> +	misc_deregister(&efi_runtime_dev);
> +}
> +
> +module_init(efi_runtime_init);
> +module_exit(efi_runtime_exit);
> +
> diff --git a/efi_runtime/efi_runtime.h b/efi_runtime/efi_runtime.h
> new file mode 100644
> index 0000000..0766916
> --- /dev/null
> +++ b/efi_runtime/efi_runtime.h
> @@ -0,0 +1,112 @@
> +/*
> + * EFI Runtime driver
> + *
> + * Copyright(C) 2012 Canonical Ltd.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#ifndef _EFI_RUNTIME_H_
> +#define _EFI_RUNTIME_H_
> +
> +typedef struct {
> +	uint32_t	Data1;
> +	uint16_t	Data2;
> +	uint16_t	Data3;
> +	uint8_t		Data4[8];
> +} __attribute__ ((packed)) EFI_GUID;
> +
> +typedef struct {
> +	uint16_t	Year;		/* 1900 – 9999 */
> +	uint8_t		Month;		/* 1 – 12 */
> +	uint8_t		Day;		/* 1 – 31 */
> +	uint8_t		Hour;		/* 0 – 23 */
> +	uint8_t		Minute;		/* 0 – 59 */
> +	uint8_t		Second;		/* 0 – 59 */
> +	uint8_t		Pad1;
> +	uint32_t	Nanosecond;	/* 0 – 999,999,999 */
> +	int16_t		TimeZone;	/* -1440 to 1440 or 2047 */
> +	uint8_t		Daylight;
> +	uint8_t		Pad2;
> +} __attribute__ ((packed)) EFI_TIME;
> +
> +typedef struct {
> +	uint32_t	Resolution;
> +	uint32_t	Accuracy;
> +	uint8_t		SetsToZero;
> +} __attribute__ ((packed)) EFI_TIME_CAPABILITIES;
> +
> +struct efi_getvariable {
> +	uint16_t	*VariableName;
> +	EFI_GUID	*VendorGuid;
> +	uint32_t	*Attributes;
> +	uint64_t	*DataSize;
> +	void		*Data;
> +} __attribute__ ((packed));
> +
> +struct efi_setvariable {
> +	uint16_t	*VariableName;
> +	EFI_GUID	*VendorGuid;
> +	uint32_t	Attributes;
> +	uint64_t	DataSize;
> +	void		*Data;
> +} __attribute__ ((packed));
> +
> +struct efi_getnextvariablename {
> +	uint64_t	*VariableNameSize;
> +	uint16_t	*VariableName;
> +	EFI_GUID	*VendorGuid;
> +} __attribute__ ((packed));
> +
> +struct efi_gettime {
> +	EFI_TIME		*Time;
> +	EFI_TIME_CAPABILITIES	*Capabilities;
> +} __attribute__ ((packed));
> +
> +struct efi_settime {
> +	EFI_TIME		*Time;
> +} __attribute__ ((packed));
> +
> +struct efi_getwakeuptime {
> +	uint8_t		*Enabled;
> +	uint8_t		*Pending;
> +	EFI_TIME	*Time;
> +} __attribute__ ((packed));
> +
> +struct efi_setwakeuptime {
> +	uint8_t		Enabled;
> +	EFI_TIME	*Time;
> +} __attribute__ ((packed));
> +
> +/* ioctl calls that are permitted to the /dev/efi_runtime interface. */
> +#define EFI_RUNTIME_GET_VARIABLE \
> +	_IOWR('p', 0x01, struct efi_getvariable)
> +#define EFI_RUNTIME_SET_VARIABLE \
> +	_IOW('p', 0x02, struct efi_setvariable)
> +
> +#define EFI_RUNTIME_GET_TIME \
> +	_IOR('p', 0x03, struct efi_gettime)
> +#define EFI_RUNTIME_SET_TIME \
> +	_IOW('p', 0x04, struct efi_settime)
> +
> +#define EFI_RUNTIME_GET_WAKETIME \
> +	_IOR('p', 0x05, struct efi_getwakeuptime)
> +#define EFI_RUNTIME_SET_WAKETIME \
> +	_IOW('p', 0x06, struct efi_setwakeuptime)
> +
> +#define EFI_RUNTIME_GET_NEXTVARIABLENAME \
> +	_IOWR('p', 0x07, struct efi_getnextvariablename)
> +
> +#endif /* _EFI_RUNTIME_H_ */
>

On 09/21/2012 01:47 AM, Colin Ian King wrote:
> On 20/09/12 16:03, Ivan Hu wrote:
>> efi_runtime kernel driver provides the runtime UEFI interfaces for fwts
>> to test the UEFI runtime service implementiation.
>> Current capabilities:
>> * provide the RT service interfaces:
>>     * GetVariable
>>     * SetVariable
>>     * GetTime
>>     * SetTime
>>     * GetWakeupTime
>>     * SetWakeupTime
>>     * GetNextVariableName
>>
>> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
>> ---
>>   efi_runtime/Makefile      |    6 +
>>   efi_runtime/efi_runtime.c |  310
>> +++++++++++++++++++++++++++++++++++++++++++++
>>   efi_runtime/efi_runtime.h |  112 ++++++++++++++++
>>   3 files changed, 428 insertions(+)
>>   create mode 100644 efi_runtime/Makefile
>>   create mode 100644 efi_runtime/efi_runtime.c
>>   create mode 100644 efi_runtime/efi_runtime.h
>>
>> diff --git a/efi_runtime/Makefile b/efi_runtime/Makefile
>> new file mode 100644
>> index 0000000..8ed7dea
>> --- /dev/null
>> +++ b/efi_runtime/Makefile
>> @@ -0,0 +1,6 @@
>> +obj-m += efi_runtime.o
>> +all:
>> +    make -C /lib/modules/`uname -r`/build M=`pwd` modules
>> +
>> +clean:
>> +    make -C /lib/modules/`uname -r`/build M=`pwd` clean
>> diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c
>> new file mode 100644
>> index 0000000..901af42
>> --- /dev/null
>> +++ b/efi_runtime/efi_runtime.c
>> @@ -0,0 +1,310 @@
>> +/*
>> + * EFI Runtime driver
>> + *
>> + * Copyright(C) 2012 Canonical Ltd.
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; either version 2 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with this program; if not, write to the Free Software
>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
>> 02111-1307  USA
>> + */
>> +
>> +#include <linux/miscdevice.h>
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/proc_fs.h>
>> +#include <linux/efi.h>
>> +
>> +#include <linux/uaccess.h>
>> +
>> +#include "efi_runtime.h"
>> +
>> +#define EFI_FWTS_EFI_VERSION    "0.1"
>> +
>> +MODULE_AUTHOR("Ivan Hu");
>> +MODULE_DESCRIPTION("EFI Runtime Driver");
>> +MODULE_LICENSE("GPL");
>> +
>> +static void convert_from_efi_time(efi_time_t *eft, EFI_TIME *time)
>> +{
>> +    memset(time, 0, sizeof(EFI_TIME));
>> +    time->Year = eft->year;
>> +    time->Month = eft->month;
>> +    time->Day  = eft->day;
>> +    time->Hour = eft->hour;
>> +    time->Minute = eft->minute;
>> +    time->Second  = eft->second;
>> +    time->Pad1 = eft->pad1;
>> +    time->Nanosecond = eft->nanosecond;
>> +    time->TimeZone = eft->timezone;
>> +    time->Daylight = eft->daylight;
>> +    time->Pad2 = eft->pad2;
>> +}
>> +
>> +static void convert_to_efi_time(efi_time_t *eft, EFI_TIME *time)
>> +{
>> +    memset(eft, 0, sizeof(eft));
>> +    eft->year = time->Year;
>> +    eft->month = time->Month;
>> +    eft->day = time->Day;
>> +    eft->hour = time->Hour;
>> +    eft->minute = time->Minute;
>> +    eft->second = time->Second;
>> +    eft->pad1 = time->Pad1;
>> +    eft->nanosecond = time->Nanosecond;
>> +    eft->timezone = time->TimeZone;
>> +    eft->daylight = time->Daylight;
>> +    eft->pad2 = time->Pad2;
>> +}
>> +
>> +static void convert_from_guid(efi_guid_t *vendor, EFI_GUID *vendor_guid)
>> +{
>> +    int i;
>> +    for (i = 0; i < 16; i++) {
>> +        if (i < 4)
>> +            vendor->b[i] = (vendor_guid->Data1 >> (8*i)) & 0xff;
>> +        else if (i < 6)
>> +            vendor->b[i] = (vendor_guid->Data2 >> (8*(i-4))) & 0xff;
>> +        else if (i < 8)
>> +            vendor->b[i] = (vendor_guid->Data3 >> (8*(i-6))) & 0xff;
>> +        else
>> +            vendor->b[i] = (vendor_guid->Data4[i-8]);
>> +    }
>> +}
>> +
>> +static void convert_to_guid(efi_guid_t *vendor, EFI_GUID *vendor_guid)
>> +{
>> +    int i;
>> +    vendor_guid->Data1 = vendor->b[0] + (vendor->b[1] << 8) +
>> +                (vendor->b[2] << 16) + (vendor->b[3] << 24);
>> +    vendor_guid->Data2 = vendor->b[4] + (vendor->b[5] << 8);
>> +    vendor_guid->Data3 = vendor->b[6] + (vendor->b[7] << 8);
>> +    for (i = 0; i < 8; i++)
>> +        vendor_guid->Data4[i] = vendor->b[i+8];
>> +}
>> +
>> +static long efi_runtime_ioctl(struct file *file, unsigned int cmd,
>> +                            unsigned long arg)
>> +{
>> +    efi_status_t status;
>> +    struct efi_getvariable __user *pgetvariable;
>> +    struct efi_setvariable __user *psetvariable;
>> +
>> +    efi_guid_t vendor;
>> +    EFI_GUID vendor_guid;
>> +    unsigned long datasize;
>> +    uint32_t attr;
>> +
>> +    efi_time_t eft;
>> +    efi_time_cap_t cap;
>> +    struct efi_gettime __user *pgettime;
>> +    struct efi_settime __user *psettime;
>> +
>> +    unsigned char enabled, pending;
>> +    EFI_TIME efi_time;
>> +    struct efi_getwakeuptime __user *pgetwakeuptime;
>> +    struct efi_setwakeuptime __user *psetwakeuptime;
>> +
>> +    struct efi_getnextvariablename __user *pgetnextvariablename;
>> +    unsigned long name_size;
>> +
>> +    switch (cmd) {
>> +    case EFI_RUNTIME_GET_VARIABLE:
>> +        pgetvariable = (struct efi_getvariable __user *)arg;
>> +
>> +        if (get_user(datasize, pgetvariable->DataSize) ||
>> +            copy_from_user(&vendor_guid, pgetvariable->VendorGuid,
>> +                            sizeof(EFI_GUID)))
>> +            return -EFAULT;
>> +
>> +        convert_from_guid(&vendor, &vendor_guid);
>> +        status = efi.get_variable(pgetvariable->VariableName, &vendor,
>> +                    &attr, &datasize, pgetvariable->Data);
>> +
>> +        if (status == EFI_SUCCESS) {
>> +            if (put_user(attr, pgetvariable->Attributes) ||
>> +                put_user(datasize, pgetvariable->DataSize))
>> +                return -EFAULT;
>> +            return status;
>> +        } else {
>> +            printk(KERN_ERR "efi_runtime: can't get variable\n");
>> +            return status;
>> +        }
>> +
>> +    case EFI_RUNTIME_SET_VARIABLE:
>> +        psetvariable = (struct efi_setvariable __user *)arg;
>> +        if (get_user(datasize, &psetvariable->DataSize) ||
>> +            get_user(attr, &psetvariable->Attributes) ||
>> +            copy_from_user(&vendor_guid, psetvariable->VendorGuid,
>> +                            sizeof(EFI_GUID)))
>> +            return -EFAULT;
>> +
>> +        convert_from_guid(&vendor, &vendor_guid);
>> +        status = efi.set_variable(psetvariable->VariableName, &vendor,
>> +                    attr, datasize, psetvariable->Data);
>> +        return status;
>> +
>> +    case EFI_RUNTIME_GET_TIME:
>> +        status = efi.get_time(&eft, &cap);
>> +        if (status != EFI_SUCCESS) {
>> +            printk(KERN_ERR "efitime: can't read time\n");
>> +            return status;
>> +        }
>> +
>> +        pgettime = (struct efi_gettime __user *)arg;
>> +        if (put_user(cap.resolution,
>> +                    &pgettime->Capabilities->Resolution) ||
>> +                    put_user(cap.accuracy,
>> +                    &pgettime->Capabilities->Accuracy) ||
>> +                    put_user(cap.sets_to_zero,
>> +                    &pgettime->Capabilities->SetsToZero))
>> +            return -EFAULT;
>> +        return copy_to_user(pgettime->Time, &eft,
>> +                sizeof(EFI_TIME)) ? -EFAULT : status;
>> +
>> +    case EFI_RUNTIME_SET_TIME:
>> +
>> +        psettime = (struct efi_settime __user *)arg;
>> +        if (copy_from_user(&efi_time, psettime->Time,
>> +                        sizeof(EFI_TIME)))
>> +            return -EFAULT;
>> +        convert_to_efi_time(&eft, &efi_time);
>> +        status = efi.set_time(&eft);
>> +        return status == EFI_SUCCESS ? status : -EINVAL;
>> +
>> +    case EFI_RUNTIME_GET_WAKETIME:
>> +
>> +        status = efi.get_wakeup_time((efi_bool_t *)&enabled,
>> +                        (efi_bool_t *)&pending, &eft);
>> +
>> +        if (status != EFI_SUCCESS)
>> +            return status;
>> +
>> +        pgetwakeuptime = (struct efi_getwakeuptime __user *)arg;
>> +
>> +        if (put_user(enabled, pgetwakeuptime->Enabled) ||
>> +                put_user(pending, pgetwakeuptime->Pending))
>> +            return -EFAULT;
>> +
>> +        convert_from_efi_time(&eft, &efi_time);
>> +
>> +        return copy_to_user(pgetwakeuptime->Time, &efi_time,
>> +                sizeof(EFI_TIME)) ? -EFAULT : status;
>> +
>> +    case EFI_RUNTIME_SET_WAKETIME:
>> +
>> +        psetwakeuptime = (struct efi_setwakeuptime __user *)arg;
>> +
>> +        if (get_user(enabled, &psetwakeuptime->Enabled) ||
>> +                    copy_from_user(&efi_time,
>> +                    psetwakeuptime->Time,
>> +                    sizeof(EFI_TIME)))
>> +            return -EFAULT;
>> +
>> +        convert_to_efi_time(&eft, &efi_time);
>> +
>> +        status = efi.set_wakeup_time(enabled, &eft);
>> +
>> +        return status == EFI_SUCCESS ? status : -EINVAL;
>> +
>> +    case EFI_RUNTIME_GET_NEXTVARIABLENAME:
>> +
>> +        pgetnextvariablename = (struct efi_getnextvariablename
>> +                                __user *)arg;
>> +
>> +        if (get_user(name_size, pgetnextvariablename->VariableNameSize)
>> +                || copy_from_user(&vendor_guid,
>> +                    pgetnextvariablename->VendorGuid,
>> +                    sizeof(EFI_GUID)))
>> +            return -EFAULT;
>> +        if (name_size > 1024)
>> +            return -EFAULT;
>> +
>> +        convert_from_guid(&vendor, &vendor_guid);
>> +
>> +        status = efi.get_next_variable(&name_size,
>> +                    pgetnextvariablename->VariableName,
>> +                                &vendor);
>> +
>> +        if (status != EFI_SUCCESS)
>> +            return status;
>> +        convert_to_guid(&vendor, &vendor_guid);
>> +
>> +        if (put_user(name_size, pgetnextvariablename->VariableNameSize))
>> +            return -EFAULT;
>> +
>> +        if (copy_to_user(pgetnextvariablename->VendorGuid,
>> +                        &vendor_guid, sizeof(EFI_GUID)))
>> +            return -EFAULT;
>> +        return status;
>> +    }
>> +
>> +    return -ENOTTY;
>> +}
>
>
> OK, so efi_runtime_ioctl() returns a signed long, but the code is
> actually returning status which is an efi_status which is an unsigned
> long.  What happens if the efi status has the top bit set? .. this would
> return a -ve value which looks like an error.  I think we need to see if
> this makes sense or not - what is the range of return values from
> underling EFI calls?
>
>

yup, I noticed this as well, actually, this is a temporary solution. The 
high bit of return value means EFI_STATUS Success Codes (High Bit Clear) 
or EFI_STATUS Warning Codes (High Bit Clear), I ignore the high bit 
here. So if we get the status in user space, we get two possibilities 
(error or warning).
I am planing to put the efi_status into each structures and return to 
user space. Is that make sense to you? or any suggestion? :)

>> +
>> +static int efi_runtime_open(struct inode *inode, struct file *file)
>> +{
>> +    /*
>> +     * nothing special to do here
>> +     * We do accept multiple open files at the same time as we
>> +     * synchronize on the per call operation.
>> +     */
>> +    return 0;
>> +}
>> +
>> +static int efi_runtime_close(struct inode *inode, struct file *file)
>> +{
>> +    return 0;
>> +}
>> +
>> +/*
>> + *    The various file operations we support.
>> + */
>> +static const struct file_operations efi_runtime_fops = {
>> +    .owner        = THIS_MODULE,
>> +    .unlocked_ioctl    = efi_runtime_ioctl,
>> +    .open        = efi_runtime_open,
>> +    .release    = efi_runtime_close,
>> +    .llseek        = no_llseek,
>> +};
>> +
>> +static struct miscdevice efi_runtime_dev = {
>> +    MISC_DYNAMIC_MINOR,
>> +    "efi_runtime",
>> +    &efi_runtime_fops
>> +};
>> +
>> +static int __init efi_runtime_init(void)
>> +{
>> +    int ret;
>> +
>> +    printk(KERN_INFO "EFI_RUNTIME Driver v%s\n", EFI_FWTS_EFI_VERSION);
>> +
>> +    ret = misc_register(&efi_runtime_dev);
>> +    if (ret) {
>> +        printk(KERN_ERR "efi_runtime: can't misc_register on
>> minor=%d\n",
>> +                MISC_DYNAMIC_MINOR);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void __exit efi_runtime_exit(void)
>> +{
>> +    printk(KERN_INFO "EFI_RUNTIME Driver Exit.\n");
>> +    misc_deregister(&efi_runtime_dev);
>> +}
>> +
>> +module_init(efi_runtime_init);
>> +module_exit(efi_runtime_exit);
>> +
>> diff --git a/efi_runtime/efi_runtime.h b/efi_runtime/efi_runtime.h
>> new file mode 100644
>> index 0000000..0766916
>> --- /dev/null
>> +++ b/efi_runtime/efi_runtime.h
>> @@ -0,0 +1,112 @@
>> +/*
>> + * EFI Runtime driver
>> + *
>> + * Copyright(C) 2012 Canonical Ltd.
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; either version 2 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with this program; if not, write to the Free Software
>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
>> 02111-1307  USA
>> + */
>> +
>> +#ifndef _EFI_RUNTIME_H_
>> +#define _EFI_RUNTIME_H_
>> +
>> +typedef struct {
>> +    uint32_t    Data1;
>> +    uint16_t    Data2;
>> +    uint16_t    Data3;
>> +    uint8_t        Data4[8];
>> +} __attribute__ ((packed)) EFI_GUID;
>> +
>> +typedef struct {
>> +    uint16_t    Year;        /* 1900 – 9999 */
>> +    uint8_t        Month;        /* 1 – 12 */
>> +    uint8_t        Day;        /* 1 – 31 */
>> +    uint8_t        Hour;        /* 0 – 23 */
>> +    uint8_t        Minute;        /* 0 – 59 */
>> +    uint8_t        Second;        /* 0 – 59 */
>> +    uint8_t        Pad1;
>> +    uint32_t    Nanosecond;    /* 0 – 999,999,999 */
>> +    int16_t        TimeZone;    /* -1440 to 1440 or 2047 */
>> +    uint8_t        Daylight;
>> +    uint8_t        Pad2;
>> +} __attribute__ ((packed)) EFI_TIME;
>> +
>> +typedef struct {
>> +    uint32_t    Resolution;
>> +    uint32_t    Accuracy;
>> +    uint8_t        SetsToZero;
>> +} __attribute__ ((packed)) EFI_TIME_CAPABILITIES;
>> +
>> +struct efi_getvariable {
>> +    uint16_t    *VariableName;
>> +    EFI_GUID    *VendorGuid;
>> +    uint32_t    *Attributes;
>> +    uint64_t    *DataSize;
>> +    void        *Data;
>> +} __attribute__ ((packed));
>> +
>> +struct efi_setvariable {
>> +    uint16_t    *VariableName;
>> +    EFI_GUID    *VendorGuid;
>> +    uint32_t    Attributes;
>> +    uint64_t    DataSize;
>> +    void        *Data;
>> +} __attribute__ ((packed));
>> +
>> +struct efi_getnextvariablename {
>> +    uint64_t    *VariableNameSize;
>> +    uint16_t    *VariableName;
>> +    EFI_GUID    *VendorGuid;
>> +} __attribute__ ((packed));
>> +
>> +struct efi_gettime {
>> +    EFI_TIME        *Time;
>> +    EFI_TIME_CAPABILITIES    *Capabilities;
>> +} __attribute__ ((packed));
>> +
>> +struct efi_settime {
>> +    EFI_TIME        *Time;
>> +} __attribute__ ((packed));
>> +
>> +struct efi_getwakeuptime {
>> +    uint8_t        *Enabled;
>> +    uint8_t        *Pending;
>> +    EFI_TIME    *Time;
>> +} __attribute__ ((packed));
>> +
>> +struct efi_setwakeuptime {
>> +    uint8_t        Enabled;
>> +    EFI_TIME    *Time;
>> +} __attribute__ ((packed));
>> +
>> +/* ioctl calls that are permitted to the /dev/efi_runtime interface. */
>> +#define EFI_RUNTIME_GET_VARIABLE \
>> +    _IOWR('p', 0x01, struct efi_getvariable)
>> +#define EFI_RUNTIME_SET_VARIABLE \
>> +    _IOW('p', 0x02, struct efi_setvariable)
>> +
>> +#define EFI_RUNTIME_GET_TIME \
>> +    _IOR('p', 0x03, struct efi_gettime)
>> +#define EFI_RUNTIME_SET_TIME \
>> +    _IOW('p', 0x04, struct efi_settime)
>> +
>> +#define EFI_RUNTIME_GET_WAKETIME \
>> +    _IOR('p', 0x05, struct efi_getwakeuptime)
>> +#define EFI_RUNTIME_SET_WAKETIME \
>> +    _IOW('p', 0x06, struct efi_setwakeuptime)
>> +
>> +#define EFI_RUNTIME_GET_NEXTVARIABLENAME \
>> +    _IOWR('p', 0x07, struct efi_getnextvariablename)
>> +
>> +#endif /* _EFI_RUNTIME_H_ */
>>
>
>

On 21/09/12 02:53, IvanHu wrote:
> On 09/21/2012 01:47 AM, Colin Ian King wrote:
>> On 20/09/12 16:03, Ivan Hu wrote:
>>> efi_runtime kernel driver provides the runtime UEFI interfaces for fwts
>>> to test the UEFI runtime service implementiation.
>>> Current capabilities:
>>> * provide the RT service interfaces:
>>>     * GetVariable
>>>     * SetVariable
>>>     * GetTime
>>>     * SetTime
>>>     * GetWakeupTime
>>>     * SetWakeupTime
>>>     * GetNextVariableName
>>>
>>> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
>>> ---
>>>   efi_runtime/Makefile      |    6 +
>>>   efi_runtime/efi_runtime.c |  310
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>   efi_runtime/efi_runtime.h |  112 ++++++++++++++++
>>>   3 files changed, 428 insertions(+)
>>>   create mode 100644 efi_runtime/Makefile
>>>   create mode 100644 efi_runtime/efi_runtime.c
>>>   create mode 100644 efi_runtime/efi_runtime.h
>>>
>>> diff --git a/efi_runtime/Makefile b/efi_runtime/Makefile
>>> new file mode 100644
>>> index 0000000..8ed7dea
>>> --- /dev/null
>>> +++ b/efi_runtime/Makefile
>>> @@ -0,0 +1,6 @@
>>> +obj-m += efi_runtime.o
>>> +all:
>>> +    make -C /lib/modules/`uname -r`/build M=`pwd` modules
>>> +
>>> +clean:
>>> +    make -C /lib/modules/`uname -r`/build M=`pwd` clean
>>> diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c
>>> new file mode 100644
>>> index 0000000..901af42
>>> --- /dev/null
>>> +++ b/efi_runtime/efi_runtime.c
>>> @@ -0,0 +1,310 @@
>>> +/*
>>> + * EFI Runtime driver
>>> + *
>>> + * Copyright(C) 2012 Canonical Ltd.
>>> + *
>>> + *  This program is free software; you can redistribute it and/or
>>> modify
>>> + *  it under the terms of the GNU General Public License as
>>> published by
>>> + *  the Free Software Foundation; either version 2 of the License, or
>>> + *  (at your option) any later version.
>>> + *
>>> + *  This program is distributed in the hope that it will be useful,
>>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + *  GNU General Public License for more details.
>>> + *
>>> + *  You should have received a copy of the GNU General Public License
>>> + *  along with this program; if not, write to the Free Software
>>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
>>> 02111-1307  USA
>>> + */
>>> +
>>> +#include <linux/miscdevice.h>
>>> +#include <linux/module.h>
>>> +#include <linux/init.h>
>>> +#include <linux/proc_fs.h>
>>> +#include <linux/efi.h>
>>> +
>>> +#include <linux/uaccess.h>
>>> +
>>> +#include "efi_runtime.h"
>>> +
>>> +#define EFI_FWTS_EFI_VERSION    "0.1"
>>> +
>>> +MODULE_AUTHOR("Ivan Hu");
>>> +MODULE_DESCRIPTION("EFI Runtime Driver");
>>> +MODULE_LICENSE("GPL");
>>> +
>>> +static void convert_from_efi_time(efi_time_t *eft, EFI_TIME *time)
>>> +{
>>> +    memset(time, 0, sizeof(EFI_TIME));
>>> +    time->Year = eft->year;
>>> +    time->Month = eft->month;
>>> +    time->Day  = eft->day;
>>> +    time->Hour = eft->hour;
>>> +    time->Minute = eft->minute;
>>> +    time->Second  = eft->second;
>>> +    time->Pad1 = eft->pad1;
>>> +    time->Nanosecond = eft->nanosecond;
>>> +    time->TimeZone = eft->timezone;
>>> +    time->Daylight = eft->daylight;
>>> +    time->Pad2 = eft->pad2;
>>> +}
>>> +
>>> +static void convert_to_efi_time(efi_time_t *eft, EFI_TIME *time)
>>> +{
>>> +    memset(eft, 0, sizeof(eft));
>>> +    eft->year = time->Year;
>>> +    eft->month = time->Month;
>>> +    eft->day = time->Day;
>>> +    eft->hour = time->Hour;
>>> +    eft->minute = time->Minute;
>>> +    eft->second = time->Second;
>>> +    eft->pad1 = time->Pad1;
>>> +    eft->nanosecond = time->Nanosecond;
>>> +    eft->timezone = time->TimeZone;
>>> +    eft->daylight = time->Daylight;
>>> +    eft->pad2 = time->Pad2;
>>> +}
>>> +
>>> +static void convert_from_guid(efi_guid_t *vendor, EFI_GUID
>>> *vendor_guid)
>>> +{
>>> +    int i;
>>> +    for (i = 0; i < 16; i++) {
>>> +        if (i < 4)
>>> +            vendor->b[i] = (vendor_guid->Data1 >> (8*i)) & 0xff;
>>> +        else if (i < 6)
>>> +            vendor->b[i] = (vendor_guid->Data2 >> (8*(i-4))) & 0xff;
>>> +        else if (i < 8)
>>> +            vendor->b[i] = (vendor_guid->Data3 >> (8*(i-6))) & 0xff;
>>> +        else
>>> +            vendor->b[i] = (vendor_guid->Data4[i-8]);
>>> +    }
>>> +}
>>> +
>>> +static void convert_to_guid(efi_guid_t *vendor, EFI_GUID *vendor_guid)
>>> +{
>>> +    int i;
>>> +    vendor_guid->Data1 = vendor->b[0] + (vendor->b[1] << 8) +
>>> +                (vendor->b[2] << 16) + (vendor->b[3] << 24);
>>> +    vendor_guid->Data2 = vendor->b[4] + (vendor->b[5] << 8);
>>> +    vendor_guid->Data3 = vendor->b[6] + (vendor->b[7] << 8);
>>> +    for (i = 0; i < 8; i++)
>>> +        vendor_guid->Data4[i] = vendor->b[i+8];
>>> +}
>>> +
>>> +static long efi_runtime_ioctl(struct file *file, unsigned int cmd,
>>> +                            unsigned long arg)
>>> +{
>>> +    efi_status_t status;
>>> +    struct efi_getvariable __user *pgetvariable;
>>> +    struct efi_setvariable __user *psetvariable;
>>> +
>>> +    efi_guid_t vendor;
>>> +    EFI_GUID vendor_guid;
>>> +    unsigned long datasize;
>>> +    uint32_t attr;
>>> +
>>> +    efi_time_t eft;
>>> +    efi_time_cap_t cap;
>>> +    struct efi_gettime __user *pgettime;
>>> +    struct efi_settime __user *psettime;
>>> +
>>> +    unsigned char enabled, pending;
>>> +    EFI_TIME efi_time;
>>> +    struct efi_getwakeuptime __user *pgetwakeuptime;
>>> +    struct efi_setwakeuptime __user *psetwakeuptime;
>>> +
>>> +    struct efi_getnextvariablename __user *pgetnextvariablename;
>>> +    unsigned long name_size;
>>> +
>>> +    switch (cmd) {
>>> +    case EFI_RUNTIME_GET_VARIABLE:
>>> +        pgetvariable = (struct efi_getvariable __user *)arg;
>>> +
>>> +        if (get_user(datasize, pgetvariable->DataSize) ||
>>> +            copy_from_user(&vendor_guid, pgetvariable->VendorGuid,
>>> +                            sizeof(EFI_GUID)))
>>> +            return -EFAULT;
>>> +
>>> +        convert_from_guid(&vendor, &vendor_guid);
>>> +        status = efi.get_variable(pgetvariable->VariableName, &vendor,
>>> +                    &attr, &datasize, pgetvariable->Data);
>>> +
>>> +        if (status == EFI_SUCCESS) {
>>> +            if (put_user(attr, pgetvariable->Attributes) ||
>>> +                put_user(datasize, pgetvariable->DataSize))
>>> +                return -EFAULT;
>>> +            return status;
>>> +        } else {
>>> +            printk(KERN_ERR "efi_runtime: can't get variable\n");
>>> +            return status;
>>> +        }
>>> +
>>> +    case EFI_RUNTIME_SET_VARIABLE:
>>> +        psetvariable = (struct efi_setvariable __user *)arg;
>>> +        if (get_user(datasize, &psetvariable->DataSize) ||
>>> +            get_user(attr, &psetvariable->Attributes) ||
>>> +            copy_from_user(&vendor_guid, psetvariable->VendorGuid,
>>> +                            sizeof(EFI_GUID)))
>>> +            return -EFAULT;
>>> +
>>> +        convert_from_guid(&vendor, &vendor_guid);
>>> +        status = efi.set_variable(psetvariable->VariableName, &vendor,
>>> +                    attr, datasize, psetvariable->Data);
>>> +        return status;
>>> +
>>> +    case EFI_RUNTIME_GET_TIME:
>>> +        status = efi.get_time(&eft, &cap);
>>> +        if (status != EFI_SUCCESS) {
>>> +            printk(KERN_ERR "efitime: can't read time\n");
>>> +            return status;
>>> +        }
>>> +
>>> +        pgettime = (struct efi_gettime __user *)arg;
>>> +        if (put_user(cap.resolution,
>>> +                    &pgettime->Capabilities->Resolution) ||
>>> +                    put_user(cap.accuracy,
>>> +                    &pgettime->Capabilities->Accuracy) ||
>>> +                    put_user(cap.sets_to_zero,
>>> +                    &pgettime->Capabilities->SetsToZero))
>>> +            return -EFAULT;
>>> +        return copy_to_user(pgettime->Time, &eft,
>>> +                sizeof(EFI_TIME)) ? -EFAULT : status;
>>> +
>>> +    case EFI_RUNTIME_SET_TIME:
>>> +
>>> +        psettime = (struct efi_settime __user *)arg;
>>> +        if (copy_from_user(&efi_time, psettime->Time,
>>> +                        sizeof(EFI_TIME)))
>>> +            return -EFAULT;
>>> +        convert_to_efi_time(&eft, &efi_time);
>>> +        status = efi.set_time(&eft);
>>> +        return status == EFI_SUCCESS ? status : -EINVAL;
>>> +
>>> +    case EFI_RUNTIME_GET_WAKETIME:
>>> +
>>> +        status = efi.get_wakeup_time((efi_bool_t *)&enabled,
>>> +                        (efi_bool_t *)&pending, &eft);
>>> +
>>> +        if (status != EFI_SUCCESS)
>>> +            return status;
>>> +
>>> +        pgetwakeuptime = (struct efi_getwakeuptime __user *)arg;
>>> +
>>> +        if (put_user(enabled, pgetwakeuptime->Enabled) ||
>>> +                put_user(pending, pgetwakeuptime->Pending))
>>> +            return -EFAULT;
>>> +
>>> +        convert_from_efi_time(&eft, &efi_time);
>>> +
>>> +        return copy_to_user(pgetwakeuptime->Time, &efi_time,
>>> +                sizeof(EFI_TIME)) ? -EFAULT : status;
>>> +
>>> +    case EFI_RUNTIME_SET_WAKETIME:
>>> +
>>> +        psetwakeuptime = (struct efi_setwakeuptime __user *)arg;
>>> +
>>> +        if (get_user(enabled, &psetwakeuptime->Enabled) ||
>>> +                    copy_from_user(&efi_time,
>>> +                    psetwakeuptime->Time,
>>> +                    sizeof(EFI_TIME)))
>>> +            return -EFAULT;
>>> +
>>> +        convert_to_efi_time(&eft, &efi_time);
>>> +
>>> +        status = efi.set_wakeup_time(enabled, &eft);
>>> +
>>> +        return status == EFI_SUCCESS ? status : -EINVAL;
>>> +
>>> +    case EFI_RUNTIME_GET_NEXTVARIABLENAME:
>>> +
>>> +        pgetnextvariablename = (struct efi_getnextvariablename
>>> +                                __user *)arg;
>>> +
>>> +        if (get_user(name_size, pgetnextvariablename->VariableNameSize)
>>> +                || copy_from_user(&vendor_guid,
>>> +                    pgetnextvariablename->VendorGuid,
>>> +                    sizeof(EFI_GUID)))
>>> +            return -EFAULT;
>>> +        if (name_size > 1024)
>>> +            return -EFAULT;
>>> +
>>> +        convert_from_guid(&vendor, &vendor_guid);
>>> +
>>> +        status = efi.get_next_variable(&name_size,
>>> +                    pgetnextvariablename->VariableName,
>>> +                                &vendor);
>>> +
>>> +        if (status != EFI_SUCCESS)
>>> +            return status;
>>> +        convert_to_guid(&vendor, &vendor_guid);
>>> +
>>> +        if (put_user(name_size,
>>> pgetnextvariablename->VariableNameSize))
>>> +            return -EFAULT;
>>> +
>>> +        if (copy_to_user(pgetnextvariablename->VendorGuid,
>>> +                        &vendor_guid, sizeof(EFI_GUID)))
>>> +            return -EFAULT;
>>> +        return status;
>>> +    }
>>> +
>>> +    return -ENOTTY;
>>> +}
>>
>>
>> OK, so efi_runtime_ioctl() returns a signed long, but the code is
>> actually returning status which is an efi_status which is an unsigned
>> long.  What happens if the efi status has the top bit set? .. this would
>> return a -ve value which looks like an error.  I think we need to see if
>> this makes sense or not - what is the range of return values from
>> underling EFI calls?
>>
>>
>
> yup, I noticed this as well, actually, this is a temporary solution.

Yep, I understand that, which is understandable.  However, temporary 
solutions turn into fixed implementations which are hard to change, so 
my preference is to do it right first time.

> The high bit of return value means EFI_STATUS Success Codes (High Bit Clear)
> or EFI_STATUS Warning Codes (High Bit Clear), I ignore the high bit
> here. So if we get the status in user space, we get two possibilities
> (error or warning).

OK - so returning the high bit will confuse the ioctl() return as this 
does indicate an error.


> I am planing to put the efi_status into each structures and return to
> user space. Is that make sense to you? or any suggestion? :)
>

Yes, indeed, that makes sense.

Apologies if it means more work. :-/

Colin

>>> +
>>> +static int efi_runtime_open(struct inode *inode, struct file *file)
>>> +{
>>> +    /*
>>> +     * nothing special to do here
>>> +     * We do accept multiple open files at the same time as we
>>> +     * synchronize on the per call operation.
>>> +     */
>>> +    return 0;
>>> +}
>>> +
>>> +static int efi_runtime_close(struct inode *inode, struct file *file)
>>> +{
>>> +    return 0;
>>> +}
>>> +
>>> +/*
>>> + *    The various file operations we support.
>>> + */
>>> +static const struct file_operations efi_runtime_fops = {
>>> +    .owner        = THIS_MODULE,
>>> +    .unlocked_ioctl    = efi_runtime_ioctl,
>>> +    .open        = efi_runtime_open,
>>> +    .release    = efi_runtime_close,
>>> +    .llseek        = no_llseek,
>>> +};
>>> +
>>> +static struct miscdevice efi_runtime_dev = {
>>> +    MISC_DYNAMIC_MINOR,
>>> +    "efi_runtime",
>>> +    &efi_runtime_fops
>>> +};
>>> +
>>> +static int __init efi_runtime_init(void)
>>> +{
>>> +    int ret;
>>> +
>>> +    printk(KERN_INFO "EFI_RUNTIME Driver v%s\n", EFI_FWTS_EFI_VERSION);
>>> +
>>> +    ret = misc_register(&efi_runtime_dev);
>>> +    if (ret) {
>>> +        printk(KERN_ERR "efi_runtime: can't misc_register on
>>> minor=%d\n",
>>> +                MISC_DYNAMIC_MINOR);
>>> +        return ret;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void __exit efi_runtime_exit(void)
>>> +{
>>> +    printk(KERN_INFO "EFI_RUNTIME Driver Exit.\n");
>>> +    misc_deregister(&efi_runtime_dev);
>>> +}
>>> +
>>> +module_init(efi_runtime_init);
>>> +module_exit(efi_runtime_exit);
>>> +
>>> diff --git a/efi_runtime/efi_runtime.h b/efi_runtime/efi_runtime.h
>>> new file mode 100644
>>> index 0000000..0766916
>>> --- /dev/null
>>> +++ b/efi_runtime/efi_runtime.h
>>> @@ -0,0 +1,112 @@
>>> +/*
>>> + * EFI Runtime driver
>>> + *
>>> + * Copyright(C) 2012 Canonical Ltd.
>>> + *
>>> + *  This program is free software; you can redistribute it and/or
>>> modify
>>> + *  it under the terms of the GNU General Public License as
>>> published by
>>> + *  the Free Software Foundation; either version 2 of the License, or
>>> + *  (at your option) any later version.
>>> + *
>>> + *  This program is distributed in the hope that it will be useful,
>>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + *  GNU General Public License for more details.
>>> + *
>>> + *  You should have received a copy of the GNU General Public License
>>> + *  along with this program; if not, write to the Free Software
>>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
>>> 02111-1307  USA
>>> + */
>>> +
>>> +#ifndef _EFI_RUNTIME_H_
>>> +#define _EFI_RUNTIME_H_
>>> +
>>> +typedef struct {
>>> +    uint32_t    Data1;
>>> +    uint16_t    Data2;
>>> +    uint16_t    Data3;
>>> +    uint8_t        Data4[8];
>>> +} __attribute__ ((packed)) EFI_GUID;
>>> +
>>> +typedef struct {
>>> +    uint16_t    Year;        /* 1900 – 9999 */
>>> +    uint8_t        Month;        /* 1 – 12 */
>>> +    uint8_t        Day;        /* 1 – 31 */
>>> +    uint8_t        Hour;        /* 0 – 23 */
>>> +    uint8_t        Minute;        /* 0 – 59 */
>>> +    uint8_t        Second;        /* 0 – 59 */
>>> +    uint8_t        Pad1;
>>> +    uint32_t    Nanosecond;    /* 0 – 999,999,999 */
>>> +    int16_t        TimeZone;    /* -1440 to 1440 or 2047 */
>>> +    uint8_t        Daylight;
>>> +    uint8_t        Pad2;
>>> +} __attribute__ ((packed)) EFI_TIME;
>>> +
>>> +typedef struct {
>>> +    uint32_t    Resolution;
>>> +    uint32_t    Accuracy;
>>> +    uint8_t        SetsToZero;
>>> +} __attribute__ ((packed)) EFI_TIME_CAPABILITIES;
>>> +
>>> +struct efi_getvariable {
>>> +    uint16_t    *VariableName;
>>> +    EFI_GUID    *VendorGuid;
>>> +    uint32_t    *Attributes;
>>> +    uint64_t    *DataSize;
>>> +    void        *Data;
>>> +} __attribute__ ((packed));
>>> +
>>> +struct efi_setvariable {
>>> +    uint16_t    *VariableName;
>>> +    EFI_GUID    *VendorGuid;
>>> +    uint32_t    Attributes;
>>> +    uint64_t    DataSize;
>>> +    void        *Data;
>>> +} __attribute__ ((packed));
>>> +
>>> +struct efi_getnextvariablename {
>>> +    uint64_t    *VariableNameSize;
>>> +    uint16_t    *VariableName;
>>> +    EFI_GUID    *VendorGuid;
>>> +} __attribute__ ((packed));
>>> +
>>> +struct efi_gettime {
>>> +    EFI_TIME        *Time;
>>> +    EFI_TIME_CAPABILITIES    *Capabilities;
>>> +} __attribute__ ((packed));
>>> +
>>> +struct efi_settime {
>>> +    EFI_TIME        *Time;
>>> +} __attribute__ ((packed));
>>> +
>>> +struct efi_getwakeuptime {
>>> +    uint8_t        *Enabled;
>>> +    uint8_t        *Pending;
>>> +    EFI_TIME    *Time;
>>> +} __attribute__ ((packed));
>>> +
>>> +struct efi_setwakeuptime {
>>> +    uint8_t        Enabled;
>>> +    EFI_TIME    *Time;
>>> +} __attribute__ ((packed));
>>> +
>>> +/* ioctl calls that are permitted to the /dev/efi_runtime interface. */
>>> +#define EFI_RUNTIME_GET_VARIABLE \
>>> +    _IOWR('p', 0x01, struct efi_getvariable)
>>> +#define EFI_RUNTIME_SET_VARIABLE \
>>> +    _IOW('p', 0x02, struct efi_setvariable)
>>> +
>>> +#define EFI_RUNTIME_GET_TIME \
>>> +    _IOR('p', 0x03, struct efi_gettime)
>>> +#define EFI_RUNTIME_SET_TIME \
>>> +    _IOW('p', 0x04, struct efi_settime)
>>> +
>>> +#define EFI_RUNTIME_GET_WAKETIME \
>>> +    _IOR('p', 0x05, struct efi_getwakeuptime)
>>> +#define EFI_RUNTIME_SET_WAKETIME \
>>> +    _IOW('p', 0x06, struct efi_setwakeuptime)
>>> +
>>> +#define EFI_RUNTIME_GET_NEXTVARIABLENAME \
>>> +    _IOWR('p', 0x07, struct efi_getnextvariablename)
>>> +
>>> +#endif /* _EFI_RUNTIME_H_ */
>>>
>>
>>
>