On Fri, Jun 04, 2021 at 09:35:27AM +0200, Vitaly Kuznetsov wrote:
> Eduardo Habkost <ehabk...@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 <vkuzn...@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.
Thanks for the clarification!
Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
--
Eduardo
