Re: [PATCH v9 7/8] xen/arm: enable dom0 to use PCI devices with pci-passthrough=no

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.04.2025 10:21, Mykyta Poturai wrote:
> On 17.03.25 17:07, Jan Beulich wrote:
>> On 14.03.2025 14:34, Mykyta Poturai wrote:
>>> --- a/xen/arch/arm/pci/pci.c
>>> +++ b/xen/arch/arm/pci/pci.c
>>> @@ -16,9 +16,18 @@
>>>   #include <xen/device_tree.h>
>>>   #include <xen/errno.h>
>>>   #include <xen/init.h>
>>> +#include <xen/iommu.h>
>>>   #include <xen/param.h>
>>>   #include <xen/pci.h>
>>>   
>>> +bool is_pci_passthrough_enabled(bool dom0)
>>> +{
>>> +    if ( dom0 )
>>> +        return pci_passthrough_enabled || iommu_enabled;
>>
>> As I think I said before - the function's name now no longer expresses
>> what it really checks. That (imo heavily) misleading at the use sites
>> of this function.
> 
> I've spent some more time thinking about how to better deal with this. 
> In the end, I think your earlier suggestion about introducing a new arch 
> specific function is the best approach, but I want to agree on the 
> naming before sending new patches. Would "arch_requires_pci_physdev" be 
> an appropriate name in your opinion?
> 
> At the call sites it will look like this:
>      case PHYSDEVOP_pci_device_remove: {
>          struct physdev_pci_device dev;
> 
>          if ( !is_pci_passthrough_enabled() && !arch_requires_pci_physdev())
>              return -EOPNOTSUPP;

There are several questions that affect naming: Is it really "requires"? Is
it really all PCI-related physdevops? Is the ordering of naming elements in
line with what we use elsewhere (arch_ first is, but perhaps either pci or
physdevop wants to move earlier)?

Jan