On 28.04.25 12:01, Jan Beulich wrote:
> 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
I understand the issue with the ordering, will
"arch_pci_requires_physdev_ops" or "arch_physdev_pci_update_required" be
better? Regarding the specific ops, only add/remove are needed, but I am
not sure how to elegantly encode this in the name. Maybe you can suggest
something better if you have something specific in mind?
--
Mykyta
