QEMU does not store the A20 setting in the SMM cpu environment area (and it does not look like real CPUs do either). So, manually backup and restore A20 on a mode switch.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/fw/smm.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/fw/smm.c b/src/fw/smm.c index 95f6ba7..178959c 100644 --- a/src/fw/smm.c +++ b/src/fw/smm.c @@ -52,7 +52,8 @@ struct smm_state { struct smm_layout { struct smm_state backup1; struct smm_state backup2; - u8 stack[0x7c00]; + u32 backup_a20; + u8 stack[0x8000 - sizeof(struct smm_state)*2 - sizeof(u32)]; u64 codeentry; u8 pad_8008[0x7df8]; struct smm_state cpu; @@ -102,10 +103,13 @@ handle_smi(u16 cs) memcpy(&smm->cpu, &smm->backup1, sizeof(smm->cpu)); memcpy(&smm->cpu.i32.eax, regs, sizeof(regs)); smm->cpu.i32.eip = regs[3]; + // Enable a20 and backup its previous state + smm->backup_a20 = set_a20(1); } else if (smm->cpu.i32.ecx == CALL32SMM_RETURNID) { dprintf(9, "smm cpu ret %x esp=%x\n", regs[3], regs[4]); memcpy(&smm->cpu, &smm->backup2, sizeof(smm->cpu)); memcpy(&smm->cpu.i32.eax, regs, sizeof(regs)); + set_a20(smm->backup_a20); smm->cpu.i32.eip = regs[3]; } } else if (rev == SMM_REV_I64) { @@ -116,9 +120,12 @@ handle_smi(u16 cs) memcpy(&smm->cpu, &smm->backup1, sizeof(smm->cpu)); memcpy(&smm->cpu.i64.rdi, regs, sizeof(regs)); smm->cpu.i64.rip = (u32)regs[4]; + // Enable a20 and backup its previous state + smm->backup_a20 = set_a20(1); } else if ((u32)smm->cpu.i64.rcx == CALL32SMM_RETURNID) { memcpy(&smm->cpu, &smm->backup2, sizeof(smm->cpu)); memcpy(&smm->cpu.i64.rdi, regs, sizeof(regs)); + set_a20(smm->backup_a20); smm->cpu.i64.rip = (u32)regs[4]; } }
Initialize the Call16Data at startup - otherwise some early yield() calls may check for interrupts without using the preferred A20 setting.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/stacks.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/stacks.c b/src/stacks.c index ef6a707..9fec2fb 100644 --- a/src/stacks.c +++ b/src/stacks.c @@ -496,6 +496,7 @@ void thread_setup(void) { CanInterrupt = 1; + call16_override(1); if (! CONFIG_THREADS) return; ThreadControl = romfile_loadint("etc/threads", 1);
The A20 setting is almost always enabled - only issue an outb() if the A20 is actually changing. This reduces the number of outb() calls.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/fw/smm.c | 6 ++++-- src/stacks.c | 4 +++- src/x86.h | 7 ++++--- 3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/fw/smm.c b/src/fw/smm.c index 178959c..d90e43a 100644 --- a/src/fw/smm.c +++ b/src/fw/smm.c @@ -109,7 +109,8 @@ handle_smi(u16 cs) dprintf(9, "smm cpu ret %x esp=%x\n", regs[3], regs[4]); memcpy(&smm->cpu, &smm->backup2, sizeof(smm->cpu)); memcpy(&smm->cpu.i32.eax, regs, sizeof(regs)); - set_a20(smm->backup_a20); + if (!smm->backup_a20) + set_a20(0); smm->cpu.i32.eip = regs[3]; } } else if (rev == SMM_REV_I64) { @@ -125,7 +126,8 @@ handle_smi(u16 cs) } else if ((u32)smm->cpu.i64.rcx == CALL32SMM_RETURNID) { memcpy(&smm->cpu, &smm->backup2, sizeof(smm->cpu)); memcpy(&smm->cpu.i64.rdi, regs, sizeof(regs)); - set_a20(smm->backup_a20); + if (!smm->backup_a20) + set_a20(0); smm->cpu.i64.rip = (u32)regs[4]; } } diff --git a/src/stacks.c b/src/stacks.c index 9fec2fb..f4d15ce 100644 --- a/src/stacks.c +++ b/src/stacks.c @@ -84,7 +84,9 @@ call32_post(void)
if (!CONFIG_CALL32_SMM || method != C16_SMM) { // Restore a20 - set_a20(GET_LOW(Call16Data.a20)); + u8 a20 = GET_LOW(Call16Data.a20); + if (!a20) + set_a20(0);
// Restore gdt and fs/gs struct descloc_s gdt; diff --git a/src/x86.h b/src/x86.h index a770e6f..4aea65c 100644 --- a/src/x86.h +++ b/src/x86.h @@ -258,9 +258,10 @@ static inline u8 get_a20(void) { }
static inline u8 set_a20(u8 cond) { - u8 val = inb(PORT_A20); - outb((val & ~A20_ENABLE_BIT) | (cond ? A20_ENABLE_BIT : 0), PORT_A20); - return (val & A20_ENABLE_BIT) != 0; + u8 val = inb(PORT_A20), a20_enabled = (val & A20_ENABLE_BIT) != 0; + if (a20_enabled != !!cond) + outb(val ^ A20_ENABLE_BIT, PORT_A20); + return a20_enabled; }
// x86.c
Don't write to the cmos index port on a mode switch if NMI is already disabled. This reduces the number of outb() calls.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/stacks.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/stacks.c b/src/stacks.c index f4d15ce..2fe1bfb 100644 --- a/src/stacks.c +++ b/src/stacks.c @@ -66,8 +66,10 @@ call32_prep(u8 method)
// Backup cmos index register and disable nmi u8 cmosindex = inb(PORT_CMOS_INDEX); - outb(cmosindex | NMI_DISABLE_BIT, PORT_CMOS_INDEX); - inb(PORT_CMOS_DATA); + if (!(cmosindex & NMI_DISABLE_BIT)) { + outb(cmosindex | NMI_DISABLE_BIT, PORT_CMOS_INDEX); + inb(PORT_CMOS_DATA); + } SET_LOW(Call16Data.cmosindex, cmosindex);
SET_LOW(Call16Data.method, method); @@ -103,8 +105,11 @@ call32_post(void) }
// Restore cmos index register - outb(GET_LOW(Call16Data.cmosindex), PORT_CMOS_INDEX); - inb(PORT_CMOS_DATA); + u8 cmosindex = GET_LOW(Call16Data.cmosindex); + if (!(cmosindex & NMI_DISABLE_BIT)) { + outb(cmosindex, PORT_CMOS_INDEX); + inb(PORT_CMOS_DATA); + } return method; }
On 16/05/2017 18:14, Kevin O'Connor wrote:
Don't write to the cmos index port on a mode switch if NMI is already disabled. This reduces the number of outb() calls.
Signed-off-by: Kevin O'Connor kevin@koconnor.net
src/stacks.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/stacks.c b/src/stacks.c index f4d15ce..2fe1bfb 100644 --- a/src/stacks.c +++ b/src/stacks.c @@ -66,8 +66,10 @@ call32_prep(u8 method)
// Backup cmos index register and disable nmi u8 cmosindex = inb(PORT_CMOS_INDEX);
- outb(cmosindex | NMI_DISABLE_BIT, PORT_CMOS_INDEX);
- inb(PORT_CMOS_DATA);
if (!(cmosindex & NMI_DISABLE_BIT)) {
outb(cmosindex | NMI_DISABLE_BIT, PORT_CMOS_INDEX);inb(PORT_CMOS_DATA);} SET_LOW(Call16Data.cmosindex, cmosindex);
SET_LOW(Call16Data.method, method);
@@ -103,8 +105,11 @@ call32_post(void) }
// Restore cmos index register
- outb(GET_LOW(Call16Data.cmosindex), PORT_CMOS_INDEX);
- inb(PORT_CMOS_DATA);
- u8 cmosindex = GET_LOW(Call16Data.cmosindex);
- if (!(cmosindex & NMI_DISABLE_BIT)) {
outb(cmosindex, PORT_CMOS_INDEX);inb(PORT_CMOS_DATA);- } return method;
}
NMI disable is actually a no-op on QEMU (as it never sends NMIs, either), we might as well skip it in that case.
Paolo
On Tue, May 16, 2017 at 06:27:03PM +0200, Paolo Bonzini wrote:
On 16/05/2017 18:14, Kevin O'Connor wrote:
Don't write to the cmos index port on a mode switch if NMI is already disabled. This reduces the number of outb() calls.
Signed-off-by: Kevin O'Connor kevin@koconnor.net
src/stacks.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/stacks.c b/src/stacks.c index f4d15ce..2fe1bfb 100644 --- a/src/stacks.c +++ b/src/stacks.c @@ -66,8 +66,10 @@ call32_prep(u8 method)
// Backup cmos index register and disable nmi u8 cmosindex = inb(PORT_CMOS_INDEX);
- outb(cmosindex | NMI_DISABLE_BIT, PORT_CMOS_INDEX);
- inb(PORT_CMOS_DATA);
if (!(cmosindex & NMI_DISABLE_BIT)) {
outb(cmosindex | NMI_DISABLE_BIT, PORT_CMOS_INDEX);inb(PORT_CMOS_DATA);} SET_LOW(Call16Data.cmosindex, cmosindex);
SET_LOW(Call16Data.method, method);
@@ -103,8 +105,11 @@ call32_post(void) }
// Restore cmos index register
- outb(GET_LOW(Call16Data.cmosindex), PORT_CMOS_INDEX);
- inb(PORT_CMOS_DATA);
- u8 cmosindex = GET_LOW(Call16Data.cmosindex);
- if (!(cmosindex & NMI_DISABLE_BIT)) {
outb(cmosindex, PORT_CMOS_INDEX);inb(PORT_CMOS_DATA);- } return method;
}
NMI disable is actually a no-op on QEMU (as it never sends NMIs, either), we might as well skip it in that case.
This particular code is also run on real hardware. We could skip the inb() on QEMU, but my preference would be to try and minimize the number of platform specific branches.
Separately, the above patch should have also had:
--- a/src/stacks.c +++ b/src/stacks.c @@ -116,6 +121,7 @@ call16_override(int big) if (getesp() > BUILD_STACK_ADDR) panic("call16_override with invalid stack\n"); memset(&Call16Data, 0, sizeof(Call16Data)); + Call16Data.cmosindex = NMI_DISABLE_BIT; if (big) { Call16Data.method = C16_BIG; Call16Data.a20 = 1;
-Kevin
On Tue, May 16, 2017 at 12:14:48PM -0400, Kevin O'Connor wrote:
QEMU does not store the A20 setting in the SMM cpu environment area (and it does not look like real CPUs do either). So, manually backup and restore A20 on a mode switch.
FYI, I committed this series.
-Kevin
-
Kevin O'Connor -
Paolo Bonzini