From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44792) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKmUi-0005EO-1r for qemu-devel@nongnu.org; Wed, 06 Jul 2016 09:04:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bKmUa-0005mJ-PK for qemu-devel@nongnu.org; Wed, 06 Jul 2016 09:03:58 -0400 Received: from mga09.intel.com ([134.134.136.24]:54401) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKmUa-0005mD-Dg for qemu-devel@nongnu.org; Wed, 06 Jul 2016 09:03:52 -0400 Date: Wed, 6 Jul 2016 21:03:36 +0800 From: Haozhong Zhang Message-ID: <20160706130336.ad4jmzvbgiyc7g5h@hz-desktop> References: <20160622065324.23812-1-haozhong.zhang@intel.com> <20160706012619.ebiizdoqngkmur7a@hz-desktop> <234271213.4215596.1467785884526.JavaMail.zimbra@redhat.com> <20160706062832.py2denwwvhw24f52@hz-desktop> <395937dc-b61d-880c-8a62-545bfc88eb84@redhat.com> <761279c8-3386-398e-f76f-919e55a81b06@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <761279c8-3386-398e-f76f-919e55a81b06@redhat.com> Subject: Re: [Qemu-devel] [SeaBIOS] [PATCH v3] fw/msr_feature_control: add support to set MSR_IA32_FEATURE_CONTROL List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: Ashok Raj , Paolo Bonzini , seabios@seabios.org, qemu-devel@nongnu.org On 07/06/16 13:04, Laszlo Ersek wrote: > On 07/06/16 08:42, Laszlo Ersek wrote: > > On 07/06/16 08:28, Haozhong Zhang wrote: > >> Hi Ashok, > >> > >> On 07/06/16 02:18, Paolo Bonzini wrote: > >>>> I forgot to restore MSR_IA32_FEATURE_CONTROL in the resume path, and > >>>> MSR_IA32_FEATURE_CONTROL is zero after S3 resume. > >>> > >>> This is a bug. Sorry Laszlo. :) > >>> > >>>> Not restore MSR_IA32_FEATURE_CONTROL during S3 resume does not affect > >>>> at least Linux guest (tested 4.5). Current QEMU may advise the guest > >>>> firmware to set bit 20 (for LMCE), bit 2 (for VMX) and bit 0 (lock > >>>> bit). > >>>> - For LMCE, Linux only checks bit 20 and bit 0 at boot time and then > >>>> keeps using the result even after resume. > >>> > >>> On real hardware, LMCE would not be enabled after resume. I'm not > >>> sure what would happen, but it wouldn't be good. > >> > >> Could you help to check if the LMCE bit in MSR_IA32_FEATURE_CONTROL is > >> set after S3 resume on the real hardware? > > > > The SDM says that IA32_FEATURE_CONTROL is zeroed on logical processor reset. > > > > 23.7 ENABLING AND ENTERING VMX OPERATION > > > > [...] VMXON is also controlled by the IA32_FEATURE_CONTROL MSR (MSR > > address 3AH). This MSR is cleared to zero when a logical processor is > > reset. [...] > > Actually, I think there is a bug in KVM at the moment. I ran the > following test: > > - modified OVMF to set the MSR to value 0x5 on just the BSP > - booted an i440fx and a Q35 (SMM-enabled) OVMF guest > - checked "rdmsr -a 0x3a" in both > - ran "pm-suspend" in both guests, woke them > - repeated the rdmsr command > > The result is that the BSP had the 0x5 MSR value both after cold boot > and after S3 resume. So, KVM does not seem to implement clearing of the MSR. > Interesting result, is setting MSR on BSP also called after S3 resume? I went through your test steps with OVMF replaced by a modified SeaBIOS which only sets MSR_IA32_FEATURE_CONTROL on BSP at boot time, the result before S3 resume is the value on BSP is 5 and others are 0, and the result after S3 resume is values on all CPUs are 0. > I checked kvm/next (currently at > 196f20ca52e8c7281932663c348fa54b82d03914), and vmx_vcpu_reset() does not > seem to zero vmx->msr_ia32_feature_control. > The function reset cpu state in QEMU after S3 resume is x86_cpu_reset(CPUState *s) in target-i386/cpu.c which is called for all vcpus and does memset(env, 0, offsetof(CPUX86State, cpuid_level)); CPUX86State.msr_ia32_feature_control is before .cpuid_level, so guest MSR_IA32_FEATURE_CONTROL on all vcpus should be zero after S3 resume. Thanks, Haozhong > Now, what I absolutely can't tell you is whether this zeroing should > happen regardless of "init_event", or just for a specific value of > "init_event". Whenever I look at "init_event", I have to track it down > to the commit that added it, then locate all the commits that fixed it, > then guess whether the SDM language "logical processor reset" implies a > specific "init_event" value or not. So, I have no idea about "init_event". > > Either way, why it is important to me: the edk2 modules that are built > into OVMF use loads of INIT-SIPI-SIPI sequences, for implementing > multiprocessing in the firmware (regardless of the feature at hand). For > example, EFI_MP_SERVICES_PROTOCOL.StartupAllAps() boots all APs with a > programmer-specified routine, after which the APs are HLTed. (Until the > next such call.) INIT-SIPI-SIPI is used by the implementation for > booting the APs. > > So I need to know whether those INIT-SIPI-SIPI sequences are supposed to > clear the MSR -- in other words, whether I have to write patches that > explicitly sustain the MSR across these IPIs. > > If the answer is "yes", then I would like to ask for a KVM patch that > zeroes vmx->msr_ia32_feature_control "under the right circumstances". > > I can only attempt to re-program the MSR in all the necessary places in > edk2 if I can trust that KVM will clear the MSR (visibly to the "rdmsr" > command in the guest) as long as I miss at least one location in edk2. > > Thanks! > Laszlo >