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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.07.2025 10:29, Mykyta Poturai wrote:
> On 04.06.25 08:52, Jan Beulich wrote:
>> On 03.06.2025 15:31, Mykyta Poturai wrote:
>>> On 02.06.25 11:11, Jan Beulich wrote:
>>>> On 28.05.2025 11:12, Mykyta Poturai wrote:
>>>>> From: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
>>>>>
>>>>> Enable the use of IOMMU + PCI in dom0 without having to specify
>>>>> "pci-passthrough=yes". Due to possible platform specific dependencies
>>>>> of the PCI host, we rely on dom0 to initialize it and perform
>>>>> a PHYSDEVOP_pci_device_add/remove call to add each device to SMMU.
>>>>> PHYSDEVOP_pci_device_reset is left untouched as it does not have the
>>>>> pci_passthrough_enabled check.
>>>>
>>>> Just to re-raise the question here: Is this actually correct?
>>>
>>> I'm afraid I don't quite understand your concerns here.
>>>
>>> The purpose of this patch is to relax the pci_passthrough_enabled checks
>>> and make PCI physdev ops work with passthrough disabled.
>>> The reset op worked independently of PCI passthrough being on or off and
>>> will continue to do so after this patch.
>>> If your concerns are about the correctness of allowing reset to always
>>> work, you specifically requested this behavior in the patches
>>> implementing it here[1].
>>
>> Right, yet even there I had already asked for possible differing opinions.
>> Plus the case I had mentioned was specifically Dom0, which fits here.
> 
> So I've done some testing to see the actual behavior with different 
> combinations of pci-passthrough and iommu switches. With passthrough=off 
> and iommu=on the reset works fine. But with both of them off, it fails 
> because PHYSDEVOP_pci_device_add is not adding anything and therefore 
> pci_get_pdev can't find the pdev.
> 
> I am not sure which behavior would be the correct one here for 
> passthrought=off and iommu=off.
> 
> 1. Leave it as is, reset returns -ENODEV and pciback probe fails
> 2. Add the same check as in add/remove, reset will return -EOPNOTSUPP 
> and pciback probe will also fail
> 3. Add the same check as in add/remove but return 0 so pciback can probe 
> the device.
> 
> Maybe you have some thoughts on this. I can't come up with an actual 
> good reason for using pciback without pci-passthrough enabled, outside 
> of maybe "not breaking some abstract scripts". And EOPNOTSUPP seems more 
> descriptive than ENODEV so I strive towards option 2 if everyone okay 
> with that.

I think I'd favor option 2, too. Without pass-through, 
PHYSDEVOP_pci_device_reset
is pretty meaningless aiui. vPCI in particular builds on top of pass-through 
aiui,
even if that isn't expressed like that right now (e.g. by having HAS_VPCI select
HAS_PASSTHROUGH).

Jan