| Message ID | 20121219132926.GA3551@amt.cnet (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Dec 22, 2012 at 02:31:10PM +0200, Avi Kivity wrote: > On Wed, Dec 19, 2012 at 3:29 PM, Marcelo Tosatti <mtosatti@redhat.com>wrote: > > > > > > > CPL is always 0 when in real mode, and always 3 when virtual 8086 mode. > > > > Using values other than those can cause failures on operations that check > > CPL. > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > index a4ecf7c..3abe433 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -3215,13 +3215,6 @@ static u64 vmx_get_segment_base(struct kvm_vcpu > > *vcpu, int seg) > > > > static int __vmx_get_cpl(struct kvm_vcpu *vcpu) > > { > > - if (!is_protmode(vcpu)) > > - return 0; > > - > > - if (!is_long_mode(vcpu) > > - && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > */ > > - return 3; > > - > > return vmx_read_guest_seg_selector(to_vmx(vcpu), VCPU_SREG_CS) & 3; > > } > > > > @@ -3229,6 +3222,13 @@ static int vmx_get_cpl(struct kvm_vcpu *vcpu) > > { > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > > > + if (!is_protmode(vcpu)) > > + return 0; > > + > > + if (!is_long_mode(vcpu) > > + && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > */ > > + return 3; > > + > > /* > > * If we enter real mode with cs.sel & 3 != 0, the normal CPL > > calculations > > * fail; use the cache instead. > > > > > This undoes the cache, now every vmx_get_cpl() in protected mode has to > VMREAD(GUEST_RFLAGS). True. Marcelo what failure do you see without the patch? -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 25, 2012 at 02:48:08PM +0200, Gleb Natapov wrote: > On Sat, Dec 22, 2012 at 02:31:10PM +0200, Avi Kivity wrote: > > On Wed, Dec 19, 2012 at 3:29 PM, Marcelo Tosatti <mtosatti@redhat.com>wrote: > > > > > > > > > > > CPL is always 0 when in real mode, and always 3 when virtual 8086 mode. > > > > > > Using values other than those can cause failures on operations that check > > > CPL. > > > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > > index a4ecf7c..3abe433 100644 > > > --- a/arch/x86/kvm/vmx.c > > > +++ b/arch/x86/kvm/vmx.c > > > @@ -3215,13 +3215,6 @@ static u64 vmx_get_segment_base(struct kvm_vcpu > > > *vcpu, int seg) > > > > > > static int __vmx_get_cpl(struct kvm_vcpu *vcpu) > > > { > > > - if (!is_protmode(vcpu)) > > > - return 0; > > > - > > > - if (!is_long_mode(vcpu) > > > - && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > */ > > > - return 3; > > > - > > > return vmx_read_guest_seg_selector(to_vmx(vcpu), VCPU_SREG_CS) & 3; > > > } > > > > > > @@ -3229,6 +3222,13 @@ static int vmx_get_cpl(struct kvm_vcpu *vcpu) > > > { > > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > > > > > + if (!is_protmode(vcpu)) > > > + return 0; > > > + > > > + if (!is_long_mode(vcpu) > > > + && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > */ > > > + return 3; > > > + > > > /* > > > * If we enter real mode with cs.sel & 3 != 0, the normal CPL > > > calculations > > > * fail; use the cache instead. > > > > > > > > > This undoes the cache, now every vmx_get_cpl() in protected mode has to > > VMREAD(GUEST_RFLAGS). > True. Marcelo what failure do you see without the patch? > > -- > Gleb. On transition _to_ real mode, linearize fails due to CPL checks (FreeBSD). I'll resend the patch with use of cache for VMREAD(GUEST_RFLAGS), which is already implemented. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 25, 2012 at 07:37:10PM -0200, Marcelo Tosatti wrote: > On Tue, Dec 25, 2012 at 02:48:08PM +0200, Gleb Natapov wrote: > > On Sat, Dec 22, 2012 at 02:31:10PM +0200, Avi Kivity wrote: > > > On Wed, Dec 19, 2012 at 3:29 PM, Marcelo Tosatti <mtosatti@redhat.com>wrote: > > > > > > > > > > > > > > > CPL is always 0 when in real mode, and always 3 when virtual 8086 mode. > > > > > > > > Using values other than those can cause failures on operations that check > > > > CPL. > > > > > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > > > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > > > index a4ecf7c..3abe433 100644 > > > > --- a/arch/x86/kvm/vmx.c > > > > +++ b/arch/x86/kvm/vmx.c > > > > @@ -3215,13 +3215,6 @@ static u64 vmx_get_segment_base(struct kvm_vcpu > > > > *vcpu, int seg) > > > > > > > > static int __vmx_get_cpl(struct kvm_vcpu *vcpu) > > > > { > > > > - if (!is_protmode(vcpu)) > > > > - return 0; > > > > - > > > > - if (!is_long_mode(vcpu) > > > > - && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > > */ > > > > - return 3; > > > > - > > > > return vmx_read_guest_seg_selector(to_vmx(vcpu), VCPU_SREG_CS) & 3; > > > > } > > > > > > > > @@ -3229,6 +3222,13 @@ static int vmx_get_cpl(struct kvm_vcpu *vcpu) > > > > { > > > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > > > > > > > + if (!is_protmode(vcpu)) > > > > + return 0; > > > > + > > > > + if (!is_long_mode(vcpu) > > > > + && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > > */ > > > > + return 3; > > > > + > > > > /* > > > > * If we enter real mode with cs.sel & 3 != 0, the normal CPL > > > > calculations > > > > * fail; use the cache instead. > > > > > > > > > > > > > This undoes the cache, now every vmx_get_cpl() in protected mode has to > > > VMREAD(GUEST_RFLAGS). > > True. Marcelo what failure do you see without the patch? > > > > -- > > Gleb. > > On transition _to_ real mode, linearize fails due to CPL checks > (FreeBSD). I'll resend the patch with use of cache for > VMREAD(GUEST_RFLAGS), which is already implemented. I am curious does it still fails with all my vmx patches applied too? The question is how does it happen that we enter real mode while cache is set to 3. It should never be 3 during boot since boot process never enters the userspace. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 26, 2012 at 07:25:49AM +0200, Gleb Natapov wrote: > On Tue, Dec 25, 2012 at 07:37:10PM -0200, Marcelo Tosatti wrote: > > On Tue, Dec 25, 2012 at 02:48:08PM +0200, Gleb Natapov wrote: > > > On Sat, Dec 22, 2012 at 02:31:10PM +0200, Avi Kivity wrote: > > > > On Wed, Dec 19, 2012 at 3:29 PM, Marcelo Tosatti <mtosatti@redhat.com>wrote: > > > > > > > > > > > > > > > > > > > CPL is always 0 when in real mode, and always 3 when virtual 8086 mode. > > > > > > > > > > Using values other than those can cause failures on operations that check > > > > > CPL. > > > > > > > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > > > > > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > > > > index a4ecf7c..3abe433 100644 > > > > > --- a/arch/x86/kvm/vmx.c > > > > > +++ b/arch/x86/kvm/vmx.c > > > > > @@ -3215,13 +3215,6 @@ static u64 vmx_get_segment_base(struct kvm_vcpu > > > > > *vcpu, int seg) > > > > > > > > > > static int __vmx_get_cpl(struct kvm_vcpu *vcpu) > > > > > { > > > > > - if (!is_protmode(vcpu)) > > > > > - return 0; > > > > > - > > > > > - if (!is_long_mode(vcpu) > > > > > - && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > > > */ > > > > > - return 3; > > > > > - > > > > > return vmx_read_guest_seg_selector(to_vmx(vcpu), VCPU_SREG_CS) & 3; > > > > > } > > > > > > > > > > @@ -3229,6 +3222,13 @@ static int vmx_get_cpl(struct kvm_vcpu *vcpu) > > > > > { > > > > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > > > > > > > > > + if (!is_protmode(vcpu)) > > > > > + return 0; > > > > > + > > > > > + if (!is_long_mode(vcpu) > > > > > + && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > > > */ > > > > > + return 3; > > > > > + > > > > > /* > > > > > * If we enter real mode with cs.sel & 3 != 0, the normal CPL > > > > > calculations > > > > > * fail; use the cache instead. > > > > > > > > > > > > > > > > > This undoes the cache, now every vmx_get_cpl() in protected mode has to > > > > VMREAD(GUEST_RFLAGS). > > > True. Marcelo what failure do you see without the patch? > > > > > > -- > > > Gleb. > > > > On transition _to_ real mode, linearize fails due to CPL checks > > (FreeBSD). I'll resend the patch with use of cache for > > VMREAD(GUEST_RFLAGS), which is already implemented. > I am curious does it still fails with all my vmx patches applied too? Yes. > The question is how does it happen that we enter real mode while cache > is set to 3. It should never be 3 during boot since boot process never > enters the userspace. Its transition _to_ real mode (from protected). -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 26, 2012 at 11:25:24AM -0200, Marcelo Tosatti wrote: > On Wed, Dec 26, 2012 at 07:25:49AM +0200, Gleb Natapov wrote: > > On Tue, Dec 25, 2012 at 07:37:10PM -0200, Marcelo Tosatti wrote: > > > On Tue, Dec 25, 2012 at 02:48:08PM +0200, Gleb Natapov wrote: > > > > On Sat, Dec 22, 2012 at 02:31:10PM +0200, Avi Kivity wrote: > > > > > On Wed, Dec 19, 2012 at 3:29 PM, Marcelo Tosatti <mtosatti@redhat.com>wrote: > > > > > > > > > > > > > > > > > > > > > > > CPL is always 0 when in real mode, and always 3 when virtual 8086 mode. > > > > > > > > > > > > Using values other than those can cause failures on operations that check > > > > > > CPL. > > > > > > > > > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > > > > > > > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > > > > > index a4ecf7c..3abe433 100644 > > > > > > --- a/arch/x86/kvm/vmx.c > > > > > > +++ b/arch/x86/kvm/vmx.c > > > > > > @@ -3215,13 +3215,6 @@ static u64 vmx_get_segment_base(struct kvm_vcpu > > > > > > *vcpu, int seg) > > > > > > > > > > > > static int __vmx_get_cpl(struct kvm_vcpu *vcpu) > > > > > > { > > > > > > - if (!is_protmode(vcpu)) > > > > > > - return 0; > > > > > > - > > > > > > - if (!is_long_mode(vcpu) > > > > > > - && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > > > > */ > > > > > > - return 3; > > > > > > - > > > > > > return vmx_read_guest_seg_selector(to_vmx(vcpu), VCPU_SREG_CS) & 3; > > > > > > } > > > > > > > > > > > > @@ -3229,6 +3222,13 @@ static int vmx_get_cpl(struct kvm_vcpu *vcpu) > > > > > > { > > > > > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > > > > > > > > > > > + if (!is_protmode(vcpu)) > > > > > > + return 0; > > > > > > + > > > > > > + if (!is_long_mode(vcpu) > > > > > > + && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > > > > */ > > > > > > + return 3; > > > > > > + > > > > > > /* > > > > > > * If we enter real mode with cs.sel & 3 != 0, the normal CPL > > > > > > calculations > > > > > > * fail; use the cache instead. > > > > > > > > > > > > > > > > > > > > > This undoes the cache, now every vmx_get_cpl() in protected mode has to > > > > > VMREAD(GUEST_RFLAGS). > > > > True. Marcelo what failure do you see without the patch? > > > > > > > > -- > > > > Gleb. > > > > > > On transition _to_ real mode, linearize fails due to CPL checks > > > (FreeBSD). I'll resend the patch with use of cache for > > > VMREAD(GUEST_RFLAGS), which is already implemented. > > I am curious does it still fails with all my vmx patches applied too? > > Yes. > Which FreeBSD exactly? Cannot reproduce with FreeBSD 9 64 bits here. > > The question is how does it happen that we enter real mode while cache > > is set to 3. It should never be 3 during boot since boot process never > > enters the userspace. > > Its transition _to_ real mode (from protected). But in protected mode CPL should be 0 during boot. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 26, 2012 at 03:33:16PM +0200, Gleb Natapov wrote: > On Wed, Dec 26, 2012 at 11:25:24AM -0200, Marcelo Tosatti wrote: > > On Wed, Dec 26, 2012 at 07:25:49AM +0200, Gleb Natapov wrote: > > > On Tue, Dec 25, 2012 at 07:37:10PM -0200, Marcelo Tosatti wrote: > > > > On Tue, Dec 25, 2012 at 02:48:08PM +0200, Gleb Natapov wrote: > > > > > On Sat, Dec 22, 2012 at 02:31:10PM +0200, Avi Kivity wrote: > > > > > > On Wed, Dec 19, 2012 at 3:29 PM, Marcelo Tosatti <mtosatti@redhat.com>wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > CPL is always 0 when in real mode, and always 3 when virtual 8086 mode. > > > > > > > > > > > > > > Using values other than those can cause failures on operations that check > > > > > > > CPL. > > > > > > > > > > > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > > > > > > > > > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > > > > > > index a4ecf7c..3abe433 100644 > > > > > > > --- a/arch/x86/kvm/vmx.c > > > > > > > +++ b/arch/x86/kvm/vmx.c > > > > > > > @@ -3215,13 +3215,6 @@ static u64 vmx_get_segment_base(struct kvm_vcpu > > > > > > > *vcpu, int seg) > > > > > > > > > > > > > > static int __vmx_get_cpl(struct kvm_vcpu *vcpu) > > > > > > > { > > > > > > > - if (!is_protmode(vcpu)) > > > > > > > - return 0; > > > > > > > - > > > > > > > - if (!is_long_mode(vcpu) > > > > > > > - && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > > > > > */ > > > > > > > - return 3; > > > > > > > - > > > > > > > return vmx_read_guest_seg_selector(to_vmx(vcpu), VCPU_SREG_CS) & 3; > > > > > > > } > > > > > > > > > > > > > > @@ -3229,6 +3222,13 @@ static int vmx_get_cpl(struct kvm_vcpu *vcpu) > > > > > > > { > > > > > > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > > > > > > > > > > > > > + if (!is_protmode(vcpu)) > > > > > > > + return 0; > > > > > > > + > > > > > > > + if (!is_long_mode(vcpu) > > > > > > > + && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > > > > > */ > > > > > > > + return 3; > > > > > > > + > > > > > > > /* > > > > > > > * If we enter real mode with cs.sel & 3 != 0, the normal CPL > > > > > > > calculations > > > > > > > * fail; use the cache instead. > > > > > > > > > > > > > > > > > > > > > > > > > This undoes the cache, now every vmx_get_cpl() in protected mode has to > > > > > > VMREAD(GUEST_RFLAGS). > > > > > True. Marcelo what failure do you see without the patch? > > > > > > > > > > -- > > > > > Gleb. > > > > > > > > On transition _to_ real mode, linearize fails due to CPL checks > > > > (FreeBSD). I'll resend the patch with use of cache for > > > > VMREAD(GUEST_RFLAGS), which is already implemented. > > > I am curious does it still fails with all my vmx patches applied too? > > > > Yes. > > > Which FreeBSD exactly? Cannot reproduce with FreeBSD 9 64 bits here. FreeBSD 9.1 with -smp 2. > > > > The question is how does it happen that we enter real mode while cache > > > is set to 3. It should never be 3 during boot since boot process never > > > enters the userspace. > > > > Its transition _to_ real mode (from protected). > But in protected mode CPL should be 0 during boot. BTX (FreeBSD's bootloader) does: http://people.freebsd.org/~jhb/docs/loader.txt. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 22, 2012 at 02:31:10PM +0200, Avi Kivity wrote: > On Wed, Dec 19, 2012 at 3:29 PM, Marcelo Tosatti <mtosatti@redhat.com>wrote: > > > > > > > CPL is always 0 when in real mode, and always 3 when virtual 8086 mode. > > > > Using values other than those can cause failures on operations that check > > CPL. > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > index a4ecf7c..3abe433 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -3215,13 +3215,6 @@ static u64 vmx_get_segment_base(struct kvm_vcpu > > *vcpu, int seg) > > > > static int __vmx_get_cpl(struct kvm_vcpu *vcpu) > > { > > - if (!is_protmode(vcpu)) > > - return 0; > > - > > - if (!is_long_mode(vcpu) > > - && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > */ > > - return 3; > > - > > return vmx_read_guest_seg_selector(to_vmx(vcpu), VCPU_SREG_CS) & 3; > > } > > > > @@ -3229,6 +3222,13 @@ static int vmx_get_cpl(struct kvm_vcpu *vcpu) > > { > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > > > + if (!is_protmode(vcpu)) > > + return 0; > > + > > + if (!is_long_mode(vcpu) > > + && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > */ > > + return 3; > > + > > /* > > * If we enter real mode with cs.sel & 3 != 0, the normal CPL > > calculations > > * fail; use the cache instead. > > > > > This undoes the cache, now every vmx_get_cpl() in protected mode has to > VMREAD(GUEST_RFLAGS). Yes, but GUEST_RFLAGS reads are cached. So its read once per exit. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 01, 2013 at 09:36:38PM -0200, Marcelo Tosatti wrote: > On Wed, Dec 26, 2012 at 03:33:16PM +0200, Gleb Natapov wrote: > > On Wed, Dec 26, 2012 at 11:25:24AM -0200, Marcelo Tosatti wrote: > > > On Wed, Dec 26, 2012 at 07:25:49AM +0200, Gleb Natapov wrote: > > > > On Tue, Dec 25, 2012 at 07:37:10PM -0200, Marcelo Tosatti wrote: > > > > > On Tue, Dec 25, 2012 at 02:48:08PM +0200, Gleb Natapov wrote: > > > > > > On Sat, Dec 22, 2012 at 02:31:10PM +0200, Avi Kivity wrote: > > > > > > > On Wed, Dec 19, 2012 at 3:29 PM, Marcelo Tosatti <mtosatti@redhat.com>wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > CPL is always 0 when in real mode, and always 3 when virtual 8086 mode. > > > > > > > > > > > > > > > > Using values other than those can cause failures on operations that check > > > > > > > > CPL. > > > > > > > > > > > > > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > > > > > > > > > > > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > > > > > > > index a4ecf7c..3abe433 100644 > > > > > > > > --- a/arch/x86/kvm/vmx.c > > > > > > > > +++ b/arch/x86/kvm/vmx.c > > > > > > > > @@ -3215,13 +3215,6 @@ static u64 vmx_get_segment_base(struct kvm_vcpu > > > > > > > > *vcpu, int seg) > > > > > > > > > > > > > > > > static int __vmx_get_cpl(struct kvm_vcpu *vcpu) > > > > > > > > { > > > > > > > > - if (!is_protmode(vcpu)) > > > > > > > > - return 0; > > > > > > > > - > > > > > > > > - if (!is_long_mode(vcpu) > > > > > > > > - && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > > > > > > */ > > > > > > > > - return 3; > > > > > > > > - > > > > > > > > return vmx_read_guest_seg_selector(to_vmx(vcpu), VCPU_SREG_CS) & 3; > > > > > > > > } > > > > > > > > > > > > > > > > @@ -3229,6 +3222,13 @@ static int vmx_get_cpl(struct kvm_vcpu *vcpu) > > > > > > > > { > > > > > > > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > > > > > > > > > > > > > > > + if (!is_protmode(vcpu)) > > > > > > > > + return 0; > > > > > > > > + > > > > > > > > + if (!is_long_mode(vcpu) > > > > > > > > + && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 > > > > > > > > */ > > > > > > > > + return 3; > > > > > > > > + > > > > > > > > /* > > > > > > > > * If we enter real mode with cs.sel & 3 != 0, the normal CPL > > > > > > > > calculations > > > > > > > > * fail; use the cache instead. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This undoes the cache, now every vmx_get_cpl() in protected mode has to > > > > > > > VMREAD(GUEST_RFLAGS). > > > > > > True. Marcelo what failure do you see without the patch? > > > > > > > > > > > > -- > > > > > > Gleb. > > > > > > > > > > On transition _to_ real mode, linearize fails due to CPL checks > > > > > (FreeBSD). I'll resend the patch with use of cache for > > > > > VMREAD(GUEST_RFLAGS), which is already implemented. > > > > I am curious does it still fails with all my vmx patches applied too? > > > > > > Yes. > > > > > Which FreeBSD exactly? Cannot reproduce with FreeBSD 9 64 bits here. > > FreeBSD 9.1 with -smp 2. I cannot reproduce. I do see boot failure on the next branch with 9.[01] 64 bit -smp 2 here, but it is caused but segment registers been all incorrect on a secondary vcpu and this patch does not help. Applying http://comments.gmane.org/gmane.comp.emulators.kvm.devel/102593 fixes it. > > > > > > The question is how does it happen that we enter real mode while cache > > > > is set to 3. It should never be 3 during boot since boot process never > > > > enters the userspace. > > > > > > Its transition _to_ real mode (from protected). > > But in protected mode CPL should be 0 during boot. > > BTX (FreeBSD's bootloader) does: > > http://people.freebsd.org/~jhb/docs/loader.txt. Crazy. Regardless, I think your patch is correct and the current code that uses cpl cache during emulation is wrong and it remains wrong even after your patch. What if last time cpl cache was updated was while vcpu ran in cpl 3? Commit message says that it tries to fix the case when CS.selector&3 != 0 during transition to protected mode, but this can be fixed by setting cpl cache to 0 in vmx_set_cr0() instead of clearing it. Two things about the patch itself. Get rid of __vmx_get_cpl() and call to vmx_read_guest_seg_selector() directly from vmx_get_cpl() and drop __clear_bit(VCPU_EXREG_CPL) from vmx_set_rflags() and vmx_set_cr0() since vmx->cpl no longer depends on rflags and cr0. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 03, 2013 at 10:11:53AM +0200, Gleb Natapov wrote: > > FreeBSD 9.1 with -smp 2. > I cannot reproduce. I do see boot failure on the next branch with 9.[01] > 64 bit -smp 2 here, but it is caused but segment registers been all > incorrect on a secondary vcpu and this patch does not help. Applying > http://comments.gmane.org/gmane.comp.emulators.kvm.devel/102593 fixes > it. > > > > > > > > > The question is how does it happen that we enter real mode while cache > > > > > is set to 3. It should never be 3 during boot since boot process never > > > > > enters the userspace. > > > > > > > > Its transition _to_ real mode (from protected). > > > But in protected mode CPL should be 0 during boot. > > > > BTX (FreeBSD's bootloader) does: > > > > http://people.freebsd.org/~jhb/docs/loader.txt. > Crazy. Regardless, I think your patch is correct and the current code > that uses cpl cache during emulation is wrong and it remains wrong even > after your patch. What if last time cpl cache was updated was while vcpu > ran in cpl 3? Commit message says that it tries to fix the case when > CS.selector&3 != 0 during transition to protected mode, but this can be > fixed by setting cpl cache to 0 in vmx_set_cr0() instead of clearing it. Yes, i suppose so, can be done by a separatch patch, though. > Two things about the patch itself. Get rid of __vmx_get_cpl() and call > to vmx_read_guest_seg_selector() directly from vmx_get_cpl() and drop > __clear_bit(VCPU_EXREG_CPL) from vmx_set_rflags() and vmx_set_cr0() > since vmx->cpl no longer depends on rflags and cr0. You still want to invalidate vmx->cpl cache on cr0 writes: think protected -> real -> protected transition. And as for EFLAGS, agreed. Sending new patch. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 03, 2013 at 11:19:18AM -0200, Marcelo Tosatti wrote: > On Thu, Jan 03, 2013 at 10:11:53AM +0200, Gleb Natapov wrote: > > > FreeBSD 9.1 with -smp 2. > > I cannot reproduce. I do see boot failure on the next branch with 9.[01] > > 64 bit -smp 2 here, but it is caused but segment registers been all > > incorrect on a secondary vcpu and this patch does not help. Applying > > http://comments.gmane.org/gmane.comp.emulators.kvm.devel/102593 fixes > > it. > > > > > > > > > > > > The question is how does it happen that we enter real mode while cache > > > > > > is set to 3. It should never be 3 during boot since boot process never > > > > > > enters the userspace. > > > > > > > > > > Its transition _to_ real mode (from protected). > > > > But in protected mode CPL should be 0 during boot. > > > > > > BTX (FreeBSD's bootloader) does: > > > > > > http://people.freebsd.org/~jhb/docs/loader.txt. > > Crazy. Regardless, I think your patch is correct and the current code > > that uses cpl cache during emulation is wrong and it remains wrong even > > after your patch. What if last time cpl cache was updated was while vcpu > > ran in cpl 3? Commit message says that it tries to fix the case when > > CS.selector&3 != 0 during transition to protected mode, but this can be > > fixed by setting cpl cache to 0 in vmx_set_cr0() instead of clearing it. > > Yes, i suppose so, can be done by a separatch patch, though. > Yes. I will send the patch. > > Two things about the patch itself. Get rid of __vmx_get_cpl() and call > > to vmx_read_guest_seg_selector() directly from vmx_get_cpl() and drop > > __clear_bit(VCPU_EXREG_CPL) from vmx_set_rflags() and vmx_set_cr0() > > since vmx->cpl no longer depends on rflags and cr0. > > You still want to invalidate vmx->cpl cache on cr0 writes: > think protected -> real -> protected transition. If CS.selector didn't change during the transition vmx->cpl will be set to the same value and if it changes cpl cache will be cleared either by vmx_set_segment() or vmexit. > And as for EFLAGS, agreed. Sending new patch. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index a4ecf7c..3abe433 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -3215,13 +3215,6 @@ static u64 vmx_get_segment_base(struct kvm_vcpu *vcpu, int seg) static int __vmx_get_cpl(struct kvm_vcpu *vcpu) { - if (!is_protmode(vcpu)) - return 0; - - if (!is_long_mode(vcpu) - && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 */ - return 3; - return vmx_read_guest_seg_selector(to_vmx(vcpu), VCPU_SREG_CS) & 3; } @@ -3229,6 +3222,13 @@ static int vmx_get_cpl(struct kvm_vcpu *vcpu) { struct vcpu_vmx *vmx = to_vmx(vcpu); + if (!is_protmode(vcpu)) + return 0; + + if (!is_long_mode(vcpu) + && (kvm_get_rflags(vcpu) & X86_EFLAGS_VM)) /* if virtual 8086 */ + return 3; + /* * If we enter real mode with cs.sel & 3 != 0, the normal CPL calculations * fail; use the cache instead.
CPL is always 0 when in real mode, and always 3 when virtual 8086 mode. Using values other than those can cause failures on operations that check CPL. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html