On Mon, Jun 12, 2023 at 10:52:00AM +0100, Nikos Nikoleris wrote: > On 12/06/2023 08:52, Andrew Jones wrote: > > On Sat, Jun 10, 2023 at 01:32:59AM -0700, Nadav Amit wrote: > > > > > > > On May 30, 2023, at 9:08 AM, Nikos Nikoleris <nikos.nikoleris@xxxxxxx> wrote: > > > > > > > > Hello, > > > > > > > > This series adds initial support for building arm64 tests as EFI > > > > apps and running them under QEMU. Much like x86_64, we import external > > > > dependencies from gnu-efi and adapt them to work with types and other > > > > assumptions from kvm-unit-tests. In addition, this series adds support > > > > for enumerating parts of the system using ACPI. > > > > > > Just an issue I encountered, which I am not sure is arm64 specific: > > > > > > All the printf’s in efi_main() are before current_thread_info() is > > > initialized (or even memset’d to zero, as done in setup_efi). > > > > > > But printf() calls puts() which checks if mmu_enabled(). And > > > mmu_enabled() uses is_user() and current_thread_info()->cpu, both > > > of which read uninitialized data from current_thread_info(). > > > > > > IOW: Any printf in efi_main() can cause a crash. > > > > > > > Nice catch, Nadav. Nikos, shouldn't we drop the memset() in setup_efi and > > put a zero_range call, similar to the one in arm/cstart64.S which zero's > > the thread-info area, in arm/efi/crt0-efi-aarch64.S? > > > > While I haven't run into any problems with this in this series, We're fine on QEMU, since QEMU zeros the memory. > I had in a > previous version and back then the solution was this patch: > > 993c37be - arm/arm64: Zero BSS and stack at startup > > So I agree we should drop the memset and call some macro like zero_range in > arm/efi/crt0-efi-aarch64.S. > > Let me know if you want me to send a patch for this. Yes, please, but we also don't have to hold this series up by it, since we agreed that this series was focused on EFI over QEMU as a first step. We'll need to worry about zeroing memory and more when we start running on bare-metal. To be clear, the patch for this can be on top of this series. Thanks, drew