Re: [RFC v1 1/2] drivers/vfio_pci_core: Change PXD_ORDER check from switch case to if/else block

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

"Christophe Leroy (CS GROUP)" <chleroy@kernel.org> writes:

> Le 27/02/2026 à 07:16, Ritesh Harjani (IBM) a écrit :
>> Architectures like PowerPC uses runtime defined values for
>> PMD_ORDER/PUD_ORDER. This is because it can use either RADIX or HASH MMU
>> at runtime using kernel cmdline. So the pXd_index_size is not known at
>> compile time. Without this fix, when we add huge pfn support on powerpc
>> in the next patch, vfio_pci_core driver compilation can fail with the
>> following errors.
>> 
>>    CC [M]  drivers/vfio/vfio_main.o
>>    CC [M]  drivers/vfio/group.o
>>    CC [M]  drivers/vfio/container.o
>>    CC [M]  drivers/vfio/virqfd.o
>>    CC [M]  drivers/vfio/vfio_iommu_spapr_tce.o
>>    CC [M]  drivers/vfio/pci/vfio_pci_core.o
>>    CC [M]  drivers/vfio/pci/vfio_pci_intrs.o
>>    CC [M]  drivers/vfio/pci/vfio_pci_rdwr.o
>>    CC [M]  drivers/vfio/pci/vfio_pci_config.o
>>    CC [M]  drivers/vfio/pci/vfio_pci.o
>>    AR      kernel/built-in.a
>> ../drivers/vfio/pci/vfio_pci_core.c: In function ‘vfio_pci_vmf_insert_pfn’:
>> ../drivers/vfio/pci/vfio_pci_core.c:1678:9: error: case label does not reduce to an integer constant
>>   1678 |         case PMD_ORDER:
>>        |         ^~~~
>> ../drivers/vfio/pci/vfio_pci_core.c:1682:9: error: case label does not reduce to an integer constant
>>   1682 |         case PUD_ORDER:
>>        |         ^~~~
>> make[6]: *** [../scripts/Makefile.build:289: drivers/vfio/pci/vfio_pci_core.o] Error 1
>> make[6]: *** Waiting for unfinished jobs....
>> make[5]: *** [../scripts/Makefile.build:546: drivers/vfio/pci] Error 2
>> make[5]: *** Waiting for unfinished jobs....
>> make[4]: *** [../scripts/Makefile.build:546: drivers/vfio] Error 2
>> make[3]: *** [../scripts/Makefile.build:546: drivers] Error 2
>> 
>> Fixes: f9e54c3a2f5b7 ("vfio/pci: implement huge_fault support")
>> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>> ---
>>   drivers/vfio/pci/vfio_pci_core.c | 15 +++++++--------
>>   1 file changed, 7 insertions(+), 8 deletions(-)
>> 
>> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>> index d43745fe4c84..5395a6f30904 100644
>> --- a/drivers/vfio/pci/vfio_pci_core.c
>> +++ b/drivers/vfio/pci/vfio_pci_core.c
>> @@ -1670,21 +1670,20 @@ vm_fault_t vfio_pci_vmf_insert_pfn(struct vfio_pci_core_device *vdev,
>>   	if (vdev->pm_runtime_engaged || !__vfio_pci_memory_enabled(vdev))
>>   		return VM_FAULT_SIGBUS;
>> 
>> -	switch (order) {
>> -	case 0:
>> +	if (order == 0) {
>>   		return vmf_insert_pfn(vmf->vma, vmf->address, pfn);
>> +	}
>
> Those braces are unneeded as all legs of the if/else are single lines
>
>>   #ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
>
> ifdef could be replaced by IS_ENABLED() because PxD_ORDER and 
> vmf_insert_pfn_xxx() are declared all the time
>
>> -	case PMD_ORDER:
>> +	 else if (order == PMD_ORDER) {
>
> 'else' is not needed because every 'if' leads to a return statement
>
>>   		return vmf_insert_pfn_pmd(vmf, pfn, false);
>> +	 }
>>   #endif
>>   #ifdef CONFIG_ARCH_SUPPORTS_PUD_PFNMAP
>> -	case PUD_ORDER:
>> +	 else if (order == PUD_ORDER) {
>>   		return vmf_insert_pfn_pud(vmf, pfn, false);
>> -		break;
>> +	 }
>>   #endif
>> -	default:
>> -		return VM_FAULT_FALLBACK;
>> -	}
>> +	return VM_FAULT_FALLBACK;
>
> So at the end we should get something like:
>
> 	if (!order)
> 		return vmf_insert_pfn(vmf->vma, vmf->address, pfn);
>
> 	if (IS_ENABLED(CONFIG_ARCH_SUPPORTS_PMD_PFNMAP) && order == PMD_ORDER)
> 		return vmf_insert_pfn_pmd(vmf, pfn, false);
>
> 	if (IS_ENABLED(CONFIG_ARCH_SUPPORTS_PUD_PFNMAP) && order == PMD_ORDER)
                                                                ^^^ PUD_ORDER

> 		return vmf_insert_pfn_pud(vmf, pfn, false);
>
> 	return VM_FAULT_FALLBACK;
>
>

Looks a lot cleaner. Thanks!
I will make that change in v2.

-ritesh