[PATCH 2/3] qemu: Skip validation of VIR_DOMAIN_HYPERV_SYNIC on i440fx

Michal Privoznik via Devel posted 3 patches 3 weeks, 5 days ago
There is a newer version of this series
[PATCH 2/3] qemu: Skip validation of VIR_DOMAIN_HYPERV_SYNIC on i440fx
Posted by Michal Privoznik via Devel 3 weeks, 5 days ago
From: Michal Privoznik <mprivozn@redhat.com>

Turns out, that synic hyperv enlightenment not always requires
vpindex. Some (older) machine types (e.g. pc-i440fx-3.0,
pc-i440fx-rhel7.6.0) can run with synic enabled and vpindex
disabled. This is because they did enable 'x-hv-synic-kvm-only'
CPU property, but starting from QEMU commit v3.1.0-rc0~44^2~9 the
property is disabled by default.

To avoid parsing machine type version, let's just skip this
dependency validation for all i440fx machine types with a note to
remove the limitation once affected machine types go out of
support.

Now, q35 has this dependency, although it might be not visible at
the first glance. Inside of target/i386/kvm/kvm.c there's
kvm_hyperv_properties[] array declared and in there
HYPERV_FEAT_SYNIC doesn't have any .dependencies set. But looking
couple of pages down at kvm_hyperv_expand_features() function,
there's the following check:

    /* Additional dependencies not covered by kvm_hyperv_properties[] */
    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
        !cpu->hyperv_synic_kvm_only &&
        !hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
        error_setg(errp, "Hyper-V %s requires Hyper-V %s",
                   kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
                   kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
        return false;
    }

And as noted above, 'hyperv_synic_kvm_only' is false by default.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/837
Resolves: https://issues.redhat.com/browse/RHEL-138689
Fixes: 1822d030c32d9857020ee8385b0a8808a29a472f
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_validate.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index da08fd17cd..a8b1029d0c 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -112,7 +112,13 @@ qemuValidateDomainDefHypervFeatures(const virDomainDef *def)
         return -1;
     }
 
-    CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_SYNIC, VIR_DOMAIN_HYPERV_VPINDEX);
+    if (!qemuDomainIsI440FX(def)) {
+        /* Some i440fx machine types don't have this dependency and unless we
+         * want to parse machine type version (we do not), just skip this
+         * check. The worst that'll happen is QEMU will error out later.
+         * TODO: Remove once pc-i440fx-3.0 is no longer supported. */
+        CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_SYNIC, VIR_DOMAIN_HYPERV_VPINDEX);
+    }
 
     if (def->hyperv.features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) {
         if (!virDomainDefHasTimer(def, VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK)) {
-- 
2.52.0
Re: [PATCH 2/3] qemu: Skip validation of VIR_DOMAIN_HYPERV_SYNIC on i440fx
Posted by Daniel P. Berrangé via Devel 3 weeks, 5 days ago
On Tue, Jan 06, 2026 at 03:25:17PM +0100, Michal Privoznik via Devel wrote:
> From: Michal Privoznik <mprivozn@redhat.com>
> 
> Turns out, that synic hyperv enlightenment not always requires
> vpindex. Some (older) machine types (e.g. pc-i440fx-3.0,
> pc-i440fx-rhel7.6.0) can run with synic enabled and vpindex
> disabled. This is because they did enable 'x-hv-synic-kvm-only'
> CPU property, but starting from QEMU commit v3.1.0-rc0~44^2~9 the
> property is disabled by default.
> 
> To avoid parsing machine type version, let's just skip this
> dependency validation for all i440fx machine types with a note to
> remove the limitation once affected machine types go out of
> support.

pc-q35-3.0  has the same behaviour as pc-i440fx-3.0, so
it isn't accurate to say the problem is specific to i440fx.
That is only the case in RHEL downstream since it strips
out upstream machine types and none of RHEL q35 machine
types in RHEL-9 are old enough to be affected.

> 
> Now, q35 has this dependency, although it might be not visible at
> the first glance. Inside of target/i386/kvm/kvm.c there's
> kvm_hyperv_properties[] array declared and in there
> HYPERV_FEAT_SYNIC doesn't have any .dependencies set. But looking
> couple of pages down at kvm_hyperv_expand_features() function,
> there's the following check:
> 
>     /* Additional dependencies not covered by kvm_hyperv_properties[] */
>     if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
>         !cpu->hyperv_synic_kvm_only &&
>         !hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
>         error_setg(errp, "Hyper-V %s requires Hyper-V %s",
>                    kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
>                    kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
>         return false;
>     }
> 
> And as noted above, 'hyperv_synic_kvm_only' is false by default.
> 
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/837
> Resolves: https://issues.redhat.com/browse/RHEL-138689
> Fixes: 1822d030c32d9857020ee8385b0a8808a29a472f
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_validate.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index da08fd17cd..a8b1029d0c 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -112,7 +112,13 @@ qemuValidateDomainDefHypervFeatures(const virDomainDef *def)
>          return -1;
>      }
>  
> -    CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_SYNIC, VIR_DOMAIN_HYPERV_VPINDEX);
> +    if (!qemuDomainIsI440FX(def)) {
> +        /* Some i440fx machine types don't have this dependency and unless we
> +         * want to parse machine type version (we do not), just skip this
> +         * check. The worst that'll happen is QEMU will error out later.
> +         * TODO: Remove once pc-i440fx-3.0 is no longer supported. */
> +        CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_SYNIC, VIR_DOMAIN_HYPERV_VPINDEX);
> +    }

I think we need to remove this check entirely until we can depend on a QEMU
upstream version that drops the -3.0 machine types.  This happened in the
QEMU 10.1.0 release which started automatically removing versioned machine
types that were >= 18 versions / 6 years old.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH 2/3] qemu: Skip validation of VIR_DOMAIN_HYPERV_SYNIC on i440fx
Posted by Michal Prívozník via Devel 3 weeks, 5 days ago
On 1/6/26 15:39, Daniel P. Berrangé wrote:
> On Tue, Jan 06, 2026 at 03:25:17PM +0100, Michal Privoznik via Devel wrote:
>> From: Michal Privoznik <mprivozn@redhat.com>
>>
>> Turns out, that synic hyperv enlightenment not always requires
>> vpindex. Some (older) machine types (e.g. pc-i440fx-3.0,
>> pc-i440fx-rhel7.6.0) can run with synic enabled and vpindex
>> disabled. This is because they did enable 'x-hv-synic-kvm-only'
>> CPU property, but starting from QEMU commit v3.1.0-rc0~44^2~9 the
>> property is disabled by default.
>>
>> To avoid parsing machine type version, let's just skip this
>> dependency validation for all i440fx machine types with a note to
>> remove the limitation once affected machine types go out of
>> support.
> 
> pc-q35-3.0  has the same behaviour as pc-i440fx-3.0, so
> it isn't accurate to say the problem is specific to i440fx.
> That is only the case in RHEL downstream since it strips
> out upstream machine types and none of RHEL q35 machine
> types in RHEL-9 are old enough to be affected.

Aaagrh. You're right.

> 
>>
>> Now, q35 has this dependency, although it might be not visible at
>> the first glance. Inside of target/i386/kvm/kvm.c there's
>> kvm_hyperv_properties[] array declared and in there
>> HYPERV_FEAT_SYNIC doesn't have any .dependencies set. But looking
>> couple of pages down at kvm_hyperv_expand_features() function,
>> there's the following check:
>>
>>     /* Additional dependencies not covered by kvm_hyperv_properties[] */
>>     if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
>>         !cpu->hyperv_synic_kvm_only &&
>>         !hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
>>         error_setg(errp, "Hyper-V %s requires Hyper-V %s",
>>                    kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
>>                    kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
>>         return false;
>>     }
>>
>> And as noted above, 'hyperv_synic_kvm_only' is false by default.
>>
>> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/837
>> Resolves: https://issues.redhat.com/browse/RHEL-138689
>> Fixes: 1822d030c32d9857020ee8385b0a8808a29a472f
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/qemu/qemu_validate.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>> index da08fd17cd..a8b1029d0c 100644
>> --- a/src/qemu/qemu_validate.c
>> +++ b/src/qemu/qemu_validate.c
>> @@ -112,7 +112,13 @@ qemuValidateDomainDefHypervFeatures(const virDomainDef *def)
>>          return -1;
>>      }
>>  
>> -    CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_SYNIC, VIR_DOMAIN_HYPERV_VPINDEX);
>> +    if (!qemuDomainIsI440FX(def)) {
>> +        /* Some i440fx machine types don't have this dependency and unless we
>> +         * want to parse machine type version (we do not), just skip this
>> +         * check. The worst that'll happen is QEMU will error out later.
>> +         * TODO: Remove once pc-i440fx-3.0 is no longer supported. */
>> +        CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_SYNIC, VIR_DOMAIN_HYPERV_VPINDEX);
>> +    }
> 
> I think we need to remove this check entirely until we can depend on a QEMU
> upstream version that drops the -3.0 machine types.  This happened in the
> QEMU 10.1.0 release which started automatically removing versioned machine
> types that were >= 18 versions / 6 years old.

Yeah. But bumping min QEMU version in libvirt to 10.1.0 is in far
future. So removing this check makes only sense. Will post v2 shortly.

Michal