Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support

[Kang, Luwei]

> > From: Chao Peng <address@hidden>
> >
> > Expose Intel Processor Trace feature to guest.
> >
> > Signed-off-by: Chao Peng <address@hidden>
> > Signed-off-by: Luwei Kang <address@hidden>
> > ---
> >  target/i386/cpu.c | 19 ++++++++++++++++++-  target/i386/cpu.h |  1 +
> > target/i386/kvm.c | 23 +++++++++++++++++++++++
> >  3 files changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c index
> > 3818d72..57f8370 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -427,7 +427,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] 
> > = {
> >              NULL, NULL, "mpx", NULL,
> >              "avx512f", "avx512dq", "rdseed", "adx",
> >              "smap", "avx512ifma", "pcommit", "clflushopt",
> > -            "clwb", NULL, "avx512pf", "avx512er",
> > +            "clwb", "intel-pt", "avx512pf", "avx512er",
> >              "avx512cd", "sha-ni", "avx512bw", "avx512vl",
> >          },
> >          .cpuid_eax = 7,
> > @@ -3006,6 +3006,23 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> > uint32_t count,
> >          }
> >          break;
> >      }
> > +    case 0x14: {
> > +        if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
> > +             kvm_enabled()) {
> > +            KVMState *s = cs->kvm_state;
> > +
> > +            *eax = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EAX);
> > +            *ebx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EBX);
> > +            *ecx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_ECX);
> > +            *edx = kvm_arch_get_supported_cpuid(s, 0x14, count,
> > + R_EDX);
> 
> If you are forwarding host info directly to the guest, the feature is not 
> migration-safe.  The new feature needs to be added to
> feature_word_info[FEAT_7_0_EBX].unmigratable_flags.
> 

Hi,
     Thanks for you review. I want to support Intel PT live migration and patch 
[2/2] to do this. I don't understand  why need to add this feature in 
feature_word_info[FEAT_7_0_EBX].unmigratable_flags first to disable live 
migration.

Thanks,
Luwei Kang

> 
> > +        } else {
> > +            *eax = 0;
> > +            *ebx = 0;
> > +            *ecx = 0;
> > +            *edx = 0;
> > +        }
> > +        break;
> > +    }
> >      case 0x40000000:
> >          /*
> >           * CPUID code in kvm_arch_init_vcpu() ignores stuff diff
> > --git a/target/i386/cpu.h b/target/i386/cpu.h index 62c4742..58a4b6c
> > 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -642,6 +642,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> > #define CPUID_7_0_EBX_PCOMMIT  (1U << 22) /* Persistent Commit */
> > #define CPUID_7_0_EBX_CLFLUSHOPT (1U << 23) /* Flush a Cache Line Optimized 
> > */
> >  #define CPUID_7_0_EBX_CLWB     (1U << 24) /* Cache Line Write Back */
> > +#define CPUID_7_0_EBX_INTEL_PT (1U << 25) /* Intel Processor Trace */
> >  #define CPUID_7_0_EBX_AVX512PF (1U << 26) /* AVX-512 Prefetch */
> > #define CPUID_7_0_EBX_AVX512ER (1U << 27) /* AVX-512 Exponential and
> > Reciprocal */  #define CPUID_7_0_EBX_AVX512CD (1U << 28) /* AVX-512
> > Conflict Detection */ diff --git a/target/i386/kvm.c
> > b/target/i386/kvm.c index 6f69e2f..e13ab58 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -863,6 +863,29 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >                  c = &cpuid_data.entries[cpuid_i++];
> >              }
> >              break;
> > +        case 0x14: {
> > +            uint32_t times;
> > +
> > +            c->function = i;
> > +            c->index = 0;
> > +            c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> > +            cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
> > +            times = c->eax;
> > +
> > +            for (j = 1; j <= times; ++j) {
> > +                if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
> > +                    fprintf(stderr, "cpuid_data is full, no space for "
> > +                                "cpuid(eax:0x14,ecx:0x%x)\n", j);
> > +                    abort();
> > +                }
> > +                c = &cpuid_data.entries[cpuid_i++];
> > +                c->function = i;
> > +                c->index = j;
> > +                c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> > +                cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, 
> > &c->edx);
> > +            }
> > +            break;
> > +        }
> >          default:
> >              c->function = i;
> >              c->flags = 0;
> > --
> > 1.8.3.1
> >
> 
> --
> Eduardo