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
