| Message ID | 20261003-aspeed_vbios_fixes-v2-5-f8e593f024ab@oss.tenstorrent.com |
|---|---|
| State | New |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v2,1/5] bios_emulator: select X86EMU_RAW_IO only on x86 | expand |
Hi Anirudh, On 2026-10-03T20:07:54, Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> wrote: > bios_emulator: extend VGA I/O ports to include ports 0x3B4-0x3DA > > The IS_VGA_PORT macro currently doesn't cover the 0x3B4, 0x3B5 and 0x3BA > I/O ports. Reads/writes to this currently fall through to LOG_inpb/outb > which are unimplemented on non x86 architectures (which don't support > I/O ports). > > Add these ports to the IS_VGA_PORT macro, and implement reads/writes to > these ports. 0x3B4 and 0x3B5 are mirrors of 0x3D4 and 0x3D5 that are > used in monochrome mode (bit 0 of 0x3C2 is not set). Implement these > with the same logic and env state as 0x3D4/0x3D5, but with the bit check > condition for 0x3C2 flipped. 0x3BA is a mirror of 0x3DA, so the > implementation for this falls back to the same logic as 0x3DA. > > The extended range also covers the non-VGA ports 0x3B6-0x3B9 and > 0x3BB-0x3BF (LPT1 on legacy PCs): reads return 0xff and writes are > dropped instead of reaching the real inb/outb path, which is harmless > from the VBIOS's perspective. > > The write side handles 0x3B4/0x3B5 but not 0x3BA, consistent with the > existing lack of a 0x3DA write handler. > > The Aspeed AST2600's VBIOS uses these I/O ports, and these changes are > needed for it's VBIOS to run. Please use 'its' rather than 'it's' here. > [...] > > drivers/bios_emulator/besys.c | 32 ++++++++++++++++++++++++++++++-- > 1 file changed, 30 insertions(+), 2 deletions(-) > diff --git a/drivers/bios_emulator/besys.c b/drivers/bios_emulator/besys.c > @@ -270,6 +270,18 @@ static u8 VGA_inpb (const int port) > + case 0x3B4: > + if (!(_BE_env.emu3C2 & 0x1)) > + return _BE_env.emu3D4; > + break; > + case 0x3B5: > + if (!(_BE_env.emu3C2 & 0x1) && (_BE_env.emu3D4 < CRT_C)) > + return _BE_env.emu3D5[_BE_env.emu3D4]; > + break; This repeats the 0x3D4/0x3D5 cases with the condition inverted, and VGA_outpb() does the same again. You could add a small helper that checks whether the CRTC port matches the current mode, for example: static bool vga_crtc_active(int port) { return (port >= 0x3d0) == !!(_BE_env.emu3C2 & 1); } Then 0x3B4 can share a case with 0x3D4, and 0x3B5 with 0x3D5, in both functions, removing the duplication. What do you think? > diff --git a/drivers/bios_emulator/besys.c b/drivers/bios_emulator/besys.c > @@ -321,6 +333,10 @@ static u8 VGA_inpb (const int port) > + /* > + * 3BA is a mirror of 3DA > + */ > + case 0x3BA: > case 0x3DA: Just to check: on real hardware 0x3BA only decodes in monochrome mode, so a read in colour mode should not reset flipFlop3C0 or toggle emu3DA. The existing 0x3DA case doesn't check the mode either, so this may not matter for the AST2600 VBIOS, but it would be more accurate to use the same mode check as 0x3B4/0x3B5. Also, a single-line comment is enough here. Regards, Simon
Hi Simon, On Tue, Oct 13, 2026 at 5:25 AM Simon Glass <sjg@chromium.org> wrote: > > Hi Anirudh, > > On 2026-10-03T20:07:54, Anirudh Srinivasan > <asrinivasan@oss.tenstorrent.com> wrote: > > bios_emulator: extend VGA I/O ports to include ports 0x3B4-0x3DA > > > > The IS_VGA_PORT macro currently doesn't cover the 0x3B4, 0x3B5 and 0x3BA > > I/O ports. Reads/writes to this currently fall through to LOG_inpb/outb > > which are unimplemented on non x86 architectures (which don't support > > I/O ports). > > > > Add these ports to the IS_VGA_PORT macro, and implement reads/writes to > > these ports. 0x3B4 and 0x3B5 are mirrors of 0x3D4 and 0x3D5 that are > > used in monochrome mode (bit 0 of 0x3C2 is not set). Implement these > > with the same logic and env state as 0x3D4/0x3D5, but with the bit check > > condition for 0x3C2 flipped. 0x3BA is a mirror of 0x3DA, so the > > implementation for this falls back to the same logic as 0x3DA. > > > > The extended range also covers the non-VGA ports 0x3B6-0x3B9 and > > 0x3BB-0x3BF (LPT1 on legacy PCs): reads return 0xff and writes are > > dropped instead of reaching the real inb/outb path, which is harmless > > from the VBIOS's perspective. > > > > The write side handles 0x3B4/0x3B5 but not 0x3BA, consistent with the > > existing lack of a 0x3DA write handler. > > > > The Aspeed AST2600's VBIOS uses these I/O ports, and these changes are > > needed for it's VBIOS to run. > > Please use 'its' rather than 'it's' here. > > > [...] > > > > drivers/bios_emulator/besys.c | 32 ++++++++++++++++++++++++++++++-- > > 1 file changed, 30 insertions(+), 2 deletions(-) > > > diff --git a/drivers/bios_emulator/besys.c b/drivers/bios_emulator/besys.c > > @@ -270,6 +270,18 @@ static u8 VGA_inpb (const int port) > > + case 0x3B4: > > + if (!(_BE_env.emu3C2 & 0x1)) > > + return _BE_env.emu3D4; > > + break; > > + case 0x3B5: > > + if (!(_BE_env.emu3C2 & 0x1) && (_BE_env.emu3D4 < CRT_C)) > > + return _BE_env.emu3D5[_BE_env.emu3D4]; > > + break; > > This repeats the 0x3D4/0x3D5 cases with the condition inverted, and > VGA_outpb() does the same again. You could add a small helper that > checks whether the CRTC port matches the current mode, for example: > > static bool vga_crtc_active(int port) > { > return (port >= 0x3d0) == !!(_BE_env.emu3C2 & 1); > } > > Then 0x3B4 can share a case with 0x3D4, and 0x3B5 with 0x3D5, in both > functions, removing the duplication. What do you think? > > > diff --git a/drivers/bios_emulator/besys.c b/drivers/bios_emulator/besys.c > > @@ -321,6 +333,10 @@ static u8 VGA_inpb (const int port) > > + /* > > + * 3BA is a mirror of 3DA > > + */ > > + case 0x3BA: > > case 0x3DA: > > Just to check: on real hardware 0x3BA only decodes in monochrome mode, > so a read in colour mode should not reset flipFlop3C0 or toggle > emu3DA. The existing 0x3DA case doesn't check the mode either, so this > may not matter for the AST2600 VBIOS, but it would be more accurate to > use the same mode check as 0x3B4/0x3B5. Also, a single-line comment is > enough here. Let me propose a different solution here: What if I just drop this and the previous commit (support 16-bit reads on VGA port)? If I implement custom ops in the Aspeed VGA driver for all BE_(in|out)(b|w|l) calls, then I don't need to touch this function at all. All the port/range checking needed can be handled in these ops that biosemu will call. The existing biosemu code will just look for the custom ops in the driver and fallback to the existing generic version. > > Regards, > Simon
diff --git a/drivers/bios_emulator/besys.c b/drivers/bios_emulator/besys.c index 76a05e74f55..c06ed34c0af 100644 --- a/drivers/bios_emulator/besys.c +++ b/drivers/bios_emulator/besys.c @@ -248,8 +248,8 @@ void X86API BE_wrl(u32 addr, u32 val) #define IS_TIMER_PORT(port) (0x40 <= port && port <= 0x43) #define IS_CMOS_PORT(port) (0x70 <= port && port <= 0x71) -/*#define IS_VGA_PORT(port) (_BE_env.emulateVGA && 0x3C0 <= port && port <= 0x3DA)*/ -#define IS_VGA_PORT(port) (0x3C0 <= port && port <= 0x3DA) +/*#define IS_VGA_PORT(port) (_BE_env.emulateVGA && 0x3B4 <= port && port <= 0x3DA)*/ +#define IS_VGA_PORT(port) (0x3B4 <= port && port <= 0x3DA) #define IS_PCI_PORT(port) (0xCF8 <= port && port <= 0xCFF) #define IS_SPKR_PORT(port) (port == 0x61) @@ -270,6 +270,18 @@ static u8 VGA_inpb (const int port) debug_io("vga_inb.%04X -> ", (u16) port); switch (port) { + /* + * 3B4 and 3B5 are mirrors of 3D4 and 3D5 that are used in + * monochrome mode (i.e., bit 0 of 3C2 is not set) + */ + case 0x3B4: + if (!(_BE_env.emu3C2 & 0x1)) + return _BE_env.emu3D4; + break; + case 0x3B5: + if (!(_BE_env.emu3C2 & 0x1) && (_BE_env.emu3D4 < CRT_C)) + return _BE_env.emu3D5[_BE_env.emu3D4]; + break; case 0x3C0: /* 3C0 has funky characteristics because it can act as either a data register or index register depending on the state @@ -321,6 +333,10 @@ static u8 VGA_inpb (const int port) if ((_BE_env.emu3C2 & 0x1) && (_BE_env.emu3D4 < CRT_C)) return _BE_env.emu3D5[_BE_env.emu3D4]; break; + /* + * 3BA is a mirror of 3DA + */ + case 0x3BA: case 0x3DA: _BE_env.flipFlop3C0 = 0; val = _BE_env.emu3DA; @@ -343,6 +359,18 @@ to use. static void VGA_outpb (int port, u8 val) { switch (port) { + /* + * 3B4 and 3B5 are mirrors of 3D4 and 3D5 that are used in + * monochrome mode (i.e., bit 0 of 3C2 is not set) + */ + case 0x3B4: + if (!(_BE_env.emu3C2 & 0x1)) + _BE_env.emu3D4 = val; + break; + case 0x3B5: + if (!(_BE_env.emu3C2 & 0x1) && (_BE_env.emu3D4 < CRT_C)) + _BE_env.emu3D5[_BE_env.emu3D4] = val; + break; case 0x3C0: /* 3C0 has funky characteristics because it can act as either a data register or index register depending on the state
The IS_VGA_PORT macro currently doesn't cover the 0x3B4, 0x3B5 and 0x3BA I/O ports. Reads/writes to this currently fall through to LOG_inpb/outb which are unimplemented on non x86 architectures (which don't support I/O ports). Add these ports to the IS_VGA_PORT macro, and implement reads/writes to these ports. 0x3B4 and 0x3B5 are mirrors of 0x3D4 and 0x3D5 that are used in monochrome mode (bit 0 of 0x3C2 is not set). Implement these with the same logic and env state as 0x3D4/0x3D5, but with the bit check condition for 0x3C2 flipped. 0x3BA is a mirror of 0x3DA, so the implementation for this falls back to the same logic as 0x3DA. The extended range also covers the non-VGA ports 0x3B6-0x3B9 and 0x3BB-0x3BF (LPT1 on legacy PCs): reads return 0xff and writes are dropped instead of reaching the real inb/outb path, which is harmless from the VBIOS's perspective. The write side handles 0x3B4/0x3B5 but not 0x3BA, consistent with the existing lack of a 0x3DA write handler. The Aspeed AST2600's VBIOS uses these I/O ports, and these changes are needed for it's VBIOS to run. Signed-off-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> --- drivers/bios_emulator/besys.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)