On Sun, Jul 12, 2020 at 03:06:20PM +0200, felix wrote:
> Hello, here’s another defect I’ve found, this time in SeaVGABIOS.
>
> Video BIOS interrupt 0x10 subfunction 0 accepts a flag that determines
> whether video memory should be preserved after mode change. SeaVGABIOS
> ignores this flag. The reason is a buggy implementation of the interrupt
> call in vgasrc/vgabios.c:
>
> > static void
> > handle_1000(struct bregs *regs)
> > {
> > int mode = regs->al & 0x7f;
> >
> > // Set regs->al
> > if (mode > 7)
> > regs->al = 0x20;
> > else if (mode == 6)
> > regs->al = 0x3f;
> > else
> > regs->al = 0x30;
> >
> > int flags = MF_LEGACY | (GET_BDA(modeset_ctl) &
> > (MF_NOPALETTE|MF_GRAYSUM));
> > if (regs->al & 0x80)
> > flags |= MF_NOCLEARMEM;
> >
> > vga_set_mode(mode, flags);
> > }
>
> The AL register’s structure member is overwritten by the chained if before
> the flag is checked, which means the flag is never picked up. This is a
> regression from commit 5108c69c47e18244206593c0c7918711311d8ef3, which
> removed the noclearmem variable that stashed the flag for later.[0]
Thanks. I agree that is a bug. I've sent a separate patch to the
mailing list.
> Sloppy refactoring aside, there was no explanation for this condition chain,
> so I became puzzled as to why the register was being overwritten in the
> first place. It seems this was meant to mimic a behaviour described in Ralf
> Brown’s:
>
> > Return: AL = video mode flag (Phoenix, AMI BIOS)
> > 20h mode > 7
> > 30h modes 0-5 and 7
> > 3Fh mode 6
> > AL = CRT controller mode byte (Phoenix 386 BIOS v1.10)
>
> I have checked this with an Intel video BIOS on a physical machine, and it
> left the register untouched, so this behaviour seems hardly universal. Not
> even DOSBox emulates this, and I think they do have some incentives to
> emulate these calls as faithfully (to old hardware) as possible, so that is
> saying something. (Not to mention RBIL itself describes two conflicting
> behaviours here.) So I doubt that this condition chain needs to be kept in
> the first place. But even if it is kept, I think it should be moved a little
> later. And probably commented too.
This was inherited from the original "GPL vga bios" code. I'd prefer
to leave the al manipulation as it currently is to reduce the risk of
introducing a regression.
-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
