Re: [PATCH v1 2/3] target/i386: Add VMX control bits for nested FRED support

[Xin Li]

On 8/7/2024 8:58 AM, Zhao Liu wrote:
> On Wed, Aug 07, 2024 at 01:18:11AM -0700, Xin Li (Intel) wrote:
> > @@ -1435,7 +1435,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> >               "vmx-exit-save-efer", "vmx-exit-load-efer",
> >                   "vmx-exit-save-preemption-timer", "vmx-exit-clear-bndcfgs",
> >               NULL, "vmx-exit-clear-rtit-ctl", NULL, NULL,
> > -            NULL, "vmx-exit-load-pkrs", NULL, NULL,
> > +            NULL, "vmx-exit-load-pkrs", NULL, "vmx-exit-secondary-ctls",
>
> Oh, the order of my reviews is mixed up.
> It's better to move VMX_VM_EXIT_ACTIVATE_SECONDARY_CONTROLS into this patch.

Usually a simple definition is added in a patch where it is used, not in
qemu?

> >           },
> >           .msr = {
> >               .index = MSR_IA32_VMX_TRUE_EXIT_CTLS,
> > @@ -1450,7 +1450,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> >               NULL, "vmx-entry-ia32e-mode", NULL, NULL,
> >               NULL, "vmx-entry-load-perf-global-ctrl", "vmx-entry-load-pat", 
> > "vmx-entry-load-efer",
> >               "vmx-entry-load-bndcfgs", NULL, "vmx-entry-load-rtit-ctl", NULL,
> > -            NULL, NULL, "vmx-entry-load-pkrs", NULL,
> > +            NULL, NULL, "vmx-entry-load-pkrs", "vmx-entry-load-fred",
>
> Should we also define VMX_VM_ENTRY_LOAD_FRED? "vmx-entry-load-rtit-ctl"
> and "vmx-entry-load-pkrs" have their corresponding bit definitions, even
> if they are not used.

I'm not sure, but why add something that is not being used (thus not
tested)?