On 11/05/2017 16:53, Kevin O'Connor wrote:
> On Thu, May 11, 2017 at 01:35:28PM +0200, Paolo Bonzini wrote:
>> Ignore env->a20_mask when running in system management mode.
>
> Thanks Paolo. I don't think this patch will help SeaBIOS though. The
> SeaBIOS SMM handler doesn't do much - it doesn't even access ram above
> 1MiB. See SeaBIOS' code in src/fw/smm.c:handle_smi().
>
> Instead, the SeaBIOS code does a cpu state backup/restore to switch
> into 32bit mode. I thought the A20 state would be part of that cpu
> backup/restore. However, looking at the Intel SDM docs now, it's not
> really clear to me how the processor "inhibits" A20 when in SMM mode -
> does it save/restore that state on SMI/RSM or does it have special
> logic to ignore A20 while in SMM mode?
There isn't any documented place for A20 in the state save map (I checked
AMD's BIOS/Kernel Developer Guide which is pretty comprehensive), so I
think the latter is more plausible. What I'm doing in this patch is
ignoring A20 while in SMM mode.
Then you would have to add an A20 save/restore in handle_smi; since
CALL32SMM_ENTERID should not nest, I think you can just do this:
diff --git a/src/fw/smm.c b/src/fw/smm.c
index 95f6ba7..711dae3 100644
--- a/src/fw/smm.c
+++ b/src/fw/smm.c
@@ -54,7 +54,8 @@ struct smm_layout {
struct smm_state backup2;
u8 stack[0x7c00];
u64 codeentry;
- u8 pad_8008[0x7df8];
+ u8 a20;
+ u8 pad_8009[0x7df7];
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->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->a20);
smm->cpu.i32.eip = regs[3];
}
} else if (rev == SMM_REV_I64) {
Paolo
