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