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