Re: can't boot OVMF guest after commit 45cfcd8

[Laszlo Ersek <lersek@redhat.com>]

On 06/26/17 21:52, Laszlo Ersek wrote:
> Hi Gabriel,
>
> On 06/26/17 19:13, Gabriel L. Somlo wrote:
>> Hi,
>>
>> Following commit 45cfcd8dccf84b8abbc1d6f587fedb5d2037ec79, a.k.a.
>> "MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol",
>> I'm no longer able to boot a VM guest with OVMF. Using command line:
>>
>> bin/qemu-system-x86_64 -machine q35,accel=kvm -m 2048 -bios OVMF.fd \
>>   -device ide-drive,bus=ide.2,drive=CD \
>>   -drive id=CD,if=none,snapshot=on,file=Fedora-Workstation-Live-x86_64-25-1.3.iso
>>
>> everything hangs at the Tianocore splash screen indefinitely.
>>
>> Reverting the commit locally gets things to work fine once again.
>>
>> Sorry for the noise if this report is a dupe, otherwise please let me
>> know if/what other information and test results I can provide to
>> help.
>
> Thank you for the report and for identifying the commit -- I guess I
> should have stumbled upon this myself, but other duties have kept me
> from building OVMF for a while now. :/
>
> Can you please add the following options to your QEMU command line,
> for capturing the OVMF debug log? Maybe we'll know better where the
> problem occurs this way.
>
>   -debugcon file:debug.log -global isa-debugcon.iobase=0x402
>
> In addition,
>
>   -serial stdio
>
> might help you catch a register dump (written to serial) from the
> default CPU exception handler in UefiCpuPkg, if an unhandled page
> fault occurs, for example.

Yes, it is a crash:

> !!!! X64 Exception Type - 0D(#GP - General Protection)  CPU Apic ID - 00000000 !!!!
> ExceptionData - 0000000000000000
> RIP  - 000000007F4CFB72, CS  - 0000000000000038, RFLAGS - 0000000000010282
> RAX  - 8000000000000010, RCX - 8000000000000010, RDX - 0000000000000000
> RBX  - 0000000000000013, RSP - 000000007F6BB670, RBP - 000000007F6BB690
> RSI  - 0000000000000000, RDI - 000000007E303D98
> R8   - 8000000000000010, R9  - 0000000000000000, R10 - 0000000FFFFFFFFF
> R11  - 0000000000000018, R12 - 0000000000000000, R13 - 0000000000000000
> R14  - 0000000000000000, R15 - 0000000000000000
> DS   - 0000000000000030, ES  - 0000000000000030, FS  - 0000000000000030
> GS   - 0000000000000030, SS  - 0000000000000030
> CR0  - 0000000080010033, CR2 - 0000000000000000, CR3 - 000000007F65A000
> CR4  - 0000000000000668, CR8 - 0000000000000000
> DR0  - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
> DR3  - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
> GDTR - 000000007F5ADA98 0000000000000047, LDTR - 0000000000000000
> IDTR - 000000007E897018 0000000000000FFF,   TR - 0000000000000000
> FXSAVE_STATE - 000000007F6BB2D0
> !!!! Find image
> /.../Build/Ovmf3264/NOOPT_GCC48/X64/MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe/DEBUG/DevicePathDxe.dll
> (ImageBase=000000007F4CF000, EntryPoint=000000007F4CF27B) !!!!

It happens like this:

The IsaBusControllerDriverStart() function in
"IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBus.c", opens two protocol
interfaces BY_DRIVER, a Device Path and an ISA ACPI one. Either of these
can fail, and the returns with an error in such cases, *except* when the
protocol opens BY_DRIVER fail with EFI_ALREADY_STARTED. (This means that
the driver has bound the controller already.)

Before commit 45cfcd8dccf8, the protocol interface addresses would be
returned on EFI_ALREADY_STARTED, and then IsaBusControllerDriverStart()
would proceed to:

- call:

  REPORT_STATUS_CODE_WITH_DEVICE_PATH (
    EFI_PROGRESS_CODE,
    (EFI_IO_BUS_LPC | EFI_IOB_PC_INIT),
    ParentDevicePath
    );

  which is what crashes with commit 45cfcd8dccf8 in place, since
  ParentDevicePath keeps its initial garbage (non-NULL) contents from
  the stack, on EFI_ALREADY_STARTED,

- call:

  IsaAcpi->InterfaceInit (IsaAcpi);

- do a bunch of similar stuff, regardless of whether the same driver has
  bound the same controller previously. It even has a comment like

    //
    // Create handle for this ISA device
    //
    // If any child device handle was created in previous call to Start() and not stopped
    // in previous call to Stop(), it will not be created again because the
    // InstallMultipleProtocolInterfaces() boot service will reject same device path.
    //

So I guess what the driver does is intentional, but the way it requires
gBS->OpenProtocol() to return a valid Interface even on error seems to
conflict with the UEFI spec. And now commit 45cfcd8dccf8 makes that show.

I'll follow up under

[edk2] [PATCH V4] MdeModulePkg/DxeCore: Fixed Interface returned by
CoreOpenProtocol

and CC you.

Thanks
Laszlo