> > On Thu, Jan 18, 2018 at 03:44:49PM +0100, Paolo Bonzini wrote: > >> On 18/01/2018 15:37, Eduardo Habkost wrote: > >>> On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote: > >>>> On 18/01/2018 14:24, Eduardo Habkost wrote: > >>>>> However, if there's a simple way to make it possible to migrate > >>>>> between hosts with different CPUID[14h] data, it would be even > >>>>> better. With the current KVM intel-pt implementation, what > >>>>> happens if the CPUID[14h] data seen by the guest doesn't match > >>>>> exactly the CPUID[14h] leaves from the host? > >>>> > >>>> Some bits in there can be treated as CPU features (e.g. EBX bit 0 > >>>> "CR3 filtering support"). Probably we should handle these in KVM right now. > >>>> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based > >>>> on CPUID, and apply it when the MSR is written. > >>> > >>> Does this mean QEMU can't set CPUID values that won't match the host > >>> with the existing implementation, or this won't matter for > >>> well-behaved guests that don't try to set reserved bits on the MSRs? > >> > >> All the features could be handled exactly like regular feature bits. > >> If QEMU sets them incorrectly and "enforce" is not used, bad things > >> happen but it's the user's fault. > > > > Oh, I mean setting the bit to 0 when it's 1 on the host (if it's > > 0 on the host, QEMU would never set it anyway). Is it safe to do it > > with the current KVM intel-pt implementation? > > It's not, but it's (very) easy to fix. Hi Paolo, Do you mean there need to add some check before setting IA32_RTIT_CTL MSR in KVM because some bits of this MSR is depend on the result of CPUID[14]. Any attempts to change these reserved bit should cause a #GP. > > > >> > >>> > >>>> It also needs to > >>>> whitelist bits like we do for other feature words. These include: > >>>> > >>>> - CPUID[EAX=14h,ECX=0].EBX > >>>> > >>>> - CPUID[EAX=14h,ECX=0].ECX except bit 31 > >>>> > >>>> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if > >>>> CPUID[EAX=14h,ECX=0].EBX[3]=1) > >>>> > >>>> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1) > >>> > >>> What do you mean by whitelist? > >> > >> KVM needs to tell QEMU the bits it knows about. I think kvm_arch_get_supported_cpuid() function can get the result of CPUID[14] from KVM. Is this the whitelist what you mentioned? Thanks, Luwei Kang > > > > So KVM isn't currently doing it on GET_SUPPORTED_CPUID? Oops. > > > > > >> > >>>> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, > >>>> there is no way to emulate the "wrong" value. > >>> > >>> In this case we could make it configurable but require the host and > >>> guest value to always match. > >>> > >>> This might be an obstacle to enabling intel-pt by default (because > >>> it could make VMs not migratable to newer hosts), but may allow the > >>> feature to be configured in a predictable way. > >> > >> Yeah, but consider that virtualized PT anyway would only be enabled > >> on Ice Lake processors. It's a few years away anyway! > >> > >>>> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric > >>>> values, and it's possible to emulate a lower value than the one in the processor. > >>> > >>> This could be handled by QEMU. There's no requirement that all > >>> GET_SUPPORTED_CPUID values should be validated by simple bit > >>> masking. > >> > >> Good! > >> > >> Paolo > >