linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Shorten constant names for EFI variable attributes
@ 2012-07-20 22:08 Khalid Aziz
  2012-07-20 22:10 ` H. Peter Anvin
  0 siblings, 1 reply; 12+ messages in thread
From: Khalid Aziz @ 2012-07-20 22:08 UTC (permalink / raw)
  To: mjg, mikew, tony.luck, keescook, gong.chen, gregkh,
	paul.gortmaker, maxin.john, rdunlap, hpa, matt.fleming, olof,
	dhowells
  Cc: linux-kernel

Replace very long constants for EFI variable attributes
with shorter and more convenient names. Also create an
alias for the current longer names so as to not break
compatibility with current API since these constants
are used by userspace programs. This patch depends on 
patch <https://lkml.org/lkml/2012/7/13/313>.


Signed-off-by: Khalid Aziz <khalid.aziz@hp.com>
---
 drivers/firmware/efivars.c     |   20 ++++++++------------
 drivers/firmware/google/gsmi.c |    8 ++------
 include/linux/efi.h            |   35 ++++++++++++++++++++---------------
 3 files changed, 30 insertions(+), 33 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index d10c987..27f4fab 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -122,10 +122,7 @@ struct efivar_attribute {
 	ssize_t (*store)(struct efivar_entry *entry, const char *buf, size_t count);
 };
 
-#define PSTORE_EFI_ATTRIBUTES \
-	(EFI_VARIABLE_NON_VOLATILE | \
-	 EFI_VARIABLE_BOOTSERVICE_ACCESS | \
-	 EFI_VARIABLE_RUNTIME_ACCESS)
+#define PSTORE_EFI_ATTRIBUTES	(EFI_VAR_NV | EFI_VAR_BOOT | EFI_VAR_RUNTIME)
 
 #define EFIVAR_ATTR(_name, _mode, _show, _store) \
 struct efivar_attribute efivar_attr_##_name = { \
@@ -435,22 +432,21 @@ efivar_attr_read(struct efivar_entry *entry, char *buf)
 	if (status != EFI_SUCCESS)
 		return -EIO;
 
-	if (var->Attributes & EFI_VARIABLE_NON_VOLATILE)
+	if (var->Attributes & EFI_VAR_NV)
 		str += sprintf(str, "EFI_VARIABLE_NON_VOLATILE\n");
-	if (var->Attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)
+	if (var->Attributes & EFI_VAR_BOOT)
 		str += sprintf(str, "EFI_VARIABLE_BOOTSERVICE_ACCESS\n");
-	if (var->Attributes & EFI_VARIABLE_RUNTIME_ACCESS)
+	if (var->Attributes & EFI_VAR_RUNTIME)
 		str += sprintf(str, "EFI_VARIABLE_RUNTIME_ACCESS\n");
-	if (var->Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD)
+	if (var->Attributes & EFI_VAR_HW_ERR)
 		str += sprintf(str, "EFI_VARIABLE_HARDWARE_ERROR_RECORD\n");
-	if (var->Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS)
+	if (var->Attributes & EFI_VAR_AUTH_WRITE)
 		str += sprintf(str,
 			"EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS\n");
-	if (var->Attributes &
-			EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)
+	if (var->Attributes & EFI_VAR_TIMED_AUTH_WRITE)
 		str += sprintf(str,
 			"EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS\n");
-	if (var->Attributes & EFI_VARIABLE_APPEND_WRITE)
+	if (var->Attributes & EFI_VAR_APPEND)
 		str += sprintf(str, "EFI_VARIABLE_APPEND_WRITE\n");
 	return str - buf;
 }
diff --git a/drivers/firmware/google/gsmi.c b/drivers/firmware/google/gsmi.c
index 91ddf0f..3e2d6d4 100644
--- a/drivers/firmware/google/gsmi.c
+++ b/drivers/firmware/google/gsmi.c
@@ -353,9 +353,7 @@ static efi_status_t gsmi_get_variable(efi_char16_t *name,
 		memcpy(data, gsmi_dev.data_buf->start, *data_size);
 
 		/* All variables are have the following attributes */
-		*attr = EFI_VARIABLE_NON_VOLATILE |
-			EFI_VARIABLE_BOOTSERVICE_ACCESS |
-			EFI_VARIABLE_RUNTIME_ACCESS;
+		*attr = EFI_VAR_NV | EFI_VAR_BOOT | EFI_VAR_RUNTIME;
 	}
 
 	spin_unlock_irqrestore(&gsmi_dev.lock, flags);
@@ -430,9 +428,7 @@ static efi_status_t gsmi_set_variable(efi_char16_t *name,
 		.name_ptr = gsmi_dev.name_buf->address,
 		.data_ptr = gsmi_dev.data_buf->address,
 		.data_len = (u32)data_size,
-		.attributes = EFI_VARIABLE_NON_VOLATILE |
-			      EFI_VARIABLE_BOOTSERVICE_ACCESS |
-			      EFI_VARIABLE_RUNTIME_ACCESS,
+		.attributes = EFI_VAR_NV | EFI_VAR_BOOT | EFI_VAR_RUNTIME,
 	};
 	size_t name_len = utf16_strlen(name, GSMI_BUF_SIZE / 2);
 	efi_status_t ret = EFI_SUCCESS;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index ec45ccd..3d2a6f0 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -551,21 +551,26 @@ extern int __init efi_setup_pcdp_console(char *);
 /*
  * Variable Attributes
  */
-#define EFI_VARIABLE_NON_VOLATILE       0x0000000000000001
-#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002
-#define EFI_VARIABLE_RUNTIME_ACCESS     0x0000000000000004
-#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x0000000000000008
-#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x0000000000000010
-#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x0000000000000020
-#define EFI_VARIABLE_APPEND_WRITE	0x0000000000000040
-
-#define EFI_VARIABLE_MASK 	(EFI_VARIABLE_NON_VOLATILE | \
-				EFI_VARIABLE_BOOTSERVICE_ACCESS | \
-				EFI_VARIABLE_RUNTIME_ACCESS | \
-				EFI_VARIABLE_HARDWARE_ERROR_RECORD | \
-				EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \
-				EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | \
-				EFI_VARIABLE_APPEND_WRITE)
+#define EFI_VAR_NV			0x0000000000000001
+#define EFI_VAR_BOOT			0x0000000000000002
+#define EFI_VAR_RUNTIME			0x0000000000000004
+#define EFI_VAR_HW_ERR			0x0000000000000008
+#define EFI_VAR_AUTH_WRITE		0x0000000000000010
+#define EFI_VAR_TIMED_AUTH_WRITE	0x0000000000000020
+#define EFI_VAR_APPEND			0x0000000000000040
+
+#define EFI_VARIABLE_NON_VOLATILE			EFI_VAR_NV
+#define EFI_VARIABLE_BOOTSERVICE_ACCESS			EFI_VAR_BOOT
+#define EFI_VARIABLE_RUNTIME_ACCESS			EFI_VAR_RUNTIME
+#define EFI_VARIABLE_HARDWARE_ERROR_RECORD		EFI_VAR_HW_ERR
+#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS		EFI_VAR_AUTH_WRITE
+#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS \
+							EFI_VAR_TIMED_AUTH_WRITE
+#define EFI_VARIABLE_APPEND_WRITE			EFI_VAR_APPEND
+
+#define EFI_VARIABLE_MASK	(EFI_VAR_NV | EFI_VAR_BOOT | EFI_VAR_RUNTIME | \
+				EFI_VAR_HW_ERR | EFI_VAR_AUTH_WRITE | \
+				EFI_VAR_TIMED_AUTH_WRITE | EFI_VAR_APPEND)
 /*
  * The type of search to perform when calling boottime->locate_handle
  */
-- 
1.7.9.5

-- 

===========
Khalid Aziz
khalid.aziz@hp.com

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] Shorten constant names for EFI variable attributes
  2012-07-20 22:08 [PATCH] Shorten constant names for EFI variable attributes Khalid Aziz
@ 2012-07-20 22:10 ` H. Peter Anvin
  2012-07-20 22:30   ` Khalid Aziz
  2012-07-23 13:26   ` Matthew Garrett
  0 siblings, 2 replies; 12+ messages in thread
From: H. Peter Anvin @ 2012-07-20 22:10 UTC (permalink / raw)
  To: Khalid Aziz
  Cc: mjg, mikew, tony.luck, keescook, gong.chen, gregkh,
	paul.gortmaker, maxin.john, rdunlap, matt.fleming, olof,
	dhowells, linux-kernel

On 07/20/2012 03:08 PM, Khalid Aziz wrote:
> Replace very long constants for EFI variable attributes
> with shorter and more convenient names. Also create an
> alias for the current longer names so as to not break
> compatibility with current API since these constants
> are used by userspace programs. This patch depends on
> patch <https://lkml.org/lkml/2012/7/13/313>.

I think these some from the EFI specifcation, so NAK IMO.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Shorten constant names for EFI variable attributes
  2012-07-20 22:10 ` H. Peter Anvin
@ 2012-07-20 22:30   ` Khalid Aziz
  2012-07-20 22:34     ` H. Peter Anvin
  2012-07-23 13:26   ` Matthew Garrett
  1 sibling, 1 reply; 12+ messages in thread
From: Khalid Aziz @ 2012-07-20 22:30 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: mjg, mikew, tony.luck, keescook, gong.chen, gregkh,
	paul.gortmaker, maxin.john, rdunlap, matt.fleming, olof,
	dhowells, linux-kernel

On 07/20/2012 04:10 PM, H. Peter Anvin wrote:
> On 07/20/2012 03:08 PM, Khalid Aziz wrote:
>> Replace very long constants for EFI variable attributes
>> with shorter and more convenient names. Also create an
>> alias for the current longer names so as to not break
>> compatibility with current API since these constants
>> are used by userspace programs. This patch depends on
>> patch <https://lkml.org/lkml/2012/7/13/313>.
>
> I think these some from the EFI specifcation, so NAK IMO.
>
>     -hpa
>
This patch is based upon earlier discussion at 
<https://lkml.org/lkml/2012/7/13/320>.

You are right that EFI specification uses exactly these long names for 
the constants, but does that mean kernel must also use the exact same 
long constant names? I can see doing that for the sake of consistency. 
At the same time, can we make the kernel code more readable and retain 
compatibility with existing API by using aliases? I slightly prefer 
making kernel code more readable, but I could go either way.

-- 

Khalid Aziz
khalid.aziz@hp.com


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Shorten constant names for EFI variable attributes
  2012-07-20 22:30   ` Khalid Aziz
@ 2012-07-20 22:34     ` H. Peter Anvin
  2012-07-20 22:46       ` Khalid Aziz
  0 siblings, 1 reply; 12+ messages in thread
From: H. Peter Anvin @ 2012-07-20 22:34 UTC (permalink / raw)
  To: Khalid Aziz
  Cc: mjg, mikew, tony.luck, keescook, gong.chen, gregkh,
	paul.gortmaker, maxin.john, rdunlap, matt.fleming, olof,
	dhowells, linux-kernel

On 07/20/2012 03:30 PM, Khalid Aziz wrote:
> On 07/20/2012 04:10 PM, H. Peter Anvin wrote:
>> On 07/20/2012 03:08 PM, Khalid Aziz wrote:
>>> Replace very long constants for EFI variable attributes
>>> with shorter and more convenient names. Also create an
>>> alias for the current longer names so as to not break
>>> compatibility with current API since these constants
>>> are used by userspace programs. This patch depends on
>>> patch <https://lkml.org/lkml/2012/7/13/313>.
>>
>> I think these some from the EFI specifcation, so NAK IMO.
>>
>>     -hpa
>>
> This patch is based upon earlier discussion at
> <https://lkml.org/lkml/2012/7/13/320>.
>
> You are right that EFI specification uses exactly these long names for
> the constants, but does that mean kernel must also use the exact same
> long constant names? I can see doing that for the sake of consistency.
> At the same time, can we make the kernel code more readable and retain
> compatibility with existing API by using aliases? I slightly prefer
> making kernel code more readable, but I could go either way.
>

I think it makes the kernel code less readable, because now you not only 
need to understand the kernel code and the EFI spec, but also how the 
two maps onto each other. The fact that you then have to introduce 
aliases indicates to me that you're doing something actively broken.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Shorten constant names for EFI variable attributes
  2012-07-20 22:34     ` H. Peter Anvin
@ 2012-07-20 22:46       ` Khalid Aziz
  0 siblings, 0 replies; 12+ messages in thread
From: Khalid Aziz @ 2012-07-20 22:46 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: mjg, mikew, tony.luck, keescook, gong.chen, gregkh,
	paul.gortmaker, maxin.john, rdunlap, matt.fleming, olof,
	dhowells, linux-kernel

On 07/20/2012 04:34 PM, H. Peter Anvin wrote:
> On 07/20/2012 03:30 PM, Khalid Aziz wrote:
>>
>> This patch is based upon earlier discussion at
>> <https://lkml.org/lkml/2012/7/13/320>.
>>
>> You are right that EFI specification uses exactly these long names for
>> the constants, but does that mean kernel must also use the exact same
>> long constant names? I can see doing that for the sake of consistency.
>> At the same time, can we make the kernel code more readable and retain
>> compatibility with existing API by using aliases? I slightly prefer
>> making kernel code more readable, but I could go either way.
>>
>
> I think it makes the kernel code less readable, because now you not 
> only need to understand the kernel code and the EFI spec, but also how 
> the two maps onto each other. The fact that you then have to introduce 
> aliases indicates to me that you're doing something actively broken.
>
>     -hpa
>
As I think more about it, existence of aliases could also potentially 
create confusion where someone adding new code to kernel chooses to use 
the long name instead. Maybe unless we can make a clean break from long 
names, it is not worth doing this and that is going to be problematic 
because of the existing usage in userspace programs.

Matthew, do you have a different point of view?

-- 
Khalid
khalid.aziz@hp.com


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Shorten constant names for EFI variable attributes
  2012-07-20 22:10 ` H. Peter Anvin
  2012-07-20 22:30   ` Khalid Aziz
@ 2012-07-23 13:26   ` Matthew Garrett
  2012-07-26 17:28     ` Khalid Aziz
  1 sibling, 1 reply; 12+ messages in thread
From: Matthew Garrett @ 2012-07-23 13:26 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Khalid Aziz, mikew, tony.luck, keescook, gong.chen, gregkh,
	paul.gortmaker, maxin.john, rdunlap, matt.fleming, olof,
	dhowells, linux-kernel

On Fri, Jul 20, 2012 at 03:10:56PM -0700, H. Peter Anvin wrote:
> On 07/20/2012 03:08 PM, Khalid Aziz wrote:
> >Replace very long constants for EFI variable attributes
> >with shorter and more convenient names. Also create an
> >alias for the current longer names so as to not break
> >compatibility with current API since these constants
> >are used by userspace programs. This patch depends on
> >patch <https://lkml.org/lkml/2012/7/13/313>.
> 
> I think these some from the EFI specifcation, so NAK IMO.

>From the point of view of making efivars more readable, I'd certainly 
prefer shorter constant names. Keeping an alias is necessary only 
because it's an existing exposed interface. The specification doesn't 
actually require the use of these specific names anywhere, and we've 
taken a more relaxed attitude in other bits of the EFI code.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Shorten constant names for EFI variable attributes
  2012-07-23 13:26   ` Matthew Garrett
@ 2012-07-26 17:28     ` Khalid Aziz
  2012-07-26 17:33       ` Matthew Garrett
  0 siblings, 1 reply; 12+ messages in thread
From: Khalid Aziz @ 2012-07-26 17:28 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: H. Peter Anvin, mikew, tony.luck, keescook, gong.chen, gregkh,
	paul.gortmaker, maxin.john, rdunlap, matt.fleming, olof,
	dhowells, linux-kernel

On 07/23/2012 07:26 AM, Matthew Garrett wrote:
> On Fri, Jul 20, 2012 at 03:10:56PM -0700, H. Peter Anvin wrote:
>> On 07/20/2012 03:08 PM, Khalid Aziz wrote:
>>> Replace very long constants for EFI variable attributes
>>> with shorter and more convenient names. Also create an
>>> alias for the current longer names so as to not break
>>> compatibility with current API since these constants
>>> are used by userspace programs. This patch depends on
>>> patch <https://lkml.org/lkml/2012/7/13/313>.
>> I think these some from the EFI specifcation, so NAK IMO.
>  From the point of view of making efivars more readable, I'd certainly
> prefer shorter constant names. Keeping an alias is necessary only
> because it's an existing exposed interface. The specification doesn't
> actually require the use of these specific names anywhere, and we've
> taken a more relaxed attitude in other bits of the EFI code.
>
Matthew,

I also do not believe that kernel must use the constant names mentioned 
in the specification especially when the name reaches 50 characters. We 
can not get away from having to create aliases. Do you think having 
aliases in efi.h can cause mixed use of long names and short names in 
future code in the kernel? Can we address this by suggesting to future 
code authors that they should use the short names in their code? Should 
we consider inclusion of this patch in the kernel?

-- 
Khalid Aziz
khalid.aziz@hp.com


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Shorten constant names for EFI variable attributes
  2012-07-26 17:28     ` Khalid Aziz
@ 2012-07-26 17:33       ` Matthew Garrett
  2012-09-25 15:41         ` [PATCH -next v2] " Khalid Aziz
  0 siblings, 1 reply; 12+ messages in thread
From: Matthew Garrett @ 2012-07-26 17:33 UTC (permalink / raw)
  To: Khalid Aziz
  Cc: H. Peter Anvin, mikew, tony.luck, keescook, gong.chen, gregkh,
	paul.gortmaker, maxin.john, rdunlap, matt.fleming, olof,
	dhowells, linux-kernel

On Thu, Jul 26, 2012 at 11:28:32AM -0600, Khalid Aziz wrote:

> I also do not believe that kernel must use the constant names
> mentioned in the specification especially when the name reaches 50
> characters. We can not get away from having to create aliases. Do
> you think having aliases in efi.h can cause mixed use of long names
> and short names in future code in the kernel? Can we address this by
> suggesting to future code authors that they should use the short
> names in their code? Should we consider inclusion of this patch in
> the kernel?

I'd be surprised if it were a problem - we should catch any of those 
cases in code review, or gate the aliases under #ifndef __KERNEL__

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH -next v2] Shorten constant names for EFI variable attributes
  2012-07-26 17:33       ` Matthew Garrett
@ 2012-09-25 15:41         ` Khalid Aziz
  2012-09-25 21:55           ` Stephen Rothwell
  0 siblings, 1 reply; 12+ messages in thread
From: Khalid Aziz @ 2012-09-25 15:41 UTC (permalink / raw)
  To: Matthew Garrett, hpa, keescook, tony.luck, Greg KH, cbouatmailru,
	ccross, paul.gortmaker, maxin.john, matt.fleming, olof, dhowells
  Cc: linux-next, linux-kernel, khalid

Replace very long constants for EFI variable attributes with
shorter and more convenient names. Also create an alias for
the current longer names so as to not break compatibility
with current API since these constants are used by
userspace programs.


Signed-off-by: Khalid Aziz <khalid.aziz@hp.com>
Cc: Khalid Aziz <khalid@gonehiking.org>
---
Change for v2: Made only the new constant names available for use in kernel by
	adding #ifndef __KERNEL__ around aliases for old constant names

 drivers/firmware/efivars.c     |   20 ++++++++------------
 drivers/firmware/google/gsmi.c |    8 ++------
 include/linux/efi.h            |   37 ++++++++++++++++++++++---------------
 3 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index d10c987..27f4fab 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -122,10 +122,7 @@ struct efivar_attribute {
 	ssize_t (*store)(struct efivar_entry *entry, const char *buf, size_t count);
 };
 
-#define PSTORE_EFI_ATTRIBUTES \
-	(EFI_VARIABLE_NON_VOLATILE | \
-	 EFI_VARIABLE_BOOTSERVICE_ACCESS | \
-	 EFI_VARIABLE_RUNTIME_ACCESS)
+#define PSTORE_EFI_ATTRIBUTES	(EFI_VAR_NV | EFI_VAR_BOOT | EFI_VAR_RUNTIME)
 
 #define EFIVAR_ATTR(_name, _mode, _show, _store) \
 struct efivar_attribute efivar_attr_##_name = { \
@@ -435,22 +432,21 @@ efivar_attr_read(struct efivar_entry *entry, char *buf)
 	if (status != EFI_SUCCESS)
 		return -EIO;
 
-	if (var->Attributes & EFI_VARIABLE_NON_VOLATILE)
+	if (var->Attributes & EFI_VAR_NV)
 		str += sprintf(str, "EFI_VARIABLE_NON_VOLATILE\n");
-	if (var->Attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)
+	if (var->Attributes & EFI_VAR_BOOT)
 		str += sprintf(str, "EFI_VARIABLE_BOOTSERVICE_ACCESS\n");
-	if (var->Attributes & EFI_VARIABLE_RUNTIME_ACCESS)
+	if (var->Attributes & EFI_VAR_RUNTIME)
 		str += sprintf(str, "EFI_VARIABLE_RUNTIME_ACCESS\n");
-	if (var->Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD)
+	if (var->Attributes & EFI_VAR_HW_ERR)
 		str += sprintf(str, "EFI_VARIABLE_HARDWARE_ERROR_RECORD\n");
-	if (var->Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS)
+	if (var->Attributes & EFI_VAR_AUTH_WRITE)
 		str += sprintf(str,
 			"EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS\n");
-	if (var->Attributes &
-			EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)
+	if (var->Attributes & EFI_VAR_TIMED_AUTH_WRITE)
 		str += sprintf(str,
 			"EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS\n");
-	if (var->Attributes & EFI_VARIABLE_APPEND_WRITE)
+	if (var->Attributes & EFI_VAR_APPEND)
 		str += sprintf(str, "EFI_VARIABLE_APPEND_WRITE\n");
 	return str - buf;
 }
diff --git a/drivers/firmware/google/gsmi.c b/drivers/firmware/google/gsmi.c
index 91ddf0f..3e2d6d4 100644
--- a/drivers/firmware/google/gsmi.c
+++ b/drivers/firmware/google/gsmi.c
@@ -353,9 +353,7 @@ static efi_status_t gsmi_get_variable(efi_char16_t *name,
 		memcpy(data, gsmi_dev.data_buf->start, *data_size);
 
 		/* All variables are have the following attributes */
-		*attr = EFI_VARIABLE_NON_VOLATILE |
-			EFI_VARIABLE_BOOTSERVICE_ACCESS |
-			EFI_VARIABLE_RUNTIME_ACCESS;
+		*attr = EFI_VAR_NV | EFI_VAR_BOOT | EFI_VAR_RUNTIME;
 	}
 
 	spin_unlock_irqrestore(&gsmi_dev.lock, flags);
@@ -430,9 +428,7 @@ static efi_status_t gsmi_set_variable(efi_char16_t *name,
 		.name_ptr = gsmi_dev.name_buf->address,
 		.data_ptr = gsmi_dev.data_buf->address,
 		.data_len = (u32)data_size,
-		.attributes = EFI_VARIABLE_NON_VOLATILE |
-			      EFI_VARIABLE_BOOTSERVICE_ACCESS |
-			      EFI_VARIABLE_RUNTIME_ACCESS,
+		.attributes = EFI_VAR_NV | EFI_VAR_BOOT | EFI_VAR_RUNTIME,
 	};
 	size_t name_len = utf16_strlen(name, GSMI_BUF_SIZE / 2);
 	efi_status_t ret = EFI_SUCCESS;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index ec45ccd..9240a5e 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -551,21 +551,28 @@ extern int __init efi_setup_pcdp_console(char *);
 /*
  * Variable Attributes
  */
-#define EFI_VARIABLE_NON_VOLATILE       0x0000000000000001
-#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002
-#define EFI_VARIABLE_RUNTIME_ACCESS     0x0000000000000004
-#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x0000000000000008
-#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x0000000000000010
-#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x0000000000000020
-#define EFI_VARIABLE_APPEND_WRITE	0x0000000000000040
-
-#define EFI_VARIABLE_MASK 	(EFI_VARIABLE_NON_VOLATILE | \
-				EFI_VARIABLE_BOOTSERVICE_ACCESS | \
-				EFI_VARIABLE_RUNTIME_ACCESS | \
-				EFI_VARIABLE_HARDWARE_ERROR_RECORD | \
-				EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \
-				EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | \
-				EFI_VARIABLE_APPEND_WRITE)
+#define EFI_VAR_NV			0x0000000000000001
+#define EFI_VAR_BOOT			0x0000000000000002
+#define EFI_VAR_RUNTIME			0x0000000000000004
+#define EFI_VAR_HW_ERR			0x0000000000000008
+#define EFI_VAR_AUTH_WRITE		0x0000000000000010
+#define EFI_VAR_TIMED_AUTH_WRITE	0x0000000000000020
+#define EFI_VAR_APPEND			0x0000000000000040
+
+#ifndef __KERNEL__
+#define EFI_VARIABLE_NON_VOLATILE			EFI_VAR_NV
+#define EFI_VARIABLE_BOOTSERVICE_ACCESS			EFI_VAR_BOOT
+#define EFI_VARIABLE_RUNTIME_ACCESS			EFI_VAR_RUNTIME
+#define EFI_VARIABLE_HARDWARE_ERROR_RECORD		EFI_VAR_HW_ERR
+#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS		EFI_VAR_AUTH_WRITE
+#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS \
+							EFI_VAR_TIMED_AUTH_WRITE
+#define EFI_VARIABLE_APPEND_WRITE			EFI_VAR_APPEND
+#endif
+
+#define EFI_VARIABLE_MASK	(EFI_VAR_NV | EFI_VAR_BOOT | EFI_VAR_RUNTIME | \
+				EFI_VAR_HW_ERR | EFI_VAR_AUTH_WRITE | \
+				EFI_VAR_TIMED_AUTH_WRITE | EFI_VAR_APPEND)
 /*
  * The type of search to perform when calling boottime->locate_handle
  */
-- 
1.7.8.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH -next v2] Shorten constant names for EFI variable attributes
  2012-09-25 15:41         ` [PATCH -next v2] " Khalid Aziz
@ 2012-09-25 21:55           ` Stephen Rothwell
  2012-09-25 23:06             ` Khalid Aziz
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Rothwell @ 2012-09-25 21:55 UTC (permalink / raw)
  To: Khalid Aziz
  Cc: Matthew Garrett, hpa, keescook, tony.luck, Greg KH, cbouatmailru,
	ccross, paul.gortmaker, maxin.john, matt.fleming, olof, dhowells,
	linux-next, linux-kernel, khalid

[-- Attachment #1: Type: text/plain, Size: 485 bytes --]

Hi,

On Tue, 25 Sep 2012 09:41:00 -0600 Khalid Aziz <khalid.aziz@hp.com> wrote:
>
> Replace very long constants for EFI variable attributes with
> shorter and more convenient names. Also create an alias for
> the current longer names so as to not break compatibility
> with current API since these constants are used by
> userspace programs.

Why do this?  It just looks like churn for no real gain.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH -next v2] Shorten constant names for EFI variable attributes
  2012-09-25 21:55           ` Stephen Rothwell
@ 2012-09-25 23:06             ` Khalid Aziz
  2012-09-25 23:12               ` Matthew Garrett
  0 siblings, 1 reply; 12+ messages in thread
From: Khalid Aziz @ 2012-09-25 23:06 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Matthew Garrett, hpa, keescook, tony.luck, Greg KH, cbouatmailru,
	ccross, paul.gortmaker, maxin.john, matt.fleming, olof, dhowells,
	linux-next, linux-kernel, khalid

On Wed, 2012-09-26 at 07:55 +1000, Stephen Rothwell wrote:
> Hi,
> 
> On Tue, 25 Sep 2012 09:41:00 -0600 Khalid Aziz <khalid.aziz@hp.com> wrote:
> >
> > Replace very long constants for EFI variable attributes with
> > shorter and more convenient names. Also create an alias for
> > the current longer names so as to not break compatibility
> > with current API since these constants are used by
> > userspace programs.
> 
> Why do this?  It just looks like churn for no real gain.
> 

This is for code cleanup and does not impact functionality. This is
based upon an earlier discussion -
<https://lkml.org/lkml/2012/7/13/320>. The goal is to make the code more
readable. V1 of this patch was discussed at
<https://lkml.org/lkml/2012/7/20/414>.

-- 
Khalid Aziz <khalid.aziz@hp.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH -next v2] Shorten constant names for EFI variable attributes
  2012-09-25 23:06             ` Khalid Aziz
@ 2012-09-25 23:12               ` Matthew Garrett
  0 siblings, 0 replies; 12+ messages in thread
From: Matthew Garrett @ 2012-09-25 23:12 UTC (permalink / raw)
  To: Khalid Aziz
  Cc: Stephen Rothwell, hpa, keescook, tony.luck, Greg KH,
	cbouatmailru, ccross, paul.gortmaker, maxin.john, matt.fleming,
	olof, dhowells, linux-next, linux-kernel, khalid

On Tue, Sep 25, 2012 at 05:06:32PM -0600, Khalid Aziz wrote:

> This is for code cleanup and does not impact functionality. This is
> based upon an earlier discussion -
> <https://lkml.org/lkml/2012/7/13/320>. The goal is to make the code more
> readable. V1 of this patch was discussed at
> <https://lkml.org/lkml/2012/7/20/414>.

Right. Keeping the spec names makes it difficult to write code in a 
readable way.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-09-25 23:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-20 22:08 [PATCH] Shorten constant names for EFI variable attributes Khalid Aziz
2012-07-20 22:10 ` H. Peter Anvin
2012-07-20 22:30   ` Khalid Aziz
2012-07-20 22:34     ` H. Peter Anvin
2012-07-20 22:46       ` Khalid Aziz
2012-07-23 13:26   ` Matthew Garrett
2012-07-26 17:28     ` Khalid Aziz
2012-07-26 17:33       ` Matthew Garrett
2012-09-25 15:41         ` [PATCH -next v2] " Khalid Aziz
2012-09-25 21:55           ` Stephen Rothwell
2012-09-25 23:06             ` Khalid Aziz
2012-09-25 23:12               ` Matthew Garrett

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).