Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <084f4de105d24f22513c14a83c2254add31f5928.camel@redhat.com>
Subject: Re: [PATCH v3 03/13] KVM: x86: emulator: introduce
 emulator_recalc_and_set_mode
From:   Maxim Levitsky <mlevitsk@redhat.com>
To:     "Yang, Weijiang" <weijiang.yang@intel.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>
Cc:     Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Wanpeng Li <wanpengli@tencent.com>,
        Ingo Molnar <mingo@redhat.com>,
        "Christopherson,, Sean" <seanjc@google.com>,
        "x86@kernel.org" <x86@kernel.org>,
        Jim Mattson <jmattson@google.com>,
        Kees Cook <keescook@chromium.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        "H. Peter Anvin" <hpa@zytor.com>, Joerg Roedel <joro@8bytes.org>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <2e191acd-0b71-784a-2c14-6e78351788cc@intel.com>
References: <20220803155011.43721-1-mlevitsk@redhat.com>
         <20220803155011.43721-4-mlevitsk@redhat.com>
         <2e191acd-0b71-784a-2c14-6e78351788cc@intel.com>
Content-Type: text/plain; charset="UTF-8"
MIME-Version: 1.0
Date:   Fri, 12 Aug 2022 09:25:04 +0300
User-Agent: Evolution 3.36.5 (3.36.5-2.fc32) 
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 2022-08-11 at 23:33 +0800, Yang, Weijiang wrote:
> On 8/3/2022 11:50 PM, Maxim Levitsky wrote:
> > [...]
> > +static inline int emulator_recalc_and_set_mode(struct x86_emulate_ctxt *ctxt)
> > +{
> > +       u64 efer;
> > +       struct desc_struct cs;
> > +       u16 selector;
> > +       u32 base3;
> > +
> > +       ctxt->ops->get_msr(ctxt, MSR_EFER, &efer);
> > +
> > +       if (!ctxt->ops->get_cr(ctxt, 0) & X86_CR0_PE) {

Ouch, thanks for catching this!!

I wonder how I didn't catch this in unit tests....
(I'll check on this Sunday)


Best regards,
	Maxim Levitsky

> Shouldn't this be:  !(ctxt->ops->get_cr(ctxt, 0) & X86_CR0_PE) ?
> > +               /* Real mode. cpu must not have long mode active */
> > +               if (efer & EFER_LMA)
> > +                       return X86EMUL_UNHANDLEABLE;
> > +               ctxt->mode = X86EMUL_MODE_REAL;
> > +               return X86EMUL_CONTINUE;
> > +       }
> > +
> [...]
> > --
> > 2.26.3
> >