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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.02.2025 12:28, Mykyta Poturai wrote:
> On 10.02.25 12:56, Jan Beulich wrote:
>> On 10.02.2025 11:30, Mykyta Poturai wrote:
>>> From: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
>>>
>>> Enable the use of IOMMU + PCI in dom0 without having to specify
>>> "pci-passthrough=yes".
>>
>> Why? It _is_ passing through, even if Dom0 is special.
> 
> Do you think it would be better to drop this completely and require
> pci-passthrough=yes for PCI to work in Dom0?

>From an abstract perspective: Yes. I don't know any of the Arm background,
though.

>>> @@ -83,9 +84,9 @@ static int __init pci_init(void)
>>>   {
>>>       /*
>>>        * Enable PCI passthrough when has been enabled explicitly
>>> -     * (pci-passthrough=on).
>>> +     * (pci-passthrough=on) or IOMMU is present and enabled.
>>>        */
>>> -    if ( !pci_passthrough_enabled )
>>> +    if ( !is_pci_passthrough_enabled() && !iommu_enabled )
>>>           return 0;
>>
>> I can't reasonably judge on this adjustment, but ...
>>
>>> --- a/xen/drivers/pci/physdev.c
>>> +++ b/xen/drivers/pci/physdev.c
>>> @@ -19,7 +19,7 @@ ret_t pci_physdev_op(int cmd, 
>>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>           struct pci_dev_info pdev_info;
>>>           nodeid_t node = NUMA_NO_NODE;
>>>   
>>> -        if ( !is_pci_passthrough_enabled() )
>>> +        if ( !is_pci_passthrough_enabled() && !iommu_enabled )
>>>               return -EOPNOTSUPP;
>>>   
>>>           ret = -EFAULT;
>>> @@ -57,7 +57,7 @@ ret_t pci_physdev_op(int cmd, 
>>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>       case PHYSDEVOP_pci_device_remove: {
>>>           struct physdev_pci_device dev;
>>>   
>>> -        if ( !is_pci_passthrough_enabled() )
>>> +        if ( !is_pci_passthrough_enabled() && !iommu_enabled )
>>>               return -EOPNOTSUPP;
>>>   
>>>           ret = -EFAULT;
>>
>> ... these two certainly look wrong to be made. If an Arm-specific adjustment
>> is needed (and can be justified), a per-arch hook may need introducing.
> 
> This should not affect x86 in any way if I'm not missing something, as
> !is_pci_passthrough_enabled() will always be false. Or are you concerned 
> about something else?

Indeed I am - the wrong look of it. Readers like me will have the immediate
impression of there being something fishy here.

Jan