| Message ID | 20230117142718.564299-1-ardb@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm64: efi: Call SetVaMap() with a 1:1 mapping | expand |
On Tue, 17 Jan 2023 at 15:27, Ard Biesheuvel <ardb@kernel.org> wrote: > > Linux on arm64 is now in the same boat as x86, where supporting laptops > that were built to run Windows and never tested beyond what is required > for the Windows Logo certification need workarounds for all kinds of > bizarre behaviors. > > On Snapdragon laptops, we cannot call SetVirtualAddressMap() from the > stub, because the firmware will crash while trying to access memory via > the virtual addresses being installed, which is explicitly unsupported > by the EFI spec. > > However, not calling SetVirtualAddressMap() results in other problems: > on Ampere Altra, it causes SetTime() to crash. On Surface and Flex5g > Windows-on-ARM laptops, it causes ResetSystem() to crash. > > So let's try to work around this while not making too much of a mess. > > First of all, install a 1:1 mapping instead of avoiding SetVaMap() > altogether - from the EFI spec pov, this should amount to the same > thing. > > Then, given that we already use a SMBIOS based hack for Altra to force > the use of SetVirtualAddressMap(), let's check for Surface systems in > the same way. > > Please test, and please report the SMBIOS type 1 family field for which > this workaround is needed. > > Also, note that these changes will not make a difference if the > EFI_RT_PROPERTIES_TABLE lists SetVirtualAddressMap() as not implemented. > > Nathan, I would appreciate it if you could give this a spin on your > Altra box (only patch #1 should make a difference), and for good > measure, double check that hwclock still works as it should. > > Cc: Johan Hovold <johan+linaro@kernel.org> > Cc: Maximilian Luz <luzmaximilian@gmail.com> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: Steev Klimaszewski <steev@kali.org> > Cc: Shawn Guo <shawn.guo@linaro.org> > > Ard Biesheuvel (2): > arm64: efi: Prefer a flat virtual mapping of the runtime services > arm64: efi: Force use of SetVirtualAddressMap() on MS Surface > Bah this does not even work on Yoga C630, so this is not going to help us. If we want ResetSystem() on these machines, we'll have to retain other memory ranges and map the in the EFI runtime map. Yuck. Nathan - still interested in whether patch #1 works on Altra,
On Tue, Jan 17, 2023 at 04:20:09PM +0100, Ard Biesheuvel wrote: > On Tue, 17 Jan 2023 at 15:27, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > Linux on arm64 is now in the same boat as x86, where supporting laptops > > that were built to run Windows and never tested beyond what is required > > for the Windows Logo certification need workarounds for all kinds of > > bizarre behaviors. > > > > On Snapdragon laptops, we cannot call SetVirtualAddressMap() from the > > stub, because the firmware will crash while trying to access memory via > > the virtual addresses being installed, which is explicitly unsupported > > by the EFI spec. > > > > However, not calling SetVirtualAddressMap() results in other problems: > > on Ampere Altra, it causes SetTime() to crash. On Surface and Flex5g > > Windows-on-ARM laptops, it causes ResetSystem() to crash. > > > > So let's try to work around this while not making too much of a mess. > > > > First of all, install a 1:1 mapping instead of avoiding SetVaMap() > > altogether - from the EFI spec pov, this should amount to the same > > thing. > > > > Then, given that we already use a SMBIOS based hack for Altra to force > > the use of SetVirtualAddressMap(), let's check for Surface systems in > > the same way. > > > > Please test, and please report the SMBIOS type 1 family field for which > > this workaround is needed. > > > > Also, note that these changes will not make a difference if the > > EFI_RT_PROPERTIES_TABLE lists SetVirtualAddressMap() as not implemented. > > > > Nathan, I would appreciate it if you could give this a spin on your > > Altra box (only patch #1 should make a difference), and for good > > measure, double check that hwclock still works as it should. > > > > Cc: Johan Hovold <johan+linaro@kernel.org> > > Cc: Maximilian Luz <luzmaximilian@gmail.com> > > Cc: Nathan Chancellor <nathan@kernel.org> > > Cc: Steev Klimaszewski <steev@kali.org> > > Cc: Shawn Guo <shawn.guo@linaro.org> > > > > Ard Biesheuvel (2): > > arm64: efi: Prefer a flat virtual mapping of the runtime services > > arm64: efi: Force use of SetVirtualAddressMap() on MS Surface > > > > Bah this does not even work on Yoga C630, so this is not going to help us. > > If we want ResetSystem() on these machines, we'll have to retain other > memory ranges and map the in the EFI runtime map. Yuck. > > Nathan - still interested in whether patch #1 works on Altra, I applied patch 1 on top of commit 6e50979a9c87 ("Merge tag 'mm-hotfixes-stable-2023-01-16-15-23' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm") in Linus' tree and everything still appears to be okay with hwclock. If there is any more specific testing that I should do, please let me know. Feel free to add Tested-by: Nathan Chancellor <nathan@kernel.org> to patch 1 in future revisions, and I am happy to test anything else that you might need in this series or future ones. Mainline: # uname -mr 6.2.0-rc4-00031-g6e50979a9c87 aarch64 # hwclock 2023-01-17 09:04:58.845411-07:00 Patch: # uname -mr 6.2.0-rc4-00032-g20165e83052e aarch64 # hwclock 2023-01-17 10:25:38.843788-07:00 Cheers, Nathan
On Tue, 17 Jan 2023 at 18:30, Nathan Chancellor <nathan@kernel.org> wrote: > > On Tue, Jan 17, 2023 at 04:20:09PM +0100, Ard Biesheuvel wrote: > > On Tue, 17 Jan 2023 at 15:27, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > Linux on arm64 is now in the same boat as x86, where supporting laptops > > > that were built to run Windows and never tested beyond what is required > > > for the Windows Logo certification need workarounds for all kinds of > > > bizarre behaviors. > > > > > > On Snapdragon laptops, we cannot call SetVirtualAddressMap() from the > > > stub, because the firmware will crash while trying to access memory via > > > the virtual addresses being installed, which is explicitly unsupported > > > by the EFI spec. > > > > > > However, not calling SetVirtualAddressMap() results in other problems: > > > on Ampere Altra, it causes SetTime() to crash. On Surface and Flex5g > > > Windows-on-ARM laptops, it causes ResetSystem() to crash. > > > > > > So let's try to work around this while not making too much of a mess. > > > > > > First of all, install a 1:1 mapping instead of avoiding SetVaMap() > > > altogether - from the EFI spec pov, this should amount to the same > > > thing. > > > > > > Then, given that we already use a SMBIOS based hack for Altra to force > > > the use of SetVirtualAddressMap(), let's check for Surface systems in > > > the same way. > > > > > > Please test, and please report the SMBIOS type 1 family field for which > > > this workaround is needed. > > > > > > Also, note that these changes will not make a difference if the > > > EFI_RT_PROPERTIES_TABLE lists SetVirtualAddressMap() as not implemented. > > > > > > Nathan, I would appreciate it if you could give this a spin on your > > > Altra box (only patch #1 should make a difference), and for good > > > measure, double check that hwclock still works as it should. > > > > > > Cc: Johan Hovold <johan+linaro@kernel.org> > > > Cc: Maximilian Luz <luzmaximilian@gmail.com> > > > Cc: Nathan Chancellor <nathan@kernel.org> > > > Cc: Steev Klimaszewski <steev@kali.org> > > > Cc: Shawn Guo <shawn.guo@linaro.org> > > > > > > Ard Biesheuvel (2): > > > arm64: efi: Prefer a flat virtual mapping of the runtime services > > > arm64: efi: Force use of SetVirtualAddressMap() on MS Surface > > > > > > > Bah this does not even work on Yoga C630, so this is not going to help us. > > > > If we want ResetSystem() on these machines, we'll have to retain other > > memory ranges and map the in the EFI runtime map. Yuck. > > > > Nathan - still interested in whether patch #1 works on Altra, > > I applied patch 1 on top of commit 6e50979a9c87 ("Merge tag > 'mm-hotfixes-stable-2023-01-16-15-23' of > git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm") in Linus' tree > and everything still appears to be okay with hwclock. If there is any > more specific testing that I should do, please let me know. Feel free to > add > > Tested-by: Nathan Chancellor <nathan@kernel.org> > > to patch 1 in future revisions, and I am happy to test anything else > that you might need in this series or future ones. > > Mainline: > > # uname -mr > 6.2.0-rc4-00031-g6e50979a9c87 aarch64 > > # hwclock > 2023-01-17 09:04:58.845411-07:00 > > Patch: > > # uname -mr > 6.2.0-rc4-00032-g20165e83052e aarch64 > > # hwclock > 2023-01-17 10:25:38.843788-07:00 > Thanks Nathan, Forgot to mention, though: it is SetTime() not GetTime() that is problematic on this platform. Could you please double check whether setting the RTC using hwclock works too?
On Wed, Jan 18, 2023 at 08:56:54AM +0100, Ard Biesheuvel wrote: > On Tue, 17 Jan 2023 at 18:30, Nathan Chancellor <nathan@kernel.org> wrote: > > > > On Tue, Jan 17, 2023 at 04:20:09PM +0100, Ard Biesheuvel wrote: > > > On Tue, 17 Jan 2023 at 15:27, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > > > Linux on arm64 is now in the same boat as x86, where supporting laptops > > > > that were built to run Windows and never tested beyond what is required > > > > for the Windows Logo certification need workarounds for all kinds of > > > > bizarre behaviors. > > > > > > > > On Snapdragon laptops, we cannot call SetVirtualAddressMap() from the > > > > stub, because the firmware will crash while trying to access memory via > > > > the virtual addresses being installed, which is explicitly unsupported > > > > by the EFI spec. > > > > > > > > However, not calling SetVirtualAddressMap() results in other problems: > > > > on Ampere Altra, it causes SetTime() to crash. On Surface and Flex5g > > > > Windows-on-ARM laptops, it causes ResetSystem() to crash. > > > > > > > > So let's try to work around this while not making too much of a mess. > > > > > > > > First of all, install a 1:1 mapping instead of avoiding SetVaMap() > > > > altogether - from the EFI spec pov, this should amount to the same > > > > thing. > > > > > > > > Then, given that we already use a SMBIOS based hack for Altra to force > > > > the use of SetVirtualAddressMap(), let's check for Surface systems in > > > > the same way. > > > > > > > > Please test, and please report the SMBIOS type 1 family field for which > > > > this workaround is needed. > > > > > > > > Also, note that these changes will not make a difference if the > > > > EFI_RT_PROPERTIES_TABLE lists SetVirtualAddressMap() as not implemented. > > > > > > > > Nathan, I would appreciate it if you could give this a spin on your > > > > Altra box (only patch #1 should make a difference), and for good > > > > measure, double check that hwclock still works as it should. > > > > > > > > Cc: Johan Hovold <johan+linaro@kernel.org> > > > > Cc: Maximilian Luz <luzmaximilian@gmail.com> > > > > Cc: Nathan Chancellor <nathan@kernel.org> > > > > Cc: Steev Klimaszewski <steev@kali.org> > > > > Cc: Shawn Guo <shawn.guo@linaro.org> > > > > > > > > Ard Biesheuvel (2): > > > > arm64: efi: Prefer a flat virtual mapping of the runtime services > > > > arm64: efi: Force use of SetVirtualAddressMap() on MS Surface > > > > > > > > > > Bah this does not even work on Yoga C630, so this is not going to help us. > > > > > > If we want ResetSystem() on these machines, we'll have to retain other > > > memory ranges and map the in the EFI runtime map. Yuck. > > > > > > Nathan - still interested in whether patch #1 works on Altra, > > > > I applied patch 1 on top of commit 6e50979a9c87 ("Merge tag > > 'mm-hotfixes-stable-2023-01-16-15-23' of > > git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm") in Linus' tree > > and everything still appears to be okay with hwclock. If there is any > > more specific testing that I should do, please let me know. Feel free to > > add > > > > Tested-by: Nathan Chancellor <nathan@kernel.org> > > > > to patch 1 in future revisions, and I am happy to test anything else > > that you might need in this series or future ones. > > > > Mainline: > > > > # uname -mr > > 6.2.0-rc4-00031-g6e50979a9c87 aarch64 > > > > # hwclock > > 2023-01-17 09:04:58.845411-07:00 > > > > Patch: > > > > # uname -mr > > 6.2.0-rc4-00032-g20165e83052e aarch64 > > > > # hwclock > > 2023-01-17 10:25:38.843788-07:00 > > > > Thanks Nathan, > > Forgot to mention, though: it is SetTime() not GetTime() that is > problematic on this platform. Could you please double check whether > setting the RTC using hwclock works too? Ah, okay, makes sense! As far as I can tell, that works too. Mainline: # uname -mr 6.2.0-rc4-00031-g6e50979a9c87 aarch64 # hwclock --systohc # echo $status 0 Patch: # uname -mr 6.2.0-rc4-00032-g20165e83052e aarch64 # hwclock --systohc # echo $status 0 Cheers, Nathan
On Wed, 18 Jan 2023 at 16:26, Nathan Chancellor <nathan@kernel.org> wrote: > > On Wed, Jan 18, 2023 at 08:56:54AM +0100, Ard Biesheuvel wrote: > > On Tue, 17 Jan 2023 at 18:30, Nathan Chancellor <nathan@kernel.org> wrote: > > > > > > On Tue, Jan 17, 2023 at 04:20:09PM +0100, Ard Biesheuvel wrote: > > > > On Tue, 17 Jan 2023 at 15:27, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > > > > > Linux on arm64 is now in the same boat as x86, where supporting laptops > > > > > that were built to run Windows and never tested beyond what is required > > > > > for the Windows Logo certification need workarounds for all kinds of > > > > > bizarre behaviors. > > > > > > > > > > On Snapdragon laptops, we cannot call SetVirtualAddressMap() from the > > > > > stub, because the firmware will crash while trying to access memory via > > > > > the virtual addresses being installed, which is explicitly unsupported > > > > > by the EFI spec. > > > > > > > > > > However, not calling SetVirtualAddressMap() results in other problems: > > > > > on Ampere Altra, it causes SetTime() to crash. On Surface and Flex5g > > > > > Windows-on-ARM laptops, it causes ResetSystem() to crash. > > > > > > > > > > So let's try to work around this while not making too much of a mess. > > > > > > > > > > First of all, install a 1:1 mapping instead of avoiding SetVaMap() > > > > > altogether - from the EFI spec pov, this should amount to the same > > > > > thing. > > > > > > > > > > Then, given that we already use a SMBIOS based hack for Altra to force > > > > > the use of SetVirtualAddressMap(), let's check for Surface systems in > > > > > the same way. > > > > > > > > > > Please test, and please report the SMBIOS type 1 family field for which > > > > > this workaround is needed. > > > > > > > > > > Also, note that these changes will not make a difference if the > > > > > EFI_RT_PROPERTIES_TABLE lists SetVirtualAddressMap() as not implemented. > > > > > > > > > > Nathan, I would appreciate it if you could give this a spin on your > > > > > Altra box (only patch #1 should make a difference), and for good > > > > > measure, double check that hwclock still works as it should. > > > > > > > > > > Cc: Johan Hovold <johan+linaro@kernel.org> > > > > > Cc: Maximilian Luz <luzmaximilian@gmail.com> > > > > > Cc: Nathan Chancellor <nathan@kernel.org> > > > > > Cc: Steev Klimaszewski <steev@kali.org> > > > > > Cc: Shawn Guo <shawn.guo@linaro.org> > > > > > > > > > > Ard Biesheuvel (2): > > > > > arm64: efi: Prefer a flat virtual mapping of the runtime services > > > > > arm64: efi: Force use of SetVirtualAddressMap() on MS Surface > > > > > > > > > > > > > Bah this does not even work on Yoga C630, so this is not going to help us. > > > > > > > > If we want ResetSystem() on these machines, we'll have to retain other > > > > memory ranges and map the in the EFI runtime map. Yuck. > > > > > > > > Nathan - still interested in whether patch #1 works on Altra, > > > > > > I applied patch 1 on top of commit 6e50979a9c87 ("Merge tag > > > 'mm-hotfixes-stable-2023-01-16-15-23' of > > > git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm") in Linus' tree > > > and everything still appears to be okay with hwclock. If there is any > > > more specific testing that I should do, please let me know. Feel free to > > > add > > > > > > Tested-by: Nathan Chancellor <nathan@kernel.org> > > > > > > to patch 1 in future revisions, and I am happy to test anything else > > > that you might need in this series or future ones. > > > > > > Mainline: > > > > > > # uname -mr > > > 6.2.0-rc4-00031-g6e50979a9c87 aarch64 > > > > > > # hwclock > > > 2023-01-17 09:04:58.845411-07:00 > > > > > > Patch: > > > > > > # uname -mr > > > 6.2.0-rc4-00032-g20165e83052e aarch64 > > > > > > # hwclock > > > 2023-01-17 10:25:38.843788-07:00 > > > > > > > Thanks Nathan, > > > > Forgot to mention, though: it is SetTime() not GetTime() that is > > problematic on this platform. Could you please double check whether > > setting the RTC using hwclock works too? > > Ah, okay, makes sense! As far as I can tell, that works too. > > Mainline: > > # uname -mr > 6.2.0-rc4-00031-g6e50979a9c87 aarch64 > > # hwclock --systohc > > # echo $status > 0 > > Patch: > > # uname -mr > 6.2.0-rc4-00032-g20165e83052e aarch64 > > # hwclock --systohc > > # echo $status > 0 > Thanks again!
On Wed, Jan 18, 2023 at 04:33:29PM +0100, Ard Biesheuvel wrote: > On Wed, 18 Jan 2023 at 16:26, Nathan Chancellor <nathan@kernel.org> wrote: > > > > On Wed, Jan 18, 2023 at 08:56:54AM +0100, Ard Biesheuvel wrote: > > > On Tue, 17 Jan 2023 at 18:30, Nathan Chancellor <nathan@kernel.org> wrote: > > > > > > > > On Tue, Jan 17, 2023 at 04:20:09PM +0100, Ard Biesheuvel wrote: > > > > > On Tue, 17 Jan 2023 at 15:27, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > > > > > > > Linux on arm64 is now in the same boat as x86, where supporting laptops > > > > > > that were built to run Windows and never tested beyond what is required > > > > > > for the Windows Logo certification need workarounds for all kinds of > > > > > > bizarre behaviors. > > > > > > > > > > > > On Snapdragon laptops, we cannot call SetVirtualAddressMap() from the > > > > > > stub, because the firmware will crash while trying to access memory via > > > > > > the virtual addresses being installed, which is explicitly unsupported > > > > > > by the EFI spec. > > > > > > > > > > > > However, not calling SetVirtualAddressMap() results in other problems: > > > > > > on Ampere Altra, it causes SetTime() to crash. On Surface and Flex5g > > > > > > Windows-on-ARM laptops, it causes ResetSystem() to crash. > > > > > > > > > > > > So let's try to work around this while not making too much of a mess. > > > > > > > > > > > > First of all, install a 1:1 mapping instead of avoiding SetVaMap() > > > > > > altogether - from the EFI spec pov, this should amount to the same > > > > > > thing. > > > > > > > > > > > > Then, given that we already use a SMBIOS based hack for Altra to force > > > > > > the use of SetVirtualAddressMap(), let's check for Surface systems in > > > > > > the same way. > > > > > > > > > > > > Please test, and please report the SMBIOS type 1 family field for which > > > > > > this workaround is needed. > > > > > > > > > > > > Also, note that these changes will not make a difference if the > > > > > > EFI_RT_PROPERTIES_TABLE lists SetVirtualAddressMap() as not implemented. > > > > > > > > > > > > Nathan, I would appreciate it if you could give this a spin on your > > > > > > Altra box (only patch #1 should make a difference), and for good > > > > > > measure, double check that hwclock still works as it should. > > > > > > > > > > > > Cc: Johan Hovold <johan+linaro@kernel.org> > > > > > > Cc: Maximilian Luz <luzmaximilian@gmail.com> > > > > > > Cc: Nathan Chancellor <nathan@kernel.org> > > > > > > Cc: Steev Klimaszewski <steev@kali.org> > > > > > > Cc: Shawn Guo <shawn.guo@linaro.org> > > > > > > > > > > > > Ard Biesheuvel (2): > > > > > > arm64: efi: Prefer a flat virtual mapping of the runtime services > > > > > > arm64: efi: Force use of SetVirtualAddressMap() on MS Surface > > > > > > > > > > > > > > > > Bah this does not even work on Yoga C630, so this is not going to help us. > > > > > > > > > > If we want ResetSystem() on these machines, we'll have to retain other > > > > > memory ranges and map the in the EFI runtime map. Yuck. > > > > > > > > > > Nathan - still interested in whether patch #1 works on Altra, > > > > > > > > I applied patch 1 on top of commit 6e50979a9c87 ("Merge tag > > > > 'mm-hotfixes-stable-2023-01-16-15-23' of > > > > git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm") in Linus' tree > > > > and everything still appears to be okay with hwclock. If there is any > > > > more specific testing that I should do, please let me know. Feel free to > > > > add > > > > > > > > Tested-by: Nathan Chancellor <nathan@kernel.org> > > > > > > > > to patch 1 in future revisions, and I am happy to test anything else > > > > that you might need in this series or future ones. > > > > > > > > Mainline: > > > > > > > > # uname -mr > > > > 6.2.0-rc4-00031-g6e50979a9c87 aarch64 > > > > > > > > # hwclock > > > > 2023-01-17 09:04:58.845411-07:00 > > > > > > > > Patch: > > > > > > > > # uname -mr > > > > 6.2.0-rc4-00032-g20165e83052e aarch64 > > > > > > > > # hwclock > > > > 2023-01-17 10:25:38.843788-07:00 > > > > > > > > > > Thanks Nathan, > > > > > > Forgot to mention, though: it is SetTime() not GetTime() that is > > > problematic on this platform. Could you please double check whether > > > setting the RTC using hwclock works too? The other runtime service that triggers this is SetVariable, which I have been testing like this. The tests still pass on systems with the affected firmware with the SMBIOS string mitigation in place and this flat va mapping applied. #!/bin/sh function set_timeout() { TO=$1 re='^[0-9]$' if ! [[ "$TO" =~ $re ]]; then echo "Timeout must be 0-9" exit 1 fi efibootmgr -t $TO > /dev/null efivar -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-Timeout | grep -q "00000000 0$TO 00" if [ $? -ne 0 ]; then echo "ERROR: timeout $TO not reflected in efivar" exit 1 fi echo "SUCCESS: timeout $TO reflected in efivar" return 0 } set_timeout 4 set_timeout 5 exit 0 Thanks, Darren > > > > Ah, okay, makes sense! As far as I can tell, that works too. > > > > Mainline: > > > > # uname -mr > > 6.2.0-rc4-00031-g6e50979a9c87 aarch64 > > > > # hwclock --systohc > > > > # echo $status > > 0 > > > > Patch: > > > > # uname -mr > > 6.2.0-rc4-00032-g20165e83052e aarch64 > > > > # hwclock --systohc > > > > # echo $status > > 0 > > > > Thanks again!