RE: [PATCH] hw/ppc: Improve build for PPC VFIO

2023-11-24 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>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


Re: [PATCH] hw/ppc: Improve build for PPC VFIO

2023-11-24 Thread Cédric Le Goater

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 */









Re: [PATCH] hw/ppc: Improve build for PPC VFIO

2023-11-23 Thread Nicholas Piggin
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 
> > Signed-off-by: Zhenzhong Duan 
>
>
> Nick,
>
> I will take this patch through the vfio tree if that's OK for you.

That's fine, thank you.

Thanks,
Nick



Re: [PATCH] hw/ppc: Improve build for PPC VFIO

2023-11-23 Thread Cédric Le Goater

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 
Signed-off-by: Zhenzhong Duan 



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 */





Re: [PATCH] hw/ppc: Improve build for PPC VFIO

2023-11-23 Thread Philippe Mathieu-Daudé

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 
Signed-off-by: Zhenzhong Duan 



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 00..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 

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', i

RE: [PATCH] hw/ppc: Improve build for PPC VFIO

2023-11-22 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>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 
>> Signed-off-by: Zhenzhong Duan 
>
>
>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 00..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 
>
>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(



Re: [PATCH] hw/ppc: Improve build for PPC VFIO

2023-11-22 Thread Cédric Le Goater

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 
Signed-off-by: Zhenzhong Duan 



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 00..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 

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(