Re: [PATCH v2 0/6] Ovmf: Drop IntelFramework[Module]Pkg dependency

["David Woodhouse" <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 6780 bytes --]

On Wed, 2019-06-12 at 13:52 +0200, Laszlo Ersek wrote:
> (2c) SeaBIOS is built *twice*, once for the CSM, and another time for
> the Cirrus VGABIOS.

We shouldn't need to rebuild the VGA BIOS, should we? Any Legacy VGA
BIOS should work — and indeed UEFI should be able to use the Legacy VGA
BIOS even if it doesn't have native support for the video hardware.

ISTR Windows 2008 even called legacy INT10 as part of the boot process
and was part of my testing at the time?

> (3b) The following out-of-tree patch is applied on top of (3a):
> 
> > diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
> > index 211750c0123b..119b5c9b6fe8 100644
> > --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
> > +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
> > @@ -1231,8 +1231,8 @@ GenericLegacyBoot (
> >    //
> >    Private->LegacyRegion->Lock (
> >                             Private->LegacyRegion,
> > -                           0xc0000,
> > -                           0x40000,
> > +                           0xe0000,
> > +                           0x10000,
> >                             &Granularity
> >                             );
> > 

I didn't need this part.


> With these, grub is booted successfully from the RHEL5 installer ISO
> (which does not support UEFI at all). Unfortunately, after grub loads
> vmlinuz + initrd, it fails with "Not enough memory to load specified
> image", and returns to the grub prompt. (If I double the argument to
> QEMU's "-m" option, to 2048, that makes no difference.)
> 
> This is at least a graceful failure.

It's because the E820 table isn't being passed through to the CSM.


Start of legacy boot
E820[ 0]: 0x0000000000000000 - 0x000000000009F000, Type = 1
E820[ 1]: 0x000000000009F000 - 0x00000000000A0000, Type = 2
E820[ 2]: 0x00000000000E0000 - 0x0000000000100000, Type = 2
E820[ 3]: 0x0000000000100000 - 0x0000000000800000, Type = 1
E820[ 4]: 0x0000000000800000 - 0x0000000000808000, Type = 4
E820[ 5]: 0x0000000000808000 - 0x0000000000810000, Type = 1
E820[ 6]: 0x0000000000810000 - 0x0000000000900000, Type = 4
E820[ 7]: 0x0000000000900000 - 0x000000007E874000, Type = 1
E820[ 8]: 0x000000007E874000 - 0x000000007E88C000, Type = 4
E820[ 9]: 0x000000007E88C000 - 0x000000007E8B0000, Type = 2
E820[10]: 0x000000007E8B0000 - 0x000000007FBEF000, Type = 1
E820[11]: 0x000000007FBEF000 - 0x000000007FBF3000, Type = 2
E820[12]: 0x000000007FBF3000 - 0x000000007FBFB000, Type = 3
E820[13]: 0x000000007FBFB000 - 0x000000007FBFF000, Type = 4
E820[14]: 0x000000007FBFF000 - 0x000000007FF78000, Type = 1
E820[15]: 0x000000007FF78000 - 0x0000000080000000, Type = 4
E820[16]: 0x0000000080000000 - 0x00000000FC000000, Type = 2
E820[17]: 0x00000000FEC00000 - 0x00000000FEC01000, Type = 2
E820[18]: 0x00000000FED00000 - 0x00000000FED00400, Type = 2
E820[19]: 0x00000000FEE00000 - 0x00000000FEF00000, Type = 2
handle_hwpic1 irq=1
handle_csm regs 0x0005ffd4 AX=0006
Legacy16GetTableAddress size 190 align 0 region 3
ebda moved from 9f000 to fffffc00
Legacy16GetTableAddress size 190 align 0 region 3 yields 0x00000000
handle_csm returning AX=0001
enter handle_15:
   a=00002401  b=00000000  c=00000190  d=00000003 ds=0000 es=0000 ss=5000
  si=00000000 di=00000000 bp=00000000 sp=0000ffc6 cs=5f00 ip=0030  f=3002
Legacy16 E820 length insufficient
...
handle_csm regs 0x0005ffd4 AX=0002
PrepareToBoot table 580a:0008
Add to e820 map: f000ff53f000ff53 f000ff53f000e2c3 -268370093
Add to e820 map: f000ff53f000ff54 f000ce45f000ff53 -268382651
Add to e820 map: f000ce45f000ce45 f000ce45f000ce45 -268382651
Add to e820 map: c0005212f000ce45 f000f841f000f84d -268377090
Add to e820 map: f000f859f000e739 f000efd2f000e82e -268382614
Add to e820 map: f000fe6ef000e6f2 f000ff53f000ff53 -268370093
Add to e820 map: c0009200f000ff53 f000ff53f000ff53 -268370093
Add to e820 map: f000ff53f000ff53 f000ff53f000ff53 -268370093
Add to e820 map: f000ff53f000ff53 f000ff53f000ff53 -268370093
Add to e820 map: f000ff53f000ff53 f000ff53f000ff53 -268370093
Add to e820 map: f000ff53f000ff53 f000ff53f000ff53 -268370093
Add to e820 map: f000ff53f000ff53 f000ff53f000ff53 -268370093
Add to e820 map: f000ff53f000ff53 f000ff53f000ff53 -268374951
Add to e820 map: f000ff53f000ff53 f000ff53c0007000 -268370093
Add to e820 map: f000ff53f000ff53 f000ff53f000ff53 -268370093
Add to e820 map: f000ff53f000ff53 f000ff53f000ff53 -268370093
Add to e820 map: f000ff53f000ff53 f000ff53f000ff53 -268370093
Add to e820 map: f000ff53f000ff53 f000ff53f000ff53 -268370093
Add to e820 map: f000ff53f000ff53 f000ff53f000ff53 -268370093
Add to e820 map: f000ff53 00000000 0
Add to e820 map: 007c0000 00040000 2
CSM PIRQ table at 0x000f6340
CSM ACPI RSDP at 0x000f6440


Completely bogus E820 information tends to lead to confusing out-of-
memory errors. Film at 11 :)

This fixes it:

diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
index 211750c012..3a84fa0258 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
@@ -929,6 +929,7 @@ GenericLegacyBoot (
     ZeroMem (&Regs, sizeof (EFI_IA32_REGISTER_SET));
     Regs.X.AX = Legacy16GetTableAddress;
     Regs.X.CX = (UINT16) CopySize;
+    Regs.X.DX = (UINT16) 1;
     Private->LegacyBios.FarCall86 (
       &Private->LegacyBios,
       Private->Legacy16Table->Compatibility16CallSegment,


I'm going to dig out the specification and see what an align field of 0
is actually intended to mean, and whether I should really be fixing it
in SeaBIOS instead. This would also fix it:

diff --git a/src/fw/csm.c b/src/fw/csm.c
index 03b4bb8..5288096 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -265,9 +265,9 @@ handle_csm_0006(struct bregs *regs)
         size, align, region);
 
     if (region & 2)
-        chunk = _malloc(&ZoneLow, size, align);
+        chunk = _malloc(&ZoneLow, size, align ? align : 1);
     if (!chunk && (region & 1))
-        chunk = _malloc(&ZoneFSeg, size, align);
+        chunk = _malloc(&ZoneFSeg, size, align ? align : 1);
 
     dprintf(3, "Legacy16GetTableAddress size %x align %x region %d yields %p\n",
         size, align, region, chunk);

I have various things booting fine with CSM again now. Thanks for
helping me work through this.

Aside from the actual allocation issue, perhaps the UEFI side should
bail out properly when it can't pass E820 information to the CSM. And
perhaps SeaBIOS should abort more cleanly if it's passed nonsense.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]