| Message ID | 1535888767-154680-3-git-send-email-robert.hu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: QEMU side support on MSR based features | expand |
Hi, Thanks for the patch and sorry for taking so long to review it. On Sun, Sep 02, 2018 at 07:46:06PM +0800, Robert Hoo wrote: > Add kvm_get_supported_feature_msrs() to get supported MSR feature index list. > Add kvm_arch_get_supported_msr_feature() to get each MSR features value. > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > --- > include/sysemu/kvm.h | 2 ++ > target/i386/cpu.c | 7 ++--- > target/i386/kvm.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 78 insertions(+), 3 deletions(-) > > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index 0b64b8e..97d8d9d 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -463,6 +463,8 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension); > > uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function, > uint32_t index, int reg); > +uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index); > + > > void kvm_set_sigmask_len(KVMState *s, unsigned int sigmask_len); > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index a252c26..0160e97 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -3670,7 +3670,7 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, > bool migratable_only) > { > FeatureWordInfo *wi = &feature_word_info[w]; > - uint32_t r; > + uint32_t r = 0; > > if (kvm_enabled()) { > switch (wi->type) { > @@ -3679,8 +3679,9 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, > wi->cpuid.ecx, > wi->cpuid.reg); > break; > - default: > - r = 0; > + case MSR_FEATURE_WORD: > + r = kvm_arch_get_supported_msr_feature(kvm_state, > + wi->msr.index); If you move this patch before patch 1/3, this hunk could be part of patch 1/3. > break; > } > } else if (hvf_enabled()) { > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index 0b2a07d..bfd8088 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -107,6 +107,7 @@ static int has_pit_state2; > static bool has_msr_mcg_ext_ctl; > > static struct kvm_cpuid2 *cpuid_cache; > +static struct kvm_msr_list *kvm_feature_msrs; > > int kvm_has_pit_state2(void) > { > @@ -420,6 +421,33 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, > return ret; > } > > +uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index) > +{ > + struct { > + struct kvm_msrs info; > + struct kvm_msr_entry entries[1]; > + } msr_data; > + uint32_t ret; > + > + if (kvm_feature_msrs == NULL) { /*ARCH doesn't support feature MSRs*/ Nit: normally comments have spaces after "/*" and before "*/". Also: what do you mean by "ARCH"? Do you mean "host kernel"? > + return 0; > + } > + > + msr_data.info.nmsrs = 1; > + msr_data.entries[0].index = index; > + > + ret = kvm_ioctl(s, KVM_GET_MSRS, &msr_data); > + > + if (ret != 1) { If the MSR is not supported by the host kernel, it must not be a fatal error. We should just return 0 on that case. Probably the best way to ensure that is to check if the MSR is listed on kvm_feature_msrs before calling KVM_GET_MSRS (and return 0 if the MSR is not on the list). > + fprintf(stderr, "KVM get MSR (index=0x%x) feature failed, %s\n", > + index, strerror(-ret)); Please use error_report() instead of fprintf(stderr). > + exit(1); I'm unsure if exit(1) is the best option here, but at least this is consistent with error handling kvm_arch_get_supported_cpuid(). > + } > + > + return msr_data.entries[0].data; > +} > + > + > typedef struct HWPoisonPage { > ram_addr_t ram_addr; > QLIST_ENTRY(HWPoisonPage) list; > @@ -1239,6 +1267,45 @@ void kvm_arch_do_init_vcpu(X86CPU *cpu) > } > } > > +static int kvm_get_supported_feature_msrs(KVMState *s) > +{ > + int ret = 0; > + > + if (kvm_feature_msrs != NULL) { > + return 0; > + } > + > + if (!kvm_check_extension(s, KVM_CAP_GET_MSR_FEATURES)) { > + return -1; There's nothing wrong with not supporting KVM_CAP_GET_MSR_FEATURES. Why not return 0? > + } > + > + struct kvm_msr_list msr_list; > + > + msr_list.nmsrs = 0; > + ret = kvm_ioctl(s, KVM_GET_MSR_FEATURE_INDEX_LIST, &msr_list); > + if (ret < 0 && ret != -E2BIG) { You print an error to stderr if (ret < 0) below, but don't print anything here. Seems inconsistent. > + return ret; > + } > + > + assert(msr_list.nmsrs > 0); > + kvm_feature_msrs = (struct kvm_msr_list *) \ > + g_malloc0(sizeof(msr_list) + > + msr_list.nmsrs * sizeof(msr_list.indices[0])); > + > + kvm_feature_msrs->nmsrs = msr_list.nmsrs; > + ret = kvm_ioctl(s, KVM_GET_MSR_FEATURE_INDEX_LIST, kvm_feature_msrs); kvm_arch_get_supported_msr_feature() is only checking if kvm_feature_msrs is NULL, and nothing else. What exactly is the point of calling KVM_GET_MSR_FEATURE_INDEX_LIST and saving data at kvm_feature_msrs, if no other code is ever looking at the returned data? > + > + if (ret < 0) { > + fprintf(stderr, "Fetch KVM feature MSRs failed: %s\n", > + strerror(-ret)); Please use error_report() instead of fprintf(stderr). > + g_free(kvm_feature_msrs); > + kvm_feature_msrs = NULL; > + return ret; > + } > + > + return 0; > +} > + > static int kvm_get_supported_msrs(KVMState *s) > { > static int kvm_supported_msrs; > @@ -1392,6 +1459,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > return ret; > } > > + ret = kvm_get_supported_feature_msrs(s); > + if (ret < 0) { /*if MSR based features aren't supported, ignore it.*/ > + warn_report("Get supported feature MSRs failed."); We must not print a warning only because KVM_CAP_GET_MSR_FEATURES isn't supported by the host kernel. If KVM_GET_MSR_FEATURE_INDEX_LIST fails, on the other hand, we probably should make it a fatal error and not a warning. > + } > + > uname(&utsname); > lm_capable_kernel = strcmp(utsname.machine, "x86_64") == 0; > > -- > 1.8.3.1 > >
On Thu, 2018-09-20 at 00:07 -0300, Eduardo Habkost wrote: > Hi, > > Thanks for the patch and sorry for taking so long to review it. Never mind. I understand you're really busy. :-) > > On Sun, Sep 02, 2018 at 07:46:06PM +0800, Robert Hoo wrote: > > Add kvm_get_supported_feature_msrs() to get supported MSR feature > > index list. > > Add kvm_arch_get_supported_msr_feature() to get each MSR features > > value. > > > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> > > --- > > include/sysemu/kvm.h | 2 ++ > > target/i386/cpu.c | 7 ++--- > > target/i386/kvm.c | 72 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 78 insertions(+), 3 deletions(-) > > > > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > > index 0b64b8e..97d8d9d 100644 > > --- a/include/sysemu/kvm.h > > +++ b/include/sysemu/kvm.h > > @@ -463,6 +463,8 @@ int kvm_vm_check_extension(KVMState *s, > > unsigned int extension); > > > > uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t > > function, > > uint32_t index, int reg); > > +uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t > > index); > > + > > > > void kvm_set_sigmask_len(KVMState *s, unsigned int sigmask_len); > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > > index a252c26..0160e97 100644 > > --- a/target/i386/cpu.c > > +++ b/target/i386/cpu.c > > @@ -3670,7 +3670,7 @@ static uint32_t > > x86_cpu_get_supported_feature_word(FeatureWord w, > > bool > > migratable_only) > > { > > FeatureWordInfo *wi = &feature_word_info[w]; > > - uint32_t r; > > + uint32_t r = 0; > > > > if (kvm_enabled()) { > > switch (wi->type) { > > @@ -3679,8 +3679,9 @@ static uint32_t > > x86_cpu_get_supported_feature_word(FeatureWord w, > > wi->cpuid.ecx, > > wi->cpuid.reg); > > break; > > - default: > > - r = 0; > > + case MSR_FEATURE_WORD: > > + r = kvm_arch_get_supported_msr_feature(kvm_state, > > + wi->msr.index); > > If you move this patch before patch 1/3, this hunk could be part > of patch 1/3. > I'm afraid that if I moved this hunk, because of the dependency, I would have to move the definition of kvm_arch_get_supported_msr_feature() to patch 1/3, then in turn, it used kvm_feature_msrs, I've to put its definition and initialization function kvm_get_supported_feature_msrs() to patch 1/3 as well. Then actually, this makes patch 1/3 and 2/3 merged into 1. Would you like me to do so? > > break; > > } > > } else if (hvf_enabled()) { > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > > index 0b2a07d..bfd8088 100644 > > --- a/target/i386/kvm.c > > +++ b/target/i386/kvm.c > > @@ -107,6 +107,7 @@ static int has_pit_state2; > > static bool has_msr_mcg_ext_ctl; > > > > static struct kvm_cpuid2 *cpuid_cache; > > +static struct kvm_msr_list *kvm_feature_msrs; > > > > int kvm_has_pit_state2(void) > > { > > @@ -420,6 +421,33 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState > > *s, uint32_t function, > > return ret; > > } > > > > +uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t > > index) > > +{ > > + struct { > > + struct kvm_msrs info; > > + struct kvm_msr_entry entries[1]; > > + } msr_data; > > + uint32_t ret; > > + > > + if (kvm_feature_msrs == NULL) { /*ARCH doesn't support feature > > MSRs*/ > > Nit: normally comments have spaces after "/*" and before "*/". > > Also: what do you mean by "ARCH"? Do you mean "host kernel"? > Going to say "Host". > > > + return 0; > > + } > > + > > + msr_data.info.nmsrs = 1; > > + msr_data.entries[0].index = index; > > + > > + ret = kvm_ioctl(s, KVM_GET_MSRS, &msr_data); > > + > > + if (ret != 1) { > > If the MSR is not supported by the host kernel, it must not be a > fatal error. We should just return 0 on that case. > > Probably the best way to ensure that is to check if the MSR is > listed on kvm_feature_msrs before calling KVM_GET_MSRS (and > return 0 if the MSR is not on the list). Yes. Will do in this way in v5. > > > > + fprintf(stderr, "KVM get MSR (index=0x%x) feature failed, > > %s\n", > > + index, strerror(-ret)); > > Please use error_report() instead of fprintf(stderr). > > > + exit(1); > > I'm unsure if exit(1) is the best option here, but at least this > is consistent with error handling kvm_arch_get_supported_cpuid(). > > > > + } > > + > > + return msr_data.entries[0].data; > > +} > > + > > + > > typedef struct HWPoisonPage { > > ram_addr_t ram_addr; > > QLIST_ENTRY(HWPoisonPage) list; > > @@ -1239,6 +1267,45 @@ void kvm_arch_do_init_vcpu(X86CPU *cpu) > > } > > } > > > > +static int kvm_get_supported_feature_msrs(KVMState *s) > > +{ > > + int ret = 0; > > + > > + if (kvm_feature_msrs != NULL) { > > + return 0; > > + } > > + > > + if (!kvm_check_extension(s, KVM_CAP_GET_MSR_FEATURES)) { > > + return -1; > > There's nothing wrong with not supporting > KVM_CAP_GET_MSR_FEATURES. Why not return 0? > OK > > > + } > > + > > + struct kvm_msr_list msr_list; > > + > > + msr_list.nmsrs = 0; > > + ret = kvm_ioctl(s, KVM_GET_MSR_FEATURE_INDEX_LIST, &msr_list); > > + if (ret < 0 && ret != -E2BIG) { > > You print an error to stderr if (ret < 0) below, but don't print > anything here. Seems inconsistent. Going to be consistent. > > > + return ret; > > + } > > + > > + assert(msr_list.nmsrs > 0); > > + kvm_feature_msrs = (struct kvm_msr_list *) \ > > + g_malloc0(sizeof(msr_list) + > > + msr_list.nmsrs * sizeof(msr_list.indices[0])); > > + > > + kvm_feature_msrs->nmsrs = msr_list.nmsrs; > > + ret = kvm_ioctl(s, KVM_GET_MSR_FEATURE_INDEX_LIST, > > kvm_feature_msrs); > > kvm_arch_get_supported_msr_feature() is only checking if > kvm_feature_msrs is NULL, and nothing else. > > What exactly is the point of calling > KVM_GET_MSR_FEATURE_INDEX_LIST and saving data at > kvm_feature_msrs, if no other code is ever looking at the > returned data? > Now above change make it useful for checking requested feature MSR's validity. > > > + > > + if (ret < 0) { > > + fprintf(stderr, "Fetch KVM feature MSRs failed: %s\n", > > + strerror(-ret)); > > Please use error_report() instead of fprintf(stderr). OK > > > + g_free(kvm_feature_msrs); > > + kvm_feature_msrs = NULL; > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > static int kvm_get_supported_msrs(KVMState *s) > > { > > static int kvm_supported_msrs; > > @@ -1392,6 +1459,11 @@ int kvm_arch_init(MachineState *ms, KVMState > > *s) > > return ret; > > } > > > > + ret = kvm_get_supported_feature_msrs(s); > > + if (ret < 0) { /*if MSR based features aren't supported, > > ignore it.*/ > > + warn_report("Get supported feature MSRs failed."); > > We must not print a warning only because KVM_CAP_GET_MSR_FEATURES > isn't supported by the host kernel. > > If KVM_GET_MSR_FEATURE_INDEX_LIST fails, on the other hand, we > probably should make it a fatal error and not a warning. > OK, remove the check. > > + } > > + > > uname(&utsname); > > lm_capable_kernel = strcmp(utsname.machine, "x86_64") == 0; > > > > -- > > 1.8.3.1 > > > > > >
On Thu, Sep 20, 2018 at 03:45:42PM +0800, Robert Hoo wrote: [...] > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > > > index a252c26..0160e97 100644 > > > --- a/target/i386/cpu.c > > > +++ b/target/i386/cpu.c > > > @@ -3670,7 +3670,7 @@ static uint32_t > > > x86_cpu_get_supported_feature_word(FeatureWord w, > > > bool > > > migratable_only) > > > { > > > FeatureWordInfo *wi = &feature_word_info[w]; > > > - uint32_t r; > > > + uint32_t r = 0; > > > > > > if (kvm_enabled()) { > > > switch (wi->type) { > > > @@ -3679,8 +3679,9 @@ static uint32_t > > > x86_cpu_get_supported_feature_word(FeatureWord w, > > > wi->cpuid.ecx, > > > wi->cpuid.reg); > > > break; > > > - default: > > > - r = 0; > > > + case MSR_FEATURE_WORD: > > > + r = kvm_arch_get_supported_msr_feature(kvm_state, > > > + wi->msr.index); > > > > If you move this patch before patch 1/3, this hunk could be part > > of patch 1/3. > > > I'm afraid that if I moved this hunk, because of the dependency, I > would have to move the definition of > kvm_arch_get_supported_msr_feature() to patch 1/3, then in turn, it > used kvm_feature_msrs, I've to put its definition and initialization > function kvm_get_supported_feature_msrs() to patch 1/3 as well. Then > actually, this makes patch 1/3 and 2/3 merged into 1. Would you like me > to do so? I don't get it. Why would you need to move the definition of kvm_arch_get_supported_cpuid() to patch 1/3, if we change the patch order and apply this patch before patch 1/3?
On Thu, 2018-09-20 at 14:22 -0300, Eduardo Habkost wrote: > On Thu, Sep 20, 2018 at 03:45:42PM +0800, Robert Hoo wrote: > [...] > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > > > > index a252c26..0160e97 100644 > > > > --- a/target/i386/cpu.c > > > > +++ b/target/i386/cpu.c > > > > @@ -3670,7 +3670,7 @@ static uint32_t > > > > x86_cpu_get_supported_feature_word(FeatureWord w, > > > > bool > > > > migratable_only) > > > > { > > > > FeatureWordInfo *wi = &feature_word_info[w]; > > > > - uint32_t r; > > > > + uint32_t r = 0; > > > > > > > > if (kvm_enabled()) { > > > > switch (wi->type) { > > > > @@ -3679,8 +3679,9 @@ static uint32_t > > > > x86_cpu_get_supported_feature_word(FeatureWord w, > > > > wi->cpuid.ecx, > > > > wi- > > > > >cpuid.reg); > > > > break; > > > > - default: > > > > - r = 0; > > > > + case MSR_FEATURE_WORD: > > > > + r = kvm_arch_get_supported_msr_feature(kvm_state, > > > > + wi->msr.index); > > > > > > If you move this patch before patch 1/3, this hunk could be part > > > of patch 1/3. > > > > > > > I'm afraid that if I moved this hunk, because of the dependency, I > > would have to move the definition of > > kvm_arch_get_supported_msr_feature() to patch 1/3, then in turn, it > > used kvm_feature_msrs, I've to put its definition and > > initialization > > function kvm_get_supported_feature_msrs() to patch 1/3 as well. > > Then > > actually, this makes patch 1/3 and 2/3 merged into 1. Would you > > like me > > to do so? > > I don't get it. Why would you need to move the definition of > kvm_arch_get_supported_cpuid() to patch 1/3, if we change the > patch order and apply this patch before patch 1/3? > OK, so you mean simply switch the 1/3 and 2/3 order, not move some hunks of 2/3 into 1/3. Am I right?
On Fri, Sep 21, 2018 at 08:28:24AM +0800, Robert Hoo wrote: > On Thu, 2018-09-20 at 14:22 -0300, Eduardo Habkost wrote: > > On Thu, Sep 20, 2018 at 03:45:42PM +0800, Robert Hoo wrote: > > [...] > > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > > > > > index a252c26..0160e97 100644 > > > > > --- a/target/i386/cpu.c > > > > > +++ b/target/i386/cpu.c > > > > > @@ -3670,7 +3670,7 @@ static uint32_t > > > > > x86_cpu_get_supported_feature_word(FeatureWord w, > > > > > bool > > > > > migratable_only) > > > > > { > > > > > FeatureWordInfo *wi = &feature_word_info[w]; > > > > > - uint32_t r; > > > > > + uint32_t r = 0; > > > > > > > > > > if (kvm_enabled()) { > > > > > switch (wi->type) { > > > > > @@ -3679,8 +3679,9 @@ static uint32_t > > > > > x86_cpu_get_supported_feature_word(FeatureWord w, > > > > > wi->cpuid.ecx, > > > > > wi- > > > > > >cpuid.reg); > > > > > break; > > > > > - default: > > > > > - r = 0; > > > > > + case MSR_FEATURE_WORD: > > > > > + r = kvm_arch_get_supported_msr_feature(kvm_state, > > > > > + wi->msr.index); > > > > > > > > If you move this patch before patch 1/3, this hunk could be part > > > > of patch 1/3. > > > > > > > > > > I'm afraid that if I moved this hunk, because of the dependency, I > > > would have to move the definition of > > > kvm_arch_get_supported_msr_feature() to patch 1/3, then in turn, it > > > used kvm_feature_msrs, I've to put its definition and > > > initialization > > > function kvm_get_supported_feature_msrs() to patch 1/3 as well. > > > Then > > > actually, this makes patch 1/3 and 2/3 merged into 1. Would you > > > like me > > > to do so? > > > > I don't get it. Why would you need to move the definition of > > kvm_arch_get_supported_cpuid() to patch 1/3, if we change the > > patch order and apply this patch before patch 1/3? > > > OK, so you mean simply switch the 1/3 and 2/3 order, not move some > hunks of 2/3 into 1/3. Am I right? Exactly.
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 0b64b8e..97d8d9d 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -463,6 +463,8 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension); uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function, uint32_t index, int reg); +uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index); + void kvm_set_sigmask_len(KVMState *s, unsigned int sigmask_len); diff --git a/target/i386/cpu.c b/target/i386/cpu.c index a252c26..0160e97 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -3670,7 +3670,7 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, bool migratable_only) { FeatureWordInfo *wi = &feature_word_info[w]; - uint32_t r; + uint32_t r = 0; if (kvm_enabled()) { switch (wi->type) { @@ -3679,8 +3679,9 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, wi->cpuid.ecx, wi->cpuid.reg); break; - default: - r = 0; + case MSR_FEATURE_WORD: + r = kvm_arch_get_supported_msr_feature(kvm_state, + wi->msr.index); break; } } else if (hvf_enabled()) { diff --git a/target/i386/kvm.c b/target/i386/kvm.c index 0b2a07d..bfd8088 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -107,6 +107,7 @@ static int has_pit_state2; static bool has_msr_mcg_ext_ctl; static struct kvm_cpuid2 *cpuid_cache; +static struct kvm_msr_list *kvm_feature_msrs; int kvm_has_pit_state2(void) { @@ -420,6 +421,33 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, return ret; } +uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index) +{ + struct { + struct kvm_msrs info; + struct kvm_msr_entry entries[1]; + } msr_data; + uint32_t ret; + + if (kvm_feature_msrs == NULL) { /*ARCH doesn't support feature MSRs*/ + return 0; + } + + msr_data.info.nmsrs = 1; + msr_data.entries[0].index = index; + + ret = kvm_ioctl(s, KVM_GET_MSRS, &msr_data); + + if (ret != 1) { + fprintf(stderr, "KVM get MSR (index=0x%x) feature failed, %s\n", + index, strerror(-ret)); + exit(1); + } + + return msr_data.entries[0].data; +} + + typedef struct HWPoisonPage { ram_addr_t ram_addr; QLIST_ENTRY(HWPoisonPage) list; @@ -1239,6 +1267,45 @@ void kvm_arch_do_init_vcpu(X86CPU *cpu) } } +static int kvm_get_supported_feature_msrs(KVMState *s) +{ + int ret = 0; + + if (kvm_feature_msrs != NULL) { + return 0; + } + + if (!kvm_check_extension(s, KVM_CAP_GET_MSR_FEATURES)) { + return -1; + } + + struct kvm_msr_list msr_list; + + msr_list.nmsrs = 0; + ret = kvm_ioctl(s, KVM_GET_MSR_FEATURE_INDEX_LIST, &msr_list); + if (ret < 0 && ret != -E2BIG) { + return ret; + } + + assert(msr_list.nmsrs > 0); + kvm_feature_msrs = (struct kvm_msr_list *) \ + g_malloc0(sizeof(msr_list) + + msr_list.nmsrs * sizeof(msr_list.indices[0])); + + kvm_feature_msrs->nmsrs = msr_list.nmsrs; + ret = kvm_ioctl(s, KVM_GET_MSR_FEATURE_INDEX_LIST, kvm_feature_msrs); + + if (ret < 0) { + fprintf(stderr, "Fetch KVM feature MSRs failed: %s\n", + strerror(-ret)); + g_free(kvm_feature_msrs); + kvm_feature_msrs = NULL; + return ret; + } + + return 0; +} + static int kvm_get_supported_msrs(KVMState *s) { static int kvm_supported_msrs; @@ -1392,6 +1459,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s) return ret; } + ret = kvm_get_supported_feature_msrs(s); + if (ret < 0) { /*if MSR based features aren't supported, ignore it.*/ + warn_report("Get supported feature MSRs failed."); + } + uname(&utsname); lm_capable_kernel = strcmp(utsname.machine, "x86_64") == 0;
Add kvm_get_supported_feature_msrs() to get supported MSR feature index list. Add kvm_arch_get_supported_msr_feature() to get each MSR features value. Signed-off-by: Robert Hoo <robert.hu@linux.intel.com> --- include/sysemu/kvm.h | 2 ++ target/i386/cpu.c | 7 ++--- target/i386/kvm.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-)