Re: [PATCH v5 15/32] efi: Update efi_mem_type() to return an error rather than 0
On 5/7/2017 12:18 PM, Borislav Petkov wrote: On Tue, Apr 18, 2017 at 04:19:00PM -0500, Tom Lendacky wrote: The efi_mem_type() function currently returns a 0, which maps to EFI_RESERVED_TYPE, if the function is unable to find a memmap entry for the supplied physical address. Returning EFI_RESERVED_TYPE implies that a memmap entry exists, when it doesn't. Instead of returning 0, change the function to return a negative error value when no memmap entry is found. Signed-off-by: Tom Lendacky--- ... diff --git a/include/linux/efi.h b/include/linux/efi.h index cd768a1..a27bb3f 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -973,7 +973,7 @@ static inline void efi_esrt_init(void) { } extern int efi_config_parse_tables(void *config_tables, int count, int sz, efi_config_table_type_t *arch_tables); extern u64 efi_get_iobase (void); -extern u32 efi_mem_type (unsigned long phys_addr); +extern int efi_mem_type (unsigned long phys_addr); WARNING: space prohibited between function name and open parenthesis '(' #101: FILE: include/linux/efi.h:976: +extern int efi_mem_type (unsigned long phys_addr); Please integrate scripts/checkpatch.pl in your patch creation workflow. Some of the warnings/errors *actually* make sense. I do/did run scripts/checkpatch.pl against all my patches. In this case I chose to keep the space in order to stay consistent with some of the surrounding functions. No problem though, I can remove the space. Thanks, Tom I know, the other function prototypes have a space too but that's not our coding style. Looks like this trickled in from ia64, from looking at arch/ia64/kernel/efi.c. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 15/32] efi: Update efi_mem_type() to return an error rather than 0
On Tue, Apr 18, 2017 at 04:19:00PM -0500, Tom Lendacky wrote: > The efi_mem_type() function currently returns a 0, which maps to > EFI_RESERVED_TYPE, if the function is unable to find a memmap entry for > the supplied physical address. Returning EFI_RESERVED_TYPE implies that > a memmap entry exists, when it doesn't. Instead of returning 0, change > the function to return a negative error value when no memmap entry is > found. > > Signed-off-by: Tom Lendacky> --- ... > diff --git a/include/linux/efi.h b/include/linux/efi.h > index cd768a1..a27bb3f 100644 > --- a/include/linux/efi.h > +++ b/include/linux/efi.h > @@ -973,7 +973,7 @@ static inline void efi_esrt_init(void) { } > extern int efi_config_parse_tables(void *config_tables, int count, int sz, > efi_config_table_type_t *arch_tables); > extern u64 efi_get_iobase (void); > -extern u32 efi_mem_type (unsigned long phys_addr); > +extern int efi_mem_type (unsigned long phys_addr); WARNING: space prohibited between function name and open parenthesis '(' #101: FILE: include/linux/efi.h:976: +extern int efi_mem_type (unsigned long phys_addr); Please integrate scripts/checkpatch.pl in your patch creation workflow. Some of the warnings/errors *actually* make sense. I know, the other function prototypes have a space too but that's not our coding style. Looks like this trickled in from ia64, from looking at arch/ia64/kernel/efi.c. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v5 15/32] efi: Update efi_mem_type() to return an error rather than 0
The efi_mem_type() function currently returns a 0, which maps to EFI_RESERVED_TYPE, if the function is unable to find a memmap entry for the supplied physical address. Returning EFI_RESERVED_TYPE implies that a memmap entry exists, when it doesn't. Instead of returning 0, change the function to return a negative error value when no memmap entry is found. Signed-off-by: Tom Lendacky--- arch/ia64/kernel/efi.c |4 ++-- arch/x86/platform/efi/efi.c |6 +++--- include/linux/efi.h |2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c index 1212956..8141600 100644 --- a/arch/ia64/kernel/efi.c +++ b/arch/ia64/kernel/efi.c @@ -757,14 +757,14 @@ static void __init handle_palo(unsigned long phys_addr) return 0; } -u32 +int efi_mem_type (unsigned long phys_addr) { efi_memory_desc_t *md = efi_memory_descriptor(phys_addr); if (md) return md->type; - return 0; + return -EINVAL; } u64 diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index a15cf81..f9b0b7a 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -1032,12 +1032,12 @@ void __init efi_enter_virtual_mode(void) /* * Convenience functions to obtain memory types and attributes */ -u32 efi_mem_type(unsigned long phys_addr) +int efi_mem_type(unsigned long phys_addr) { efi_memory_desc_t *md; if (!efi_enabled(EFI_MEMMAP)) - return 0; + return -ENOTSUPP; for_each_efi_memory_desc(md) { if ((md->phys_addr <= phys_addr) && @@ -1045,7 +1045,7 @@ u32 efi_mem_type(unsigned long phys_addr) (md->num_pages << EFI_PAGE_SHIFT return md->type; } - return 0; + return -EINVAL; } static int __init arch_parse_efi_cmdline(char *str) diff --git a/include/linux/efi.h b/include/linux/efi.h index cd768a1..a27bb3f 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -973,7 +973,7 @@ static inline void efi_esrt_init(void) { } extern int efi_config_parse_tables(void *config_tables, int count, int sz, efi_config_table_type_t *arch_tables); extern u64 efi_get_iobase (void); -extern u32 efi_mem_type (unsigned long phys_addr); +extern int efi_mem_type (unsigned long phys_addr); extern u64 efi_mem_attributes (unsigned long phys_addr); extern u64 efi_mem_attribute (unsigned long phys_addr, unsigned long size); extern int __init efi_uart_console_only (void); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu