Currently, we support 'hv-passthrough,hv-feature=on' enablement, this
is supposed to mean "hv-feature is mandatory, don't start without it". Add
support for 'hv-passthrough,hv-feature=off' meaning "enable everything
supported by the host except for hv-feature".
While on it, make 'hv-passthrough' parse semantics in-line with other
options in qemu: when specified, it overrides what was previously set with
what's supported by the host. This can later be modified with 'hv-feature=on'/
'hv-feature=off'.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
target/i386/cpu.c | 28 +++++++++++++++++++++++++++-
target/i386/kvm/kvm.c | 4 ++++
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index e8a004c39d04..f8df2caed779 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4725,6 +4725,29 @@ static void x86_hv_stimer_direct_set(Object *obj, bool value, Error **errp)
x86_hv_feature_set(obj, value, HYPERV_FEAT_STIMER_DIRECT);
}
+static bool x86_hv_passthrough_get(Object *obj, Error **errp)
+{
+ X86CPU *cpu = X86_CPU(obj);
+
+ return cpu->hyperv_passthrough;
+}
+
+static void x86_hv_passthrough_set(Object *obj, bool value, Error **errp)
+{
+ X86CPU *cpu = X86_CPU(obj);
+
+ cpu->hyperv_passthrough = value;
+
+ /* hv-passthrough overrides everything with what's supported by the host */
+ if (value) {
+ cpu->hyperv_features = 0;
+ cpu->hyperv_features_on = 0;
+ cpu->hyperv_features_off = 0;
+ }
+
+ return;
+}
+
/* Generic getter for "feature-words" and "filtered-features" properties */
static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
const char *name, void *opaque,
@@ -7281,7 +7304,6 @@ static Property x86_cpu_properties[] = {
HYPERV_SPINLOCK_NEVER_NOTIFY),
DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
- DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
@@ -7460,6 +7482,10 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
x86_hv_stimer_direct_get,
x86_hv_stimer_direct_set);
+ object_class_property_add_bool(oc, "hv-passthrough",
+ x86_hv_passthrough_get,
+ x86_hv_passthrough_set);
+
for (w = 0; w < FEATURE_WORDS; w++) {
int bitnr;
for (bitnr = 0; bitnr < 64; bitnr++) {
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 30013f0d7cee..fca088d4d3b5 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1153,6 +1153,10 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature, Error **errp)
return 0;
}
+ if (cpu->hyperv_passthrough && (cpu->hyperv_features_off & BIT(feature))) {
+ return 0;
+ }
+
deps = kvm_hyperv_properties[feature].dependencies;
while (deps) {
dep_feat = ctz64(deps);
--
2.29.2
On Wed, 10 Feb 2021 17:40:29 +0100
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> Currently, we support 'hv-passthrough,hv-feature=on' enablement, this
> is supposed to mean "hv-feature is mandatory, don't start without it". Add
> support for 'hv-passthrough,hv-feature=off' meaning "enable everything
> supported by the host except for hv-feature".
>
> While on it, make 'hv-passthrough' parse semantics in-line with other
> options in qemu: when specified, it overrides what was previously set with
> what's supported by the host. This can later be modified with 'hv-feature=on'/
> 'hv-feature=off'.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> target/i386/cpu.c | 28 +++++++++++++++++++++++++++-
> target/i386/kvm/kvm.c | 4 ++++
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index e8a004c39d04..f8df2caed779 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4725,6 +4725,29 @@ static void x86_hv_stimer_direct_set(Object *obj, bool value, Error **errp)
> x86_hv_feature_set(obj, value, HYPERV_FEAT_STIMER_DIRECT);
> }
>
> +static bool x86_hv_passthrough_get(Object *obj, Error **errp)
> +{
> + X86CPU *cpu = X86_CPU(obj);
> +
> + return cpu->hyperv_passthrough;
> +}
> +
> +static void x86_hv_passthrough_set(Object *obj, bool value, Error **errp)
> +{
> + X86CPU *cpu = X86_CPU(obj);
> +
> + cpu->hyperv_passthrough = value;
> +
> + /* hv-passthrough overrides everything with what's supported by the host */
> + if (value) {
> + cpu->hyperv_features = 0;
> + cpu->hyperv_features_on = 0;
> + cpu->hyperv_features_off = 0;
why do we have _on|_off fields?
> + }
> +
> + return;
> +}
> +
> /* Generic getter for "feature-words" and "filtered-features" properties */
> static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
> const char *name, void *opaque,
> @@ -7281,7 +7304,6 @@ static Property x86_cpu_properties[] = {
> HYPERV_SPINLOCK_NEVER_NOTIFY),
> DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
> hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
> - DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
>
> DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
> DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
> @@ -7460,6 +7482,10 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> x86_hv_stimer_direct_get,
> x86_hv_stimer_direct_set);
>
> + object_class_property_add_bool(oc, "hv-passthrough",
> + x86_hv_passthrough_get,
> + x86_hv_passthrough_set);
> +
> for (w = 0; w < FEATURE_WORDS; w++) {
> int bitnr;
> for (bitnr = 0; bitnr < 64; bitnr++) {
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 30013f0d7cee..fca088d4d3b5 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -1153,6 +1153,10 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature, Error **errp)
> return 0;
> }
>
> + if (cpu->hyperv_passthrough && (cpu->hyperv_features_off & BIT(feature))) {
> + return 0;
> + }
> +
> deps = kvm_hyperv_properties[feature].dependencies;
> while (deps) {
> dep_feat = ctz64(deps);
Igor Mammedov <imammedo@redhat.com> writes:
> On Wed, 10 Feb 2021 17:40:29 +0100
> Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
>> Currently, we support 'hv-passthrough,hv-feature=on' enablement, this
>> is supposed to mean "hv-feature is mandatory, don't start without it". Add
>> support for 'hv-passthrough,hv-feature=off' meaning "enable everything
>> supported by the host except for hv-feature".
>>
>> While on it, make 'hv-passthrough' parse semantics in-line with other
>> options in qemu: when specified, it overrides what was previously set with
>> what's supported by the host. This can later be modified with 'hv-feature=on'/
>> 'hv-feature=off'.
>>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>> target/i386/cpu.c | 28 +++++++++++++++++++++++++++-
>> target/i386/kvm/kvm.c | 4 ++++
>> 2 files changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index e8a004c39d04..f8df2caed779 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -4725,6 +4725,29 @@ static void x86_hv_stimer_direct_set(Object *obj, bool value, Error **errp)
>> x86_hv_feature_set(obj, value, HYPERV_FEAT_STIMER_DIRECT);
>> }
>>
>> +static bool x86_hv_passthrough_get(Object *obj, Error **errp)
>> +{
>> + X86CPU *cpu = X86_CPU(obj);
>> +
>> + return cpu->hyperv_passthrough;
>> +}
>> +
>> +static void x86_hv_passthrough_set(Object *obj, bool value, Error **errp)
>> +{
>> + X86CPU *cpu = X86_CPU(obj);
>> +
>> + cpu->hyperv_passthrough = value;
>> +
>> + /* hv-passthrough overrides everything with what's supported by the host */
>> + if (value) {
>> + cpu->hyperv_features = 0;
>> + cpu->hyperv_features_on = 0;
>> + cpu->hyperv_features_off = 0;
>
> why do we have _on|_off fields?
>
You mean 'why do we have them at all' or 'why do we reset them here'?
For the former, we need to distinguish between
'hv-passthroug,hv-feature=off' and just 'hv-passthrough';
'hv-passthrough,hv-evmcs=on' and just 'hv-passthrough'. For the later,
I'd like to make the samentics you've asked for:
'hv-feature,hv-passthrough' == 'hv-passthrough'
(though I still see it as a gotcha for an unprepared user)
>> + }
>> +
>> + return;
>> +}
>> +
>> /* Generic getter for "feature-words" and "filtered-features" properties */
>> static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
>> const char *name, void *opaque,
>> @@ -7281,7 +7304,6 @@ static Property x86_cpu_properties[] = {
>> HYPERV_SPINLOCK_NEVER_NOTIFY),
>> DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
>> hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
>> - DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
>>
>> DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
>> DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>> @@ -7460,6 +7482,10 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>> x86_hv_stimer_direct_get,
>> x86_hv_stimer_direct_set);
>>
>> + object_class_property_add_bool(oc, "hv-passthrough",
>> + x86_hv_passthrough_get,
>> + x86_hv_passthrough_set);
>> +
>> for (w = 0; w < FEATURE_WORDS; w++) {
>> int bitnr;
>> for (bitnr = 0; bitnr < 64; bitnr++) {
>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
>> index 30013f0d7cee..fca088d4d3b5 100644
>> --- a/target/i386/kvm/kvm.c
>> +++ b/target/i386/kvm/kvm.c
>> @@ -1153,6 +1153,10 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature, Error **errp)
>> return 0;
>> }
>>
>> + if (cpu->hyperv_passthrough && (cpu->hyperv_features_off & BIT(feature))) {
>> + return 0;
>> + }
>> +
>> deps = kvm_hyperv_properties[feature].dependencies;
>> while (deps) {
>> dep_feat = ctz64(deps);
>
--
Vitaly
On Friday, 2021-02-12 at 09:49:46 +01, Vitaly Kuznetsov wrote:
> Igor Mammedov <imammedo@redhat.com> writes:
>
>> On Wed, 10 Feb 2021 17:40:29 +0100
>> Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>>> Currently, we support 'hv-passthrough,hv-feature=on' enablement, this
>>> is supposed to mean "hv-feature is mandatory, don't start without it". Add
>>> support for 'hv-passthrough,hv-feature=off' meaning "enable everything
>>> supported by the host except for hv-feature".
>>>
>>> While on it, make 'hv-passthrough' parse semantics in-line with other
>>> options in qemu: when specified, it overrides what was previously set with
>>> what's supported by the host. This can later be modified with 'hv-feature=on'/
>>> 'hv-feature=off'.
>>>
>>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>>> ---
>>> target/i386/cpu.c | 28 +++++++++++++++++++++++++++-
>>> target/i386/kvm/kvm.c | 4 ++++
>>> 2 files changed, 31 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>>> index e8a004c39d04..f8df2caed779 100644
>>> --- a/target/i386/cpu.c
>>> +++ b/target/i386/cpu.c
>>> @@ -4725,6 +4725,29 @@ static void x86_hv_stimer_direct_set(Object *obj, bool value, Error **errp)
>>> x86_hv_feature_set(obj, value, HYPERV_FEAT_STIMER_DIRECT);
>>> }
>>>
>>> +static bool x86_hv_passthrough_get(Object *obj, Error **errp)
>>> +{
>>> + X86CPU *cpu = X86_CPU(obj);
>>> +
>>> + return cpu->hyperv_passthrough;
>>> +}
>>> +
>>> +static void x86_hv_passthrough_set(Object *obj, bool value, Error **errp)
>>> +{
>>> + X86CPU *cpu = X86_CPU(obj);
>>> +
>>> + cpu->hyperv_passthrough = value;
>>> +
>>> + /* hv-passthrough overrides everything with what's supported by the host */
>>> + if (value) {
>>> + cpu->hyperv_features = 0;
>>> + cpu->hyperv_features_on = 0;
>>> + cpu->hyperv_features_off = 0;
>>
>> why do we have _on|_off fields?
>>
>
> You mean 'why do we have them at all' or 'why do we reset them here'?
> For the former, we need to distinguish between
> 'hv-passthroug,hv-feature=off' and just 'hv-passthrough';
> 'hv-passthrough,hv-evmcs=on' and just 'hv-passthrough'. For the later,
> I'd like to make the samentics you've asked for:
> 'hv-feature,hv-passthrough' == 'hv-passthrough'
> (though I still see it as a gotcha for an unprepared user)
Either approach will confuse *someone*, I think.
This way at least behaves better if someone/something is composing the
feature strings via concatenation.
>>> + }
>>> +
>>> + return;
>>> +}
>>> +
>>> /* Generic getter for "feature-words" and "filtered-features" properties */
>>> static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
>>> const char *name, void *opaque,
>>> @@ -7281,7 +7304,6 @@ static Property x86_cpu_properties[] = {
>>> HYPERV_SPINLOCK_NEVER_NOTIFY),
>>> DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
>>> hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
>>> - DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
>>>
>>> DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
>>> DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>>> @@ -7460,6 +7482,10 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>>> x86_hv_stimer_direct_get,
>>> x86_hv_stimer_direct_set);
>>>
>>> + object_class_property_add_bool(oc, "hv-passthrough",
>>> + x86_hv_passthrough_get,
>>> + x86_hv_passthrough_set);
>>> +
>>> for (w = 0; w < FEATURE_WORDS; w++) {
>>> int bitnr;
>>> for (bitnr = 0; bitnr < 64; bitnr++) {
>>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
>>> index 30013f0d7cee..fca088d4d3b5 100644
>>> --- a/target/i386/kvm/kvm.c
>>> +++ b/target/i386/kvm/kvm.c
>>> @@ -1153,6 +1153,10 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature, Error **errp)
>>> return 0;
>>> }
>>>
>>> + if (cpu->hyperv_passthrough && (cpu->hyperv_features_off & BIT(feature))) {
>>> + return 0;
>>> + }
>>> +
>>> deps = kvm_hyperv_properties[feature].dependencies;
>>> while (deps) {
>>> dep_feat = ctz64(deps);
>>
>
> --
> Vitaly
dme.
--
J'aurais toujours faim de toi.
On Fri, 12 Feb 2021 09:49:46 +0100
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> Igor Mammedov <imammedo@redhat.com> writes:
>
> > On Wed, 10 Feb 2021 17:40:29 +0100
> > Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> >
> >> Currently, we support 'hv-passthrough,hv-feature=on' enablement, this
> >> is supposed to mean "hv-feature is mandatory, don't start without it". Add
> >> support for 'hv-passthrough,hv-feature=off' meaning "enable everything
> >> supported by the host except for hv-feature".
> >>
> >> While on it, make 'hv-passthrough' parse semantics in-line with other
> >> options in qemu: when specified, it overrides what was previously set with
> >> what's supported by the host. This can later be modified with 'hv-feature=on'/
> >> 'hv-feature=off'.
> >>
> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> ---
> >> target/i386/cpu.c | 28 +++++++++++++++++++++++++++-
> >> target/i386/kvm/kvm.c | 4 ++++
> >> 2 files changed, 31 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> >> index e8a004c39d04..f8df2caed779 100644
> >> --- a/target/i386/cpu.c
> >> +++ b/target/i386/cpu.c
> >> @@ -4725,6 +4725,29 @@ static void x86_hv_stimer_direct_set(Object *obj, bool value, Error **errp)
> >> x86_hv_feature_set(obj, value, HYPERV_FEAT_STIMER_DIRECT);
> >> }
> >>
> >> +static bool x86_hv_passthrough_get(Object *obj, Error **errp)
> >> +{
> >> + X86CPU *cpu = X86_CPU(obj);
> >> +
> >> + return cpu->hyperv_passthrough;
> >> +}
> >> +
> >> +static void x86_hv_passthrough_set(Object *obj, bool value, Error **errp)
> >> +{
> >> + X86CPU *cpu = X86_CPU(obj);
> >> +
> >> + cpu->hyperv_passthrough = value;
> >> +
> >> + /* hv-passthrough overrides everything with what's supported by the host */
> >> + if (value) {
> >> + cpu->hyperv_features = 0;
> >> + cpu->hyperv_features_on = 0;
> >> + cpu->hyperv_features_off = 0;
> >
> > why do we have _on|_off fields?
> >
>
> You mean 'why do we have them at all' or 'why do we reset them here'?
> For the former, we need to distinguish between
> 'hv-passthroug,hv-feature=off' and just 'hv-passthrough';
> 'hv-passthrough,hv-evmcs=on' and just 'hv-passthrough'. For the later,
that's what I was asking for, eventually I found it being buried in kvm code
> I'd like to make the samentics you've asked for:
> 'hv-feature,hv-passthrough' == 'hv-passthrough'
you essentially you wrote your own hv-foo parser in kvm_hyperv_expand_features(),
which is a bit complicated for my taste.
With scratch CPU you can simplify and make it easier to read.
> (though I still see it as a gotcha for an unprepared user)
this way it at least works the same way like any other property.
> >> + }
> >> +
> >> + return;
> >> +}
> >> +
> >> /* Generic getter for "feature-words" and "filtered-features" properties */
> >> static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
> >> const char *name, void *opaque,
> >> @@ -7281,7 +7304,6 @@ static Property x86_cpu_properties[] = {
> >> HYPERV_SPINLOCK_NEVER_NOTIFY),
> >> DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
> >> hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
> >> - DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
> >>
> >> DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
> >> DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
> >> @@ -7460,6 +7482,10 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> >> x86_hv_stimer_direct_get,
> >> x86_hv_stimer_direct_set);
> >>
> >> + object_class_property_add_bool(oc, "hv-passthrough",
> >> + x86_hv_passthrough_get,
> >> + x86_hv_passthrough_set);
> >> +
> >> for (w = 0; w < FEATURE_WORDS; w++) {
> >> int bitnr;
> >> for (bitnr = 0; bitnr < 64; bitnr++) {
> >> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> >> index 30013f0d7cee..fca088d4d3b5 100644
> >> --- a/target/i386/kvm/kvm.c
> >> +++ b/target/i386/kvm/kvm.c
> >> @@ -1153,6 +1153,10 @@ static int hv_cpuid_check_and_set(CPUState *cs, int feature, Error **errp)
> >> return 0;
> >> }
> >>
> >> + if (cpu->hyperv_passthrough && (cpu->hyperv_features_off & BIT(feature))) {
> >> + return 0;
> >> + }
> >> +
> >> deps = kvm_hyperv_properties[feature].dependencies;
> >> while (deps) {
> >> dep_feat = ctz64(deps);
> >
>
© 2016 - 2026 Red Hat, Inc.