VFIO is not a required subsystem for the pseries machine but it's
force enabled currently. When --without-default-devices is used
to drop some default devices including vfio-pci, vfio core code
is still kept which is unnecessary.

Introduce a stub file to hold stub functions of VFIO EEH hooks,
then vfio core could be compiled out.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
Based on vfio-next/vfio-8.2

 hw/ppc/spapr_pci_vfio_stub.c | 33 +++++++++++++++++++++++++++++++++
 hw/ppc/Kconfig               |  2 +-
 hw/ppc/meson.build           |  6 +++---
 3 files changed, 37 insertions(+), 4 deletions(-)
 create mode 100644 hw/ppc/spapr_pci_vfio_stub.c

diff --git a/hw/ppc/spapr_pci_vfio_stub.c b/hw/ppc/spapr_pci_vfio_stub.c
new file mode 100644
index 0000000000..adb3fb5e35
--- /dev/null
+++ b/hw/ppc/spapr_pci_vfio_stub.c
@@ -0,0 +1,33 @@
+#include "qemu/osdep.h"
+#include "hw/pci-host/spapr.h"
+
+bool spapr_phb_eeh_available(SpaprPhbState *sphb)
+{
+    return false;
+}
+
+void spapr_phb_vfio_reset(DeviceState *qdev)
+{
+}
+
+int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
+                                  unsigned int addr, int option)
+{
+    return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state)
+{
+    return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
+{
+    return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
+{
+    return RTAS_OUT_NOT_SUPPORTED;
+}
+
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index edc6d2d139..b8dabdbfbe 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -3,11 +3,11 @@ config PSERIES
     imply PCI_DEVICES
     imply TEST_DEVICES
     imply VIRTIO_VGA
+    imply VFIO if LINUX   # needed by spapr_pci_vfio.c
     select NVDIMM
     select DIMM
     select PCI
     select SPAPR_VSCSI
-    select VFIO_PCI if LINUX   # needed by spapr_pci_vfio.c
     select XICS
     select XIVE
     select MSI_NONBROKEN
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index ea44856d43..2df5db2eef 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -34,9 +34,9 @@ ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], if_true: files(
   'spapr_softmmu.c',
 ))
 ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
-ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
-  'spapr_pci_vfio.c',
-))
+ppc_ss.add(when: [ 'CONFIG_VFIO_PCI', 'CONFIG_PSERIES', 'CONFIG_LINUX'],
+           if_true: files('spapr_pci_vfio.c'),
+           if_false: files('spapr_pci_vfio_stub.c'))
 
 # IBM PowerNV
 ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
-- 
2.34.1

On 11/23/23 07:01, Zhenzhong Duan wrote:
> VFIO is not a required subsystem for the pseries machine but it's
> force enabled currently. When --without-default-devices is used
> to drop some default devices including vfio-pci, vfio core code
> is still kept which is unnecessary.
> 
> Introduce a stub file to hold stub functions of VFIO EEH hooks,
> then vfio core could be compiled out.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


Nick,

I will take this patch through the vfio tree if that's OK for you.

> ---
> Based on vfio-next/vfio-8.2
> 
>   hw/ppc/spapr_pci_vfio_stub.c | 33 +++++++++++++++++++++++++++++++++
>   hw/ppc/Kconfig               |  2 +-
>   hw/ppc/meson.build           |  6 +++---
>   3 files changed, 37 insertions(+), 4 deletions(-)
>   create mode 100644 hw/ppc/spapr_pci_vfio_stub.c
> 
> diff --git a/hw/ppc/spapr_pci_vfio_stub.c b/hw/ppc/spapr_pci_vfio_stub.c
> new file mode 100644
> index 0000000000..adb3fb5e35
> --- /dev/null
> +++ b/hw/ppc/spapr_pci_vfio_stub.c
> @@ -0,0 +1,33 @@
> +#include "qemu/osdep.h"
> +#include "hw/pci-host/spapr.h"
> +
> +bool spapr_phb_eeh_available(SpaprPhbState *sphb)
> +{
> +    return false;
> +}
> +
> +void spapr_phb_vfio_reset(DeviceState *qdev)
> +{
> +}
> +
> +int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
> +                                  unsigned int addr, int option)
> +{
> +    return RTAS_OUT_NOT_SUPPORTED;
> +}
> +
> +int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state)
> +{
> +    return RTAS_OUT_NOT_SUPPORTED;
> +}
> +
> +int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
> +{
> +    return RTAS_OUT_NOT_SUPPORTED;
> +}
> +
> +int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
> +{
> +    return RTAS_OUT_NOT_SUPPORTED;
> +}
> +
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index edc6d2d139..b8dabdbfbe 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -3,11 +3,11 @@ config PSERIES
>       imply PCI_DEVICES
>       imply TEST_DEVICES
>       imply VIRTIO_VGA
> +    imply VFIO if LINUX   # needed by spapr_pci_vfio.c

Zhenzhong,

I changed VFIO to VFIO_PCI because PPC only supports this type
of passthrough devices.

With that,

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.



>       select NVDIMM
>       select DIMM
>       select PCI
>       select SPAPR_VSCSI
> -    select VFIO_PCI if LINUX   # needed by spapr_pci_vfio.c
>       select XICS
>       select XIVE
>       select MSI_NONBROKEN
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index ea44856d43..2df5db2eef 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -34,9 +34,9 @@ ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], if_true: files(
>     'spapr_softmmu.c',
>   ))
>   ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
> -ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
> -  'spapr_pci_vfio.c',
> -))
> +ppc_ss.add(when: [ 'CONFIG_VFIO_PCI', 'CONFIG_PSERIES', 'CONFIG_LINUX'],
> +           if_true: files('spapr_pci_vfio.c'),
> +           if_false: files('spapr_pci_vfio_stub.c'))
>   
>   # IBM PowerNV
>   ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(

On Thu Nov 23, 2023 at 5:33 PM AEST, Cédric Le Goater wrote:
> On 11/23/23 07:01, Zhenzhong Duan wrote:
> > VFIO is not a required subsystem for the pseries machine but it's
> > force enabled currently. When --without-default-devices is used
> > to drop some default devices including vfio-pci, vfio core code
> > is still kept which is unnecessary.
> > 
> > Introduce a stub file to hold stub functions of VFIO EEH hooks,
> > then vfio core could be compiled out.
> > 
> > Suggested-by: Cédric Le Goater <clg@redhat.com>
> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>
>
> Nick,
>
> I will take this patch through the vfio tree if that's OK for you.

That's fine, thank you.

Thanks,
Nick

Hi Cédric,

On 23/11/23 08:33, Cédric Le Goater wrote:
> On 11/23/23 07:01, Zhenzhong Duan wrote:
>> VFIO is not a required subsystem for the pseries machine but it's
>> force enabled currently. When --without-default-devices is used
>> to drop some default devices including vfio-pci, vfio core code
>> is still kept which is unnecessary.
>>
>> Introduce a stub file to hold stub functions of VFIO EEH hooks,
>> then vfio core could be compiled out.
>>
>> Suggested-by: Cédric Le Goater <clg@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> 
> 
> Nick,
> 
> I will take this patch through the vfio tree if that's OK for you.
> 
>> ---
>> Based on vfio-next/vfio-8.2
>>
>>   hw/ppc/spapr_pci_vfio_stub.c | 33 +++++++++++++++++++++++++++++++++
>>   hw/ppc/Kconfig               |  2 +-
>>   hw/ppc/meson.build           |  6 +++---
>>   3 files changed, 37 insertions(+), 4 deletions(-)
>>   create mode 100644 hw/ppc/spapr_pci_vfio_stub.c

We are trying to remove stubs: instead of checking late in the callee,
we shouldn't let the caller call functions depending on an unavailable
feature. So I'm a bit reluctant with this patch.

Can we add a simple 'bool pci_vfio_available(void);' helper? Or rework a
bit. For example looking quickly, we already have:

     #ifdef CONFIG_LINUX
     int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
     #else
     static inline bool spapr_phb_eeh_available(SpaprPhbState *sphb)
     {
         return false;
     }
     #endif

This should be enough to protect the other calls.

Maybe we just need:

-- >8 --
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -122,41 +122,20 @@ int spapr_pci_dt_populate(SpaprDrc *drc, 
SpaprMachineState *spapr,
                            void *fdt, int *fdt_start_offset, Error **errp);

  /* VFIO EEH hooks */
-#ifdef CONFIG_LINUX
+#if defined(CONFIG_LINUX) && defined(CONFIG_VFIO_PCI)
  bool spapr_phb_eeh_available(SpaprPhbState *sphb);
+#else
+static inline bool spapr_phb_eeh_available(SpaprPhbState *sphb)
+{
+    return false;
+}
+#endif
  int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
                                    unsigned int addr, int option);
  int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state);
  int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option);
  int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb);
  void spapr_phb_vfio_reset(DeviceState *qdev);
-#else
-static inline bool spapr_phb_eeh_available(SpaprPhbState *sphb)
-{
-    return false;
-}
-static inline int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
-                                                unsigned int addr, int 
option)
-{
-    return RTAS_OUT_HW_ERROR;
-}
-static inline int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb,
-                                               int *state)
-{
-    return RTAS_OUT_HW_ERROR;
-}
-static inline int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
-{
-    return RTAS_OUT_HW_ERROR;
-}
-static inline int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
-{
-    return RTAS_OUT_HW_ERROR;
-}
-static inline void spapr_phb_vfio_reset(DeviceState *qdev)
-{
-}
-#endif

  void spapr_phb_dma_reset(SpaprPhbState *sphb);
---

and massage a bit the calls not protected by spapr_phb_eeh_available().

>> diff --git a/hw/ppc/spapr_pci_vfio_stub.c b/hw/ppc/spapr_pci_vfio_stub.c
>> new file mode 100644
>> index 0000000000..adb3fb5e35
>> --- /dev/null
>> +++ b/hw/ppc/spapr_pci_vfio_stub.c
>> @@ -0,0 +1,33 @@
>> +#include "qemu/osdep.h"
>> +#include "hw/pci-host/spapr.h"
>> +
>> +bool spapr_phb_eeh_available(SpaprPhbState *sphb)
>> +{
>> +    return false;
>> +}
>> +
>> +void spapr_phb_vfio_reset(DeviceState *qdev)
>> +{
>> +}
>> +
>> +int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
>> +                                  unsigned int addr, int option)
>> +{
>> +    return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> +int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state)
>> +{
>> +    return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> +int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
>> +{
>> +    return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> +int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
>> +{
>> +    return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
>> index edc6d2d139..b8dabdbfbe 100644
>> --- a/hw/ppc/Kconfig
>> +++ b/hw/ppc/Kconfig
>> @@ -3,11 +3,11 @@ config PSERIES
>>       imply PCI_DEVICES
>>       imply TEST_DEVICES
>>       imply VIRTIO_VGA
>> +    imply VFIO if LINUX   # needed by spapr_pci_vfio.c
> 
> Zhenzhong,
> 
> I changed VFIO to VFIO_PCI because PPC only supports this type
> of passthrough devices.
> 
> With that,
> 
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> 
> Thanks,
> 
> C.
> 
> 
> 
>>       select NVDIMM
>>       select DIMM
>>       select PCI
>>       select SPAPR_VSCSI
>> -    select VFIO_PCI if LINUX   # needed by spapr_pci_vfio.c
>>       select XICS
>>       select XIVE
>>       select MSI_NONBROKEN
>> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
>> index ea44856d43..2df5db2eef 100644
>> --- a/hw/ppc/meson.build
>> +++ b/hw/ppc/meson.build
>> @@ -34,9 +34,9 @@ ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], 
>> if_true: files(
>>     'spapr_softmmu.c',
>>   ))
>>   ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
>> -ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
>> -  'spapr_pci_vfio.c',
>> -))
>> +ppc_ss.add(when: [ 'CONFIG_VFIO_PCI', 'CONFIG_PSERIES', 'CONFIG_LINUX'],
>> +           if_true: files('spapr_pci_vfio.c'),
>> +           if_false: files('spapr_pci_vfio_stub.c'))
>>   # IBM PowerNV
>>   ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
> 
> 

On 11/23/23 11:19, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
> 
> On 23/11/23 08:33, Cédric Le Goater wrote:
>> On 11/23/23 07:01, Zhenzhong Duan wrote:
>>> VFIO is not a required subsystem for the pseries machine but it's
>>> force enabled currently. When --without-default-devices is used
>>> to drop some default devices including vfio-pci, vfio core code
>>> is still kept which is unnecessary.
>>>
>>> Introduce a stub file to hold stub functions of VFIO EEH hooks,
>>> then vfio core could be compiled out.
>>>
>>> Suggested-by: Cédric Le Goater <clg@redhat.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>
>>
>> Nick,
>>
>> I will take this patch through the vfio tree if that's OK for you.
>>
>>> ---
>>> Based on vfio-next/vfio-8.2
>>>
>>>   hw/ppc/spapr_pci_vfio_stub.c | 33 +++++++++++++++++++++++++++++++++
>>>   hw/ppc/Kconfig               |  2 +-
>>>   hw/ppc/meson.build           |  6 +++---
>>>   3 files changed, 37 insertions(+), 4 deletions(-)
>>>   create mode 100644 hw/ppc/spapr_pci_vfio_stub.c
> 
> We are trying to remove stubs: instead of checking late in the callee,
> we shouldn't let the caller call functions depending on an unavailable
> feature. So I'm a bit reluctant with this patch.
>
> Can we add a simple 'bool pci_vfio_available(void);' helper? Or rework a
> bit. For example looking quickly, we already have:
> 
>      #ifdef CONFIG_LINUX
>      int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
>      #else
>      static inline bool spapr_phb_eeh_available(SpaprPhbState *sphb)
>      {
>          return false;
>      }
>      #endif
> 
> This should be enough to protect the other calls.

The problem is that CONFIG_VFIO_PCI is not a target option and you can't
use the define as we do with CONFIG_LINUX. The define poisoning does its
job there.


> 
> Maybe we just need:> 
> -- >8 --
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -122,41 +122,20 @@ int spapr_pci_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
>                             void *fdt, int *fdt_start_offset, Error **errp);
> 
>   /* VFIO EEH hooks */
> -#ifdef CONFIG_LINUX
> +#if defined(CONFIG_LINUX) && defined(CONFIG_VFIO_PCI)
>   bool spapr_phb_eeh_available(SpaprPhbState *sphb);
> +#else
> +static inline bool spapr_phb_eeh_available(SpaprPhbState *sphb)
> +{
> +    return false;
> +}
> +#endif
>   int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
>                                     unsigned int addr, int option);
>   int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state);
>   int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option);
>   int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb);
>   void spapr_phb_vfio_reset(DeviceState *qdev);
> -#else
> -static inline bool spapr_phb_eeh_available(SpaprPhbState *sphb)
> -{
> -    return false;
> -}
> -static inline int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
> -                                                unsigned int addr, int option)
> -{
> -    return RTAS_OUT_HW_ERROR;
> -}
> -static inline int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb,
> -                                               int *state)
> -{
> -    return RTAS_OUT_HW_ERROR;
> -}
> -static inline int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
> -{
> -    return RTAS_OUT_HW_ERROR;
> -}
> -static inline int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
> -{
> -    return RTAS_OUT_HW_ERROR;
> -}
> -static inline void spapr_phb_vfio_reset(DeviceState *qdev)
> -{
> -}
> -#endif
> 
>   void spapr_phb_dma_reset(SpaprPhbState *sphb);
> ---
> 
> and massage a bit the calls not protected by spapr_phb_eeh_available().

How about what's below  instead ?


Thanks,

C.



--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -26,10 +26,12 @@
  #include "hw/pci/pci_device.h"
  #include "hw/vfio/vfio-common.h"
  #include "qemu/error-report.h"
+#include CONFIG_DEVICES /* CONFIG_VFIO_PCI */
  
  /*
   * Interfaces for IBM EEH (Enhanced Error Handling)
   */
+#ifdef CONFIG_VFIO_PCI
  static bool vfio_eeh_container_ok(VFIOContainer *container)
  {
      /*
@@ -314,3 +316,37 @@ int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
  
      return RTAS_OUT_SUCCESS;
  }
+
+#else
+
+bool spapr_phb_eeh_available(SpaprPhbState *sphb)
+{
+    return false;
+}
+
+void spapr_phb_vfio_reset(DeviceState *qdev)
+{
+}
+
+int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
+                                  unsigned int addr, int option)
+{
+    return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state)
+{
+    return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
+{
+    return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
+{
+    return RTAS_OUT_NOT_SUPPORTED;
+}
+
+#endif /* CONFIG_VFIO_PCI */

Zhenzhong,

> How about what's below  instead ?
> 
> 
> Thanks,
> 
> C.

I will resend the build fix with the proposal below since it addresses
Phil's concerns.

Thanks,

C.

  
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -26,10 +26,12 @@
>   #include "hw/pci/pci_device.h"
>   #include "hw/vfio/vfio-common.h"
>   #include "qemu/error-report.h"
> +#include CONFIG_DEVICES /* CONFIG_VFIO_PCI */
> 
>   /*
>    * Interfaces for IBM EEH (Enhanced Error Handling)
>    */
> +#ifdef CONFIG_VFIO_PCI
>   static bool vfio_eeh_container_ok(VFIOContainer *container)
>   {
>       /*
> @@ -314,3 +316,37 @@ int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
> 
>       return RTAS_OUT_SUCCESS;
>   }
> +
> +#else
> +
> +bool spapr_phb_eeh_available(SpaprPhbState *sphb)
> +{
> +    return false;
> +}
> +
> +void spapr_phb_vfio_reset(DeviceState *qdev)
> +{
> +}
> +
> +int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
> +                                  unsigned int addr, int option)
> +{
> +    return RTAS_OUT_NOT_SUPPORTED;
> +}
> +
> +int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state)
> +{
> +    return RTAS_OUT_NOT_SUPPORTED;
> +}
> +
> +int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
> +{
> +    return RTAS_OUT_NOT_SUPPORTED;
> +}
> +
> +int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
> +{
> +    return RTAS_OUT_NOT_SUPPORTED;
> +}
> +
> +#endif /* CONFIG_VFIO_PCI */

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Friday, November 24, 2023 3:59 PM
>Subject: Re: [PATCH] hw/ppc: Improve build for PPC VFIO
>
>Zhenzhong,
>
>> How about what's below  instead ?
>>
>>
>> Thanks,
>>
>> C.
>
>I will resend the build fix with the proposal below since it addresses
>Phil's concerns.

Sure, appreciated😊

Thanks
Zhenzhong

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Thursday, November 23, 2023 3:33 PM
>Subject: Re: [PATCH] hw/ppc: Improve build for PPC VFIO
>
>On 11/23/23 07:01, Zhenzhong Duan wrote:
>> VFIO is not a required subsystem for the pseries machine but it's
>> force enabled currently. When --without-default-devices is used
>> to drop some default devices including vfio-pci, vfio core code
>> is still kept which is unnecessary.
>>
>> Introduce a stub file to hold stub functions of VFIO EEH hooks,
>> then vfio core could be compiled out.
>>
>> Suggested-by: Cédric Le Goater <clg@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>
>
>Nick,
>
>I will take this patch through the vfio tree if that's OK for you.

Sure.

>
>> ---
>> Based on vfio-next/vfio-8.2
>>
>>   hw/ppc/spapr_pci_vfio_stub.c | 33 +++++++++++++++++++++++++++++++++
>>   hw/ppc/Kconfig               |  2 +-
>>   hw/ppc/meson.build           |  6 +++---
>>   3 files changed, 37 insertions(+), 4 deletions(-)
>>   create mode 100644 hw/ppc/spapr_pci_vfio_stub.c
>>
>> diff --git a/hw/ppc/spapr_pci_vfio_stub.c b/hw/ppc/spapr_pci_vfio_stub.c
>> new file mode 100644
>> index 0000000000..adb3fb5e35
>> --- /dev/null
>> +++ b/hw/ppc/spapr_pci_vfio_stub.c
>> @@ -0,0 +1,33 @@
>> +#include "qemu/osdep.h"
>> +#include "hw/pci-host/spapr.h"
>> +
>> +bool spapr_phb_eeh_available(SpaprPhbState *sphb)
>> +{
>> +    return false;
>> +}
>> +
>> +void spapr_phb_vfio_reset(DeviceState *qdev)
>> +{
>> +}
>> +
>> +int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
>> +                                  unsigned int addr, int option)
>> +{
>> +    return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> +int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state)
>> +{
>> +    return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> +int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
>> +{
>> +    return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> +int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
>> +{
>> +    return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
>> index edc6d2d139..b8dabdbfbe 100644
>> --- a/hw/ppc/Kconfig
>> +++ b/hw/ppc/Kconfig
>> @@ -3,11 +3,11 @@ config PSERIES
>>       imply PCI_DEVICES
>>       imply TEST_DEVICES
>>       imply VIRTIO_VGA
>> +    imply VFIO if LINUX   # needed by spapr_pci_vfio.c
>
>Zhenzhong,
>
>I changed VFIO to VFIO_PCI because PPC only supports this type
>of passthrough devices.

Oh, I see, thanks

BRs.
Zhenzhong

>
>With that,
>
>Reviewed-by: Cédric Le Goater <clg@redhat.com>
>
>Thanks,
>
>C.
>
>
>
>>       select NVDIMM
>>       select DIMM
>>       select PCI
>>       select SPAPR_VSCSI
>> -    select VFIO_PCI if LINUX   # needed by spapr_pci_vfio.c
>>       select XICS
>>       select XIVE
>>       select MSI_NONBROKEN
>> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
>> index ea44856d43..2df5db2eef 100644
>> --- a/hw/ppc/meson.build
>> +++ b/hw/ppc/meson.build
>> @@ -34,9 +34,9 @@ ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'],
>if_true: files(
>>     'spapr_softmmu.c',
>>   ))
>>   ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
>> -ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
>> -  'spapr_pci_vfio.c',
>> -))
>> +ppc_ss.add(when: [ 'CONFIG_VFIO_PCI', 'CONFIG_PSERIES', 'CONFIG_LINUX'],
>> +           if_true: files('spapr_pci_vfio.c'),
>> +           if_false: files('spapr_pci_vfio_stub.c'))
>>
>>   # IBM PowerNV
>>   ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(