[U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes

[U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes

[Heinrich Schuchardt]

Since TianoCore EDK2 commit d65f2cea36d1 ("ShellPkg/CommandLib: Locate
proper UnicodeCollation instance") in edk2 the UEFI Shell crashes if EFI
variable PlatformLang is not defined.

As this variable is anyway prescribed in the UEFI 2.7 spec let's define it
to L"en-US". Use the same value for PlatformLangCodes that defines the list
of all supported languages.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_setup.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 8266d06c2e..e431c1c053 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -10,6 +10,9 @@

 #define OBJ_LIST_NOT_INITIALIZED 1

+/* Language code for American English according to RFC 4646 */
+#define EN_US L"en-US"
+
 static efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;

 /* Initialize and populate EFI object list */
@@ -24,6 +27,30 @@ efi_status_t efi_init_obj_list(void)
 	 */
 	efi_save_gd();

+	/*
+	 * Variable PlatformLang defines the language that the machine has been
+	 * configured for.
+	 */
+	ret = EFI_CALL(efi_set_variable(L"PlatformLang",
+					&efi_global_variable_guid,
+					EFI_VARIABLE_BOOTSERVICE_ACCESS |
+					EFI_VARIABLE_RUNTIME_ACCESS,
+					sizeof(EN_US), EN_US));
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	/*
+	 * Variable PlatformLangCodes defines the language codes that the
+	 * machine can support.
+	 */
+	ret = EFI_CALL(efi_set_variable(L"PlatformLangCodes",
+					&efi_global_variable_guid,
+					EFI_VARIABLE_BOOTSERVICE_ACCESS |
+					EFI_VARIABLE_RUNTIME_ACCESS,
+					sizeof(EN_US), EN_US));
+	if (ret != EFI_SUCCESS)
+		goto out;
+
 	/* Initialize once only */
 	if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
 		return efi_obj_list_initialized;
--
2.20.1

[U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes

[AKASHI Takahiro]

On Fri, Apr 05, 2019 at 03:33:54AM +0200, Heinrich Schuchardt wrote:
> Since TianoCore EDK2 commit d65f2cea36d1 ("ShellPkg/CommandLib: Locate
> proper UnicodeCollation instance") in edk2 the UEFI Shell crashes if EFI
> variable PlatformLang is not defined.
> 
> As this variable is anyway prescribed in the UEFI 2.7 spec let's define it
> to L"en-US". Use the same value for PlatformLangCodes that defines the list
> of all supported languages.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_setup.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 8266d06c2e..e431c1c053 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -10,6 +10,9 @@
> 
>  #define OBJ_LIST_NOT_INITIALIZED 1
> 
> +/* Language code for American English according to RFC 4646 */
> +#define EN_US L"en-US"
> +
>  static efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;
> 
>  /* Initialize and populate EFI object list */
> @@ -24,6 +27,30 @@ efi_status_t efi_init_obj_list(void)
>  	 */
>  	efi_save_gd();
> 
> +	/*
> +	 * Variable PlatformLang defines the language that the machine has been
> +	 * configured for.
> +	 */
> +	ret = EFI_CALL(efi_set_variable(L"PlatformLang",
> +					&efi_global_variable_guid,
> +					EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +					EFI_VARIABLE_RUNTIME_ACCESS,
> +					sizeof(EN_US), EN_US));
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	/*
> +	 * Variable PlatformLangCodes defines the language codes that the
> +	 * machine can support.
> +	 */
> +	ret = EFI_CALL(efi_set_variable(L"PlatformLangCodes",
> +					&efi_global_variable_guid,
> +					EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +					EFI_VARIABLE_RUNTIME_ACCESS,
> +					sizeof(EN_US), EN_US));
> +	if (ret != EFI_SUCCESS)
> +		goto out;

I don't think that it is a good idea to make a hard-coding like this
in init code. Instead, we should add some kind of framework so that
initial (default) variables can be configured.

-Takahiro Akashi


>  	/* Initialize once only */
>  	if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
>  		return efi_obj_list_initialized;
> --
> 2.20.1
> 

[U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes

[Heinrich Schuchardt]

On 4/9/19 3:18 AM, AKASHI Takahiro wrote:
> On Fri, Apr 05, 2019 at 03:33:54AM +0200, Heinrich Schuchardt wrote:
>> Since TianoCore EDK2 commit d65f2cea36d1 ("ShellPkg/CommandLib: Locate
>> proper UnicodeCollation instance") in edk2 the UEFI Shell crashes if EFI
>> variable PlatformLang is not defined.
>>
>> As this variable is anyway prescribed in the UEFI 2.7 spec let's define it
>> to L"en-US". Use the same value for PlatformLangCodes that defines the list
>> of all supported languages.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  lib/efi_loader/efi_setup.c | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>>
>> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
>> index 8266d06c2e..e431c1c053 100644
>> --- a/lib/efi_loader/efi_setup.c
>> +++ b/lib/efi_loader/efi_setup.c
>> @@ -10,6 +10,9 @@
>>
>>  #define OBJ_LIST_NOT_INITIALIZED 1
>>
>> +/* Language code for American English according to RFC 4646 */
>> +#define EN_US L"en-US"
>> +
>>  static efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;
>>
>>  /* Initialize and populate EFI object list */
>> @@ -24,6 +27,30 @@ efi_status_t efi_init_obj_list(void)
>>  	 */
>>  	efi_save_gd();
>>
>> +	/*
>> +	 * Variable PlatformLang defines the language that the machine has been
>> +	 * configured for.
>> +	 */
>> +	ret = EFI_CALL(efi_set_variable(L"PlatformLang",
>> +					&efi_global_variable_guid,
>> +					EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> +					EFI_VARIABLE_RUNTIME_ACCESS,
>> +					sizeof(EN_US), EN_US));
>> +	if (ret != EFI_SUCCESS)
>> +		goto out;
>> +
>> +	/*
>> +	 * Variable PlatformLangCodes defines the language codes that the
>> +	 * machine can support.
>> +	 */
>> +	ret = EFI_CALL(efi_set_variable(L"PlatformLangCodes",
>> +					&efi_global_variable_guid,
>> +					EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> +					EFI_VARIABLE_RUNTIME_ACCESS,
>> +					sizeof(EN_US), EN_US));
>> +	if (ret != EFI_SUCCESS)
>> +		goto out;
>
> I don't think that it is a good idea to make a hard-coding like this
> in init code. Instead, we should add some kind of framework so that
> initial (default) variables can be configured.

The patch is sufficient to make EFI shell happy but it does not support
internationalization.

With internationalization we have several problem:

The U-Boot video output only supports characters of code page 437.
The USB keyboard driver supports only US keyboards.
The matrix keyboard driver supports only US and German keyboards.
Our Unicode collation protocol supports either of codepage 1250 or
codepage 437 for FAT filenames.

I have chosen EN_US because code page 437 because supports it
completely. CP437 has some European letters but it is a misery:

For DE_DE U+DF (eszett) is missing.
For FR_FR many accented letters are missing.

Given the restrictions above what value would you use for PlatformLangCodes?

So to make things more flexible what we could do is:

Define PlatformLangCodes by a Kconfig variable defaulting to EN_US.
Intialize PlatformLang by the first language from PlatformLangCodes only
if the variable is not yet set (e.g. by saving a value in the environment).

Does this logic match your expectations? If yes, I could put it into a
separate patch on top of the current one.

Best regards

Heinrich

>
> -Takahiro Akashi
>
>
>>  	/* Initialize once only */
>>  	if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
>>  		return efi_obj_list_initialized;
>> --
>> 2.20.1
>>
>

[U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes

[AKASHI Takahiro]

On Tue, Apr 09, 2019 at 07:10:11AM +0200, Heinrich Schuchardt wrote:
> On 4/9/19 3:18 AM, AKASHI Takahiro wrote:
> > On Fri, Apr 05, 2019 at 03:33:54AM +0200, Heinrich Schuchardt wrote:
> >> Since TianoCore EDK2 commit d65f2cea36d1 ("ShellPkg/CommandLib: Locate
> >> proper UnicodeCollation instance") in edk2 the UEFI Shell crashes if EFI
> >> variable PlatformLang is not defined.
> >>
> >> As this variable is anyway prescribed in the UEFI 2.7 spec let's define it
> >> to L"en-US". Use the same value for PlatformLangCodes that defines the list
> >> of all supported languages.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >>  lib/efi_loader/efi_setup.c | 27 +++++++++++++++++++++++++++
> >>  1 file changed, 27 insertions(+)
> >>
> >> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> >> index 8266d06c2e..e431c1c053 100644
> >> --- a/lib/efi_loader/efi_setup.c
> >> +++ b/lib/efi_loader/efi_setup.c
> >> @@ -10,6 +10,9 @@
> >>
> >>  #define OBJ_LIST_NOT_INITIALIZED 1
> >>
> >> +/* Language code for American English according to RFC 4646 */
> >> +#define EN_US L"en-US"
> >> +
> >>  static efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;
> >>
> >>  /* Initialize and populate EFI object list */
> >> @@ -24,6 +27,30 @@ efi_status_t efi_init_obj_list(void)
> >>  	 */
> >>  	efi_save_gd();
> >>
> >> +	/*
> >> +	 * Variable PlatformLang defines the language that the machine has been
> >> +	 * configured for.
> >> +	 */
> >> +	ret = EFI_CALL(efi_set_variable(L"PlatformLang",
> >> +					&efi_global_variable_guid,
> >> +					EFI_VARIABLE_BOOTSERVICE_ACCESS |
> >> +					EFI_VARIABLE_RUNTIME_ACCESS,
> >> +					sizeof(EN_US), EN_US));
> >> +	if (ret != EFI_SUCCESS)
> >> +		goto out;
> >> +
> >> +	/*
> >> +	 * Variable PlatformLangCodes defines the language codes that the
> >> +	 * machine can support.
> >> +	 */
> >> +	ret = EFI_CALL(efi_set_variable(L"PlatformLangCodes",
> >> +					&efi_global_variable_guid,
> >> +					EFI_VARIABLE_BOOTSERVICE_ACCESS |
> >> +					EFI_VARIABLE_RUNTIME_ACCESS,
> >> +					sizeof(EN_US), EN_US));
> >> +	if (ret != EFI_SUCCESS)
> >> +		goto out;
> >
> > I don't think that it is a good idea to make a hard-coding like this
> > in init code. Instead, we should add some kind of framework so that
> > initial (default) variables can be configured.
> 
> The patch is sufficient to make EFI shell happy but it does not support
> internationalization.
> 
> With internationalization we have several problem:

The issue is not what code-set be used here, but that default variables
should be configurable without any hard-coding in generic *.c file
for easier customization.

Something, at least, like U-Boot's include/env_default.h

-Takahiro Akashi

> The U-Boot video output only supports characters of code page 437.
> The USB keyboard driver supports only US keyboards.
> The matrix keyboard driver supports only US and German keyboards.
> Our Unicode collation protocol supports either of codepage 1250 or
> codepage 437 for FAT filenames.
> 
> I have chosen EN_US because code page 437 because supports it
> completely. CP437 has some European letters but it is a misery:
> 
> For DE_DE U+DF (eszett) is missing.
> For FR_FR many accented letters are missing.
> 
> Given the restrictions above what value would you use for PlatformLangCodes?
> 
> So to make things more flexible what we could do is:
> 
> Define PlatformLangCodes by a Kconfig variable defaulting to EN_US.
> Intialize PlatformLang by the first language from PlatformLangCodes only
> if the variable is not yet set (e.g. by saving a value in the environment).
> 
> Does this logic match your expectations? If yes, I could put it into a
> separate patch on top of the current one.
> 
> Best regards
> 
> Heinrich
> 
> >
> > -Takahiro Akashi
> >
> >
> >>  	/* Initialize once only */
> >>  	if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
> >>  		return efi_obj_list_initialized;
> >> --
> >> 2.20.1
> >>
> >
> 

[U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes

[Heinrich Schuchardt]

On 4/11/19 6:49 AM, AKASHI Takahiro wrote:
> On Tue, Apr 09, 2019 at 07:10:11AM +0200, Heinrich Schuchardt wrote:
>> On 4/9/19 3:18 AM, AKASHI Takahiro wrote:
>>> On Fri, Apr 05, 2019 at 03:33:54AM +0200, Heinrich Schuchardt wrote:
>>>> Since TianoCore EDK2 commit d65f2cea36d1 ("ShellPkg/CommandLib: Locate
>>>> proper UnicodeCollation instance") in edk2 the UEFI Shell crashes if EFI
>>>> variable PlatformLang is not defined.
>>>>
>>>> As this variable is anyway prescribed in the UEFI 2.7 spec let's define it
>>>> to L"en-US". Use the same value for PlatformLangCodes that defines the list
>>>> of all supported languages.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>> ---
>>>>   lib/efi_loader/efi_setup.c | 27 +++++++++++++++++++++++++++
>>>>   1 file changed, 27 insertions(+)
>>>>
>>>> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
>>>> index 8266d06c2e..e431c1c053 100644
>>>> --- a/lib/efi_loader/efi_setup.c
>>>> +++ b/lib/efi_loader/efi_setup.c
>>>> @@ -10,6 +10,9 @@
>>>>
>>>>   #define OBJ_LIST_NOT_INITIALIZED 1
>>>>
>>>> +/* Language code for American English according to RFC 4646 */
>>>> +#define EN_US L"en-US"
>>>> +
>>>>   static efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;
>>>>
>>>>   /* Initialize and populate EFI object list */
>>>> @@ -24,6 +27,30 @@ efi_status_t efi_init_obj_list(void)
>>>>   	 */
>>>>   	efi_save_gd();
>>>>
>>>> +	/*
>>>> +	 * Variable PlatformLang defines the language that the machine has been
>>>> +	 * configured for.
>>>> +	 */
>>>> +	ret = EFI_CALL(efi_set_variable(L"PlatformLang",
>>>> +					&efi_global_variable_guid,
>>>> +					EFI_VARIABLE_BOOTSERVICE_ACCESS |
>>>> +					EFI_VARIABLE_RUNTIME_ACCESS,
>>>> +					sizeof(EN_US), EN_US));
>>>> +	if (ret != EFI_SUCCESS)
>>>> +		goto out;
>>>> +
>>>> +	/*
>>>> +	 * Variable PlatformLangCodes defines the language codes that the
>>>> +	 * machine can support.
>>>> +	 */
>>>> +	ret = EFI_CALL(efi_set_variable(L"PlatformLangCodes",
>>>> +					&efi_global_variable_guid,
>>>> +					EFI_VARIABLE_BOOTSERVICE_ACCESS |
>>>> +					EFI_VARIABLE_RUNTIME_ACCESS,
>>>> +					sizeof(EN_US), EN_US));
>>>> +	if (ret != EFI_SUCCESS)
>>>> +		goto out;
>>>
>>> I don't think that it is a good idea to make a hard-coding like this
>>> in init code. Instead, we should add some kind of framework so that
>>> initial (default) variables can be configured.
>>
>> The patch is sufficient to make EFI shell happy but it does not support
>> internationalization.
>>
>> With internationalization we have several problem:
>
> The issue is not what code-set be used here, but that default variables
> should be configurable without any hard-coding in generic *.c file
> for easier customization.
>
> Something, at least, like U-Boot's include/env_default.h
>
> -Takahiro Akashi

Thanks for the clarification.

I think the value I used was anyway wrong. The spec says: The
PlatformLangCodes variable contains a null-terminated *ASCII* string. I
used UTF-16.

Best regards

Heinrich

>
>> The U-Boot video output only supports characters of code page 437.
>> The USB keyboard driver supports only US keyboards.
>> The matrix keyboard driver supports only US and German keyboards.
>> Our Unicode collation protocol supports either of codepage 1250 or
>> codepage 437 for FAT filenames.
>>
>> I have chosen EN_US because code page 437 because supports it
>> completely. CP437 has some European letters but it is a misery:
>>
>> For DE_DE U+DF (eszett) is missing.
>> For FR_FR many accented letters are missing.
>>
>> Given the restrictions above what value would you use for PlatformLangCodes?
>>
>> So to make things more flexible what we could do is:
>>
>> Define PlatformLangCodes by a Kconfig variable defaulting to EN_US.
>> Intialize PlatformLang by the first language from PlatformLangCodes only
>> if the variable is not yet set (e.g. by saving a value in the environment).
>>
>> Does this logic match your expectations? If yes, I could put it into a
>> separate patch on top of the current one.
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> -Takahiro Akashi
>>>
>>>
>>>>   	/* Initialize once only */
>>>>   	if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
>>>>   		return efi_obj_list_initialized;
>>>> --
>>>> 2.20.1
>>>>
>>>
>>
>