| Message ID | 20210201225404.3941395-2-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | i386: Ensure feature names are always defined | expand |
This is intentional, because there's no way that any hypervisor can run if this feature is disabled. Paolo Il lun 1 feb 2021, 23:54 Eduardo Habkost <ehabkost@redhat.com> ha scritto: > Not having a feature name in feature_word_info breaks error > reporting and query-cpu-model-expansion. Add the missing feature > name to feature_word_info[FEAT_VMX_EPT_VPID_CAPS].feat_names[14]. > > Fixes: 0723cc8a5558 ("target/i386: add VMX features to named CPU models") > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > target/i386/cpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index ae89024d366..2bf3ab78056 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -1262,7 +1262,7 @@ static FeatureWordInfo > feature_word_info[FEATURE_WORDS] = { > "vmx-ept-execonly", NULL, NULL, NULL, > NULL, NULL, "vmx-page-walk-4", "vmx-page-walk-5", > NULL, NULL, NULL, NULL, > - NULL, NULL, NULL, NULL, > + NULL, NULL, "vmx-ept-wb", NULL, > "vmx-ept-2mb", "vmx-ept-1gb", NULL, NULL, > "vmx-invept", "vmx-eptad", "vmx-ept-advanced-exitinfo", NULL, > NULL, "vmx-invept-single-context", "vmx-invept-all-context", > NULL, > -- > 2.28.0 > >
On Mon, Feb 01, 2021 at 11:59:48PM +0100, Paolo Bonzini wrote: > Il lun 1 feb 2021, 23:54 Eduardo Habkost <ehabkost@redhat.com> ha scritto: > > > Not having a feature name in feature_word_info breaks error > > reporting and query-cpu-model-expansion. Add the missing feature > > name to feature_word_info[FEAT_VMX_EPT_VPID_CAPS].feat_names[14]. > > > This is intentional, because there's no way that any hypervisor can run if > this feature is disabled. If leaving the feature without name enables some desirable behavior, that's by accident and not by design. Which part of the existing behavior is intentional?
Il mar 2 feb 2021, 00:05 Eduardo Habkost <ehabkost@redhat.com> ha scritto: > On Mon, Feb 01, 2021 at 11:59:48PM +0100, Paolo Bonzini wrote: > > Il lun 1 feb 2021, 23:54 Eduardo Habkost <ehabkost@redhat.com> ha > scritto: > > > > > Not having a feature name in feature_word_info breaks error > > > reporting and query-cpu-model-expansion. Add the missing feature > > > name to feature_word_info[FEAT_VMX_EPT_VPID_CAPS].feat_names[14]. > > > > > This is intentional, because there's no way that any hypervisor can run > if > > this feature is disabled. > > If leaving the feature without name enables some desirable > behavior, that's by accident and not by design. Which part of > the existing behavior is intentional? > Not being able to disable it. Paolo > -- > Eduardo > >
On Tue, Feb 02, 2021 at 12:28:38AM +0100, Paolo Bonzini wrote: > Il mar 2 feb 2021, 00:05 Eduardo Habkost <ehabkost@redhat.com> ha scritto: > > > On Mon, Feb 01, 2021 at 11:59:48PM +0100, Paolo Bonzini wrote: > > > Il lun 1 feb 2021, 23:54 Eduardo Habkost <ehabkost@redhat.com> ha > > scritto: > > > > > > > Not having a feature name in feature_word_info breaks error > > > > reporting and query-cpu-model-expansion. Add the missing feature > > > > name to feature_word_info[FEAT_VMX_EPT_VPID_CAPS].feat_names[14]. > > > > > > > This is intentional, because there's no way that any hypervisor can run > > if > > > this feature is disabled. > > > > If leaving the feature without name enables some desirable > > behavior, that's by accident and not by design. Which part of > > the existing behavior is intentional? > > > > Not being able to disable it. We can make it a hard dependency of vmx, then. We shouldn't leave it without a name, though.
On 02/02/21 01:18, Eduardo Habkost wrote: > On Tue, Feb 02, 2021 at 12:28:38AM +0100, Paolo Bonzini wrote: >> Il mar 2 feb 2021, 00:05 Eduardo Habkost <ehabkost@redhat.com> ha scritto: >> >>> On Mon, Feb 01, 2021 at 11:59:48PM +0100, Paolo Bonzini wrote: >>>> Il lun 1 feb 2021, 23:54 Eduardo Habkost <ehabkost@redhat.com> ha >>> scritto: >>>> >>>>> Not having a feature name in feature_word_info breaks error >>>>> reporting and query-cpu-model-expansion. Add the missing feature >>>>> name to feature_word_info[FEAT_VMX_EPT_VPID_CAPS].feat_names[14]. >>>>> >>>> This is intentional, because there's no way that any hypervisor can run >>> if >>>> this feature is disabled. >>> >>> If leaving the feature without name enables some desirable >>> behavior, that's by accident and not by design. Which part of >>> the existing behavior is intentional? >>> >> >> Not being able to disable it. > > We can make it a hard dependency of vmx, then. We shouldn't > leave it without a name, though. The feature is already added to the MSRs unconditionally in kvm_msr_entry_add_vmx. I think we can just remove it from the models instead. Paolo
On Tue, Feb 02, 2021 at 08:54:30AM +0100, Paolo Bonzini wrote: > On 02/02/21 01:18, Eduardo Habkost wrote: > > On Tue, Feb 02, 2021 at 12:28:38AM +0100, Paolo Bonzini wrote: > > > Il mar 2 feb 2021, 00:05 Eduardo Habkost <ehabkost@redhat.com> ha scritto: > > > > > > > On Mon, Feb 01, 2021 at 11:59:48PM +0100, Paolo Bonzini wrote: > > > > > Il lun 1 feb 2021, 23:54 Eduardo Habkost <ehabkost@redhat.com> ha > > > > scritto: > > > > > > > > > > > Not having a feature name in feature_word_info breaks error > > > > > > reporting and query-cpu-model-expansion. Add the missing feature > > > > > > name to feature_word_info[FEAT_VMX_EPT_VPID_CAPS].feat_names[14]. > > > > > > > > > > > This is intentional, because there's no way that any hypervisor can run > > > > if > > > > > this feature is disabled. > > > > > > > > If leaving the feature without name enables some desirable > > > > behavior, that's by accident and not by design. Which part of > > > > the existing behavior is intentional? > > > > > > > > > > Not being able to disable it. > > > > We can make it a hard dependency of vmx, then. We shouldn't > > leave it without a name, though. > > The feature is already added to the MSRs unconditionally in > kvm_msr_entry_add_vmx. I think we can just remove it from the models > instead. Sounds even simpler, and better. I'll do it in v2.
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index ae89024d366..2bf3ab78056 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1262,7 +1262,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { "vmx-ept-execonly", NULL, NULL, NULL, NULL, NULL, "vmx-page-walk-4", "vmx-page-walk-5", NULL, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, + NULL, NULL, "vmx-ept-wb", NULL, "vmx-ept-2mb", "vmx-ept-1gb", NULL, NULL, "vmx-invept", "vmx-eptad", "vmx-ept-advanced-exitinfo", NULL, NULL, "vmx-invept-single-context", "vmx-invept-all-context", NULL,
Not having a feature name in feature_word_info breaks error reporting and query-cpu-model-expansion. Add the missing feature name to feature_word_info[FEAT_VMX_EPT_VPID_CAPS].feat_names[14]. Fixes: 0723cc8a5558 ("target/i386: add VMX features to named CPU models") Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target/i386/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)