On 18 February 2016 at 11:44, Matt Fleming <matt@xxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, 15 Feb, at 12:32:32PM, Ard Biesheuvel wrote:
>> On ARM and arm64, ioremap() and memremap() are not interchangeable like
>> on x86, and the use of ioremap() on ordinary RAM is typically flagged
>> as an error if the memory region being mapped is also covered by the
>> linear mapping, since that would lead to aliases with conflicting
>> cacheability attributes.
>>
>> Since what we are dealing with is not an I/O region with side effects,
>> using ioremap() here is arguably incorrect anyway, so let's replace
>> it with memremap instead. Also add a missing unmap on the success path,
>> and drop a memblock_remove() call which does not belong here, this far
>> into the boot sequence.
>>
>> Cc: Peter Jones <pjones@xxxxxxxxxx>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>> ---
>> drivers/firmware/efi/esrt.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>
> [...]
>
>> @@ -432,8 +434,6 @@ static int __init esrt_sysfs_init(void)
>> if (error)
>> goto err_cleanup_list;
>>
>> - memblock_remove(esrt_data, esrt_data_size);
>> -
>> pr_debug("esrt-sysfs: loaded.\n");
>>
>> return 0;
>
> Shouldn't we be replacing memblock_remove() with free_bootmem_late()?
> The original ESRT region is still reserved at this point, so we should
> do our best to release it to the page allocator.
I'd rather we keep it reserved. That way, the config table entry still
points to something valid, which could be useful for kexec(), I think?
At least, that is how I intended to handle config tables on ARM ...
--
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
