> > The CPUID level need to be set to 0x14 manually on old machine-type if
> > Intel PT is enabled in guest. e.g. in Qemu 3.1 -machine pc-i440fx-3.1
> > -cpu qemu64,+intel-pt will be CPUID[0].EAX(level)=7 and
> > CPUID[7].EBX[25](intel-pt)=1.
> >
> > Some Intel PT capabilities are exposed by leaf 0x14 and the missing
> > capabilities will cause some MSRs access failed.
> > This patch add a warning message to inform the user to extend the
> > CPUID level.
>
> Note that a warning is not an acceptable fix for a QEMU crash.
> We still need to fix the QEMU crash reported at:
> https://lore.kernel.org/qemu-devel/20191024141536.gu6...@habkost.net/
>
>
> >
> > Suggested-by: Eduardo Habkost <ehabk...@redhat.com>
> > Signed-off-by: Luwei Kang <luwei.k...@intel.com>
>
> The subject line says "v1", but this patch is different from the
> v1 you sent earlier.
>
> If you are sending a different patch, please indicate it is a new version.
> Please also
> indicate what changed between different patch versions, to help review.
Got it. I fix a code style problem in resending patch (remove the '\n').
ERROR: Error messages should not contain newlines
#36: FILE: target/i386/cpu.c:5448:
+ "by \"-cpu ...,+intel-pt,level=0x14\"\n");
total: 1 errors, 0 warnings, 14 lines checked
>
> > ---
> > target/i386/cpu.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c index
> > a624163..f67c479 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -5440,8 +5440,12 @@ static void x86_cpu_expand_features(X86CPU
> > *cpu, Error **errp)
> >
> > /* Intel Processor Trace requires CPUID[0x14] */
> > if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
> > - kvm_enabled() && cpu->intel_pt_auto_level) {
>
> Not directly related to the warning: do you know why we have a
> kvm_enabled() check here? It seems unnecessary. We want CPUID level to be
> correct
> for all accelerators.
Intel PT virtualization enabling in KVM guest need some hardware enhancement and
EPT must be enabled in KVM. I think it can't work for e.g. tcg pure simulation
accelerator.
>
> > - x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14);
> > + kvm_enabled()) {
> > + if (cpu->intel_pt_auto_level)
> > + x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14);
> > + else
> > + warn_report("Intel PT need CPUID leaf 0x14, please set "
> > + "by \"-cpu ...,+intel-pt,level=0x14\"");
>
> The warning shouldn't be triggered if level is already >= 0x14.
>
> It is probably a good idea to mention that this happens only on
> pc-*-3.1 and older, as updating the machine-type is a better solution to the
> problem
> than manually setting the "level"
> property.
>
> This will print the warning multiple times if there are multiple VCPUs. You
> can use
> warn_report_once() to avoid that.
Got it. Will fix.
As you mentioned in this email " a warning is not an acceptable fix for a QEMU
crash."
We can't change the configuration of the old machine type because it may break
the
ABI compatibility. May I add more check on Intel PT, if CPUID[7].EBX[25]
(intel-pt) = 1
and level is <0x14, mask off this feature? Or do you have any other suggestions?
Thanks,
Luwei Kang
>
> > }
> >
> > /* CPU topology with multi-dies support requires CPUID[0x1F]
> > */
> > --
> > 1.8.3.1
> >
>
> --
> Eduardo
