Re: [edk2-devel] [PATCH] Emulator/X86EmulatorDxe: Replace with MultiArchUefiPkg build

["Andrei Warkentin" <andrei.warkentin@intel.com>]

Hey folks,

Sorry for the late response.

- My suggestion is not to replace, but augment. MultiArchUefiPkg is fairly new, not particularly well adopted, surely with some flaws lurking here and there and for certain with no guarantee of fitness or proof that it will work 100% or 100% better than X86EmulatorPkg for every single use case of X86EmulatorPkg in the wild. X86EmulatorPkg has been around for a lot longer than its rewrite, and I think the people actively using X86EmulatorPkg should continue to have the option to continue using it.

- Unicorn has been a blessing and a curse, although abstracting the emulator with an API seems like a winner. An API-compatible emulator or JIT is something I've been thinking about, but not really doing anything about it for the moment. So yes, actual licensing of binaries is cursed.

- All memory but the zero page is visible to the emulator. See CpuNullReadCb/CpuNullWriteCb. See also TestNullDeref in Application/EmulatorTest. I don't remember if I added the behavior because this is what the original did, or because I actively tripped on the NULL accesses from some x86 code I was testing...

A

> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Sunday, September 1, 2024 3:05 AM
> To: Rebecca Cran <rebecca@bsdio.com>; Warkentin, Andrei
> <andrei.warkentin@intel.com>
> Cc: devel@edk2.groups.io; quic_llindhol@quicinc.com; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [PATCH] Emulator/X86EmulatorDxe: Replace with
> MultiArchUefiPkg build
> 
> Hi Rebecca,
> 
> On Sun, 1 Sept 2024 at 00:33, Rebecca Cran <rebecca@bsdio.com> wrote:
> >
> > Replace the old X86EmulatorDxe with one built from
> > https://github.com/intel/MultiArchUefiPkg. This is a much more modern,
> > recent implementation that's more reliable and is actively maintained.
> >
> > Add driver binaries for both AArch64 and RISCV64, along with the
> > LoadOpRom application.
> >
> > Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> > ---
> >  Emulator/X86EmulatorDxe/AArch64/EmulatorDxe.depex | Bin 0 -> 54
> bytes
> >  Emulator/X86EmulatorDxe/AArch64/EmulatorDxe.efi   | Bin 0 -> 573440
> bytes
> >  Emulator/X86EmulatorDxe/AArch64/LoadOpRom.efi     | Bin 0 -> 28672
> bytes
> >  Emulator/X86EmulatorDxe/README.md                 |  11 +++++++----
> >  Emulator/X86EmulatorDxe/RISCV64/EmulatorDxe.depex | Bin 0 -> 54
> bytes
> >  Emulator/X86EmulatorDxe/RISCV64/EmulatorDxe.efi   | Bin 0 -> 561216
> bytes
> >  Emulator/X86EmulatorDxe/RISCV64/LoadOpRom.efi     | Bin 0 -> 30848
> bytes
> >  Emulator/X86EmulatorDxe/X86EmulatorDxe.depex      | Bin 36 -> 0 bytes
> >  Emulator/X86EmulatorDxe/X86EmulatorDxe.efi        | Bin 913408 -> 0
> bytes
> >  Emulator/X86EmulatorDxe/X86EmulatorDxe.inf        |  10 +++++++---
> >  10 files changed, 14 insertions(+), 7 deletions(-)
> >
> 
> Happy to see that this work has been absorbed into a project that will
> improve and maintain it going forward.
> 
> However, according to the github.com repo Readme of
> MultiArchUefiCpuPkg, these binaries include statically linked UniCorn builds,
> which are a mix of LPGL and GPLv2, so you will need to update the LICENSE
> file accordingly.
> 
> Also, glancing over the code, it seems like it removes the NULL pointer
> dereference handling that the original X86EmulatorPkg has - this code was
> added for a purpose, as some Nvidia option ROMs will happily dereference
> NULL pointers, which happens to work on most older
> X64 firmware because page 0x0 happens to be mapped. Perhaps Andrei can
> comment on this point?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120518): https://edk2.groups.io/g/devel/message/120518
Mute This Topic: https://groups.io/mt/108202804/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-