All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes
@ 2019-04-05  1:33 Heinrich Schuchardt
  2019-04-09  1:18 ` AKASHI Takahiro
  0 siblings, 1 reply; 5+ messages in thread
From: Heinrich Schuchardt @ 2019-04-05  1:33 UTC (permalink / raw)
  To: u-boot

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

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

* [U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes
  2019-04-05  1:33 [U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes Heinrich Schuchardt
@ 2019-04-09  1:18 ` AKASHI Takahiro
  2019-04-09  5:10   ` Heinrich Schuchardt
  0 siblings, 1 reply; 5+ messages in thread
From: AKASHI Takahiro @ 2019-04-09  1:18 UTC (permalink / raw)
  To: u-boot

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
> 

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

* [U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes
  2019-04-09  1:18 ` AKASHI Takahiro
@ 2019-04-09  5:10   ` Heinrich Schuchardt
  2019-04-11  4:49     ` AKASHI Takahiro
  0 siblings, 1 reply; 5+ messages in thread
From: Heinrich Schuchardt @ 2019-04-09  5:10 UTC (permalink / raw)
  To: u-boot

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
>>
>

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

* [U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes
  2019-04-09  5:10   ` Heinrich Schuchardt
@ 2019-04-11  4:49     ` AKASHI Takahiro
  2019-04-11  5:12       ` Heinrich Schuchardt
  0 siblings, 1 reply; 5+ messages in thread
From: AKASHI Takahiro @ 2019-04-11  4:49 UTC (permalink / raw)
  To: u-boot

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
> >>
> >
> 

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

* [U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes
  2019-04-11  4:49     ` AKASHI Takahiro
@ 2019-04-11  5:12       ` Heinrich Schuchardt
  0 siblings, 0 replies; 5+ messages in thread
From: Heinrich Schuchardt @ 2019-04-11  5:12 UTC (permalink / raw)
  To: u-boot

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
>>>>
>>>
>>
>

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

end of thread, other threads:[~2019-04-11  5:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-05  1:33 [U-Boot] [PATCH 1/1] efi_loader: variables PlatformLang and PlatformLangCodes Heinrich Schuchardt
2019-04-09  1:18 ` AKASHI Takahiro
2019-04-09  5:10   ` Heinrich Schuchardt
2019-04-11  4:49     ` AKASHI Takahiro
2019-04-11  5:12       ` Heinrich Schuchardt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.