| Message ID | 20190502040441.30372-1-jlee@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2,v2] efi: add a function to convert the status value to string | expand |
On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote: > > This function can be used to convert EFI status value to string > for printing out debug message. Using this function can improve > the readability of log. > > v2. Please move the changelog out of the commit log (move it below the --- further down) > - Changed the wording in subject and description. > - Moved the marco immediately after the status value definitions. > - Turned into a proper function instead of inline. > You missed my point here. A proper function means the function in a .c file, and only the declaration in a .h file. This way, you are still duplicating the literal strings into every object file that references this function. > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Kees Cook <keescook@chromium.org> > Cc: Anton Vorontsov <anton@enomsg.org> > Cc: Colin Cross <ccross@android.com> > Cc: Tony Luck <tony.luck@intel.com> > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> > --- > include/linux/efi.h | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/include/linux/efi.h b/include/linux/efi.h > index 54357a258b35..6f3f89a32eef 100644 > --- a/include/linux/efi.h > +++ b/include/linux/efi.h > @@ -42,6 +42,34 @@ > #define EFI_ABORTED (21 | (1UL << (BITS_PER_LONG-1))) > #define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1))) > > +#define EFI_STATUS_STR(_status) \ > +case EFI_##_status: \ > + return "EFI_" __stringify(_status); > + > +static __attribute__((unused)) char * > +efi_status_to_str(unsigned long status) > +{ > + switch (status) { > + EFI_STATUS_STR(SUCCESS) > + EFI_STATUS_STR(LOAD_ERROR) > + EFI_STATUS_STR(INVALID_PARAMETER) > + EFI_STATUS_STR(UNSUPPORTED) > + EFI_STATUS_STR(BAD_BUFFER_SIZE) > + EFI_STATUS_STR(BUFFER_TOO_SMALL) > + EFI_STATUS_STR(NOT_READY) > + EFI_STATUS_STR(DEVICE_ERROR) > + EFI_STATUS_STR(WRITE_PROTECTED) > + EFI_STATUS_STR(OUT_OF_RESOURCES) > + EFI_STATUS_STR(NOT_FOUND) > + EFI_STATUS_STR(ABORTED) > + EFI_STATUS_STR(SECURITY_VIOLATION) > + default: > + pr_warn("Unknown efi status: 0x%lx", status); > + } > + > + return "Unknown efi status"; > +} > + > typedef unsigned long efi_status_t; > typedef u8 efi_bool_t; > typedef u16 efi_char16_t; /* UNICODE character */ > -- > 2.16.4 >
On Thu, May 02, 2019 at 10:53:31AM +0200, Ard Biesheuvel wrote: > On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote: > > > > This function can be used to convert EFI status value to string > > for printing out debug message. Using this function can improve > > the readability of log. > > > > v2. > > Please move the changelog out of the commit log (move it below the --- > further down) > OK! I will moved the changelog out of the commit log. > > - Changed the wording in subject and description. > > - Moved the marco immediately after the status value definitions. > > - Turned into a proper function instead of inline. > > > > You missed my point here. A proper function means the function in a .c > file, and only the declaration in a .h file. This way, you are still > duplicating the literal strings into every object file that references > this function. > Sorry for I missunderstood. I will move the function to load_uefi.c. Thanks a lot! Joey Lee
On Fri, 3 May 2019 at 08:15, joeyli <jlee@suse.com> wrote: > > On Thu, May 02, 2019 at 10:53:31AM +0200, Ard Biesheuvel wrote: > > On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote: > > > > > > This function can be used to convert EFI status value to string > > > for printing out debug message. Using this function can improve > > > the readability of log. > > > > > > v2. > > > > Please move the changelog out of the commit log (move it below the --- > > further down) > > > > OK! I will moved the changelog out of the commit log. > > > > - Changed the wording in subject and description. > > > - Moved the marco immediately after the status value definitions. > > > - Turned into a proper function instead of inline. > > > > > > > You missed my point here. A proper function means the function in a .c > > file, and only the declaration in a .h file. This way, you are still > > duplicating the literal strings into every object file that references > > this function. > > > > Sorry for I missunderstood. I will move the function to load_uefi.c. > No, please move it to a file that is shared between all EFI code.
On Fri, May 03, 2019 at 08:16:42AM +0200, Ard Biesheuvel wrote: > On Fri, 3 May 2019 at 08:15, joeyli <jlee@suse.com> wrote: > > > > On Thu, May 02, 2019 at 10:53:31AM +0200, Ard Biesheuvel wrote: > > > On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote: > > > > > > > > This function can be used to convert EFI status value to string > > > > for printing out debug message. Using this function can improve > > > > the readability of log. > > > > > > > > v2. > > > > > > Please move the changelog out of the commit log (move it below the --- > > > further down) > > > > > > > OK! I will moved the changelog out of the commit log. > > > > > > - Changed the wording in subject and description. > > > > - Moved the marco immediately after the status value definitions. > > > > - Turned into a proper function instead of inline. > > > > > > > > > > You missed my point here. A proper function means the function in a .c > > > file, and only the declaration in a .h file. This way, you are still > > > duplicating the literal strings into every object file that references > > > this function. > > > > > > > Sorry for I missunderstood. I will move the function to load_uefi.c. > > > > No, please move it to a file that is shared between all EFI code. I see! I will move the function to the position that it is just behind efi_status_to_str() in drivers/firmware/efi/efi.c. Please let me know if the position is not suitable. Thanks a lot! Joey Lee
diff --git a/include/linux/efi.h b/include/linux/efi.h index 54357a258b35..6f3f89a32eef 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -42,6 +42,34 @@ #define EFI_ABORTED (21 | (1UL << (BITS_PER_LONG-1))) #define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1))) +#define EFI_STATUS_STR(_status) \ +case EFI_##_status: \ + return "EFI_" __stringify(_status); + +static __attribute__((unused)) char * +efi_status_to_str(unsigned long status) +{ + switch (status) { + EFI_STATUS_STR(SUCCESS) + EFI_STATUS_STR(LOAD_ERROR) + EFI_STATUS_STR(INVALID_PARAMETER) + EFI_STATUS_STR(UNSUPPORTED) + EFI_STATUS_STR(BAD_BUFFER_SIZE) + EFI_STATUS_STR(BUFFER_TOO_SMALL) + EFI_STATUS_STR(NOT_READY) + EFI_STATUS_STR(DEVICE_ERROR) + EFI_STATUS_STR(WRITE_PROTECTED) + EFI_STATUS_STR(OUT_OF_RESOURCES) + EFI_STATUS_STR(NOT_FOUND) + EFI_STATUS_STR(ABORTED) + EFI_STATUS_STR(SECURITY_VIOLATION) + default: + pr_warn("Unknown efi status: 0x%lx", status); + } + + return "Unknown efi status"; +} + typedef unsigned long efi_status_t; typedef u8 efi_bool_t; typedef u16 efi_char16_t; /* UNICODE character */
This function can be used to convert EFI status value to string for printing out debug message. Using this function can improve the readability of log. v2. - Changed the wording in subject and description. - Moved the marco immediately after the status value definitions. - Turned into a proper function instead of inline. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Kees Cook <keescook@chromium.org> Cc: Anton Vorontsov <anton@enomsg.org> Cc: Colin Cross <ccross@android.com> Cc: Tony Luck <tony.luck@intel.com> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> --- include/linux/efi.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)