Re: [Resend Patch] KVM/x86: Fix wrong macro references of X86_CR0_PG_BIT and X86_CR4_PAE_BIT in kvm_valid_sregs()

From: Lan Tianyu
Date: Thu Jan 18 2018 - 21:41:26 EST


Hi Eric:
Great thanks for your review.
On Thu, Jan 18, 2018 at 10:39:04AM -0800, Eric Biggers wrote:
> On Tue, Jan 16, 2018 at 05:34:07PM +0800, Tianyu Lan wrote:
> > kvm_valid_sregs() should use X86_CR0_PG and X86_CR4_PAE to check bit
> > status rather than X86_CR0_PG_BIT and X86_CR4_PAE_BIT. This patch is
> > to fix it.
> >
> > Fixes: f29810335965a(KVM/x86: Check input paging mode when cs.l is set)
> > Reported-by: Jeremi Piotrowski <jeremi.piotrowski@xxxxxxxxx>
> > Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > Cc: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
> > Signed-off-by: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>
> > ---
> > Sorry for noise. Missed kvm maillist.
> >
> > arch/x86/kvm/x86.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 1cec2c6..c53298d 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -7496,13 +7496,13 @@ EXPORT_SYMBOL_GPL(kvm_task_switch);
> >
> > int kvm_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
> > {
> > - if ((sregs->efer & EFER_LME) && (sregs->cr0 & X86_CR0_PG_BIT)) {
> > + if ((sregs->efer & EFER_LME) && (sregs->cr0 & X86_CR0_PG)) {
> > /*
> > * When EFER.LME and CR0.PG are set, the processor is in
> > * 64-bit mode (though maybe in a 32-bit code segment).
> > * CR4.PAE and EFER.LMA must be set.
> > */
> > - if (!(sregs->cr4 & X86_CR4_PAE_BIT)
> > + if (!(sregs->cr4 & X86_CR4_PAE)
> > || !(sregs->efer & EFER_LMA))
> > return -EINVAL;
> > } else {
> > --
> > 2.7.4
> >
>
> I came across this too and was just about to send the exact same patch. It
> looks good to me as long as the bits it's supposed to be checking were correct
> in the first place. Patch title could maybe be shortened a bit, e.g. "KVM/x86:
> Fix references to CR0.PG and CR4.PAE in kvm_valid_sregs()". The "Fixes:" line
> is also formatted incorrectly.

That will be better and will update.

>
> Thanks,
>
> Eric