Re: [PATCH] KVM: Fix EPT identity IOCTL on 32pae

From: Sheng Yang <sheng@linux.intel.com>
To: Avi Kivity <avi@redhat.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: Fix EPT identity IOCTL on 32pae
Date: Mon, 3 Aug 2009 21:38:28 +0800	[thread overview]
Message-ID: <200908032138.28658.sheng@linux.intel.com> (raw)
In-Reply-To: <4A76D747.2060506@redhat.com>

On Monday 03 August 2009 20:25:43 Avi Kivity wrote:
> On 08/03/2009 09:17 AM, Sheng Yang wrote:
> > Copy u64 from guest result in chaos.
> >
> > Also fix a mistake of still using old macro rather than new variable().
> >
> > Signed-off-by: Sheng Yang<sheng@linux.intel.com>
> > ---
> >   arch/x86/kvm/vmx.c |    2 +-
> >   arch/x86/kvm/x86.c |    2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 7931c72..c5aaa1b 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -2199,7 +2199,7 @@ static int alloc_identity_pagetable(struct kvm
> > *kvm) goto out;
> >
> >   	kvm->arch.ept_identity_pagetable = gfn_to_page(kvm,
> > -			VMX_EPT_IDENTITY_PAGETABLE_ADDR>>  PAGE_SHIFT);
> > +			kvm->arch.ept_identity_map_addr>>  PAGE_SHIFT);
>
> This is a fix, but as Jan says, separate patch.

Sure. (and thanks Jan's comment :) )
>
> >   out:
> >   	up_write(&kvm->slots_lock);
> >   	return r;
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 2539e9a..977b705 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -2178,7 +2178,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
> >   			goto out;
> >   		break;
> >   	case KVM_SET_IDENTITY_MAP_ADDR: {
> > -		u64 ident_addr;
> > +		unsigned long ident_addr;
> >
> >   		r = -EFAULT;
> >   		if (copy_from_user(&ident_addr, argp, sizeof ident_addr))
>
> This doesn't look right.  Consider 32-bit userspace running on a 32-bit
> kernel and the same userspace running on a 64-bit kernel.  We need to
> copy the same size in both cases.

Yeah... Then I think I should fix the userspace. Would update the patch.

-- 
regards
Yang, Sheng