Re: [PATCH v7 8/9] i386: Hyper-V SynIC requires POST_MESSAGES/SIGNAL_EVENTS priviliges

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Thu, Jun 03, 2021 at 01:48:34PM +0200, Vitaly Kuznetsov wrote:
>> When Hyper-V SynIC is enabled, we may need to allow Windows guests to make
>> hypercalls (POST_MESSAGES/SIGNAL_EVENTS). No issue is currently observed
>> because KVM is very permissive, allowing these hypercalls regarding of
>> guest visible CPUid bits.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>>  target/i386/kvm/hyperv-proto.h | 6 ++++++
>>  target/i386/kvm/kvm.c          | 6 ++++++
>>  2 files changed, 12 insertions(+)
>> 
>> diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h
>> index e30d64b4ade4..5fbb385cc136 100644
>> --- a/target/i386/kvm/hyperv-proto.h
>> +++ b/target/i386/kvm/hyperv-proto.h
>> @@ -38,6 +38,12 @@
>>  #define HV_ACCESS_FREQUENCY_MSRS     (1u << 11)
>>  #define HV_ACCESS_REENLIGHTENMENTS_CONTROL  (1u << 13)
>>  
>> +/*
>> + * HV_CPUID_FEATURES.EBX bits
>> + */
>> +#define HV_POST_MESSAGES             (1u << 4)
>> +#define HV_SIGNAL_EVENTS             (1u << 5)
>> +
>>  /*
>>   * HV_CPUID_FEATURES.EDX bits
>>   */
>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
>> index a3897d4d8788..6a32d43e6ec1 100644
>> --- a/target/i386/kvm/kvm.c
>> +++ b/target/i386/kvm/kvm.c
>> @@ -1343,6 +1343,12 @@ static int hyperv_fill_cpuids(CPUState *cs,
>>      /* Unconditionally required with any Hyper-V enlightenment */
>>      c->eax |= HV_HYPERCALL_AVAILABLE;
>>  
>> +    /* SynIC and Vmbus devices require messages/signals hypercalls */
>> +    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
>> +        !cpu->hyperv_synic_kvm_only) {
>> +        c->ebx |= HV_POST_MESSAGES | HV_SIGNAL_EVENTS;
>
> Why exactly is the hyperv_synic_kvm_only check needed?
>

'hyperv_synic_kvm_only' means SynIC is only used for in-KVM stimers and
in this case Post Messages / Signal Events hypercalls are not used. KVM
will also return an error to the guest directly (as it can't forward
them to QEMU). No need to expose HV_POST_MESSAGES | HV_SIGNAL_EVENTS.

> Is the hyperv_synic_kvm_only check the only reason this was done
> here and not at kvm_hyperv_properties?
>

Yes, basically.

>
>> +    }
>> +
>>      /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
>>      c->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
>>  
>> -- 
>> 2.31.1
>> 

-- 
Vitaly