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.hildebr...@amd.com> >>>>> >>>>> 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
