Re: [PATCH 08/13] KVM: x86: stubs for SMM support

From: Bandan Das <bsd@redhat.com>
To: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	guangrong.xiao@linux.intel.com,
	Yang Zhang <yang.z.zhang@intel.com>,
	wanpeng.li@linux.intel.com
Subject: Re: [PATCH 08/13] KVM: x86: stubs for SMM support
Date: Tue, 05 May 2015 14:38:40 -0400	[thread overview]
Message-ID: <jpgsiba50pb.fsf@redhat.com> (raw)
In-Reply-To: <20150504175102.GC11234@potion.brq.redhat.com> ("Radim \=\?utf-8\?B\?S3LEjW3DocWZIidz\?\= message of "Mon, 4 May 2015 19:51:02 +0200")

Radim Krčmář <rkrcmar@redhat.com> writes:
...
>> +		break;
>
> (I'm not sure if this is supported if IA32_VMX_BASIC[49] = 0.
>  34.15.6.4 Saving Guest State
>    The SMM-transfer monitor (STM) can also discover the current value of
>    the SMBASE register by using the RDMSR
>
>  but it's not possible to get into STM without having a support for it
>  noted in IA32_VMX_BASIC[49] and more magic we also don't emulate to
>  actually enable it.)

Where does it mention IA32_VMX_BASIC[49] ? I only see "IA32_VMX_MISC[15] should be 1"
in 34.15.6.4. Anyway, I think we should do what the spec says..

>> @@ -7208,6 +7240,8 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu)
>>  	vcpu->arch.regs_avail = ~0;
>>  	vcpu->arch.regs_dirty = ~0;
>>  
>> +	vcpu->arch.smbase = 0x30000;
>
> It's not reset on INIT, only on RESET.  (34.11 SMBASE RELOCATION)
I remember mentioning it elsewhere - IMO kvm_vcpu_reset() and kvm_vcpu_init()
should really be two different interfaces. I don't mean code duplication - one
can just call the other but different names will be of some help when it comes
to the million places where the spec mentions INIT and RESET have different
behavior.

Bandan