| Message ID | 20220803155011.43721-6-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SMM emulation and interrupt shadow fixes | expand |
On Wed, Aug 03, 2022, Maxim Levitsky wrote: > CR0.PE toggles real/protected mode, thus its update Uber nit, I like using title case for Real Mode, Protected Mode, etc... so that it's more obvious that a changelog/comment is referring to the architectural modes. > should update the emulation mode. > > This is likely a benign bug because there is no writeback > of state, other than the RIP increment, and when toggling > CR0.PE, the CPU has to execute code from a very low memory address. > > Also CR0.PG toggle when EFER.LMA is set, toggles the long mode. This last sentence is jumbled, and it probably fits better with the opening sentence. And it's technically EFER.LME; EFER.LMA=1 indicates the Long Mode is fully active. E.g. something like Update the emulation mode when handling writes to CR0, toggling CR0.PE switches between Real and Protected Mode, and toggling CR0.PG when EFER.LME=1 switches between Long and Protected Mode. > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > arch/x86/kvm/emulate.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 5e91b26cc1d8aa..765ec65b2861ba 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -3658,11 +3658,23 @@ static int em_movbe(struct x86_emulate_ctxt *ctxt) > > static int em_cr_write(struct x86_emulate_ctxt *ctxt) > { > - if (ctxt->ops->set_cr(ctxt, ctxt->modrm_reg, ctxt->src.val)) > + int cr_num = ctxt->modrm_reg; > + int r; > + > + if (ctxt->ops->set_cr(ctxt, cr_num, ctxt->src.val)) > return emulate_gp(ctxt, 0); > > /* Disable writeback. */ > ctxt->dst.type = OP_NONE; > + > + if (cr_num == 0) { > + /* CR0 write might have updated CR0.PE and/or CR0.PG > + * which can affect the cpu execution mode */ /* * Multi-line comment format should look like this. I need more * words to make this multiple lines. */ > + r = emulator_recalc_and_set_mode(ctxt); > + if (r != X86EMUL_CONTINUE) > + return r; > + } > + > return X86EMUL_CONTINUE; > } > > -- > 2.26.3 >
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 5e91b26cc1d8aa..765ec65b2861ba 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -3658,11 +3658,23 @@ static int em_movbe(struct x86_emulate_ctxt *ctxt) static int em_cr_write(struct x86_emulate_ctxt *ctxt) { - if (ctxt->ops->set_cr(ctxt, ctxt->modrm_reg, ctxt->src.val)) + int cr_num = ctxt->modrm_reg; + int r; + + if (ctxt->ops->set_cr(ctxt, cr_num, ctxt->src.val)) return emulate_gp(ctxt, 0); /* Disable writeback. */ ctxt->dst.type = OP_NONE; + + if (cr_num == 0) { + /* CR0 write might have updated CR0.PE and/or CR0.PG + * which can affect the cpu execution mode */ + r = emulator_recalc_and_set_mode(ctxt); + if (r != X86EMUL_CONTINUE) + return r; + } + return X86EMUL_CONTINUE; }
CR0.PE toggles real/protected mode, thus its update should update the emulation mode. This is likely a benign bug because there is no writeback of state, other than the RIP increment, and when toggling CR0.PE, the CPU has to execute code from a very low memory address. Also CR0.PG toggle when EFER.LMA is set, toggles the long mode. Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- arch/x86/kvm/emulate.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)