From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f72.google.com (mail-pg0-f72.google.com [74.125.83.72]) by kanga.kvack.org (Postfix) with ESMTP id 9CAC56B0038 for ; Thu, 23 Feb 2017 12:28:04 -0500 (EST) Received: by mail-pg0-f72.google.com with SMTP id t184so57036505pgt.1 for ; Thu, 23 Feb 2017 09:28:04 -0800 (PST) Received: from NAM03-CO1-obe.outbound.protection.outlook.com (mail-co1nam03on0077.outbound.protection.outlook.com. [104.47.40.77]) by mx.google.com with ESMTPS id q9si4873752pli.125.2017.02.23.09.28.03 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 23 Feb 2017 09:28:03 -0800 (PST) Subject: Re: [RFC PATCH v4 13/28] efi: Update efi_mem_type() to return defined EFI mem types References: <20170216154158.19244.66630.stgit@tlendack-t1.amdoffice.net> <20170216154457.19244.5369.stgit@tlendack-t1.amdoffice.net> <20170221120505.GQ28416@codeblueprint.co.uk> From: Tom Lendacky Message-ID: <41d5df05-14be-ff33-a7e2-6b2f51e2605a@amd.com> Date: Thu, 23 Feb 2017 11:27:55 -0600 MIME-Version: 1.0 In-Reply-To: <20170221120505.GQ28416@codeblueprint.co.uk> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Matt Fleming Cc: linux-arch@vger.kernel.org, linux-efi@vger.kernel.org, kvm@vger.kernel.org, linux-doc@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com, linux-mm@kvack.org, iommu@lists.linux-foundation.org, Rik van Riel , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Toshimitsu Kani , Arnd Bergmann , Jonathan Corbet , "Michael S. Tsirkin" , Joerg Roedel , Konrad Rzeszutek Wilk , Paolo Bonzini , Brijesh Singh , Ingo Molnar , Alexander Potapenko , Andy Lutomirski , "H. Peter Anvin" , Borislav Petkov , Andrey Ryabinin , Thomas Gleixner , Larry Woodman , Dmitry Vyukov On 2/21/2017 6:05 AM, Matt Fleming wrote: > On Thu, 16 Feb, at 09:44:57AM, Tom Lendacky wrote: >> Update the efi_mem_type() to return EFI_RESERVED_TYPE instead of a >> hardcoded 0. >> >> Signed-off-by: Tom Lendacky >> --- >> arch/x86/platform/efi/efi.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c >> index a15cf81..6407103 100644 >> --- a/arch/x86/platform/efi/efi.c >> +++ b/arch/x86/platform/efi/efi.c >> @@ -1037,7 +1037,7 @@ u32 efi_mem_type(unsigned long phys_addr) >> efi_memory_desc_t *md; >> >> if (!efi_enabled(EFI_MEMMAP)) >> - return 0; >> + return EFI_RESERVED_TYPE; >> >> 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 EFI_RESERVED_TYPE; >> } > > I see what you're getting at here, but arguably the return value in > these cases never should have been zero to begin with (your change > just makes that more obvious). > > Returning EFI_RESERVED_TYPE implies an EFI memmap entry exists for > this address, which is misleading because it doesn't in the hunks > you've modified above. > > Instead, could you look at returning a negative error value in the > usual way we do in the Linux kernel, and update the function prototype > to match? I don't think any callers actually require the return type > to be u32. I can do that, I'll change the return type to an int. For the !efi_enabled I can return -ENOTSUPP and for when an entry isn't found I can return -EINVAL. Sound good? The ia64 arch is the only other arch that defines the function. It has just a single return 0 that I'll change to -EINVAL. Thanks, Tom > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org