Re: [PATCH v2 05/11] KVM: x86: emulator: update the emulation mode after CR0 write

[Maxim Levitsky]

On Wed, 2022-07-20 at 23:50 +0000, Sean Christopherson wrote:
> On Tue, Jun 21, 2022, Maxim Levitsky wrote:
> > 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.
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
> > ---
> >  arch/x86/kvm/emulate.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> > index 6f4632babc4cd8..002687d17f9364 100644
> > --- a/arch/x86/kvm/emulate.c
> > +++ b/arch/x86/kvm/emulate.c
> > @@ -3659,11 +3659,22 @@ 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 */
> 
> Or toggled CR0.PG.  

I thought about it but paging actually does not affect the CPU mode.

E.g if you are in protected mode, instructions execute the same regardless
if you have paging or not.

(There are probably some exceptions but you understand what I mean).

Best regards,
	Maxim Levitsky

> It's probably also worth noting that ->set_cr() handles side
> effects to other registers, e.g. the lack of an EFER.LMA update makes this look
> suspicious at first glance.

> 
> > +		r = update_emulation_mode(ctxt);
> > +		if (r != X86EMUL_CONTINUE)
> > +			return r;
> > +	}
> > +
> >  	return X86EMUL_CONTINUE;
> >  }
> >  
> > -- 
> > 2.26.3
> >