Re: [edk2-devel] [PATCH 1/1] OvmfPkg/QemuVideoDxe: purge VbeShim

["Gerd Hoffmann" <kraxel@redhat.com>]

On Tue, Feb 13, 2024 at 11:44:17AM +0100, Laszlo Ersek wrote:
> Hi Gerd,
> 
> On 2/13/24 09:59, Gerd Hoffmann wrote:
> > Removing CSM support broke VbeShim.
> > 
> > I see ASSERTS being triggered, both in vbeshim code and elsewhere.
> > Didn't investigate in detail, but apparently the real mode address space
> > is not considered being special any more, which invalidates some
> > assumptions of the vbeshim code.
> > 
> > The guest os which depends on vbeshim for video support is -- according
> > to the comments -- Windows 2008 R2.  Which went EOL in January 2020,
> > more than four years ago.
> > 
> > Time to retire VbeShim.  RIP.
> 
> I don't understand this, and/or disagree.
> 
> (1) Please collect more information about the ASSERT()s (the failure mode you are seeing).
> Because, (a) in the code that's being removed in this patch, I don't see any ASSERT() that should fire, and (b) I cannot reproduce the issue. (In fact my log contains "InstallVbeShim: VBE shim installed".)

Fair enough.  The ASSERT() I see firing is this:

ASSERT /home/kraxel/projects/edk2/OvmfPkg/QemuVideoDxe/VbeShim.c(142): Int0x10->Segment == 0x0000

It reproduces for me when booting with a bunch of hot-pluggable CPUs (qemu -smp 2,maxcpus=32).

I bisected this, and it seems indeed totally unrelated to CSM removal:

   725acd0b9cc047b20a1b9379e46f05d3ebcf485e is the first bad commit
   commit 725acd0b9cc047b20a1b9379e46f05d3ebcf485e
   Author: Dun Tan <dun.tan@intel.com>
   Date:   Fri Dec 1 10:53:32 2023 +0800

       UefiCpuPkg: Avoid assuming only one smmbasehob

New theory: This is a NULL pointer dereference corrupting the zero page.

I still think it makes sense to retire VbeShim, but of course with a
different commit message and also the leftover function declaration
removed.

Can we unmap the zero page to avoid NULL pointer dereferences going
unnoticed (after removing VbeShim)?  IIRC I've seen an config option
for that a while back, but can't find it now in the source tree ...

take care,
  Gerd



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