On 25 August 2016 at 12:53, Leif Lindholm <leif.lindholm@xxxxxxxxxx> wrote: > On Wed, Aug 24, 2016 at 05:24:16PM +0200, Ard Biesheuvel wrote: >> Currently, memory regions are only recorded in the memblock memory table >> if they have the EFI_MEMORY_WB memory type attribute set. In case the >> region is of a reserved type, it is also marked as MEMBLOCK_NOMAP, which >> will leave it out of the linear mapping. >> >> However, memory regions may legally have the EFI_MEMORY_WT or EFI_MEMORY_WC >> attributes set, and the EFI_MEMORY_WB cleared, in which case the region in >> question is obviously backed by normal memory, but is not recorded in the >> memblock memory table at all. Since it would be useful to be able to >> identify any UEFI reported memory region using memblock_is_memory(), it >> makes sense to add all memory to the memblock memory table, and simply mark >> it as MEMBLOCK_NOMAP if it lacks the EFI_MEMORY_WB attribute. >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> >> --- >> >> Note that this will also result in regions with EFI_MEMORY_WB cleared to >> be listed in /proc/iomem as 'System RAM', which may be incorrect. However, >> we already display this incorrect behavior for runtime services code/data >> regions, so this should be fixed in a separate patch, of which an example >> has been proposed here: >> https://www.spinics.net/lists/arm-kernel/msg525369.html >> >> drivers/firmware/efi/arm-init.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c >> index c49d50e68aee..678672d332f8 100644 >> --- a/drivers/firmware/efi/arm-init.c >> +++ b/drivers/firmware/efi/arm-init.c >> @@ -28,7 +28,7 @@ u64 efi_system_table; >> >> static int __init is_normal_ram(efi_memory_desc_t *md) >> { >> - if (md->attribute & EFI_MEMORY_WB) >> + if (md->attribute & (EFI_MEMORY_WB|EFI_MEMORY_WT|EFI_MEMORY_WC)) > > The code change makes a lot of sense, but this makes the call sites a > bit confusing. It would make sense to rename this function. > > Would supports_unaligned() be too silly? That's basically what the > above makes it mean. > How about is_memory() ? And we could invert is_reserve_region, and call it is_usable_memory() ... > / > Leif > >> return 1; >> return 0; >> } >> @@ -163,7 +163,13 @@ static __init int is_reserve_region(efi_memory_desc_t *md) >> case EFI_BOOT_SERVICES_DATA: >> case EFI_CONVENTIONAL_MEMORY: >> case EFI_PERSISTENT_MEMORY: >> - return 0; >> + /* >> + * According to the spec, these regions are no longer reserved >> + * after calling ExitBootServices(). However, we can only use >> + * them as System RAM if they can be mapped writeback cacheable. >> + * Otherwise, treat them as reserved. >> + */ >> + return (md->type & EFI_MEMORY_WB) == 0; >> default: >> break; >> } >> -- >> 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
