Re: [PATCH] KVM: unbreak userspace that does not sets tss address

[Gleb Natapov <gleb@redhat.com>]

On Fri, Mar 11, 2011 at 02:30:32PM -0300, Marcelo Tosatti wrote:
> On Mon, Mar 07, 2011 at 05:19:36PM +0200, Gleb Natapov wrote:
> > Commit 6440e5967bc broke old userspaces that do not set tss address
> > before entering vcpu. Unbreak it by setting tss address to a safe
> > value on the first vcpu entry. New userspaces should set tss address,
> > so print warning in case it doesn't.
> > 
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index e2b8c6b..280b6ea 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -180,6 +180,7 @@ static u64 construct_eptp(unsigned long root_hpa);
> >  static void kvm_cpu_vmxon(u64 addr);
> >  static void kvm_cpu_vmxoff(void);
> >  static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3);
> > +static int vmx_set_tss_addr(struct kvm *kvm, unsigned int addr);
> >  
> >  static DEFINE_PER_CPU(struct vmcs *, vmxarea);
> >  static DEFINE_PER_CPU(struct vmcs *, current_vmcs);
> > @@ -1762,6 +1763,16 @@ static void enter_rmode(struct kvm_vcpu *vcpu)
> >  	vmx->emulation_required = 1;
> >  	vmx->rmode.vm86_active = 1;
> >  
> > +	/*
> > +	 * Very old userspace does not call KVM_SET_TSS_ADDR before entering
> > +	 * vcpu. Call it here with phys address pointing 16M below 4G.
> > +	 */
> > +	if (!vcpu->kvm->arch.tss_addr) {
> > +		printk_once(KERN_WARNING "kvm: KVM_SET_TSS_ADDR need to be "
> > +			     "called before entering vcpu\n");
> > +		vmx_set_tss_addr(vcpu->kvm, 0xfeffd000);
> > +	}
> > +
> >  	vmx->rmode.tr.selector = vmcs_read16(GUEST_TR_SELECTOR);
> >  	vmx->rmode.tr.base = vmcs_readl(GUEST_TR_BASE);
> >  	vmcs_writel(GUEST_TR_BASE, rmode_tss_base(vcpu->kvm));
> 
> Should leave/enter srcu section before/after set_memory_region. 
> 
OK.

> Actually, for older userspace, isnt the base_gfn + slot[0].npages - 3
> address (rmode_tss_base) used? That is, initializing the TSS page at 
> that address at vcpu_reset time should be enough?
No, since this points to real memory and nothing prevents guest to write
to this memory at any time (we do not even mark it reserved in e820 map)
it needs to be reinitialized at every rmode entry and this is was we are
trying to avoid. And initialization in vcpu_reset() has the same race
that is fixed by 6440e5967bc007827de2ba57a9928e120c6af86d. When one
cpu resets another may run in rmode and use tss.

--
			Gleb.