diff mbox series

[RFC,12/22] i386: always fill Hyper-V CPUID feature leaves from X86CPU data

Message ID 20200904145431.196885-13-vkuznets@redhat.com
State Superseded
Headers show
Series i386: KVM: expand Hyper-V features early | expand

Commit Message

Vitaly Kuznetsov Sept. 4, 2020, 2:54 p.m. UTC
We have all the required data in X86CPU already and as we are about to
split hyperv_handle_properties() into hyperv_expand_features()/
hyperv_fill_cpuids() we can remove the blind copy. The functional change
is that QEMU won't pass CPUID leaves it doesn't currently know about
to the guest but arguably this is a good change.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 target/i386/kvm.c | 9 ---------
 1 file changed, 9 deletions(-)

Comments

Eduardo Habkost Sept. 18, 2020, 10:32 p.m. UTC | #1
On Fri, Sep 04, 2020 at 04:54:21PM +0200, Vitaly Kuznetsov wrote:
> We have all the required data in X86CPU already and as we are about to
> split hyperv_handle_properties() into hyperv_expand_features()/
> hyperv_fill_cpuids() we can remove the blind copy. The functional change
> is that QEMU won't pass CPUID leaves it doesn't currently know about
> to the guest but arguably this is a good change.

Is it, though?  Maybe we don't want it to be the default, but a
blind passthrough mode that copies all CPUID leaves would be
useful for testing new features.

> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  target/i386/kvm.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 87b83a2aa2cb..b7f0e200a75f 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1221,9 +1221,6 @@ static int hyperv_handle_properties(CPUState *cs,
>      }
>  
>      if (cpu->hyperv_passthrough) {
> -        memcpy(cpuid_ent, &cpuid->entries[0],
> -               cpuid->nent * sizeof(cpuid->entries[0]));
> -
>          c = cpuid_find_entry(cpuid, HV_CPUID_VENDOR_AND_MAX_FUNCTIONS, 0);
>          if (c) {
>              cpu->hyperv_vendor_id[0] = c->ebx;
> @@ -1325,12 +1322,6 @@ static int hyperv_handle_properties(CPUState *cs,
>          goto free;
>      }
>  
> -    if (cpu->hyperv_passthrough) {
> -        /* We already copied all feature words from KVM as is */
> -        r = cpuid->nent;
> -        goto free;
> -    }
> -
>      c = &cpuid_ent[cpuid_i++];
>      c->function = HV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
>      c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?
> -- 
> 2.25.4
>
Vitaly Kuznetsov Sept. 22, 2020, 10:30 a.m. UTC | #2
Eduardo Habkost <ehabkost@redhat.com> writes:

> On Fri, Sep 04, 2020 at 04:54:21PM +0200, Vitaly Kuznetsov wrote:

>> We have all the required data in X86CPU already and as we are about to

>> split hyperv_handle_properties() into hyperv_expand_features()/

>> hyperv_fill_cpuids() we can remove the blind copy. The functional change

>> is that QEMU won't pass CPUID leaves it doesn't currently know about

>> to the guest but arguably this is a good change.

>

> Is it, though?  Maybe we don't want it to be the default, but a

> blind passthrough mode that copies all CPUID leaves would be

> useful for testing new features.

>


We already have Hyper-V features which require extra work to get enabled
(SynIC, EVMCS). Passing corresponding feature bits to guests without
actually enabling these features in KVM will likely break them. So a pure
'blind passthrough' mode is not possible. 

>> 

>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

>> ---

>>  target/i386/kvm.c | 9 ---------

>>  1 file changed, 9 deletions(-)

>> 

>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c

>> index 87b83a2aa2cb..b7f0e200a75f 100644

>> --- a/target/i386/kvm.c

>> +++ b/target/i386/kvm.c

>> @@ -1221,9 +1221,6 @@ static int hyperv_handle_properties(CPUState *cs,

>>      }

>>  

>>      if (cpu->hyperv_passthrough) {

>> -        memcpy(cpuid_ent, &cpuid->entries[0],

>> -               cpuid->nent * sizeof(cpuid->entries[0]));

>> -

>>          c = cpuid_find_entry(cpuid, HV_CPUID_VENDOR_AND_MAX_FUNCTIONS, 0);

>>          if (c) {

>>              cpu->hyperv_vendor_id[0] = c->ebx;

>> @@ -1325,12 +1322,6 @@ static int hyperv_handle_properties(CPUState *cs,

>>          goto free;

>>      }

>>  

>> -    if (cpu->hyperv_passthrough) {

>> -        /* We already copied all feature words from KVM as is */

>> -        r = cpuid->nent;

>> -        goto free;

>> -    }

>> -

>>      c = &cpuid_ent[cpuid_i++];

>>      c->function = HV_CPUID_VENDOR_AND_MAX_FUNCTIONS;

>>      c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?

>> -- 

>> 2.25.4

>> 


-- 
Vitaly
diff mbox series

Patch

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 87b83a2aa2cb..b7f0e200a75f 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1221,9 +1221,6 @@  static int hyperv_handle_properties(CPUState *cs,
     }
 
     if (cpu->hyperv_passthrough) {
-        memcpy(cpuid_ent, &cpuid->entries[0],
-               cpuid->nent * sizeof(cpuid->entries[0]));
-
         c = cpuid_find_entry(cpuid, HV_CPUID_VENDOR_AND_MAX_FUNCTIONS, 0);
         if (c) {
             cpu->hyperv_vendor_id[0] = c->ebx;
@@ -1325,12 +1322,6 @@  static int hyperv_handle_properties(CPUState *cs,
         goto free;
     }
 
-    if (cpu->hyperv_passthrough) {
-        /* We already copied all feature words from KVM as is */
-        r = cpuid->nent;
-        goto free;
-    }
-
     c = &cpuid_ent[cpuid_i++];
     c->function = HV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
     c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?