[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 5/8] pci/arm: Use iommu_add_dt_pci_device()
On 5/15/23 03:30, Jan Beulich wrote: > On 12.05.2023 23:03, Stewart Hildebrand wrote: >> On 5/12/23 03:25, Jan Beulich wrote: >>> On 11.05.2023 21:16, Stewart Hildebrand wrote: >>>> @@ -762,9 +767,20 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn, >>>> pdev->domain = NULL; >>>> goto out; >>>> } >>>> +#ifdef CONFIG_HAS_DEVICE_TREE >>>> + ret = iommu_add_dt_pci_device(pdev); >>>> + if ( ret < 0 ) >>>> + { >>>> + printk(XENLOG_ERR "pci-iommu translation failed: %d\n", ret); >>>> + goto out; >>>> + } >>>> +#endif >>>> ret = iommu_add_device(pdev); >>> >>> Hmm, am I misremembering that in the earlier patch you had #else to >>> invoke the alternative behavior? >> >> You are remembering correctly. v1 had an #else, v2 does not. >> >>> Now you end up calling both functions; >>> if that's indeed intended, >> >> Yes, this is intentional. >> >>> this may still want doing differently. >>> Looking at the earlier patch introducing the function, I can't infer >>> though whether that's intended: iommu_add_dt_pci_device() checks that >>> the add_device hook is present, but then I didn't find any use of this >>> hook. The revlog there suggests the check might be stale. >> >> Ah, right, the ops->add_device check is stale in the other patch. Good >> catch, I'll remove it there. >> >>> If indeed the function does only preparatory work, I don't see why it >>> would need naming "iommu_..."; I'd rather consider pci_add_dt_device() >>> then. >> >> The function has now been reduced to reading SMMU configuration data from DT >> and mapping RID/BDF -> AXI stream ID. However, it is still SMMU related, and >> it is still invoking another iommu_ops hook function, dt_xlate (which is yet >> another AXI stream ID translation, separate from what is being discussed >> here). Does this justify keeping "iommu_..." in the name? I'm not convinced >> pci_add_dt_device() is a good name for it either (more on this below). > > The function being SMMU-related pretty strongly suggests it wants to be > invoked via a hook. If the add_device() one isn't suitable, perhaps we > need a new (optional) prepare_device() one? With pci_add_device() then > calling iommu_prepare_device(), wrapping the hook invocation? > > But just to be clear: A new hook would need enough justification as to > the existing one being unsuitable. I'll move it to the add_device hook in v3
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |