Re: [PATCH] efi/libstub: arm64: avoid SetVirtualAddressMap() when possible

[Thorsten Leemhuis <regressions@leemhuis.info>]

[TLDR: I'm adding this regression report to the list of tracked
regressions; all text from me you find below is based on a few templates
paragraphs you might have encountered already already in similar form.]

Hi, this is your Linux kernel regression tracker. CCing the regression
mailing list, as it should be in the loop for all regressions, as
explained here:
https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html
On 19.10.22 05:24, Yicong Yang wrote:
> 
> After entering 6.1-rc1 the efi runtime services is not working on my platform:
> 
> [    0.054039] Remapping and enabling EFI services.
> [    0.054043] UEFI virtual mapping missing or invalid -- runtime services will not be available
> 
> Not sure this patch is the root cause since I see some refactor of efi codes in 6.1-rc1,
> but simply reverting this make EFI runtime services works again. Tested on HiSilicon's
> Kunpeng 920 arm64 server using 48 bit VA address:
> 
> CONFIG_ARM64_VA_BITS_48=y
> CONFIG_ARM64_VA_BITS=48
> 
> Thanks.

Thanks for the report. To be sure below issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
tracking bot:

#regzbot ^introduced d3549a938b73f203ef522562ae9f2d38aa43d234
#regzbot title efi/libstub: arm64: efi runtime services stopped working
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply -- ideally with also
telling regzbot about it, as explained here:
https://linux-regtracking.leemhuis.info/tracked-regression/

Reminder for developers: When fixing the issue, add 'Link:' tags
pointing to the report (the mail this one replies to), as explained for
in the Linux kernel's documentation; above webpage explains why this is
important for tracked regressions.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

> On 2022/9/16 18:18, Ard Biesheuvel wrote:
>> EFI's SetVirtualAddressMap() runtime service is a horrid hack that we'd
>> like to avoid using, if possible. For 64-bit architectures such as
>> arm64, the user and kernel mappings are entirely disjoint, and given
>> that we use the user region for mapping the UEFI runtime regions when
>> running under the OS, we don't rely on SetVirtualAddressMap() in the
>> conventional way, i.e., to permit kernel mappings of the OS to coexist
>> with kernel region mappings of the firmware regions. This means that, in
>> principle, we should be able to avoid SetVirtualAddressMap() altogether,
>> and simply use the 1:1 mapping that UEFI uses at boot time. (Note that
>> omitting SetVirtualAddressMap() is explicitly permitted by the UEFI
>> spec).
>>
>> However, there is a corner case on arm64, which, if configured for
>> 3-level paging (or 2-level paging when using 64k pages), may not be able
>> to cover the entire range of firmware mappings (which might contain both
>> memory and MMIO peripheral mappings).
>>
>> So let's avoid SetVirtualAddressMap() on arm64, but only if the VA space
>> is guaranteed to be of sufficient size.
>>
>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>> ---
>>  drivers/firmware/efi/libstub/arm64-stub.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
>> index cd3bea25c762..4fff6c32899e 100644
>> --- a/drivers/firmware/efi/libstub/arm64-stub.c
>> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
>> @@ -31,6 +31,15 @@ efi_status_t check_platform_features(void)
>>  			efi_err("This 16 KB granular kernel is not supported by your CPU\n");
>>  		return EFI_UNSUPPORTED;
>>  	}
>> +
>> +	/*
>> +	 * If we have 48 bits of VA space for TTBR0 mappings, we can map the
>> +	 * UEFI runtime regions 1:1 and so calling SetVirtualAddressMap() is
>> +	 * unnecessary.
>> +	 */
>> +	if (VA_BITS_MIN >= 48)
>> +		efi_novamap = true;
>> +
>>  	return EFI_SUCCESS;
>>  }
>>  
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel