| Message ID | 20210603114835.847451-9-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i386: KVM: expand Hyper-V features early | expand |
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? Is the hyperv_synic_kvm_only check the only reason this was done here and not at kvm_hyperv_properties? > + } > + > /* Not exposed by KVM but needed to make CPU hotplug in Windows work */ > c->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE; > > -- > 2.31.1 >
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 >>
On Thu, Jun 03, 2021 at 01:48:34PM +0200, Vitaly Kuznetsov wrote: In the subject, s/priviliges/privileges/ > 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>
On Fri, Jun 04, 2021 at 09:35:27AM +0200, Vitaly Kuznetsov wrote: > 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. Thanks for the clarification! Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
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; + } + /* Not exposed by KVM but needed to make CPU hotplug in Windows work */ c->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
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(+)