[PATCH 5/8] hw/vfio/pci.c: eradicate CONFIG_KVM

Pierrick Bouvier posted 8 patches 2 weeks ago
Maintainers: Thomas Huth <thuth@redhat.com>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, Tony Krowiak <akrowiak@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Jason Herne <jjherne@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Paolo Bonzini <pbonzini@redhat.com>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Richard Henderson <richard.henderson@linaro.org>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>
There is a newer version of this series
[PATCH 5/8] hw/vfio/pci.c: eradicate CONFIG_KVM
Posted by Pierrick Bouvier 2 weeks ago
We just need to add kvm_enabled() guard on concerned functions, but no
need to extract those kvm functions since they are not using any kvm
specific types that would not be visible at compilation time.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 hw/vfio/pci.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index df617f1fe46..62c52407cf9 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -152,7 +152,10 @@ void vfio_pci_intx_eoi(VFIODevice *vbasedev)
 
 static bool vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
 {
-#ifdef CONFIG_KVM
+    if (!kvm_enabled()) {
+        return true;
+    }
+
     PCIDevice *pdev = PCI_DEVICE(vdev);
     int irq_fd = event_notifier_get_fd(&vdev->intx.interrupt);
 
@@ -206,14 +209,14 @@ fail:
     qemu_set_fd_handler(irq_fd, vfio_intx_interrupt, NULL, vdev);
     vfio_device_irq_unmask(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX);
     return false;
-#else
-    return true;
-#endif
 }
 
 static bool vfio_cpr_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
 {
-#ifdef CONFIG_KVM
+    if (!kvm_enabled()) {
+        return true;
+    }
+
     if (vdev->no_kvm_intx || !kvm_irqfds_enabled() ||
         vdev->intx.route.mode != PCI_INTX_ENABLED ||
         !kvm_resamplefds_enabled()) {
@@ -236,14 +239,14 @@ static bool vfio_cpr_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
     vdev->intx.kvm_accel = true;
     trace_vfio_intx_enable_kvm(vdev->vbasedev.name);
     return true;
-#else
-    return true;
-#endif
 }
 
 static void vfio_intx_disable_kvm(VFIOPCIDevice *vdev)
 {
-#ifdef CONFIG_KVM
+    if (!kvm_enabled()) {
+        return;
+    }
+
     PCIDevice *pdev = PCI_DEVICE(vdev);
 
     if (!vdev->intx.kvm_accel) {
@@ -277,7 +280,6 @@ static void vfio_intx_disable_kvm(VFIOPCIDevice *vdev)
     vfio_device_irq_unmask(&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)
@@ -350,16 +352,14 @@ static bool vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
     vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
     pci_config_set_interrupt_pin(pdev->config, pin);
 
-#ifdef CONFIG_KVM
     /*
      * Only conditional to avoid generating error messages on platforms
      * where we won't actually use the result anyway.
      */
-    if (kvm_irqfds_enabled() && kvm_resamplefds_enabled()) {
+    if (kvm_enabled() && kvm_irqfds_enabled() && kvm_resamplefds_enabled()) {
         vdev->intx.route = pci_device_route_intx_to_irq(pdev,
                                                         vdev->intx.pin);
     }
-#endif
 
     if (!vfio_notifier_init(vdev, &vdev->intx.interrupt, "intx-interrupt", 0,
                             errp)) {
-- 
2.47.3
Re: [PATCH 5/8] hw/vfio/pci.c: eradicate CONFIG_KVM
Posted by Philippe Mathieu-Daudé 1 week, 6 days ago
On 12/3/26 23:44, Pierrick Bouvier wrote:
> We just need to add kvm_enabled() guard on concerned functions, but no
> need to extract those kvm functions since they are not using any kvm
> specific types that would not be visible at compilation time.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   hw/vfio/pci.c | 26 +++++++++++++-------------
>   1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index df617f1fe46..62c52407cf9 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -152,7 +152,10 @@ void vfio_pci_intx_eoi(VFIODevice *vbasedev)
>   
>   static bool vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
>   {
> -#ifdef CONFIG_KVM
> +    if (!kvm_enabled()) {

I'd rather this check done in callers, and here assert on entry:

        assert(kvm_enabled());

(Applies to other changes in this patch).

But this "call KVM-specific helper even if KVM isn't used" pattern
is pre-existing, so can be done later:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +        return true;
> +    }


Re: [PATCH 5/8] hw/vfio/pci.c: eradicate CONFIG_KVM
Posted by Pierrick Bouvier 1 week, 6 days ago
On 3/12/26 8:24 PM, Philippe Mathieu-Daudé wrote:
> On 12/3/26 23:44, Pierrick Bouvier wrote:
>> We just need to add kvm_enabled() guard on concerned functions, but no
>> need to extract those kvm functions since they are not using any kvm
>> specific types that would not be visible at compilation time.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    hw/vfio/pci.c | 26 +++++++++++++-------------
>>    1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index df617f1fe46..62c52407cf9 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -152,7 +152,10 @@ void vfio_pci_intx_eoi(VFIODevice *vbasedev)
>>    
>>    static bool vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
>>    {
>> -#ifdef CONFIG_KVM
>> +    if (!kvm_enabled()) {
> 
> I'd rather this check done in callers, and here assert on entry:
> 
>          assert(kvm_enabled());
> 
> (Applies to other changes in this patch).
> 
> But this "call KVM-specific helper even if KVM isn't used" pattern
> is pre-existing, so can be done later:
>

Indeed, I followed the existing convention.
But over all, it's safer, and would allow to implement the stubs as 
g_assert_not_reached(), so I'll do the change.

> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
>> +        return true;
>> +    }
>