Re: [PATCH 5/6] hw/ppc: Rename spapr_pci_vfio.c to spapr_pci_vfio_eeh.c

[Narayana Murty N <nnmlinux@linux.ibm.com>]

On 12/05/26 1:24 PM, Cédric Le Goater wrote:
> + Pierrick, Philippe
>
> On 5/12/26 09:11, Narayana Murty N wrote:
>> The file spapr_pci_vfio.c contains exclusively EEH (Enhanced Error
>> Handling) related functions for VFIO devices on sPAPR platforms.
>> To better reflect its specific purpose and improve code organization,
>> this commit:
>>
>> 1. Renames spapr_pci_vfio.c to spapr_pci_vfio_eeh.c
>> 2. Moves spapr_phb_vfio_eeh_reenable() from spapr_pci_vfio_eeh.c to
>>     spapr_pci.c, as it's a general PHB operation not specific to
>>     VFIO EEH error injection/recovery
>>
>> After this change, spapr_pci_vfio_eeh.c contains only the core VFIO
>> EEH error handling functions, making the file's purpose more focused
>> and clear.
>>
>> This is a refactoring change with no functional impact.
>>
>> Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
>> ---
>>   hw/ppc/Kconfig                                    |  2 +-
>>   hw/ppc/meson.build                                |  2 +-
>>   hw/ppc/spapr_pci.c                                | 14 ++++++++++++++
>>   hw/ppc/{spapr_pci_vfio.c => spapr_pci_vfio_eeh.c} | 12 +-----------
>
>
> There is a CONFIG_VFIO_PCI in this file that could be replaced
> with stub definitions.
>
Hi Cedric,
We are using CONFIG_VFIO_PCI in the new eeh specific file as is for eeh 
specific routines and stubs.
Are you suggesting to retain spapr_pci_vfio.c with the remaining single 
routine "spapr_phb_vfio_reset"
and its stub?

Regards,
Narayana Murty

> Thanks,
>
> C.
>
>
>> include/hw/pci-host/spapr.h                       |  1 +
>>   5 files changed, 18 insertions(+), 13 deletions(-)
>>   rename hw/ppc/{spapr_pci_vfio.c => spapr_pci_vfio_eeh.c} (96%)
>>
>> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
>> index 347dcce690..886ce71ef8 100644
>> --- a/hw/ppc/Kconfig
>> +++ b/hw/ppc/Kconfig
>> @@ -6,7 +6,7 @@ config PSERIES
>>       imply PCI_DEVICES
>>       imply TEST_DEVICES
>>       imply VIRTIO_VGA
>> -    imply VFIO_PCI if LINUX   # needed by spapr_pci_vfio.c
>> +    imply VFIO_PCI if LINUX   # needed by spapr_pci_vfio_eeh.c
>>       select NVDIMM
>>       select DIMM
>>       select PCI
>> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
>> index 37aa535db2..7cf0226b5e 100644
>> --- a/hw/ppc/meson.build
>> +++ b/hw/ppc/meson.build
>> @@ -35,7 +35,7 @@ ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], 
>> if_true: files(
>>   ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
>>   if host_os == 'linux'
>>     ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files(
>> -    'spapr_pci_vfio.c',
>> +    'spapr_pci_vfio_eeh.c',
>>     ))
>>   endif
>>   diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index b00f71d92a..fcb7b850c0 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -2215,6 +2215,20 @@ void spapr_phb_dma_reset(SpaprPhbState *sphb)
>>       tcet->def_win = true;
>>   }
>>   +void spapr_phb_vfio_reset(DeviceState *qdev)
>> +{
>> +    /*
>> +     * The PE might be in frozen state. To reenable the EEH
>> +     * functionality on it will clean the frozen state, which
>> +     * ensures that the contained PCI devices will work properly
>> +     * after reboot.
>> +     */
>> +    spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
>> +}
>> +
>> +
>> +
>> +
>>   static void spapr_phb_reset(DeviceState *qdev)
>>   {
>>       SpaprPhbState *sphb = SPAPR_PCI_HOST_BRIDGE(qdev);
>> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio_eeh.c
>> similarity index 96%
>> rename from hw/ppc/spapr_pci_vfio.c
>> rename to hw/ppc/spapr_pci_vfio_eeh.c
>> index ed0b22a84a..c00cf1d24b 100644
>> --- a/hw/ppc/spapr_pci_vfio.c
>> +++ b/hw/ppc/spapr_pci_vfio_eeh.c
>> @@ -131,21 +131,11 @@ bool spapr_phb_eeh_available(SpaprPhbState *sphb)
>>       return vfio_eeh_as_ok(&sphb->iommu_as);
>>   }
>>   -static void spapr_phb_vfio_eeh_reenable(SpaprPhbState *sphb)
>> +void spapr_phb_vfio_eeh_reenable(SpaprPhbState *sphb)
>>   {
>>       vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_ENABLE);
>>   }
>>   -void spapr_phb_vfio_reset(DeviceState *qdev)
>> -{
>> -    /*
>> -     * The PE might be in frozen state. To reenable the EEH
>> -     * functionality on it will clean the frozen state, which
>> -     * ensures that the contained PCI devices will work properly
>> -     * after reboot.
>> -     */
>> -    spapr_phb_vfio_eeh_reenable(SPAPR_PCI_HOST_BRIDGE(qdev));
>> -}
>>     static void spapr_eeh_pci_find_device(PCIBus *bus, PCIDevice *pdev,
>>                                         void *opaque)
>> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
>> index 417d1f6c31..5ca122a5f1 100644
>> --- a/include/hw/pci-host/spapr.h
>> +++ b/include/hw/pci-host/spapr.h
>> @@ -125,6 +125,7 @@ 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);
>> +void spapr_phb_vfio_eeh_reenable(SpaprPhbState *sphb);
>>   int spapr_phb_vfio_errinjct(SpaprPhbState *sphb, uint32_t func,
>>                               uint64_t addr, uint64_t mask, uint32_t 
>> type);
>>   #else
>