diff mbox series

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

Message ID 20200619073114.24303-2-chenyi.qiang@intel.com (mailing list archive)
State New, archived
Headers show
Series modify CPU model info | expand

Commit Message

Chenyi Qiang June 19, 2020, 7:31 a.m. UTC
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>
---
 target/i386/cpu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Eduardo Habkost July 9, 2020, 10:12 p.m. UTC | #1
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?

> ---
>  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
> 
>
Chenyi Qiang July 10, 2020, 1:45 a.m. UTC | #2
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
>>
>>
>
Eduardo Habkost July 10, 2020, 4:48 p.m. UTC | #3
On Fri, Jul 10, 2020 at 09:45:49AM +0800, Chenyi Qiang wrote:
> 
> 
> 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?

Yes, it would be OK to add only one new version with all the new
features.
Chenyi Qiang July 11, 2020, 2 a.m. UTC | #4
On 7/11/2020 12:48 AM, Eduardo Habkost wrote:
> On Fri, Jul 10, 2020 at 09:45:49AM +0800, Chenyi Qiang wrote:
>>
>>
>> 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?
> 
> Yes, it would be OK to add only one new version with all the new
> features.
> 

OK, I'll do it in the next version of patch.
Chenyi Qiang July 13, 2020, 7:23 a.m. UTC | #5
On 7/11/2020 12:48 AM, Eduardo Habkost wrote:
> On Fri, Jul 10, 2020 at 09:45:49AM +0800, Chenyi Qiang wrote:
>>
>>
>> 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?
> 
> Yes, it would be OK to add only one new version with all the new
> features.
> 

During the coding, I prefer to split the missing vmx features into a new 
version of CPU model, because the vmx features depends on CPUID_EXT_VMX. 
I think It would be better to distinguish it instead of enabling the vmx 
transparently. i.e.
{
	.version = 4,
	.props = (PropValue[]) {
		{ "sha-ni", "on" },
		... ...
		{ "model", "106" },
                 { /* end of list */ }
	},
},
{
	.version = 5,
	.props = (PropValue[]) {
		{ "vmx", "on" },
                 { "vmx-eptp-switching", "on" },
                 { /* end of list */ }
	},
},

What do you think about?
Xiaoyao Li July 13, 2020, 7:45 a.m. UTC | #6
On 7/13/2020 3:23 PM, Chenyi Qiang wrote:
> 
> 
> On 7/11/2020 12:48 AM, Eduardo Habkost wrote:
>> On Fri, Jul 10, 2020 at 09:45:49AM +0800, Chenyi Qiang wrote:
>>>
>>>
>>> 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?
>>
>> Yes, it would be OK to add only one new version with all the new
>> features.
>>
> 
> During the coding, I prefer to split the missing vmx features into a new 
> version of CPU model, because the vmx features depends on CPUID_EXT_VMX. 
> I think It would be better to distinguish it instead of enabling the vmx 
> transparently. i.e.
> {
>      .version = 4,
>      .props = (PropValue[]) {
>          { "sha-ni", "on" },
>          ... ...
>          { "model", "106" },
>                  { /* end of list */ }
>      },
> },
> {
>      .version = 5,
>      .props = (PropValue[]) {
>          { "vmx", "on" }

Chenyi,

This is not we have discussed. I prefer to changing the logic of 
versioned CPU model to not add the features in versioned CPU model to 
env->user_features[]. They're not supposed to be added to 
env->user_features[] since they're not set by user through -feature/+feature

Eduardo,

What do you think?
Eduardo Habkost July 13, 2020, 2:44 p.m. UTC | #7
On Mon, Jul 13, 2020 at 03:45:55PM +0800, Xiaoyao Li wrote:
> On 7/13/2020 3:23 PM, Chenyi Qiang wrote:
> > 
> > 
> > On 7/11/2020 12:48 AM, Eduardo Habkost wrote:
> > > On Fri, Jul 10, 2020 at 09:45:49AM +0800, Chenyi Qiang wrote:
> > > > 
> > > > 
> > > > 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?
> > > 
> > > Yes, it would be OK to add only one new version with all the new
> > > features.
> > > 
> > 
> > During the coding, I prefer to split the missing vmx features into a new
> > version of CPU model, because the vmx features depends on CPUID_EXT_VMX.
> > I think It would be better to distinguish it instead of enabling the vmx
> > transparently. i.e.
> > {
> >      .version = 4,
> >      .props = (PropValue[]) {
> >          { "sha-ni", "on" },
> >          ... ...
> >          { "model", "106" },
> >                  { /* end of list */ }
> >      },
> > },
> > {
> >      .version = 5,
> >      .props = (PropValue[]) {
> >          { "vmx", "on" }
> 
> Chenyi,
> 
> This is not we have discussed. I prefer to changing the logic of versioned
> CPU model to not add the features in versioned CPU model to
> env->user_features[]. They're not supposed to be added to
> env->user_features[] since they're not set by user through -feature/+feature
> 
> Eduardo,
> 
> What do you think?

If features added by the CPU model versions appear in
user_features, that's a bug.  What's the user-visible symptom you
are seeing because of it?
Xiaoyao Li July 13, 2020, 3:07 p.m. UTC | #8
On 7/13/2020 10:44 PM, Eduardo Habkost wrote:
> On Mon, Jul 13, 2020 at 03:45:55PM +0800, Xiaoyao Li wrote:
>> On 7/13/2020 3:23 PM, Chenyi Qiang wrote:
>>>
>>>
>>> On 7/11/2020 12:48 AM, Eduardo Habkost wrote:
>>>> On Fri, Jul 10, 2020 at 09:45:49AM +0800, Chenyi Qiang wrote:
>>>>>
>>>>>
>>>>> 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?
>>>>
>>>> Yes, it would be OK to add only one new version with all the new
>>>> features.
>>>>
>>>
>>> During the coding, I prefer to split the missing vmx features into a new
>>> version of CPU model, because the vmx features depends on CPUID_EXT_VMX.
>>> I think It would be better to distinguish it instead of enabling the vmx
>>> transparently. i.e.
>>> {
>>>       .version = 4,
>>>       .props = (PropValue[]) {
>>>           { "sha-ni", "on" },
>>>           ... ...
>>>           { "model", "106" },
>>>                   { /* end of list */ }
>>>       },
>>> },
>>> {
>>>       .version = 5,
>>>       .props = (PropValue[]) {
>>>           { "vmx", "on" }
>>
>> Chenyi,
>>
>> This is not we have discussed. I prefer to changing the logic of versioned
>> CPU model to not add the features in versioned CPU model to
>> env->user_features[]. They're not supposed to be added to
>> env->user_features[] since they're not set by user through -feature/+feature
>>
>> Eduardo,
>>
>> What do you think?
> 
> If features added by the CPU model versions appear in
> user_features, that's a bug.  What's the user-visible symptom you
> are seeing because of it?
> 

It's the vmx features that the PATCH 1 wants to add. They require VMX to 
be there because feature_dependencies[] checking in 
x86_cpu_expand_features().

Paolo didn't met this issue because he added VMX features to named CPU 
models directly without adding new versions. Chenyi have to deal with it 
since you require them to be added in a new version. He wants to add 
{vmx, on} in the new version to avoid the warning. But I don't think 
that's a good idea since other CPU models don't have VMX.
Eduardo Habkost July 13, 2020, 4:35 p.m. UTC | #9
On Mon, Jul 13, 2020 at 11:07:41PM +0800, Xiaoyao Li wrote:
> On 7/13/2020 10:44 PM, Eduardo Habkost wrote:
> > On Mon, Jul 13, 2020 at 03:45:55PM +0800, Xiaoyao Li wrote:
> > > On 7/13/2020 3:23 PM, Chenyi Qiang wrote:
> > > > 
> > > > 
> > > > On 7/11/2020 12:48 AM, Eduardo Habkost wrote:
> > > > > On Fri, Jul 10, 2020 at 09:45:49AM +0800, Chenyi Qiang wrote:
> > > > > > 
> > > > > > 
> > > > > > 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?
> > > > > 
> > > > > Yes, it would be OK to add only one new version with all the new
> > > > > features.
> > > > > 
> > > > 
> > > > During the coding, I prefer to split the missing vmx features into a new
> > > > version of CPU model, because the vmx features depends on CPUID_EXT_VMX.
> > > > I think It would be better to distinguish it instead of enabling the vmx
> > > > transparently. i.e.
> > > > {
> > > >       .version = 4,
> > > >       .props = (PropValue[]) {
> > > >           { "sha-ni", "on" },
> > > >           ... ...
> > > >           { "model", "106" },
> > > >                   { /* end of list */ }
> > > >       },
> > > > },
> > > > {
> > > >       .version = 5,
> > > >       .props = (PropValue[]) {
> > > >           { "vmx", "on" }
> > > 
> > > Chenyi,
> > > 
> > > This is not we have discussed. I prefer to changing the logic of versioned
> > > CPU model to not add the features in versioned CPU model to
> > > env->user_features[]. They're not supposed to be added to
> > > env->user_features[] since they're not set by user through -feature/+feature
> > > 
> > > Eduardo,
> > > 
> > > What do you think?
> > 
> > If features added by the CPU model versions appear in
> > user_features, that's a bug.  What's the user-visible symptom you
> > are seeing because of it?
> > 
> 
> It's the vmx features that the PATCH 1 wants to add. They require VMX to be
> there because feature_dependencies[] checking in x86_cpu_expand_features().
> 
> Paolo didn't met this issue because he added VMX features to named CPU
> models directly without adding new versions. Chenyi have to deal with it
> since you require them to be added in a new version. He wants to add {vmx,
> on} in the new version to avoid the warning. But I don't think that's a good
> idea since other CPU models don't have VMX.

Right, we don't add VMX by default to any CPU models (even if
they are separate versions).

I think I see the issue, now: adding vmx-* to a new CPU version
will trigger the feature dependency warnings because VMX is
disabled by default.  This is a bug we must fix.  It should be
enough to simply clear env->user_features[] at the end of
x86_cpu_load_model().
diff mbox series

Patch

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[]) {