Re: [PATCH v5 1/4] target/i386: add missing vmx features for several CPU models

[Chenyi Qiang]

On 7/10/2020 6:12 AM, Eduardo Habkost wrote:
> I'm very sorry for taking so long to review this.  Question
> below:
>
> On Fri, Jun 19, 2020 at 03:31:11PM +0800, Chenyi Qiang wrote:
> > Add some missing VMX features in Skylake-Server, Cascadelake-Server and
> > Icelake-Server CPU models based on the output of Paolo's script.
> >
> > Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
>
> Why are you changing the v1 definition instead adding those new
> features in a new version of the CPU model, just like you did in
> patch 3/4?

I suppose these missing vmx features are not quite necessary for 
customers. Just post it here to see if they are worth being added.
Adding a new version is reasonable. Is it appropriate to put all the 
missing features in patch 1/4, 3/4, 4/4 in a same version?

> > ---
> >   target/i386/cpu.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index b1b311baa2..0b309ef3ab 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -3002,6 +3002,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
> >                VMX_SECONDARY_EXEC_RDRAND_EXITING | 
> > VMX_SECONDARY_EXEC_ENABLE_INVPCID |
> >                VMX_SECONDARY_EXEC_ENABLE_VMFUNC | 
> > VMX_SECONDARY_EXEC_SHADOW_VMCS |
> >                VMX_SECONDARY_EXEC_RDSEED_EXITING | 
> > VMX_SECONDARY_EXEC_ENABLE_PML,
> > +        .features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
> >           .xlevel = 0x80000008,
> >           .model_id = "Intel Xeon Processor (Skylake)",
> >           .versions = (X86CPUVersionDefinition[]) {
> > @@ -3130,6 +3131,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
> >                VMX_SECONDARY_EXEC_RDRAND_EXITING | 
> > VMX_SECONDARY_EXEC_ENABLE_INVPCID |
> >                VMX_SECONDARY_EXEC_ENABLE_VMFUNC | 
> > VMX_SECONDARY_EXEC_SHADOW_VMCS |
> >                VMX_SECONDARY_EXEC_RDSEED_EXITING | 
> > VMX_SECONDARY_EXEC_ENABLE_PML,
> > +        .features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
> >           .xlevel = 0x80000008,
> >           .model_id = "Intel Xeon Processor (Cascadelake)",
> >           .versions = (X86CPUVersionDefinition[]) {
> > @@ -3477,7 +3479,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
> >                VMX_SECONDARY_EXEC_APIC_REGISTER_VIRT |
> >                VMX_SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
> >                VMX_SECONDARY_EXEC_RDRAND_EXITING | 
> > VMX_SECONDARY_EXEC_ENABLE_INVPCID |
> > -             VMX_SECONDARY_EXEC_ENABLE_VMFUNC | VMX_SECONDARY_EXEC_SHADOW_VMCS,
> > +             VMX_SECONDARY_EXEC_ENABLE_VMFUNC | VMX_SECONDARY_EXEC_SHADOW_VMCS 
> > |
> > +             VMX_SECONDARY_EXEC_RDSEED_EXITING | VMX_SECONDARY_EXEC_ENABLE_PML,
> > +        .features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
> >           .xlevel = 0x80000008,
> >           .model_id = "Intel Xeon Processor (Icelake)",
> >           .versions = (X86CPUVersionDefinition[]) {
> > --
> > 2.17.1