On 11/06/2019 10:31 PM, Ard Biesheuvel wrote: > On Tue, 5 Nov 2019 at 21:39, Anshuman Khandual > <anshuman.khandual@xxxxxxx> wrote: >> >> The function efi_mem_type() is expected (per documentation above) to return >> EFI_RESERVED_TYPE when a given physical address is not present in the EFI >> memory map. Even though EFI_RESERVED_TYPE is not getting checked explicitly >> anywhere in the callers, it is always better to return expected values. >> >> Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> >> Cc: linux-efi@xxxxxxxxxxxxxxx >> Cc: linux-kernel@xxxxxxxxxxxxxxx >> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx> > > This reverts commit f99afd08a45fbbd9ce35a7624ffd1d850a1906c0. > > Could you explain why it is better to fix the code than fix the comment? >From the above commit message, its not clear how returning EFI_RESERVED_TYPE would have meant that a memory descriptor really exists. Just wondering if firmware itself can send across memory descriptors with EFI_RESERVED_TYPE attribute or it is only a software specific attribute ? Currently it is being used while merging two adjacent regions with similar type and attributes. Searching for physical addresses within those merged memory descriptors will return EFI_RESERVED_TYPE but that is different than the entry not being present at all. So I think the previous commit which introduced -EINVAL/-ENOTSUPP in place for EFI_RESERVED_TYPE did the right thing. We should fix the comment instead, will send a patch. > >> --- >> drivers/firmware/efi/efi.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c >> index 69f00f7453a3..bdda90a4601e 100644 >> --- a/drivers/firmware/efi/efi.c >> +++ b/drivers/firmware/efi/efi.c >> @@ -914,7 +914,7 @@ int efi_mem_type(unsigned long phys_addr) >> (md->num_pages << EFI_PAGE_SHIFT)))) >> return md->type; >> } >> - return -EINVAL; >> + return EFI_RESERVED_TYPE; >> } >> #endif >> >> -- >> 2.20.1 >>
