Use the runtime kvm_enabled() helper to check whether
KVM is available or not.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/pci.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index fdbc15885d4..9872884ff8a 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -118,8 +118,13 @@ static void vfio_intx_eoi(VFIODevice *vbasedev)
 
 static bool vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
 {
-#ifdef CONFIG_KVM
-    int irq_fd = event_notifier_get_fd(&vdev->intx.interrupt);
+    int irq_fd;
+
+    if (!kvm_enabled()) {
+        return true;
+    }
+
+    irq_fd = event_notifier_get_fd(&vdev->intx.interrupt);
 
     if (vdev->no_kvm_intx || !kvm_irqfds_enabled() ||
         vdev->intx.route.mode != PCI_INTX_ENABLED ||
@@ -171,16 +176,13 @@ fail_irqfd:
 fail:
     qemu_set_fd_handler(irq_fd, vfio_intx_interrupt, NULL, vdev);
     vfio_unmask_single_irqindex(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX);
+
     return false;
-#else
-    return true;
-#endif
 }
 
 static void vfio_intx_disable_kvm(VFIOPCIDevice *vdev)
 {
-#ifdef CONFIG_KVM
-    if (!vdev->intx.kvm_accel) {
+    if (!kvm_enabled() || !vdev->intx.kvm_accel) {
         return;
     }
 
@@ -211,7 +213,6 @@ static void vfio_intx_disable_kvm(VFIOPCIDevice *vdev)
     vfio_unmask_single_irqindex(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX);
 
     trace_vfio_intx_disable_kvm(vdev->vbasedev.name);
-#endif
 }
 
 static void vfio_intx_update(VFIOPCIDevice *vdev, PCIINTxRoute *route)
@@ -278,7 +279,6 @@ static bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
     vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
     pci_config_set_interrupt_pin(vdev->pdev.config, pin);
 
-#ifdef CONFIG_KVM
     /*
      * Only conditional to avoid generating error messages on platforms
      * where we won't actually use the result anyway.
@@ -287,7 +287,6 @@ static bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
         vdev->intx.route = pci_device_route_intx_to_irq(&vdev->pdev,
                                                         vdev->intx.pin);
     }
-#endif
 
     ret = event_notifier_init(&vdev->intx.interrupt, 0);
     if (ret) {
-- 
2.47.1

Hi Philippe,

On 3/9/25 12:09 AM, Philippe Mathieu-Daudé wrote:
> Use the runtime kvm_enabled() helper to check whether
> KVM is available or not.

Miss the "why" of this patch.

By the way I fail to remember/see where kvm_allowed is set.

I am also confused because we still have some code, like in
vfio/common.c which does both checks:
#ifdef CONFIG_KVM
        if (kvm_enabled()) {
            max_memslots = kvm_get_max_memslots();
        }
#endif


Thanks

Eric

>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> ---
>  hw/vfio/pci.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index fdbc15885d4..9872884ff8a 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -118,8 +118,13 @@ static void vfio_intx_eoi(VFIODevice *vbasedev)
>  
>  static bool vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
>  {
> -#ifdef CONFIG_KVM
> -    int irq_fd = event_notifier_get_fd(&vdev->intx.interrupt);
> +    int irq_fd;
> +
> +    if (!kvm_enabled()) {
> +        return true;
> +    }
> +
> +    irq_fd = event_notifier_get_fd(&vdev->intx.interrupt);
>  
>      if (vdev->no_kvm_intx || !kvm_irqfds_enabled() ||
>          vdev->intx.route.mode != PCI_INTX_ENABLED ||
> @@ -171,16 +176,13 @@ fail_irqfd:
>  fail:
>      qemu_set_fd_handler(irq_fd, vfio_intx_interrupt, NULL, vdev);
>      vfio_unmask_single_irqindex(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX);
> +
>      return false;
> -#else
> -    return true;
> -#endif
>  }
>  
>  static void vfio_intx_disable_kvm(VFIOPCIDevice *vdev)
>  {
> -#ifdef CONFIG_KVM
> -    if (!vdev->intx.kvm_accel) {
> +    if (!kvm_enabled() || !vdev->intx.kvm_accel) {
>          return;
>      }
>  
> @@ -211,7 +213,6 @@ static void vfio_intx_disable_kvm(VFIOPCIDevice *vdev)
>      vfio_unmask_single_irqindex(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX);
>  
>      trace_vfio_intx_disable_kvm(vdev->vbasedev.name);
> -#endif
>  }
>  
>  static void vfio_intx_update(VFIOPCIDevice *vdev, PCIINTxRoute *route)
> @@ -278,7 +279,6 @@ static bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
>      vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
>      pci_config_set_interrupt_pin(vdev->pdev.config, pin);
>  
> -#ifdef CONFIG_KVM
>      /*
>       * Only conditional to avoid generating error messages on platforms
>       * where we won't actually use the result anyway.
> @@ -287,7 +287,6 @@ static bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
>          vdev->intx.route = pci_device_route_intx_to_irq(&vdev->pdev,
>                                                          vdev->intx.pin);
>      }
> -#endif
>  
>      ret = event_notifier_init(&vdev->intx.interrupt, 0);
>      if (ret) {

On Mon, 10 Mar 2025, Eric Auger wrote:
> Hi Philippe,
>
> On 3/9/25 12:09 AM, Philippe Mathieu-Daudé wrote:
>> Use the runtime kvm_enabled() helper to check whether
>> KVM is available or not.
>
> Miss the "why" of this patch.
>
> By the way I fail to remember/see where kvm_allowed is set.

It's in include/system/kvm.h

> I am also confused because we still have some code, like in
> vfio/common.c which does both checks:
> #ifdef CONFIG_KVM
>         if (kvm_enabled()) {
>             max_memslots = kvm_get_max_memslots();
>         }
> #endif

I think this is because if KVM is not available the if cannot be true so 
it can be left out altogether. This may make sense on platforms like 
Windows and macOS where QEMU is compiled without KVM so basically 
everywhere except Linux.

Regards,
BALATON Zoltan

On 10/3/25 13:54, BALATON Zoltan wrote:
> On Mon, 10 Mar 2025, Eric Auger wrote:
>> Hi Philippe,
>>
>> On 3/9/25 12:09 AM, Philippe Mathieu-Daudé wrote:
>>> Use the runtime kvm_enabled() helper to check whether
>>> KVM is available or not.
>>
>> Miss the "why" of this patch.
>>
>> By the way I fail to remember/see where kvm_allowed is set.

In accel/accel-system.c:

     int accel_init_machine(AccelState *accel, MachineState *ms)
     {
         AccelClass *acc = ACCEL_GET_CLASS(accel);
         int ret;
         ms->accelerator = accel;
         *(acc->allowed) = true;
         ret = acc->init_machine(ms);
         if (ret < 0) {
             ms->accelerator = NULL;
             *(acc->allowed) = false;
             object_unref(OBJECT(accel));
         } else {
             object_set_accelerator_compat_props(acc->compat_props);
         }
         return ret;
     }

> 
> It's in include/system/kvm.h
> 
>> I am also confused because we still have some code, like in
>> vfio/common.c which does both checks:
>> #ifdef CONFIG_KVM
>>         if (kvm_enabled()) {
>>             max_memslots = kvm_get_max_memslots();
>>         }
>> #endif

We should prefer kvm_enabled() over CONFIG_KVM, but for kvm_enabled()
we need the prototypes declared, which sometimes aren't.

Hi,


On 3/10/25 1:54 PM, BALATON Zoltan wrote:
> On Mon, 10 Mar 2025, Eric Auger wrote:
>> Hi Philippe,
>>
>> On 3/9/25 12:09 AM, Philippe Mathieu-Daudé wrote:
>>> Use the runtime kvm_enabled() helper to check whether
>>> KVM is available or not.
>>
>> Miss the "why" of this patch.
>>
>> By the way I fail to remember/see where kvm_allowed is set.
>
> It's in include/system/kvm.h

There you can only find the kvm_enabled() macro definition.

I was eventually able to locate it:
accel/accel-system.c:    *(acc->allowed) = true;
in accel_init_machine()


>
>> I am also confused because we still have some code, like in
>> vfio/common.c which does both checks:
>> #ifdef CONFIG_KVM
>>         if (kvm_enabled()) {
>>             max_memslots = kvm_get_max_memslots();
>>         }
>> #endif
>
> I think this is because if KVM is not available the if cannot be true
> so it can be left out altogether. This may make sense on platforms
> like Windows and macOS where QEMU is compiled without KVM so basically
> everywhere except Linux.
But in practice we have a stub for kvm_get_max_memslots in
accel/stubs/kvm-stub.c.

Eric
>
> Regards,
> BALATON Zoltan