| Message ID | 20170910132236.14318-4-robdclark@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,01/12] efi_loader: add stub EFI_DEVICE_PATH_UTILITIES_PROTOCOL | expand |
On 09/10/2017 03:22 PM, Rob Clark wrote: > From: Leif Lindholm <leif.lindholm@linaro.org> The commit message is missing. Fix all issues reported by checkpatch. > > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> > --- > include/efi_api.h | 33 +++++++++++++++++++ > include/efi_loader.h | 2 ++ > lib/efi_loader/Makefile | 2 +- > lib/efi_loader/efi_boottime.c | 3 ++ > lib/efi_loader/efi_unicode.c | 73 +++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 112 insertions(+), 1 deletion(-) > create mode 100644 lib/efi_loader/efi_unicode.c > > diff --git a/include/efi_api.h b/include/efi_api.h > index 932a3429a8..25f774f253 100644 > --- a/include/efi_api.h > +++ b/include/efi_api.h > @@ -740,6 +740,39 @@ struct efi_hii_string_protocol > UINTN *secondary_languages_size); > }; > > +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \ > + EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \ > + 0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49) > + > +struct efi_unicode_collation_protocol > +{ ERROR: open brace '{' following struct go on the same line #30: FILE: include/efi_api.h:748: +struct efi_unicode_collation_protocol +{ > + INTN(EFIAPI *stri_coll)( > + struct efi_unicode_collation_protocol *this, > + efi_string_t s1, > + efi_string_t s2); > + bool(EFIAPI *metai_match)( > + struct efi_unicode_collation_protocol *this, > + efi_string_t string, > + efi_string_t pattern); > + void(EFIAPI *str_lwr)( > + struct efi_unicode_collation_protocol *this, > + efi_string_t string); > + void(EFIAPI *str_upr)( > + struct efi_unicode_collation_protocol *this, > + efi_string_t string); > + void(EFIAPI *fat_to_str)( > + struct efi_unicode_collation_protocol *this, > + UINTN fat_size, > + uint8_t *fat, > + efi_string_t string); > + bool(EFIAPI *str_to_fat)( > + struct efi_unicode_collation_protocol *this, > + efi_string_t string, > + UINTN fat_size, > + uint8_t *fat); > + uint8_t *supported_languages; > +}; > + > #define EFI_GOP_GUID \ > EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \ > 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a) > diff --git a/include/efi_loader.h b/include/efi_loader.h > index a89bb2fcd9..6668338d0b 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -62,6 +62,7 @@ extern const struct efi_device_path_utilities_protocol efi_device_path_utilities > extern const struct efi_hii_config_routing_protocol efi_hii_config_routing; > extern const struct efi_hii_database_protocol efi_hii_database; > extern const struct efi_hii_string_protocol efi_hii_string; > +extern const struct efi_unicode_collation_protocol efi_unicode_collation; > > uint16_t *efi_dp_str(struct efi_device_path *dp); > > @@ -76,6 +77,7 @@ extern const efi_guid_t efi_guid_device_path_utilities_protocol; > extern const efi_guid_t efi_guid_hii_config_routing_protocol; > extern const efi_guid_t efi_guid_hii_database_protocol; > extern const efi_guid_t efi_guid_hii_string_protocol; > +extern const efi_guid_t efi_guid_unicode_collation_protocol2; > > extern unsigned int __efi_runtime_start, __efi_runtime_stop; > extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile > index e8fd6823a3..82b703bb39 100644 > --- a/lib/efi_loader/Makefile > +++ b/lib/efi_loader/Makefile > @@ -16,7 +16,7 @@ always := $(efiprogs-y) > obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o > obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o > obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o > -obj-y += efi_device_path_utilities.o efi_hii.o > +obj-y += efi_device_path_utilities.o efi_hii.o efi_unicode.o > obj-y += efi_file.o efi_variable.o efi_bootmgr.o > obj-$(CONFIG_LCD) += efi_gop.o > obj-$(CONFIG_DM_VIDEO) += efi_gop.o > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index 4d1a16051b..04358e8aca 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -788,6 +788,9 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob > obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol; > obj->protocols[7].protocol_interface = (void *)&efi_hii_config_routing; > Do not add a protocol that is not properly implemented yet. > + obj->protocols[8].guid = &efi_guid_unicode_collation_protocol2; > + obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation; > + > info->file_path = file_path; > info->device_handle = efi_dp_find_obj(device_path, NULL); > > diff --git a/lib/efi_loader/efi_unicode.c b/lib/efi_loader/efi_unicode.c > new file mode 100644 > index 0000000000..fdf1a99812 > --- /dev/null > +++ b/lib/efi_loader/efi_unicode.c > @@ -0,0 +1,73 @@ > +/* > +* EFI Unicode interface > + * > + * Copyright (c) 2017 Leif Lindholm > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <efi_loader.h> > + > +const efi_guid_t efi_guid_unicode_collation_protocol2 = > + EFI_UNICODE_COLLATION_PROTOCOL2_GUID; > + None of the functions matches the definitions in the structure. Add the missing EFIAPI. Regards Heinrich > +INTN stri_coll(struct efi_unicode_collation_protocol *this, > + efi_string_t s1, > + efi_string_t s2) > +{ > + EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2); > + return EFI_EXIT(0); > +} > + > +bool metai_match(struct efi_unicode_collation_protocol *this, > + efi_string_t string, > + efi_string_t pattern) > +{ > + EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, string, pattern); > + return EFI_EXIT(false); > +} > + > +void str_lwr(struct efi_unicode_collation_protocol *this, > + efi_string_t string) > +{ > + EFI_ENTRY("%p, \"%ls\"", this, string); > + EFI_EXIT(0); EFI_EXIT(EFI_SUCCESS); > + return; > +} > + > +void str_upr(struct efi_unicode_collation_protocol *this, > + efi_string_t string) > +{ > + EFI_ENTRY("%p, \"%ls\"", this, string); > + EFI_EXIT(0); EFI_EXIT(EFI_SUCCESS); > + return; > +} > + > +void fat_to_str(struct efi_unicode_collation_protocol *this, > + UINTN fat_size, > + uint8_t *fat, > + efi_string_t string) > +{ > + EFI_ENTRY("%p, %lu, \"%s\", %p", this, fat_size, fat, string); > + EFI_EXIT(0); EFI_EXIT(EFI_SUCCESS); > + return; > +} > + > +bool str_to_fat(struct efi_unicode_collation_protocol *this, > + efi_string_t string, > + UINTN fat_size, > + uint8_t *fat) > +{ > + EFI_ENTRY("%p, \"%ls\", %lu, %p", this, string, fat_size, fat); > + return EFI_EXIT(false); > +} > + > +const struct efi_unicode_collation_protocol efi_unicode_collation = { > + .stri_coll = stri_coll, > + .metai_match = metai_match, > + .str_lwr = str_lwr, > + .str_upr = str_upr, > + .fat_to_str = fat_to_str, > + .str_to_fat = str_to_fat > +}; >
On Wed, Oct 4, 2017 at 1:57 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > On 09/10/2017 03:22 PM, Rob Clark wrote: >> From: Leif Lindholm <leif.lindholm@linaro.org> > > The commit message is missing. > > Fix all issues reported by checkpatch. > >> >> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> >> --- >> include/efi_api.h | 33 +++++++++++++++++++ >> include/efi_loader.h | 2 ++ >> lib/efi_loader/Makefile | 2 +- >> lib/efi_loader/efi_boottime.c | 3 ++ >> lib/efi_loader/efi_unicode.c | 73 +++++++++++++++++++++++++++++++++++++++++++ >> 5 files changed, 112 insertions(+), 1 deletion(-) >> create mode 100644 lib/efi_loader/efi_unicode.c >> >> diff --git a/include/efi_api.h b/include/efi_api.h >> index 932a3429a8..25f774f253 100644 >> --- a/include/efi_api.h >> +++ b/include/efi_api.h >> @@ -740,6 +740,39 @@ struct efi_hii_string_protocol >> UINTN *secondary_languages_size); >> }; >> >> +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \ >> + EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \ >> + 0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49) >> + >> +struct efi_unicode_collation_protocol >> +{ > > ERROR: open brace '{' following struct go on the same line > #30: FILE: include/efi_api.h:748: > +struct efi_unicode_collation_protocol > +{ > >> + INTN(EFIAPI *stri_coll)( >> + struct efi_unicode_collation_protocol *this, >> + efi_string_t s1, >> + efi_string_t s2); >> + bool(EFIAPI *metai_match)( >> + struct efi_unicode_collation_protocol *this, >> + efi_string_t string, >> + efi_string_t pattern); >> + void(EFIAPI *str_lwr)( >> + struct efi_unicode_collation_protocol *this, >> + efi_string_t string); >> + void(EFIAPI *str_upr)( >> + struct efi_unicode_collation_protocol *this, >> + efi_string_t string); >> + void(EFIAPI *fat_to_str)( >> + struct efi_unicode_collation_protocol *this, >> + UINTN fat_size, >> + uint8_t *fat, >> + efi_string_t string); >> + bool(EFIAPI *str_to_fat)( >> + struct efi_unicode_collation_protocol *this, >> + efi_string_t string, >> + UINTN fat_size, >> + uint8_t *fat); >> + uint8_t *supported_languages; >> +}; >> + >> #define EFI_GOP_GUID \ >> EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \ >> 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a) >> diff --git a/include/efi_loader.h b/include/efi_loader.h >> index a89bb2fcd9..6668338d0b 100644 >> --- a/include/efi_loader.h >> +++ b/include/efi_loader.h >> @@ -62,6 +62,7 @@ extern const struct efi_device_path_utilities_protocol efi_device_path_utilities >> extern const struct efi_hii_config_routing_protocol efi_hii_config_routing; >> extern const struct efi_hii_database_protocol efi_hii_database; >> extern const struct efi_hii_string_protocol efi_hii_string; >> +extern const struct efi_unicode_collation_protocol efi_unicode_collation; >> >> uint16_t *efi_dp_str(struct efi_device_path *dp); >> >> @@ -76,6 +77,7 @@ extern const efi_guid_t efi_guid_device_path_utilities_protocol; >> extern const efi_guid_t efi_guid_hii_config_routing_protocol; >> extern const efi_guid_t efi_guid_hii_database_protocol; >> extern const efi_guid_t efi_guid_hii_string_protocol; >> +extern const efi_guid_t efi_guid_unicode_collation_protocol2; >> >> extern unsigned int __efi_runtime_start, __efi_runtime_stop; >> extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; >> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile >> index e8fd6823a3..82b703bb39 100644 >> --- a/lib/efi_loader/Makefile >> +++ b/lib/efi_loader/Makefile >> @@ -16,7 +16,7 @@ always := $(efiprogs-y) >> obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o >> obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o >> obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o >> -obj-y += efi_device_path_utilities.o efi_hii.o >> +obj-y += efi_device_path_utilities.o efi_hii.o efi_unicode.o >> obj-y += efi_file.o efi_variable.o efi_bootmgr.o >> obj-$(CONFIG_LCD) += efi_gop.o >> obj-$(CONFIG_DM_VIDEO) += efi_gop.o >> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >> index 4d1a16051b..04358e8aca 100644 >> --- a/lib/efi_loader/efi_boottime.c >> +++ b/lib/efi_loader/efi_boottime.c >> @@ -788,6 +788,9 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob >> obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol; >> obj->protocols[7].protocol_interface = (void *)&efi_hii_config_routing; >> > > Do not add a protocol that is not properly implemented yet. There is *no possible way* that I am going to completely implement HII, so big nak on this comment! In general, my plan was to implement the minimal parts of these protocols to get to the point where we can run SCT before implementing the rest, so we at least have a way to test implementation against compliance tests written against the spec (instead of our own tests written against our interpretation of the spec). In the particular case of HII, it includes things like forms based UI rendering, so I don't think we'll ever implement the entire thing. Or if someone did, we'd probably want to make it something that can be disabled at compile time to keep footprint down. Anyways, until we get to the point of loading option ROMs from PCI cards in u-boot I don't think we really need a setup menu or complete HII implementation ;-) >> + obj->protocols[8].guid = &efi_guid_unicode_collation_protocol2; >> + obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation; >> + >> info->file_path = file_path; >> info->device_handle = efi_dp_find_obj(device_path, NULL); >> >> diff --git a/lib/efi_loader/efi_unicode.c b/lib/efi_loader/efi_unicode.c >> new file mode 100644 >> index 0000000000..fdf1a99812 >> --- /dev/null >> +++ b/lib/efi_loader/efi_unicode.c >> @@ -0,0 +1,73 @@ >> +/* >> +* EFI Unicode interface >> + * >> + * Copyright (c) 2017 Leif Lindholm >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include <common.h> >> +#include <efi_loader.h> >> + >> +const efi_guid_t efi_guid_unicode_collation_protocol2 = >> + EFI_UNICODE_COLLATION_PROTOCOL2_GUID; >> + > > None of the functions matches the definitions in the structure. > > Add the missing EFIAPI. I was talking to Peter Jones about this, and he suggested something that makes *way* more sense.. just build u-boot with '-mabi=ms_abi' on x86_64 (at least conditionally if EFI_LOADER is enabled). That would take a bit of work to annotate the os layer in sandbox to use sysv abi, but that is a lot smaller of an interface than EFI_LOADER (which is only going to continue to grow). Anyways, other than that, I'll double check the function order issues you mentioned, I mostly assumed that Leif had it right (and I don't remember encountering any issues with that so far with Shell.efi/SCT.efi). Some of your comments could be resolved by just squashing my "implement" patches with Leif's "stub" patches, but I didn't want to do that without Leif's permission. BR, -R > Regards > > Heinrich > >> +INTN stri_coll(struct efi_unicode_collation_protocol *this, >> + efi_string_t s1, >> + efi_string_t s2) >> +{ >> + EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2); >> + return EFI_EXIT(0); >> +} >> + >> +bool metai_match(struct efi_unicode_collation_protocol *this, >> + efi_string_t string, >> + efi_string_t pattern) >> +{ >> + EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, string, pattern); >> + return EFI_EXIT(false); >> +} >> + >> +void str_lwr(struct efi_unicode_collation_protocol *this, >> + efi_string_t string) >> +{ >> + EFI_ENTRY("%p, \"%ls\"", this, string); >> + EFI_EXIT(0); > > EFI_EXIT(EFI_SUCCESS); > >> + return; >> +} >> + >> +void str_upr(struct efi_unicode_collation_protocol *this, >> + efi_string_t string) >> +{ >> + EFI_ENTRY("%p, \"%ls\"", this, string); >> + EFI_EXIT(0); > > EFI_EXIT(EFI_SUCCESS); > >> + return; >> +} >> + >> +void fat_to_str(struct efi_unicode_collation_protocol *this, >> + UINTN fat_size, >> + uint8_t *fat, >> + efi_string_t string) >> +{ >> + EFI_ENTRY("%p, %lu, \"%s\", %p", this, fat_size, fat, string); >> + EFI_EXIT(0); > > EFI_EXIT(EFI_SUCCESS); > >> + return; >> +} >> + >> +bool str_to_fat(struct efi_unicode_collation_protocol *this, >> + efi_string_t string, >> + UINTN fat_size, >> + uint8_t *fat) >> +{ >> + EFI_ENTRY("%p, \"%ls\", %lu, %p", this, string, fat_size, fat); >> + return EFI_EXIT(false); >> +} >> + >> +const struct efi_unicode_collation_protocol efi_unicode_collation = { >> + .stri_coll = stri_coll, >> + .metai_match = metai_match, >> + .str_lwr = str_lwr, >> + .str_upr = str_upr, >> + .fat_to_str = fat_to_str, >> + .str_to_fat = str_to_fat >> +}; >> >
On 10/04/2017 08:35 PM, Rob Clark wrote: > On Wed, Oct 4, 2017 at 1:57 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> On 09/10/2017 03:22 PM, Rob Clark wrote: >>> From: Leif Lindholm <leif.lindholm@linaro.org> >> >> The commit message is missing. >> >> Fix all issues reported by checkpatch. >> >>> >>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> >>> --- >>> include/efi_api.h | 33 +++++++++++++++++++ >>> include/efi_loader.h | 2 ++ >>> lib/efi_loader/Makefile | 2 +- >>> lib/efi_loader/efi_boottime.c | 3 ++ >>> lib/efi_loader/efi_unicode.c | 73 +++++++++++++++++++++++++++++++++++++++++++ >>> 5 files changed, 112 insertions(+), 1 deletion(-) >>> create mode 100644 lib/efi_loader/efi_unicode.c >>> >>> diff --git a/include/efi_api.h b/include/efi_api.h >>> index 932a3429a8..25f774f253 100644 >>> --- a/include/efi_api.h >>> +++ b/include/efi_api.h >>> @@ -740,6 +740,39 @@ struct efi_hii_string_protocol >>> UINTN *secondary_languages_size); >>> }; >>> >>> +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \ >>> + EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \ >>> + 0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49) >>> + >>> +struct efi_unicode_collation_protocol >>> +{ >> >> ERROR: open brace '{' following struct go on the same line >> #30: FILE: include/efi_api.h:748: >> +struct efi_unicode_collation_protocol >> +{ >> >>> + INTN(EFIAPI *stri_coll)( >>> + struct efi_unicode_collation_protocol *this, >>> + efi_string_t s1, >>> + efi_string_t s2); >>> + bool(EFIAPI *metai_match)( >>> + struct efi_unicode_collation_protocol *this, >>> + efi_string_t string, >>> + efi_string_t pattern); >>> + void(EFIAPI *str_lwr)( >>> + struct efi_unicode_collation_protocol *this, >>> + efi_string_t string); >>> + void(EFIAPI *str_upr)( >>> + struct efi_unicode_collation_protocol *this, >>> + efi_string_t string); >>> + void(EFIAPI *fat_to_str)( >>> + struct efi_unicode_collation_protocol *this, >>> + UINTN fat_size, >>> + uint8_t *fat, >>> + efi_string_t string); >>> + bool(EFIAPI *str_to_fat)( >>> + struct efi_unicode_collation_protocol *this, >>> + efi_string_t string, >>> + UINTN fat_size, >>> + uint8_t *fat); >>> + uint8_t *supported_languages; >>> +}; >>> + >>> #define EFI_GOP_GUID \ >>> EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \ >>> 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a) >>> diff --git a/include/efi_loader.h b/include/efi_loader.h >>> index a89bb2fcd9..6668338d0b 100644 >>> --- a/include/efi_loader.h >>> +++ b/include/efi_loader.h >>> @@ -62,6 +62,7 @@ extern const struct efi_device_path_utilities_protocol efi_device_path_utilities >>> extern const struct efi_hii_config_routing_protocol efi_hii_config_routing; >>> extern const struct efi_hii_database_protocol efi_hii_database; >>> extern const struct efi_hii_string_protocol efi_hii_string; >>> +extern const struct efi_unicode_collation_protocol efi_unicode_collation; >>> >>> uint16_t *efi_dp_str(struct efi_device_path *dp); >>> >>> @@ -76,6 +77,7 @@ extern const efi_guid_t efi_guid_device_path_utilities_protocol; >>> extern const efi_guid_t efi_guid_hii_config_routing_protocol; >>> extern const efi_guid_t efi_guid_hii_database_protocol; >>> extern const efi_guid_t efi_guid_hii_string_protocol; >>> +extern const efi_guid_t efi_guid_unicode_collation_protocol2; >>> >>> extern unsigned int __efi_runtime_start, __efi_runtime_stop; >>> extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; >>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile >>> index e8fd6823a3..82b703bb39 100644 >>> --- a/lib/efi_loader/Makefile >>> +++ b/lib/efi_loader/Makefile >>> @@ -16,7 +16,7 @@ always := $(efiprogs-y) >>> obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o >>> obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o >>> obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o >>> -obj-y += efi_device_path_utilities.o efi_hii.o >>> +obj-y += efi_device_path_utilities.o efi_hii.o efi_unicode.o >>> obj-y += efi_file.o efi_variable.o efi_bootmgr.o >>> obj-$(CONFIG_LCD) += efi_gop.o >>> obj-$(CONFIG_DM_VIDEO) += efi_gop.o >>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >>> index 4d1a16051b..04358e8aca 100644 >>> --- a/lib/efi_loader/efi_boottime.c >>> +++ b/lib/efi_loader/efi_boottime.c >>> @@ -788,6 +788,9 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob >>> obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol; >>> obj->protocols[7].protocol_interface = (void *)&efi_hii_config_routing; >>> >> >> Do not add a protocol that is not properly implemented yet. > > There is *no possible way* that I am going to completely implement > HII, so big nak on this comment! > > In general, my plan was to implement the minimal parts of these > protocols to get to the point where we can run SCT before implementing > the rest, so we at least have a way to test implementation against > compliance tests written against the spec (instead of our own tests > written against our interpretation of the spec). In the particular > case of HII, it includes things like forms based UI rendering, so I > don't think we'll ever implement the entire thing. Or if someone did, > we'd probably want to make it something that can be disabled at > compile time to keep footprint down. > > Anyways, until we get to the point of loading option ROMs from PCI > cards in u-boot I don't think we really need a setup menu or complete > HII implementation ;-) > >>> + obj->protocols[8].guid = &efi_guid_unicode_collation_protocol2; >>> + obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation; >>> + >>> info->file_path = file_path; >>> info->device_handle = efi_dp_find_obj(device_path, NULL); >>> >>> diff --git a/lib/efi_loader/efi_unicode.c b/lib/efi_loader/efi_unicode.c >>> new file mode 100644 >>> index 0000000000..fdf1a99812 >>> --- /dev/null >>> +++ b/lib/efi_loader/efi_unicode.c >>> @@ -0,0 +1,73 @@ >>> +/* >>> +* EFI Unicode interface >>> + * >>> + * Copyright (c) 2017 Leif Lindholm >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#include <common.h> >>> +#include <efi_loader.h> >>> + >>> +const efi_guid_t efi_guid_unicode_collation_protocol2 = >>> + EFI_UNICODE_COLLATION_PROTOCOL2_GUID; >>> + >> >> None of the functions matches the definitions in the structure. >> >> Add the missing EFIAPI. > > I was talking to Peter Jones about this, and he suggested something > that makes *way* more sense.. just build u-boot with '-mabi=ms_abi' on > x86_64 (at least conditionally if EFI_LOADER is enabled). That would > take a bit of work to annotate the os layer in sandbox to use sysv > abi, but that is a lot smaller of an interface than EFI_LOADER (which > is only going to continue to grow). Let's first get your patch series in before starting a new job. Otherwise we will never stop blocking each other. Regards Heinrich > > Anyways, other than that, I'll double check the function order issues > you mentioned, I mostly assumed that Leif had it right (and I don't > remember encountering any issues with that so far with > Shell.efi/SCT.efi). > > Some of your comments could be resolved by just squashing my > "implement" patches with Leif's "stub" patches, but I didn't want to > do that without Leif's permission. > > BR, > -R > >> Regards >> >> Heinrich >> >>> +INTN stri_coll(struct efi_unicode_collation_protocol *this, >>> + efi_string_t s1, >>> + efi_string_t s2) >>> +{ >>> + EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2); >>> + return EFI_EXIT(0); >>> +} >>> + >>> +bool metai_match(struct efi_unicode_collation_protocol *this, >>> + efi_string_t string, >>> + efi_string_t pattern) >>> +{ >>> + EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, string, pattern); >>> + return EFI_EXIT(false); >>> +} >>> + >>> +void str_lwr(struct efi_unicode_collation_protocol *this, >>> + efi_string_t string) >>> +{ >>> + EFI_ENTRY("%p, \"%ls\"", this, string); >>> + EFI_EXIT(0); >> >> EFI_EXIT(EFI_SUCCESS); >> >>> + return; >>> +} >>> + >>> +void str_upr(struct efi_unicode_collation_protocol *this, >>> + efi_string_t string) >>> +{ >>> + EFI_ENTRY("%p, \"%ls\"", this, string); >>> + EFI_EXIT(0); >> >> EFI_EXIT(EFI_SUCCESS); >> >>> + return; >>> +} >>> + >>> +void fat_to_str(struct efi_unicode_collation_protocol *this, >>> + UINTN fat_size, >>> + uint8_t *fat, >>> + efi_string_t string) >>> +{ >>> + EFI_ENTRY("%p, %lu, \"%s\", %p", this, fat_size, fat, string); >>> + EFI_EXIT(0); >> >> EFI_EXIT(EFI_SUCCESS); >> >>> + return; >>> +} >>> + >>> +bool str_to_fat(struct efi_unicode_collation_protocol *this, >>> + efi_string_t string, >>> + UINTN fat_size, >>> + uint8_t *fat) >>> +{ >>> + EFI_ENTRY("%p, \"%ls\", %lu, %p", this, string, fat_size, fat); >>> + return EFI_EXIT(false); >>> +} >>> + >>> +const struct efi_unicode_collation_protocol efi_unicode_collation = { >>> + .stri_coll = stri_coll, >>> + .metai_match = metai_match, >>> + .str_lwr = str_lwr, >>> + .str_upr = str_upr, >>> + .fat_to_str = fat_to_str, >>> + .str_to_fat = str_to_fat >>> +}; >>> >> >
On Wed, Oct 04, 2017 at 06:35:46PM +0000, Rob Clark wrote: > > Add the missing EFIAPI. > > I was talking to Peter Jones about this, and he suggested something > that makes *way* more sense.. just build u-boot with '-mabi=ms_abi' on > x86_64 (at least conditionally if EFI_LOADER is enabled). That would > take a bit of work to annotate the os layer in sandbox to use sysv > abi, but that is a lot smaller of an interface than EFI_LOADER (which > is only going to continue to grow). Worth noting that if we went this route, we'd also need to do -mregparm=0 on 32-bit x86.
On 10/04/2017 08:57 PM, Heinrich Schuchardt wrote: > On 10/04/2017 08:35 PM, Rob Clark wrote: >> On Wed, Oct 4, 2017 at 1:57 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >>> On 09/10/2017 03:22 PM, Rob Clark wrote: >>>> From: Leif Lindholm <leif.lindholm@linaro.org> >>> >>> The commit message is missing. >>> >>> Fix all issues reported by checkpatch. >>> >>>> >>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> >>>> --- >>>> include/efi_api.h | 33 +++++++++++++++++++ >>>> include/efi_loader.h | 2 ++ >>>> lib/efi_loader/Makefile | 2 +- >>>> lib/efi_loader/efi_boottime.c | 3 ++ >>>> lib/efi_loader/efi_unicode.c | 73 +++++++++++++++++++++++++++++++++++++++++++ >>>> 5 files changed, 112 insertions(+), 1 deletion(-) >>>> create mode 100644 lib/efi_loader/efi_unicode.c >>>> >>>> diff --git a/include/efi_api.h b/include/efi_api.h >>>> index 932a3429a8..25f774f253 100644 >>>> --- a/include/efi_api.h >>>> +++ b/include/efi_api.h >>>> @@ -740,6 +740,39 @@ struct efi_hii_string_protocol >>>> UINTN *secondary_languages_size); >>>> }; >>>> >>>> +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \ >>>> + EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \ >>>> + 0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49) >>>> + >>>> +struct efi_unicode_collation_protocol >>>> +{ >>> >>> ERROR: open brace '{' following struct go on the same line >>> #30: FILE: include/efi_api.h:748: >>> +struct efi_unicode_collation_protocol >>> +{ >>> >>>> + INTN(EFIAPI *stri_coll)( >>>> + struct efi_unicode_collation_protocol *this, >>>> + efi_string_t s1, >>>> + efi_string_t s2); >>>> + bool(EFIAPI *metai_match)( >>>> + struct efi_unicode_collation_protocol *this, >>>> + efi_string_t string, >>>> + efi_string_t pattern); >>>> + void(EFIAPI *str_lwr)( >>>> + struct efi_unicode_collation_protocol *this, >>>> + efi_string_t string); >>>> + void(EFIAPI *str_upr)( >>>> + struct efi_unicode_collation_protocol *this, >>>> + efi_string_t string); >>>> + void(EFIAPI *fat_to_str)( >>>> + struct efi_unicode_collation_protocol *this, >>>> + UINTN fat_size, >>>> + uint8_t *fat, >>>> + efi_string_t string); >>>> + bool(EFIAPI *str_to_fat)( >>>> + struct efi_unicode_collation_protocol *this, >>>> + efi_string_t string, >>>> + UINTN fat_size, >>>> + uint8_t *fat); >>>> + uint8_t *supported_languages; >>>> +}; >>>> + >>>> #define EFI_GOP_GUID \ >>>> EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \ >>>> 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a) >>>> diff --git a/include/efi_loader.h b/include/efi_loader.h >>>> index a89bb2fcd9..6668338d0b 100644 >>>> --- a/include/efi_loader.h >>>> +++ b/include/efi_loader.h >>>> @@ -62,6 +62,7 @@ extern const struct efi_device_path_utilities_protocol efi_device_path_utilities >>>> extern const struct efi_hii_config_routing_protocol efi_hii_config_routing; >>>> extern const struct efi_hii_database_protocol efi_hii_database; >>>> extern const struct efi_hii_string_protocol efi_hii_string; >>>> +extern const struct efi_unicode_collation_protocol efi_unicode_collation; >>>> >>>> uint16_t *efi_dp_str(struct efi_device_path *dp); >>>> >>>> @@ -76,6 +77,7 @@ extern const efi_guid_t efi_guid_device_path_utilities_protocol; >>>> extern const efi_guid_t efi_guid_hii_config_routing_protocol; >>>> extern const efi_guid_t efi_guid_hii_database_protocol; >>>> extern const efi_guid_t efi_guid_hii_string_protocol; >>>> +extern const efi_guid_t efi_guid_unicode_collation_protocol2; >>>> >>>> extern unsigned int __efi_runtime_start, __efi_runtime_stop; >>>> extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; >>>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile >>>> index e8fd6823a3..82b703bb39 100644 >>>> --- a/lib/efi_loader/Makefile >>>> +++ b/lib/efi_loader/Makefile >>>> @@ -16,7 +16,7 @@ always := $(efiprogs-y) >>>> obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o >>>> obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o >>>> obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o >>>> -obj-y += efi_device_path_utilities.o efi_hii.o >>>> +obj-y += efi_device_path_utilities.o efi_hii.o efi_unicode.o >>>> obj-y += efi_file.o efi_variable.o efi_bootmgr.o >>>> obj-$(CONFIG_LCD) += efi_gop.o >>>> obj-$(CONFIG_DM_VIDEO) += efi_gop.o >>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >>>> index 4d1a16051b..04358e8aca 100644 >>>> --- a/lib/efi_loader/efi_boottime.c >>>> +++ b/lib/efi_loader/efi_boottime.c >>>> @@ -788,6 +788,9 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob >>>> obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol; >>>> obj->protocols[7].protocol_interface = (void *)&efi_hii_config_routing; >>>> >>> >>> Do not add a protocol that is not properly implemented yet. >> >> There is *no possible way* that I am going to completely implement >> HII, so big nak on this comment! But this patch implements nothing. So move adding the protocol to the image to your follow-up patch. Regards Heinrich >> >> In general, my plan was to implement the minimal parts of these >> protocols to get to the point where we can run SCT before implementing >> the rest, so we at least have a way to test implementation against >> compliance tests written against the spec (instead of our own tests >> written against our interpretation of the spec). In the particular >> case of HII, it includes things like forms based UI rendering, so I >> don't think we'll ever implement the entire thing. Or if someone did, >> we'd probably want to make it something that can be disabled at >> compile time to keep footprint down. >> >> Anyways, until we get to the point of loading option ROMs from PCI >> cards in u-boot I don't think we really need a setup menu or complete >> HII implementation ;-) >> >>>> + obj->protocols[8].guid = &efi_guid_unicode_collation_protocol2; >>>> + obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation; >>>> + >>>> info->file_path = file_path; >>>> info->device_handle = efi_dp_find_obj(device_path, NULL); >>>> >>>> diff --git a/lib/efi_loader/efi_unicode.c b/lib/efi_loader/efi_unicode.c >>>> new file mode 100644 >>>> index 0000000000..fdf1a99812 >>>> --- /dev/null >>>> +++ b/lib/efi_loader/efi_unicode.c >>>> @@ -0,0 +1,73 @@ >>>> +/* >>>> +* EFI Unicode interface >>>> + * >>>> + * Copyright (c) 2017 Leif Lindholm >>>> + * >>>> + * SPDX-License-Identifier: GPL-2.0+ >>>> + */ >>>> + >>>> +#include <common.h> >>>> +#include <efi_loader.h> >>>> + >>>> +const efi_guid_t efi_guid_unicode_collation_protocol2 = >>>> + EFI_UNICODE_COLLATION_PROTOCOL2_GUID; >>>> + >>> >>> None of the functions matches the definitions in the structure. >>> >>> Add the missing EFIAPI. >> >> I was talking to Peter Jones about this, and he suggested something >> that makes *way* more sense.. just build u-boot with '-mabi=ms_abi' on >> x86_64 (at least conditionally if EFI_LOADER is enabled). That would >> take a bit of work to annotate the os layer in sandbox to use sysv >> abi, but that is a lot smaller of an interface than EFI_LOADER (which >> is only going to continue to grow). > > Let's first get your patch series in before starting a new job. > > Otherwise we will never stop blocking each other. > > Regards > > Heinrich > >> >> Anyways, other than that, I'll double check the function order issues >> you mentioned, I mostly assumed that Leif had it right (and I don't >> remember encountering any issues with that so far with >> Shell.efi/SCT.efi). >> >> Some of your comments could be resolved by just squashing my >> "implement" patches with Leif's "stub" patches, but I didn't want to >> do that without Leif's permission. >> >> BR, >> -R >> >>> Regards >>> >>> Heinrich >>> >>>> +INTN stri_coll(struct efi_unicode_collation_protocol *this, >>>> + efi_string_t s1, >>>> + efi_string_t s2) >>>> +{ >>>> + EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2); >>>> + return EFI_EXIT(0); >>>> +} >>>> + >>>> +bool metai_match(struct efi_unicode_collation_protocol *this, >>>> + efi_string_t string, >>>> + efi_string_t pattern) >>>> +{ >>>> + EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, string, pattern); >>>> + return EFI_EXIT(false); >>>> +} >>>> + >>>> +void str_lwr(struct efi_unicode_collation_protocol *this, >>>> + efi_string_t string) >>>> +{ >>>> + EFI_ENTRY("%p, \"%ls\"", this, string); >>>> + EFI_EXIT(0); >>> >>> EFI_EXIT(EFI_SUCCESS); >>> >>>> + return; >>>> +} >>>> + >>>> +void str_upr(struct efi_unicode_collation_protocol *this, >>>> + efi_string_t string) >>>> +{ >>>> + EFI_ENTRY("%p, \"%ls\"", this, string); >>>> + EFI_EXIT(0); >>> >>> EFI_EXIT(EFI_SUCCESS); >>> >>>> + return; >>>> +} >>>> + >>>> +void fat_to_str(struct efi_unicode_collation_protocol *this, >>>> + UINTN fat_size, >>>> + uint8_t *fat, >>>> + efi_string_t string) >>>> +{ >>>> + EFI_ENTRY("%p, %lu, \"%s\", %p", this, fat_size, fat, string); >>>> + EFI_EXIT(0); >>> >>> EFI_EXIT(EFI_SUCCESS); >>> >>>> + return; >>>> +} >>>> + >>>> +bool str_to_fat(struct efi_unicode_collation_protocol *this, >>>> + efi_string_t string, >>>> + UINTN fat_size, >>>> + uint8_t *fat) >>>> +{ >>>> + EFI_ENTRY("%p, \"%ls\", %lu, %p", this, string, fat_size, fat); >>>> + return EFI_EXIT(false); >>>> +} >>>> + >>>> +const struct efi_unicode_collation_protocol efi_unicode_collation = { >>>> + .stri_coll = stri_coll, >>>> + .metai_match = metai_match, >>>> + .str_lwr = str_lwr, >>>> + .str_upr = str_upr, >>>> + .fat_to_str = fat_to_str, >>>> + .str_to_fat = str_to_fat >>>> +}; >>>> >>> >> > >
diff --git a/include/efi_api.h b/include/efi_api.h index 932a3429a8..25f774f253 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -740,6 +740,39 @@ struct efi_hii_string_protocol UINTN *secondary_languages_size); }; +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \ + EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \ + 0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49) + +struct efi_unicode_collation_protocol +{ + INTN(EFIAPI *stri_coll)( + struct efi_unicode_collation_protocol *this, + efi_string_t s1, + efi_string_t s2); + bool(EFIAPI *metai_match)( + struct efi_unicode_collation_protocol *this, + efi_string_t string, + efi_string_t pattern); + void(EFIAPI *str_lwr)( + struct efi_unicode_collation_protocol *this, + efi_string_t string); + void(EFIAPI *str_upr)( + struct efi_unicode_collation_protocol *this, + efi_string_t string); + void(EFIAPI *fat_to_str)( + struct efi_unicode_collation_protocol *this, + UINTN fat_size, + uint8_t *fat, + efi_string_t string); + bool(EFIAPI *str_to_fat)( + struct efi_unicode_collation_protocol *this, + efi_string_t string, + UINTN fat_size, + uint8_t *fat); + uint8_t *supported_languages; +}; + #define EFI_GOP_GUID \ EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \ 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a) diff --git a/include/efi_loader.h b/include/efi_loader.h index a89bb2fcd9..6668338d0b 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -62,6 +62,7 @@ extern const struct efi_device_path_utilities_protocol efi_device_path_utilities extern const struct efi_hii_config_routing_protocol efi_hii_config_routing; extern const struct efi_hii_database_protocol efi_hii_database; extern const struct efi_hii_string_protocol efi_hii_string; +extern const struct efi_unicode_collation_protocol efi_unicode_collation; uint16_t *efi_dp_str(struct efi_device_path *dp); @@ -76,6 +77,7 @@ extern const efi_guid_t efi_guid_device_path_utilities_protocol; extern const efi_guid_t efi_guid_hii_config_routing_protocol; extern const efi_guid_t efi_guid_hii_database_protocol; extern const efi_guid_t efi_guid_hii_string_protocol; +extern const efi_guid_t efi_guid_unicode_collation_protocol2; extern unsigned int __efi_runtime_start, __efi_runtime_stop; extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index e8fd6823a3..82b703bb39 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -16,7 +16,7 @@ always := $(efiprogs-y) obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o -obj-y += efi_device_path_utilities.o efi_hii.o +obj-y += efi_device_path_utilities.o efi_hii.o efi_unicode.o obj-y += efi_file.o efi_variable.o efi_bootmgr.o obj-$(CONFIG_LCD) += efi_gop.o obj-$(CONFIG_DM_VIDEO) += efi_gop.o diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 4d1a16051b..04358e8aca 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -788,6 +788,9 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol; obj->protocols[7].protocol_interface = (void *)&efi_hii_config_routing; + obj->protocols[8].guid = &efi_guid_unicode_collation_protocol2; + obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation; + info->file_path = file_path; info->device_handle = efi_dp_find_obj(device_path, NULL); diff --git a/lib/efi_loader/efi_unicode.c b/lib/efi_loader/efi_unicode.c new file mode 100644 index 0000000000..fdf1a99812 --- /dev/null +++ b/lib/efi_loader/efi_unicode.c @@ -0,0 +1,73 @@ +/* +* EFI Unicode interface + * + * Copyright (c) 2017 Leif Lindholm + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <efi_loader.h> + +const efi_guid_t efi_guid_unicode_collation_protocol2 = + EFI_UNICODE_COLLATION_PROTOCOL2_GUID; + +INTN stri_coll(struct efi_unicode_collation_protocol *this, + efi_string_t s1, + efi_string_t s2) +{ + EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2); + return EFI_EXIT(0); +} + +bool metai_match(struct efi_unicode_collation_protocol *this, + efi_string_t string, + efi_string_t pattern) +{ + EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, string, pattern); + return EFI_EXIT(false); +} + +void str_lwr(struct efi_unicode_collation_protocol *this, + efi_string_t string) +{ + EFI_ENTRY("%p, \"%ls\"", this, string); + EFI_EXIT(0); + return; +} + +void str_upr(struct efi_unicode_collation_protocol *this, + efi_string_t string) +{ + EFI_ENTRY("%p, \"%ls\"", this, string); + EFI_EXIT(0); + return; +} + +void fat_to_str(struct efi_unicode_collation_protocol *this, + UINTN fat_size, + uint8_t *fat, + efi_string_t string) +{ + EFI_ENTRY("%p, %lu, \"%s\", %p", this, fat_size, fat, string); + EFI_EXIT(0); + return; +} + +bool str_to_fat(struct efi_unicode_collation_protocol *this, + efi_string_t string, + UINTN fat_size, + uint8_t *fat) +{ + EFI_ENTRY("%p, \"%ls\", %lu, %p", this, string, fat_size, fat); + return EFI_EXIT(false); +} + +const struct efi_unicode_collation_protocol efi_unicode_collation = { + .stri_coll = stri_coll, + .metai_match = metai_match, + .str_lwr = str_lwr, + .str_upr = str_upr, + .fat_to_str = fat_to_str, + .str_to_fat = str_to_fat +};