Re: [git pull] IOMMU Fixes for Linux v5.19-rc3
The pull request you sent on Fri, 24 Jun 2022 15:56:14 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git > tags/iommu-fixes-v5.19-rc3 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/bc3b8977e3747ab8aa54a0760dce5cdfa37ad4d6 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/2] vfio: Use device_iommu_capable()
On Fri, Jun 24, 2022 at 06:59:35PM +0100, Robin Murphy wrote: > Use the new interface to check the capabilities for our device > specifically. > > Reviewed-by: Lu Baolu > Signed-off-by: Robin Murphy > --- > drivers/vfio/vfio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Jason Gunthorpe Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 1/2] vfio/type1: Simplify bus_type determination
On Fri, Jun 24, 2022 at 06:51:44PM +0100, Robin Murphy wrote: > Since IOMMU groups are mandatory for drivers to support, it stands to > reason that any device which has been successfully added to a group > must be on a bus supported by that IOMMU driver, and therefore a domain > viable for any device in the group must be viable for all devices in > the group. This already has to be the case for the IOMMU API's internal > default domain, for instance. Thus even if the group contains devices on > different buses, that can only mean that the IOMMU driver actually > supports such an odd topology, and so without loss of generality we can > expect the bus type of any device in a group to be suitable for IOMMU > API calls. > > Furthermore, scrutiny reveals a lack of protection for the bus being > removed while vfio_iommu_type1_attach_group() is using it; the reference > that VFIO holds on the iommu_group ensures that data remains valid, but > does not prevent the group's membership changing underfoot. > > We can address both concerns by recycling vfio_bus_type() into some > superficially similar logic to indirect the IOMMU API calls themselves. > Each call is thus protected from races by the IOMMU group's own locking, > and we no longer need to hold group-derived pointers beyond that scope. > It also gives us an easy path for the IOMMU API's migration of bus-based > interfaces to device-based, of which we can already take the first step > with device_iommu_capable(). As with domains, any capability must in > practice be consistent for devices in a given group - and after all it's > still the same capability which was expected to be consistent across an > entire bus! - so there's no need for any complicated validation. > > Signed-off-by: Robin Murphy > --- > > v3: Complete rewrite yet again, and finally it doesn't feel like we're > stretching any abstraction boundaries the wrong way, and the diffstat > looks right too. I did think about embedding IOMMU_CAP_INTR_REMAP > directly in the callback, but decided I like the consistency of minimal > generic wrappers. And yes, if the capability isn't supported then it > does end up getting tested for the whole group, but meh, it's harmless. I think this is really weird, but it works and isn't worth debating further, so OK: Reviewed-by: Jason Gunthorpe Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 3/5] vfio/iommu_type1: Remove the domain->ops comparison
On Fri, Jun 24, 2022 at 03:28:33PM -0300, Jason Gunthorpe wrote: > On Thu, Jun 23, 2022 at 01:00:27PM -0700, Nicolin Chen wrote: > > The domain->ops validation was added, as a precaution, for mixed-driver > > systems. > > > > Per Robin's remarks, > > * While bus_set_iommu() still exists, the core code prevents multiple > > drivers from registering, so we can't really run into a situation of > > having a mixed-driver system: > > https://lore.kernel.org/kvm/6e1280c5-4b22-ebb3-3912-6c72bc169...@arm.com/ > > > > * And there's plenty more significant problems than this to fix; in future > > when many can be permitted, we will rely on the IOMMU core code to check > > the domain->ops: > > https://lore.kernel.org/kvm/6575de6d-94ba-c427-5b1e-967750ddf...@arm.com/ > > > > So remove the check in VFIO for simplicity. > > As discussed we can't remove this check, multiple iommus on different > busses are allowed today and VFIO does not restrict its attempts to > attach a pre-existing domain to a single group or bus. > > Please go back to the original version with the ops check in the core > code. Robin convinced me, so never mind :) Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/5] vfio/iommu_type1: Remove the domain->ops comparison
On Fri, Jun 24, 2022 at 07:31:47PM +0100, Robin Murphy wrote: > > > Oh, physical platforms with mixed IOMMUs definitely exist already. The > > > main > > > point is that while bus_set_iommu still exists, the core code effectively > > > *does* prevent multiple drivers from registering - even in emulated cases > > > like the example above, virtio-iommu and VT-d would both try to > > > bus_set_iommu(&pci_bus_type), and one of them will lose. The aspect which > > > might warrant clarification is that there's no combination of supported > > > drivers which claim non-overlapping buses *and* could appear in the same > > > system - even if you tried to contrive something by emulating, say, VT-d > > > (PCI) alongside rockchip-iommu (platform), you could still only describe > > > one > > > or the other due to ACPI vs. Devicetree. > > > > Right, and that is still something we need to protect against with > > this ops check. VFIO is not checking that the bus's are the same > > before attempting to re-use a domain. > > > > So it is actually functional and does protect against systems with > > multiple iommu drivers on different busses. > > But as above, which systems *are* those? IDK it seems wrong that the system today will allow different buses to have different IOMMU drivers and not provide a trivial protection check. > FWIW my iommu/bus dev branch has got as far as the final bus ops removal and > allowing multiple driver registrations, and before it allows that, it does > now have the common attach check that I sketched out in the previous > discussion of this. If you want to put the check in your series that seems fine too, as long as we get it in the end. > It's probably also noteworthy that domain->ops is no longer the same > domain->ops that this code was written to check, and may now be different > between domains from the same driver. Yes, the vfio check is not good anymore. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/5] vfio/iommu_type1: Remove the domain->ops comparison
On 2022-06-24 14:16, Jason Gunthorpe wrote: On Wed, Jun 22, 2022 at 08:54:45AM +0100, Robin Murphy wrote: On 2022-06-16 23:23, Nicolin Chen wrote: On Thu, Jun 16, 2022 at 06:40:14AM +, Tian, Kevin wrote: The domain->ops validation was added, as a precaution, for mixed-driver systems. However, at this moment only one iommu driver is possible. So remove it. It's true on a physical platform. But I'm not sure whether a virtual platform is allowed to include multiple e.g. one virtio-iommu alongside a virtual VT-d or a virtual smmu. It might be clearer to claim that (as Robin pointed out) there is plenty more significant problems than this to solve instead of simply saying that only one iommu driver is possible if we don't have explicit code to reject such configuration. 😊 Will edit this part. Thanks! Oh, physical platforms with mixed IOMMUs definitely exist already. The main point is that while bus_set_iommu still exists, the core code effectively *does* prevent multiple drivers from registering - even in emulated cases like the example above, virtio-iommu and VT-d would both try to bus_set_iommu(&pci_bus_type), and one of them will lose. The aspect which might warrant clarification is that there's no combination of supported drivers which claim non-overlapping buses *and* could appear in the same system - even if you tried to contrive something by emulating, say, VT-d (PCI) alongside rockchip-iommu (platform), you could still only describe one or the other due to ACPI vs. Devicetree. Right, and that is still something we need to protect against with this ops check. VFIO is not checking that the bus's are the same before attempting to re-use a domain. So it is actually functional and does protect against systems with multiple iommu drivers on different busses. But as above, which systems *are* those? Everything that's on my radar would have drivers all competing for the platform bus - Intel and s390 are somewhat the odd ones out in that respect, but are also non-issues as above. FWIW my iommu/bus dev branch has got as far as the final bus ops removal and allowing multiple driver registrations, and before it allows that, it does now have the common attach check that I sketched out in the previous discussion of this. It's probably also noteworthy that domain->ops is no longer the same domain->ops that this code was written to check, and may now be different between domains from the same driver. Thanks, Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 3/5] vfio/iommu_type1: Remove the domain->ops comparison
On Thu, Jun 23, 2022 at 01:00:27PM -0700, Nicolin Chen wrote: > The domain->ops validation was added, as a precaution, for mixed-driver > systems. > > Per Robin's remarks, > * While bus_set_iommu() still exists, the core code prevents multiple > drivers from registering, so we can't really run into a situation of > having a mixed-driver system: > https://lore.kernel.org/kvm/6e1280c5-4b22-ebb3-3912-6c72bc169...@arm.com/ > > * And there's plenty more significant problems than this to fix; in future > when many can be permitted, we will rely on the IOMMU core code to check > the domain->ops: > https://lore.kernel.org/kvm/6575de6d-94ba-c427-5b1e-967750ddf...@arm.com/ > > So remove the check in VFIO for simplicity. As discussed we can't remove this check, multiple iommus on different busses are allowed today and VFIO does not restrict its attempts to attach a pre-existing domain to a single group or bus. Please go back to the original version with the ops check in the core code. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 1/5] iommu: Return -EMEDIUMTYPE for incompatible domain and device/group
On Fri, Jun 24, 2022 at 06:35:49PM +0800, Yong Wu wrote: > > > It's not used in VFIO context. "return 0" just satisfy the iommu > > > framework to go ahead. and yes, here we only allow the shared > > > "mapping-domain" (All the devices share a domain created > > > internally). What part of the iommu framework is trying to attach a domain and wants to see success when the domain was not actually attached ? > > What prevent this driver from being used in VFIO context? > > Nothing prevent this. Just I didn't test. This is why it is wrong to return success here. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 2/2] vfio: Use device_iommu_capable()
Use the new interface to check the capabilities for our device specifically. Reviewed-by: Lu Baolu Signed-off-by: Robin Murphy --- drivers/vfio/vfio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 61e71c1154be..4c06b571eaba 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -605,7 +605,7 @@ int vfio_register_group_dev(struct vfio_device *device) * VFIO always sets IOMMU_CACHE because we offer no way for userspace to * restore cache coherency. */ - if (!iommu_capable(device->dev->bus, IOMMU_CAP_CACHE_COHERENCY)) + if (!device_iommu_capable(device->dev, IOMMU_CAP_CACHE_COHERENCY)) return -EINVAL; return __vfio_register_dev(device, -- 2.36.1.dirty ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 1/2] vfio/type1: Simplify bus_type determination
Since IOMMU groups are mandatory for drivers to support, it stands to reason that any device which has been successfully added to a group must be on a bus supported by that IOMMU driver, and therefore a domain viable for any device in the group must be viable for all devices in the group. This already has to be the case for the IOMMU API's internal default domain, for instance. Thus even if the group contains devices on different buses, that can only mean that the IOMMU driver actually supports such an odd topology, and so without loss of generality we can expect the bus type of any device in a group to be suitable for IOMMU API calls. Furthermore, scrutiny reveals a lack of protection for the bus being removed while vfio_iommu_type1_attach_group() is using it; the reference that VFIO holds on the iommu_group ensures that data remains valid, but does not prevent the group's membership changing underfoot. We can address both concerns by recycling vfio_bus_type() into some superficially similar logic to indirect the IOMMU API calls themselves. Each call is thus protected from races by the IOMMU group's own locking, and we no longer need to hold group-derived pointers beyond that scope. It also gives us an easy path for the IOMMU API's migration of bus-based interfaces to device-based, of which we can already take the first step with device_iommu_capable(). As with domains, any capability must in practice be consistent for devices in a given group - and after all it's still the same capability which was expected to be consistent across an entire bus! - so there's no need for any complicated validation. Signed-off-by: Robin Murphy --- v3: Complete rewrite yet again, and finally it doesn't feel like we're stretching any abstraction boundaries the wrong way, and the diffstat looks right too. I did think about embedding IOMMU_CAP_INTR_REMAP directly in the callback, but decided I like the consistency of minimal generic wrappers. And yes, if the capability isn't supported then it does end up getting tested for the whole group, but meh, it's harmless. drivers/vfio/vfio_iommu_type1.c | 42 + 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index c13b9290e357..a77ff00c677b 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -1679,18 +1679,6 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu, return ret; } -static int vfio_bus_type(struct device *dev, void *data) -{ - struct bus_type **bus = data; - - if (*bus && *bus != dev->bus) - return -EINVAL; - - *bus = dev->bus; - - return 0; -} - static int vfio_iommu_replay(struct vfio_iommu *iommu, struct vfio_domain *domain) { @@ -2153,13 +2141,25 @@ static void vfio_iommu_iova_insert_copy(struct vfio_iommu *iommu, list_splice_tail(iova_copy, iova); } +static int vfio_iommu_device_capable(struct device *dev, void *data) +{ + return device_iommu_capable(dev, (enum iommu_cap)data); +} + +static int vfio_iommu_domain_alloc(struct device *dev, void *data) +{ + struct iommu_domain **domain = data; + + *domain = iommu_domain_alloc(dev->bus); + return 1; /* Don't iterate */ +} + static int vfio_iommu_type1_attach_group(void *iommu_data, struct iommu_group *iommu_group, enum vfio_group_type type) { struct vfio_iommu *iommu = iommu_data; struct vfio_iommu_group *group; struct vfio_domain *domain, *d; - struct bus_type *bus = NULL; bool resv_msi, msi_remap; phys_addr_t resv_msi_base = 0; struct iommu_domain_geometry *geo; @@ -2192,18 +2192,19 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, goto out_unlock; } - /* Determine bus_type in order to allocate a domain */ - ret = iommu_group_for_each_dev(iommu_group, &bus, vfio_bus_type); - if (ret) - goto out_free_group; - ret = -ENOMEM; domain = kzalloc(sizeof(*domain), GFP_KERNEL); if (!domain) goto out_free_group; + /* +* Going via the iommu_group iterator avoids races, and trivially gives +* us a representative device for the IOMMU API call. We don't actually +* want to iterate beyond the first device (if any). +*/ ret = -EIO; - domain->domain = iommu_domain_alloc(bus); + iommu_group_for_each_dev(iommu_group, &domain->domain, +vfio_iommu_domain_alloc); if (!domain->domain) goto out_free_domain; @@ -2258,7 +2259,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, list_add(&group->next, &domain->group_list); msi_remap = irq_domain_check_msi_remap() || - iommu_capable(bus, IOMMU_CAP_INTR_REMAP); + iommu_group
Re: [PATCH v6 02/10] dt-bindings: display: tegra: Convert to json-schema
On Tue, 21 Jun 2022 18:10:14 +0300, Mikko Perttunen wrote: > From: Thierry Reding > > Convert the Tegra host1x controller bindings from the free-form text > format to json-schema. > > This also adds the missing display-hub DT bindings that were not > previously documented. > > Reviewed-by: Rob Herring > Signed-off-by: Thierry Reding > --- > .../display/tegra/nvidia,tegra114-mipi.txt| 41 -- > .../display/tegra/nvidia,tegra114-mipi.yaml | 74 ++ > .../display/tegra/nvidia,tegra124-dpaux.yaml | 149 > .../display/tegra/nvidia,tegra124-sor.yaml| 206 ++ > .../display/tegra/nvidia,tegra124-vic.yaml| 71 ++ > .../display/tegra/nvidia,tegra186-dc.yaml | 85 +++ > .../tegra/nvidia,tegra186-display.yaml| 310 > .../tegra/nvidia,tegra186-dsi-padctl.yaml | 45 ++ > .../display/tegra/nvidia,tegra20-dc.yaml | 181 + > .../display/tegra/nvidia,tegra20-dsi.yaml | 159 + > .../display/tegra/nvidia,tegra20-epp.yaml | 70 ++ > .../display/tegra/nvidia,tegra20-gr2d.yaml| 73 ++ > .../display/tegra/nvidia,tegra20-gr3d.yaml| 214 ++ > .../display/tegra/nvidia,tegra20-hdmi.yaml| 126 > .../display/tegra/nvidia,tegra20-host1x.txt | 675 -- > .../display/tegra/nvidia,tegra20-host1x.yaml | 347 + > .../display/tegra/nvidia,tegra20-isp.yaml | 67 ++ > .../display/tegra/nvidia,tegra20-mpe.yaml | 73 ++ > .../display/tegra/nvidia,tegra20-tvo.yaml | 58 ++ > .../display/tegra/nvidia,tegra20-vi.yaml | 163 + > .../display/tegra/nvidia,tegra210-csi.yaml| 52 ++ > .../pinctrl/nvidia,tegra124-dpaux-padctl.txt | 59 -- > 22 files changed, 2523 insertions(+), 775 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra114-mipi.txt > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra114-mipi.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-dpaux.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-sor.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-dc.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-display.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-dsi-padctl.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-dc.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-dsi.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-epp.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-gr2d.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-gr3d.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-hdmi.yaml > delete mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-isp.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-mpe.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-tvo.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml > create mode 100644 > Documentation/devicetree/bindings/display/tegra/nvidia,tegra210-csi.yaml > delete mode 100644 > Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-sor.yaml: allOf:1:if:not:properties: {'contains': {'const': 'nvidia,panel'}} should not be valid under {'$ref': '#/definitions/sub-schemas'} hint: A json-schema keyword was found instead of a DT property name. from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-sor.yaml: ignoring, error in schema: allOf: 1: if: not: properties /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-gr3d.example.dtb: gr3d@5418: resets: [[4294967295, 24]] is too short From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-gr3d.yaml /builds/robherring/linux-dt-review/Documentation/device
Re: [PATCH v3 0/3] phase out CONFIG_VIRT_TO_BUS
On Fri, Jun 24, 2022 at 5:52 PM Arnd Bergmann wrote: > Arnd Bergmann (3): > scsi: BusLogic remove bus_to_virt > scsi: dpt_i2o: remove obsolete driver The dpt_i2o removal is overly large and got dropped by some of the mailing lists, if anyone wants to see the full patch, it did make it through to the linux-scsi list at least: https://lore.kernel.org/all/20220624155226.2889613-3-a...@kernel.org/ Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/2] dma-mapping, remoteproc: Fix dma_mem leak after rproc_shutdown
On Fri, Apr 22, 2022 at 02:24:34PM +0800, Mark-PK Tsai wrote: > Release dma coherent memory before rvdev is free in > rproc_rvdev_release(). > > Below is the kmemleak report: > unreferenced object 0xff8051c1a980 (size 128): > comm "sh", pid 4895, jiffies 4295026604 (age 15481.896s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > backtrace: > [<3a0f3ec0>] dma_declare_coherent_memory+0x44/0x11c > [] rproc_add_virtio_dev+0xb8/0x20c > [ ] rproc_vdev_do_start+0x18/0x24 > [ ] rproc_start+0x22c/0x3e0 > [<0b938941>] rproc_boot+0x4a4/0x860 > [<3c4dc532>] state_store.52856+0x10c/0x1b8 > [ ] dev_attr_store+0x34/0x84 > [<83a53bdb>] sysfs_kf_write+0x60/0xbc > [<8ed830df>] kernfs_fop_write+0x198/0x458 > [<72b9ad06>] __vfs_write+0x50/0x210 > [<377d7469>] vfs_write+0xe4/0x1a8 > [ ] ksys_write+0x78/0x144 > [<9aef6f4b>] __arm64_sys_write+0x1c/0x28 > [<03496a98>] el0_svc_common+0xc8/0x22c > [ ] el0_svc_compat_handler+0x1c/0x28 > [ ] el0_svc_compat+0x8/0x24 > > Mark-PK Tsai (2): > dma-mapping: Add dma_release_coherent_memory to DMA API > remoteproc: Fix dma_mem leak after rproc_shutdown > > drivers/remoteproc/remoteproc_core.c | 1 + > include/linux/dma-map-ops.h | 3 +++ > kernel/dma/coherent.c| 10 -- > 3 files changed, 12 insertions(+), 2 deletions(-) Applied. Thanks, Mathieu > > -- > 2.18.0 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 1/3] scsi: BusLogic remove bus_to_virt
On 6/24/22 09:52, Arnd Bergmann wrote: From: Arnd Bergmann The BusLogic driver is the last remaining driver that relies on the deprecated bus_to_virt() function, which in turn only works on a few architectures, and is incompatible with both swiotlb and iommu support. Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."), the driver had a dependency on x86-32, presumably because of this problem. However, the change introduced another bug that made it still impossible to use the driver on any 64-bit machine. This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix 64-bit system enumeration error for Buslogic"), 8 years later, which shows that there are not a lot of users. Maciej is still using the driver on 32-bit hardware, and Khalid mentioned that the driver works with the device emulation used in VirtualBox and VMware. Both of those only emulate it for Windows 2000 and older operating systems that did not ship with the better LSI logic driver. Do a minimum fix that searches through the list of descriptors to find one that matches the bus address. This is clearly as inefficient as was indicated in the code comment about the lack of a bus_to_virt() replacement. A better fix would likely involve changing out the entire descriptor allocation for a simpler one, but that would be much more invasive. Cc: Maciej W. Rozycki Cc: Matt Wang Tested-by: Khalid Aziz Reviewed-by: Robin Murphy Reviewed-by: Hannes Reinecke Signed-off-by: Arnd Bergmann --- v3: Address issues pointed out by Khalid Aziz v2: Attempt to fix the driver instead of removing it --- drivers/scsi/BusLogic.c | 35 +++ drivers/scsi/Kconfig| 2 +- 2 files changed, 24 insertions(+), 13 deletions(-) This looks good. Acked-by: Khalid Aziz diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c index a897c8f914cf..f2abffce2659 100644 --- a/drivers/scsi/BusLogic.c +++ b/drivers/scsi/BusLogic.c @@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter *adapter, return (hoststatus << 16) | tgt_status; } +/* + * turn the dma address from an inbox into a ccb pointer + * This is rather inefficient. + */ +static struct blogic_ccb * +blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox) +{ + struct blogic_ccb *ccb; + + for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all) + if (inbox->ccb == ccb->dma_handle) + break; + + return ccb; +} /* blogic_scan_inbox scans the Incoming Mailboxes saving any Incoming Mailbox entries for completion processing. */ - static void blogic_scan_inbox(struct blogic_adapter *adapter) { /* @@ -2540,17 +2554,14 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter) enum blogic_cmplt_code comp_code; while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) { - /* - We are only allowed to do this because we limit our - architectures we run on to machines where bus_to_virt( - actually works. There *needs* to be a dma_addr_to_virt() - in the new PCI DMA mapping interface to replace - bus_to_virt() or else this code is going to become very - innefficient. -*/ - struct blogic_ccb *ccb = - (struct blogic_ccb *) bus_to_virt(next_inbox->ccb); - if (comp_code != BLOGIC_CMD_NOTFOUND) { + struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, next_inbox); + if (!ccb) { + /* +* This should never happen, unless the CCB list is +* corrupted in memory. +*/ + blogic_warn("Could not find CCB for dma address %x\n", adapter, next_inbox->ccb); + } else if (comp_code != BLOGIC_CMD_NOTFOUND) { if (ccb->status == BLOGIC_CCB_ACTIVE || ccb->status == BLOGIC_CCB_RESET) { /* diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index 6e3a04107bb6..689186f3a908 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -514,7 +514,7 @@ config SCSI_HPTIOP config SCSI_BUSLOGIC tristate "BusLogic SCSI support" - depends on PCI && SCSI && VIRT_TO_BUS + depends on PCI && SCSI help This is support for BusLogic MultiMaster and FlashPoint SCSI Host Adapters. Consult the SCSI-HOWTO, available from ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] vfio/type1: Simplify bus_type determination
On Fri, 24 Jun 2022 16:12:55 +0100 Robin Murphy wrote: > On 2022-06-24 15:28, Alex Williamson wrote: > > On Fri, 24 Jun 2022 11:18:36 -0300 > > Jason Gunthorpe wrote: > > > >> On Fri, Jun 24, 2022 at 08:11:59AM -0600, Alex Williamson wrote: > >>> On Thu, 23 Jun 2022 22:50:30 -0300 > >>> Jason Gunthorpe wrote: > >>> > On Thu, Jun 23, 2022 at 05:00:44PM -0600, Alex Williamson wrote: > > +struct vfio_device *vfio_device_get_from_iommu(struct iommu_group > *iommu_group) > +{ > +struct vfio_group *group = > vfio_group_get_from_iommu(iommu_group); > +struct vfio_device *device; > >>> > >>> Check group for NULL. > >> > >> OK - FWIW in context this should only ever make sense to call with an > >> iommu_group which has already been derived from a vfio_group, and I did > >> initially consider a check with a WARN_ON(), but then decided that the > >> unguarded dereference would be a sufficiently strong message. No > >> problem > >> with bringing that back to make it more defensive if that's what you > >> prefer. > > > > A while down the road, that's a bit too much implicit knowledge of the > > intent and single purpose of this function just to simply avoid a test. > > > > I think we should just pass the 'struct vfio_group *' into the > attach_group op and have this API take that type in and forget the > vfio_group_get_from_iommu(). > >>> > >>> That's essentially what I'm suggesting, the vfio_group is passed as an > >>> opaque pointer which type1 can use for a > >>> vfio_group_for_each_vfio_device() type call. Thanks, > >> > >> I don't want to add a whole vfio_group_for_each_vfio_device() > >> machinery that isn't actually needed by anything.. This is all > >> internal, we don't need to design more than exactly what is needed. > >> > >> At this point if we change the signature of the attach then we may as > >> well just pass in the representative vfio_device, that is probably > >> less LOC overall. > > > > That means that vfio core still needs to pick an arbitrary > > representative device, which I find in fundamental conflict to the > > nature of groups. Type1 is the interface to the IOMMU API, if through > > the IOMMU API we can make an assumption that all devices within the > > group are equivalent for a given operation, that should be done in type1 > > code, not in vfio core. A for-each interface is commonplace and not > > significantly more code or design than already proposed. Thanks, > > It also occurred to me this morning that there's another middle-ground > option staring out from the call-wrapping notion I mentioned yesterday - > while I'm not keen to provide it from the IOMMU API, there's absolutely > no reason that VFIO couldn't just use the building blocks by itself, and > in fact it works out almost absurdly simple: > > static bool vfio_device_capable(struct device *dev, void *data) > { > return device_iommu_capable(dev, (enum iommu_cap)data); > } > > bool vfio_group_capable(struct iommu_group *group, enum iommu_cap cap) > { > return iommu_group_for_each_dev(group, (void *)cap, > vfio_device_capable); > } > > and much the same for iommu_domain_alloc() once I get that far. The > locking concern neatly disappears because we're no longer holding any > bus or device pointer that can go stale. How does that seem as a > compromise for now, looking forward to Jason's longer-term view of > rearranging the attach_group process such that a vfio_device falls > naturally to hand? Yup, that seems like another way to do it, a slight iteration on the current bus_type flow, and also avoids any sort of arbitrary representative device being passed around as an API. For clarity of the principle that all devices within the group should have the same capabilities, we could even further follow the existing bus_type and do a sanity test here at the same time, or perhaps simply stop after the first device to avoid the if-any-device-is-capable semantics implied above. Thanks, Alex ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 3/3] arch/*/: remove CONFIG_VIRT_TO_BUS
From: Arnd Bergmann All architecture-independent users of virt_to_bus() and bus_to_virt() have been fixed to use the dma mapping interfaces or have been removed now. This means the definitions on most architectures, and the CONFIG_VIRT_TO_BUS symbol are now obsolete and can be removed. The only exceptions to this are a few network and scsi drivers for m68k Amiga and VME machines and ppc32 Macintosh. These drivers work correctly with the old interfaces and are probably not worth changing. On alpha and parisc, virt_to_bus() were still used in asm/floppy.h. alpha can use isa_virt_to_bus() like x86 does, and parisc can just open-code the virt_to_phys() here, as this is architecture specific code. I tried updating the bus-virt-phys-mapping.rst documentation, which started as an email from Linus to explain some details of the Linux-2.0 driver interfaces. The bits about virt_to_bus() were declared obsolete backin 2000, and the rest is not all that relevant any more, so in the end I just decided to remove the file completely. Reviewed-by: Geert Uytterhoeven Acked-by: Geert Uytterhoeven Acked-by: Michael Ellerman (powerpc) Signed-off-by: Arnd Bergmann --- .../core-api/bus-virt-phys-mapping.rst| 220 -- Documentation/core-api/dma-api-howto.rst | 14 -- Documentation/core-api/index.rst | 1 - .../translations/zh_CN/core-api/index.rst | 1 - arch/alpha/Kconfig| 1 - arch/alpha/include/asm/floppy.h | 2 +- arch/alpha/include/asm/io.h | 8 +- arch/ia64/Kconfig | 1 - arch/ia64/include/asm/io.h| 8 - arch/m68k/Kconfig | 1 - arch/m68k/include/asm/virtconvert.h | 4 +- arch/microblaze/Kconfig | 1 - arch/microblaze/include/asm/io.h | 2 - arch/mips/Kconfig | 1 - arch/mips/include/asm/io.h| 9 - arch/parisc/Kconfig | 1 - arch/parisc/include/asm/floppy.h | 4 +- arch/parisc/include/asm/io.h | 2 - arch/powerpc/Kconfig | 1 - arch/powerpc/include/asm/io.h | 2 - arch/riscv/include/asm/page.h | 1 - arch/x86/Kconfig | 1 - arch/x86/include/asm/io.h | 9 - arch/xtensa/Kconfig | 1 - arch/xtensa/include/asm/io.h | 3 - include/asm-generic/io.h | 14 -- mm/Kconfig| 8 - 27 files changed, 10 insertions(+), 311 deletions(-) delete mode 100644 Documentation/core-api/bus-virt-phys-mapping.rst diff --git a/Documentation/core-api/bus-virt-phys-mapping.rst b/Documentation/core-api/bus-virt-phys-mapping.rst deleted file mode 100644 index c72b24a7d52c.. --- a/Documentation/core-api/bus-virt-phys-mapping.rst +++ /dev/null @@ -1,220 +0,0 @@ -== -How to access I/O mapped memory from within device drivers -== - -:Author: Linus - -.. warning:: - - The virt_to_bus() and bus_to_virt() functions have been - superseded by the functionality provided by the PCI DMA interface - (see Documentation/core-api/dma-api-howto.rst). They continue - to be documented below for historical purposes, but new code - must not use them. --davidm 00/12/12 - -:: - - [ This is a mail message in response to a query on IO mapping, thus the -strange format for a "document" ] - -The AHA-1542 is a bus-master device, and your patch makes the driver give the -controller the physical address of the buffers, which is correct on x86 -(because all bus master devices see the physical memory mappings directly). - -However, on many setups, there are actually **three** different ways of looking -at memory addresses, and in this case we actually want the third, the -so-called "bus address". - -Essentially, the three ways of addressing memory are (this is "real memory", -that is, normal RAM--see later about other details): - - - CPU untranslated. This is the "physical" address. Physical address - 0 is what the CPU sees when it drives zeroes on the memory bus. - - - CPU translated address. This is the "virtual" address, and is - completely internal to the CPU itself with the CPU doing the appropriate - translations into "CPU untranslated". - - - bus address. This is the address of memory as seen by OTHER devices, - not the CPU. Now, in theory there could be many different bus - addresses, with each device seeing memory in some device-specific way, but - happily most hardware designers aren't actually actively trying to make - things any more complex than necessary, so you can assume that all - external hardwar
[PATCH v3 1/3] scsi: BusLogic remove bus_to_virt
From: Arnd Bergmann The BusLogic driver is the last remaining driver that relies on the deprecated bus_to_virt() function, which in turn only works on a few architectures, and is incompatible with both swiotlb and iommu support. Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."), the driver had a dependency on x86-32, presumably because of this problem. However, the change introduced another bug that made it still impossible to use the driver on any 64-bit machine. This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix 64-bit system enumeration error for Buslogic"), 8 years later, which shows that there are not a lot of users. Maciej is still using the driver on 32-bit hardware, and Khalid mentioned that the driver works with the device emulation used in VirtualBox and VMware. Both of those only emulate it for Windows 2000 and older operating systems that did not ship with the better LSI logic driver. Do a minimum fix that searches through the list of descriptors to find one that matches the bus address. This is clearly as inefficient as was indicated in the code comment about the lack of a bus_to_virt() replacement. A better fix would likely involve changing out the entire descriptor allocation for a simpler one, but that would be much more invasive. Cc: Maciej W. Rozycki Cc: Matt Wang Tested-by: Khalid Aziz Reviewed-by: Robin Murphy Reviewed-by: Hannes Reinecke Signed-off-by: Arnd Bergmann --- v3: Address issues pointed out by Khalid Aziz v2: Attempt to fix the driver instead of removing it --- drivers/scsi/BusLogic.c | 35 +++ drivers/scsi/Kconfig| 2 +- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c index a897c8f914cf..f2abffce2659 100644 --- a/drivers/scsi/BusLogic.c +++ b/drivers/scsi/BusLogic.c @@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter *adapter, return (hoststatus << 16) | tgt_status; } +/* + * turn the dma address from an inbox into a ccb pointer + * This is rather inefficient. + */ +static struct blogic_ccb * +blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox) +{ + struct blogic_ccb *ccb; + + for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all) + if (inbox->ccb == ccb->dma_handle) + break; + + return ccb; +} /* blogic_scan_inbox scans the Incoming Mailboxes saving any Incoming Mailbox entries for completion processing. */ - static void blogic_scan_inbox(struct blogic_adapter *adapter) { /* @@ -2540,17 +2554,14 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter) enum blogic_cmplt_code comp_code; while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) { - /* - We are only allowed to do this because we limit our - architectures we run on to machines where bus_to_virt( - actually works. There *needs* to be a dma_addr_to_virt() - in the new PCI DMA mapping interface to replace - bus_to_virt() or else this code is going to become very - innefficient. -*/ - struct blogic_ccb *ccb = - (struct blogic_ccb *) bus_to_virt(next_inbox->ccb); - if (comp_code != BLOGIC_CMD_NOTFOUND) { + struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, next_inbox); + if (!ccb) { + /* +* This should never happen, unless the CCB list is +* corrupted in memory. +*/ + blogic_warn("Could not find CCB for dma address %x\n", adapter, next_inbox->ccb); + } else if (comp_code != BLOGIC_CMD_NOTFOUND) { if (ccb->status == BLOGIC_CCB_ACTIVE || ccb->status == BLOGIC_CCB_RESET) { /* diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index 6e3a04107bb6..689186f3a908 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -514,7 +514,7 @@ config SCSI_HPTIOP config SCSI_BUSLOGIC tristate "BusLogic SCSI support" - depends on PCI && SCSI && VIRT_TO_BUS + depends on PCI && SCSI help This is support for BusLogic MultiMaster and FlashPoint SCSI Host Adapters. Consult the SCSI-HOWTO, available from -- 2.29.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 0/3] phase out CONFIG_VIRT_TO_BUS
From: Arnd Bergmann The virt_to_bus/bus_to_virt interface has been deprecated for decades. After Jakub Kicinski put a lot of work into cleaning out the network drivers using them, there are only a couple of other drivers left, which can all be removed or otherwise cleaned up, to remove the old interface for good. Any out of tree drivers using virt_to_bus() should be converted to using the dma-mapping interfaces, typically dma_alloc_coherent() or dma_map_single()). There are a few m68k and ppc32 specific drivers that keep using the interfaces, but these are all guarded with architecture-specific Kconfig dependencies, and are not actually broken. It might be helpful as a follow-up to replace them with platform specific helpers for amiga, m68k-vme and powermac. There are still a number of drivers that are using virt_to_phys() and phys_to_virt() in place of dma-mapping operations, and these are often broken, but they are out of scope for this series. If there are no more issues identified with this series, I'll merge it through the asm-generic tree. The SCSI patches can also get merged separately through the SCSI maintainers' tree if they prefer. Arnd --- Changes since v2: - Drop the dpt_i2o driver completely rather than fixing it - fix mistake in BusLogic patch Changes since v1: - dropped VME patches that are already in staging-next - dropped media patch that gets merged independently - added a networking patch and dropped it again after it got merged - replace BusLogic removal with a workaround Cc: Jakub Kicinski Cc: Christoph Hellwig # dma-mapping Cc: Marek Szyprowski # dma-mapping Cc: Robin Murphy # dma-mapping Cc: iommu@lists.linux-foundation.org Cc: Khalid Aziz # buslogic Cc: Maciej W. Rozycki # buslogic Cc: Matt Wang # buslogic Cc: Miquel van Smoorenburg # dpt_i2o Cc: Mark Salyzyn # dpt_i2o Cc: linux-s...@vger.kernel.org Cc: linuxppc-...@lists.ozlabs.org Cc: linux-a...@vger.kernel.org Cc: linux-al...@vger.kernel.org Cc: linux-m...@lists.linux-m68k.org Cc: linux-par...@vger.kernel.org Cc: Denis Efremov # floppy Arnd Bergmann (3): scsi: BusLogic remove bus_to_virt scsi: dpt_i2o: remove obsolete driver arch/*/: remove CONFIG_VIRT_TO_BUS .../core-api/bus-virt-phys-mapping.rst| 220 - Documentation/core-api/dma-api-howto.rst | 14 - Documentation/core-api/index.rst |1 - .../translations/zh_CN/core-api/index.rst |1 - .../userspace-api/ioctl/ioctl-number.rst |2 +- MAINTAINERS |8 - arch/alpha/Kconfig|1 - arch/alpha/include/asm/floppy.h |2 +- arch/alpha/include/asm/io.h |8 +- arch/ia64/Kconfig |1 - arch/ia64/include/asm/io.h|8 - arch/m68k/Kconfig |1 - arch/m68k/include/asm/virtconvert.h |4 +- arch/microblaze/Kconfig |1 - arch/microblaze/include/asm/io.h |2 - arch/mips/Kconfig |1 - arch/mips/include/asm/io.h|9 - arch/parisc/Kconfig |1 - arch/parisc/include/asm/floppy.h |4 +- arch/parisc/include/asm/io.h |2 - arch/powerpc/Kconfig |1 - arch/powerpc/include/asm/io.h |2 - arch/riscv/include/asm/page.h |1 - arch/x86/Kconfig |1 - arch/x86/include/asm/io.h |9 - arch/xtensa/Kconfig |1 - arch/xtensa/include/asm/io.h |3 - drivers/scsi/BusLogic.c | 35 +- drivers/scsi/Kconfig | 13 +- drivers/scsi/Makefile |1 - drivers/scsi/dpt/dpti_i2o.h | 441 -- drivers/scsi/dpt/dpti_ioctl.h | 136 - drivers/scsi/dpt/dptsig.h | 336 -- drivers/scsi/dpt/osd_defs.h | 79 - drivers/scsi/dpt/osd_util.h | 358 -- drivers/scsi/dpt/sys_info.h | 417 -- drivers/scsi/dpt_i2o.c| 3546 - drivers/scsi/dpti.h | 331 -- include/asm-generic/io.h | 14 - mm/Kconfig|8 - 40 files changed, 35 insertions(+), 5989 deletions(-) delete mode 100644 Documentation/core-api/bus-virt-phys-mapping.rst delete mode 100644 drivers/scsi/dpt/dpti_i2o.h delete mode 100644 drivers/scsi/dpt/dpti_ioctl.h delete mode 100644 drivers/scsi/dpt/dptsig.h delete mode 100644 drivers/scsi/dpt/osd_defs.h delete mode 100644 drivers/scsi/dpt/osd_util.h delete mode 100644 drivers/scsi/dpt/sys_info.h delete mode 100644 drivers/scsi/dpt_i2o.c delete mode 100644 drivers/scsi/dpti.h -- 2.29.2
RE: [PATCH v13 0/9] ACPI/IORT: Support for IORT RMR node
> -Original Message- > From: Steven Price [mailto:steven.pr...@arm.com] > Sent: 17 June 2022 13:42 > To: Shameerali Kolothum Thodi ; > linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org; > iommu@lists.linux-foundation.org > Cc: Linuxarm ; lorenzo.pieral...@arm.com; > j...@8bytes.org; robin.mur...@arm.com; w...@kernel.org; wanghuiqiang > ; Guohanjun (Hanjun Guo) > ; sami.muja...@arm.com; j...@solid-run.com; > eric.au...@redhat.com; laurentiu.tu...@nxp.com; h...@infradead.org > Subject: Re: [PATCH v13 0/9] ACPI/IORT: Support for IORT RMR node > > On 15/06/2022 11:10, Shameer Kolothum wrote: > > Hi > > > > v12 --> v13 > > -No changes. Rebased to 5.19-rc1. > > -Picked up tags received from Laurentiu, Hanjun and Will. Thanks!. > > You've already got my Tested-by tags, but just to confirm I gave this a > spin and it works fine. Thanks Steve. I think the series is now in a good shape to be merged. Hi Will/Robin, Appreciate, if you could please take a look at the remaining SMMU related patches(7-9) and provide your approval? Thanks, Shameer > > Thanks, > > Steve > > > > > Thanks, > > Shameer > > > > From old: > > We have faced issues with 3408iMR RAID controller cards which > > fail to boot when SMMU is enabled. This is because these > > controllers make use of host memory for various caching related > > purposes and when SMMU is enabled the iMR firmware fails to > > access these memory regions as there is no mapping for them. > > IORT RMR provides a way for UEFI to describe and report these > > memory regions so that the kernel can make a unity mapping for > > these in SMMU. > > > > Change History: > > > > v11 --> v12 > > -Minor fix in patch #4 to address the issue reported by the kernel test > robot. > > -Added R-by tags by Christoph(patch #1) and Lorenzo(patch #4). > > -Added T-by from Steve to all relevant patches. Many thanks!. > > > > v10 --> v11 > > -Addressed Christoph's comments. We now have a callback to > > struct iommu_resv_region to free all related memory and also dropped > > the FW specific union and now has a container struct > iommu_iort_rmr_data. > > See patches #1 & #4 > > -Added R-by from Christoph. > > -Dropped R-by from Lorenzo for patches #4 & #5 due to the above > changes. > > -Also dropped T-by from Steve and Laurentiu. Many thanks for your test > > efforts. I have done basic sanity testing on my platform but please > > do it again at your end. > > > > v9 --> v10 > > - Dropped patch #1 ("Add temporary RMR node flag definitions") since > >the ACPICA header updates patch is now in the mailing list > > - Based on the suggestion from Christoph, introduced a > >resv_region_free_fw_data() callback in struct iommu_resv_region and > >used that to free RMR specific memory allocations. > > > > v8 --> v9 > > - Adressed comments from Robin on interfaces. > > - Addressed comments from Lorenzo. > > > > v7 --> v8 > > - Patch #1 has temp definitions for RMR related changes till > > the ACPICA header changes are part of kernel. > > - No early parsing of RMR node info and is only parsed at the > > time of use. > > - Changes to the RMR get/put API format compared to the > > previous version. > > - Support for RMR descriptor shared by multiple stream IDs. > > > > v6 --> v7 > > -fix pointed out by Steve to the SMMUv2 SMR bypass install in patch #8. > > > > v5 --> v6 > > - Addressed comments from Robin & Lorenzo. > > : Moved iort_parse_rmr() to acpi_iort_init() from > > iort_init_platform_devices(). > > : Removed use of struct iort_rmr_entry during the initial > > parse. Using struct iommu_resv_region instead. > > : Report RMR address alignment and overlap errors, but continue. > > : Reworked arm_smmu_init_bypass_stes() (patch # 6). > > - Updated SMMUv2 bypass SMR code. Thanks to Jon N (patch #8). > > - Set IOMMU protection flags(IOMMU_CACHE, IOMMU_MMIO) based > > on Type of RMR region. Suggested by Jon N. > > > > v4 --> v5 > > -Added a fw_data union to struct iommu_resv_region and removed > > struct iommu_rmr (Based on comments from Joerg/Robin). > > -Added iommu_put_rmrs() to release mem. > > -Thanks to Steve for verifying on SMMUv2, but not added the Tested-by > > yet because of the above changes. > > > > v3 -->v4 > > -Included the SMMUv2 SMR bypass install changes suggested by > > Steve(patch #7) > > -As per Robin's comments, RMR reserve implementation is now > > more generic (patch #8) and dropped v3 patches 8 and 10. > > -Rebase to 5.13-rc1 > > > > RFC v2 --> v3 > > -Dropped RFC tag as the ACPICA header changes are now ready to be > > part of 5.13[0]. But this series still has a dependency on that patch. > > -Added IORT E.b related changes(node flags, _DSM function 5 checks for > > PCIe). > > -Changed RMR to stream id mapping from M:N to M:1 as per the spec > and > > discussion here[1]. > > -Last two patches add support for SMMUv2(Thanks to Jon Nettleton!) > > > > Jon Net
Re: [PATCH v2 2/3] scsi: BusLogic remove bus_to_virt
On Fri, Jun 24, 2022 at 5:38 PM Khalid Aziz wrote: > On 6/23/22 08:47, Arnd Bergmann wrote: > > Driver works with this change. next_inbox is the correct pointer to pass. Ok, great! I'll add a 'Tested-by' line then. I was already in the process of preparing a v3 patch set, will send out the fixed patch in a bit. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/3] scsi: BusLogic remove bus_to_virt
On 6/23/22 08:47, Arnd Bergmann wrote: Can you test it again with this patch on top? diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c index d057abfcdd5c..9e67f2ee25ee 100644 --- a/drivers/scsi/BusLogic.c +++ b/drivers/scsi/BusLogic.c @@ -2554,8 +2554,14 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter) enum blogic_cmplt_code comp_code; while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) { - struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, adapter->next_inbox); - if (comp_code != BLOGIC_CMD_NOTFOUND) { + struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, next_inbox); + if (!ccb) { + /* +* This should never happen, unless the CCB list is +* corrupted in memory. +*/ + blogic_warn("Could not find CCB for dma address 0x%x\n", adapter, next_inbox->ccb); + } else if (comp_code != BLOGIC_CMD_NOTFOUND) { if (ccb->status == BLOGIC_CCB_ACTIVE || ccb->status == BLOGIC_CCB_RESET) { Hi Arnd, Driver works with this change. next_inbox is the correct pointer to pass. Thanks, Khalid ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] vfio/type1: Simplify bus_type determination
On 2022-06-24 15:28, Alex Williamson wrote: On Fri, 24 Jun 2022 11:18:36 -0300 Jason Gunthorpe wrote: On Fri, Jun 24, 2022 at 08:11:59AM -0600, Alex Williamson wrote: On Thu, 23 Jun 2022 22:50:30 -0300 Jason Gunthorpe wrote: On Thu, Jun 23, 2022 at 05:00:44PM -0600, Alex Williamson wrote: +struct vfio_device *vfio_device_get_from_iommu(struct iommu_group *iommu_group) +{ + struct vfio_group *group = vfio_group_get_from_iommu(iommu_group); + struct vfio_device *device; Check group for NULL. OK - FWIW in context this should only ever make sense to call with an iommu_group which has already been derived from a vfio_group, and I did initially consider a check with a WARN_ON(), but then decided that the unguarded dereference would be a sufficiently strong message. No problem with bringing that back to make it more defensive if that's what you prefer. A while down the road, that's a bit too much implicit knowledge of the intent and single purpose of this function just to simply avoid a test. I think we should just pass the 'struct vfio_group *' into the attach_group op and have this API take that type in and forget the vfio_group_get_from_iommu(). That's essentially what I'm suggesting, the vfio_group is passed as an opaque pointer which type1 can use for a vfio_group_for_each_vfio_device() type call. Thanks, I don't want to add a whole vfio_group_for_each_vfio_device() machinery that isn't actually needed by anything.. This is all internal, we don't need to design more than exactly what is needed. At this point if we change the signature of the attach then we may as well just pass in the representative vfio_device, that is probably less LOC overall. That means that vfio core still needs to pick an arbitrary representative device, which I find in fundamental conflict to the nature of groups. Type1 is the interface to the IOMMU API, if through the IOMMU API we can make an assumption that all devices within the group are equivalent for a given operation, that should be done in type1 code, not in vfio core. A for-each interface is commonplace and not significantly more code or design than already proposed. Thanks, It also occurred to me this morning that there's another middle-ground option staring out from the call-wrapping notion I mentioned yesterday - while I'm not keen to provide it from the IOMMU API, there's absolutely no reason that VFIO couldn't just use the building blocks by itself, and in fact it works out almost absurdly simple: static bool vfio_device_capable(struct device *dev, void *data) { return device_iommu_capable(dev, (enum iommu_cap)data); } bool vfio_group_capable(struct iommu_group *group, enum iommu_cap cap) { return iommu_group_for_each_dev(group, (void *)cap, vfio_device_capable); } and much the same for iommu_domain_alloc() once I get that far. The locking concern neatly disappears because we're no longer holding any bus or device pointer that can go stale. How does that seem as a compromise for now, looking forward to Jason's longer-term view of rearranging the attach_group process such that a vfio_device falls naturally to hand? Cheers, Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] vfio/type1: Simplify bus_type determination
On Fri, Jun 24, 2022 at 08:28:31AM -0600, Alex Williamson wrote: > > > That's essentially what I'm suggesting, the vfio_group is passed as an > > > opaque pointer which type1 can use for a > > > vfio_group_for_each_vfio_device() type call. Thanks, > > > > I don't want to add a whole vfio_group_for_each_vfio_device() > > machinery that isn't actually needed by anything.. This is all > > internal, we don't need to design more than exactly what is needed. > > > > At this point if we change the signature of the attach then we may as > > well just pass in the representative vfio_device, that is probably > > less LOC overall. > > That means that vfio core still needs to pick an arbitrary > representative device, which I find in fundamental conflict to the > nature of groups. Well, this is where iommu is going, I think Robin has explained this view well enough. Ideally we'd move VFIO away from trying to attach groups and attach when the device FD is opened, I view this as a micro step in that direction. > Type1 is the interface to the IOMMU API, if through the IOMMU API we > can make an assumption that all devices within the group are > equivalent for a given operation, that should be done in type1 code, > not in vfio core. iommu_group is part of the core code, if the representative device assumption stems from the iommu_group then the core code can safely make it. > A for-each interface is commonplace and not significantly more code > or design than already proposed. Except that someone else might get the idea to use it for something completely inappropriate. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/dma: Add config for PCI SAC address trick
On 2022-06-24 14:28, Joerg Roedel wrote: On Thu, Jun 23, 2022 at 12:41:00PM +0100, Robin Murphy wrote: On 2022-06-23 12:33, Joerg Roedel wrote: On Wed, Jun 22, 2022 at 02:12:39PM +0100, Robin Murphy wrote: Thanks for your bravery! It already starts, with that patch I am getting: xhci_hcd :02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000f address=0xff00ffefe000 flags=0x] In my kernel log. The device is an AMD XHCI controller and seems to funciton normally after boot. The message disappears with iommu.forcedac=0. Need to look more into that... Given how amd_iommu_domain_alloc() sets the domain aperture, presumably the DMA address allocated was 0xffefe000? Odd that it gets bits punched out in the middle rather than simply truncated off the top as I would have expected :/ So even more weird, as a workaround I changed the AMD IOMMU driver to allocate a 4-level page-table and limit the DMA aperture to 48 bits. I still get the same message. Hmm, in that case my best guess would be that somewhere between the device itself and the IOMMU input it's trying to sign-extend the address from bit 47 or lower, but for whatever reason bits 55:48 get lost. Comparing the PCI xHCI I have to hand, mine (with nothing plugged in) only has 6 pages mapped for its command ring and other stuff. Thus unless it's sharing that domain with other devices, to be accessing something down in the second MB of IOVA space suggests that this probably isn't the very first access it's made, and therefore it would almost certainly have to be the endpoint emitting a corrupted address, but only for certain operations. FWIW I'd be inclined to turn on DMA debug and call debug_dma_dump_mappings() from the IOMMU fault handler, and/or add a bit of tracing to all the DMA mapping/allocation sites in the xHCI driver, to see what the offending address most likely represents. Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: iommu_sva_bind_device question
On Fri, Jun 24, 2022 at 06:41:02AM -0700, Jerry Snitselaar wrote: > On Fri, Jun 24, 2022 at 09:43:30AM +0800, Baolu Lu wrote: > > On 2022/6/24 09:14, Jerry Snitselaar wrote: > > > On Fri, Jun 24, 2022 at 08:55:08AM +0800, Baolu Lu wrote: > > > > On 2022/6/24 01:02, Jerry Snitselaar wrote: > > > > > Hi Baolu & Dave, > > > > > > > > > > I noticed last night that on a Sapphire Rapids system if you boot > > > > > without > > > > > intel_iommu=on, the idxd driver will crash during probe in > > > > > iommu_sva_bind_device(). > > > > > Should there be a sanity check before calling dev_iommu_ops(), or is > > > > > the expectation > > > > > that the caller would verify it is safe to call? This seemed to be > > > > > uncovered by > > > > > the combination of 3f6634d997db ("iommu: Use right way to retrieve > > > > > iommu_ops"), and > > > > > 42a1b73852c4 ("dmaengine: idxd: Separate user and kernel pasid > > > > > enabling"). > > > > > > > > > > [ 21.423729] BUG: kernel NULL pointer dereference, address: > > > > > 0038 > > > > > [ 21.445108] #PF: supervisor read access in kernel mode > > > > > [ 21.450912] #PF: error_code(0x) - not-present page > > > > > [ 21.456706] PGD 0 > > > > > [ 21.459047] Oops: [#1] PREEMPT SMP NOPTI > > > > > [ 21.464004] CPU: 0 PID: 1420 Comm: kworker/0:3 Not tainted > > > > > 5.19.0-0.rc3.27.eln120.x86_64 #1 > > > > > [ 21.464011] Hardware name: Intel Corporation > > > > > EAGLESTREAM/EAGLESTREAM, BIOS EGSDCRB1.SYS.0067.D12.2110190954 > > > > > 10/19/2021 > > > > > [ 21.464015] Workqueue: events work_for_cpu_fn > > > > > [ 21.464030] RIP: 0010:iommu_sva_bind_device+0x1d/0xe0 > > > > > [ 21.464046] Code: c3 cc 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 > > > > > 00 00 41 57 41 56 49 89 d6 41 55 41 54 55 53 48 83 ec 08 48 8b 87 d8 > > > > > 02 00 00 <48> 8b 40 38 48 8b 50 10 48 83 7a 70 00 48 89 14 24 0f 84 > > > > > 91 00 00 > > > > > [ 21.464050] RSP: 0018:ff7245d9096b7db8 EFLAGS: 00010296 > > > > > [ 21.464054] RAX: RBX: ff1eadeec8a51000 RCX: > > > > > > > > > > [ 21.464058] RDX: ff7245d9096b7e24 RSI: RDI: > > > > > ff1eadeec8a510d0 > > > > > [ 21.464060] RBP: ff1eadeec8a51000 R08: b1a12300 R09: > > > > > ff1eadffbfce25b4 > > > > > [ 21.464062] R10: R11: 0038 R12: > > > > > c09f8000 > > > > > [ 21.464065] R13: ff1eadeec8a510d0 R14: ff7245d9096b7e24 R15: > > > > > ff1eaddf54429000 > > > > > [ 21.464067] FS: () GS:ff1eadee7f60() > > > > > knlGS: > > > > > [ 21.464070] CS: 0010 DS: ES: CR0: 80050033 > > > > > [ 21.464072] CR2: 0038 CR3: 0008c0e10006 CR4: > > > > > 00771ef0 > > > > > [ 21.464074] DR0: DR1: DR2: > > > > > > > > > > [ 21.464076] DR3: DR6: fffe07f0 DR7: > > > > > 0400 > > > > > [ 21.464078] PKRU: 5554 > > > > > [ 21.464079] Call Trace: > > > > > [ 21.464083] > > > > > [ 21.464092] idxd_pci_probe+0x259/0x1070 [idxd] > > > > > [ 21.464121] local_pci_probe+0x3e/0x80 > > > > > [ 21.464132] work_for_cpu_fn+0x13/0x20 > > > > > [ 21.464136] process_one_work+0x1c4/0x380 > > > > > [ 21.464143] worker_thread+0x1ab/0x380 > > > > > [ 21.464147] ? _raw_spin_lock_irqsave+0x23/0x50 > > > > > [ 21.464158] ? process_one_work+0x380/0x380 > > > > > [ 21.464161] kthread+0xe6/0x110 > > > > > [ 21.464168] ? kthread_complete_and_exit+0x20/0x20 > > > > > [ 21.464172] ret_from_fork+0x1f/0x30 > > > > > > > > > > I figure either there needs to be a check in iommu_sva_bind_device, or > > > > > idxd needs to check in idxd_enable_system_pasid that that > > > > > idxd->pdev->dev.iommu is not null before it tries calling > > > > > iommu_sva_bind_device. > > > > > > > > As documented around the iommu_sva_bind_device() interface: > > > > > > > > * iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) must be called > > > > first, > > > > to > > > > * initialize the required SVA features. > > > > > > > > idxd->pdev->dev.iommu should be checked in there. > > > > > > > > Dave, any thoughts? > > > > > > > > Best regards, > > > > baolu > > > > > > Duh, sorry I missed that in the comments. > > > > > > It calls iommu_dev_enable_feature(), but then goes into code that > > > calls iommu_sva_bind_device whether or not iommu_dev_enable_feature() > > > fails. > > > > > > You also will get the following warning if you don't have scalable > > > mode enabled (either not enabled by default, or if enabled by default > > > and passed intel_iommu=on,sm_off): > > > > If scalable mode is disabled, iommu_dev_enable_feature(IOMMU_SVA) will > > return failure, hence driver should not call iommu_sva_bind_device(). > > I guess below will disappear if above is fixed in the idxd driver. > > > > Best regards, > > baolu > > > > It looks
Re: [PATCH v2 1/2] vfio/type1: Simplify bus_type determination
On Fri, 24 Jun 2022 11:18:36 -0300 Jason Gunthorpe wrote: > On Fri, Jun 24, 2022 at 08:11:59AM -0600, Alex Williamson wrote: > > On Thu, 23 Jun 2022 22:50:30 -0300 > > Jason Gunthorpe wrote: > > > > > On Thu, Jun 23, 2022 at 05:00:44PM -0600, Alex Williamson wrote: > > > > > > > > >> +struct vfio_device *vfio_device_get_from_iommu(struct iommu_group > > > > > >> *iommu_group) > > > > > >> +{ > > > > > >> + struct vfio_group *group = > > > > > >> vfio_group_get_from_iommu(iommu_group); > > > > > >> + struct vfio_device *device; > > > > > > > > > > > > Check group for NULL. > > > > > > > > > > OK - FWIW in context this should only ever make sense to call with an > > > > > iommu_group which has already been derived from a vfio_group, and I > > > > > did > > > > > initially consider a check with a WARN_ON(), but then decided that > > > > > the > > > > > unguarded dereference would be a sufficiently strong message. No > > > > > problem > > > > > with bringing that back to make it more defensive if that's what you > > > > > prefer. > > > > > > > > A while down the road, that's a bit too much implicit knowledge of the > > > > intent and single purpose of this function just to simply avoid a test. > > > > > > > > > > I think we should just pass the 'struct vfio_group *' into the > > > attach_group op and have this API take that type in and forget the > > > vfio_group_get_from_iommu(). > > > > That's essentially what I'm suggesting, the vfio_group is passed as an > > opaque pointer which type1 can use for a > > vfio_group_for_each_vfio_device() type call. Thanks, > > I don't want to add a whole vfio_group_for_each_vfio_device() > machinery that isn't actually needed by anything.. This is all > internal, we don't need to design more than exactly what is needed. > > At this point if we change the signature of the attach then we may as > well just pass in the representative vfio_device, that is probably > less LOC overall. That means that vfio core still needs to pick an arbitrary representative device, which I find in fundamental conflict to the nature of groups. Type1 is the interface to the IOMMU API, if through the IOMMU API we can make an assumption that all devices within the group are equivalent for a given operation, that should be done in type1 code, not in vfio core. A for-each interface is commonplace and not significantly more code or design than already proposed. Thanks, Alex ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] vfio/type1: Simplify bus_type determination
On Fri, Jun 24, 2022 at 08:11:59AM -0600, Alex Williamson wrote: > On Thu, 23 Jun 2022 22:50:30 -0300 > Jason Gunthorpe wrote: > > > On Thu, Jun 23, 2022 at 05:00:44PM -0600, Alex Williamson wrote: > > > > > > >> +struct vfio_device *vfio_device_get_from_iommu(struct iommu_group > > > > >> *iommu_group) > > > > >> +{ > > > > >> +struct vfio_group *group = > > > > >> vfio_group_get_from_iommu(iommu_group); > > > > >> +struct vfio_device *device; > > > > > > > > > > Check group for NULL. > > > > > > > > OK - FWIW in context this should only ever make sense to call with an > > > > iommu_group which has already been derived from a vfio_group, and I did > > > > initially consider a check with a WARN_ON(), but then decided that the > > > > unguarded dereference would be a sufficiently strong message. No > > > > problem > > > > with bringing that back to make it more defensive if that's what you > > > > prefer. > > > > > > A while down the road, that's a bit too much implicit knowledge of the > > > intent and single purpose of this function just to simply avoid a test. > > > > I think we should just pass the 'struct vfio_group *' into the > > attach_group op and have this API take that type in and forget the > > vfio_group_get_from_iommu(). > > That's essentially what I'm suggesting, the vfio_group is passed as an > opaque pointer which type1 can use for a > vfio_group_for_each_vfio_device() type call. Thanks, I don't want to add a whole vfio_group_for_each_vfio_device() machinery that isn't actually needed by anything.. This is all internal, we don't need to design more than exactly what is needed. At this point if we change the signature of the attach then we may as well just pass in the representative vfio_device, that is probably less LOC overall. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] vfio/type1: Simplify bus_type determination
On Tue, Jun 21, 2022 at 08:09:20PM +0100, Robin Murphy wrote: > Quick consensus then: does anyone have a particular preference between > changing the .attach_group signature vs. adding a helper based on > vfio_group_get_from_iommu() for type1 to call from within its callback? They > seem about equal (but opposite) in terms of the simplicity vs. impact > tradeoff to me, so I can't quite decide conclusively... Ah, I've been away and only just saw this.. Given Alex's remarks and the need to re-get the vfio_group in the helper, it may be very slightly nicer to pass in the vfio_device as an argument. But either works for me. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] vfio/type1: Simplify bus_type determination
On Thu, 23 Jun 2022 22:50:30 -0300 Jason Gunthorpe wrote: > On Thu, Jun 23, 2022 at 05:00:44PM -0600, Alex Williamson wrote: > > > > >> +struct vfio_device *vfio_device_get_from_iommu(struct iommu_group > > > >> *iommu_group) > > > >> +{ > > > >> + struct vfio_group *group = > > > >> vfio_group_get_from_iommu(iommu_group); > > > >> + struct vfio_device *device; > > > > > > > > Check group for NULL. > > > > > > OK - FWIW in context this should only ever make sense to call with an > > > iommu_group which has already been derived from a vfio_group, and I did > > > initially consider a check with a WARN_ON(), but then decided that the > > > unguarded dereference would be a sufficiently strong message. No problem > > > with bringing that back to make it more defensive if that's what you > > > prefer. > > > > A while down the road, that's a bit too much implicit knowledge of the > > intent and single purpose of this function just to simply avoid a test. > > I think we should just pass the 'struct vfio_group *' into the > attach_group op and have this API take that type in and forget the > vfio_group_get_from_iommu(). That's essentially what I'm suggesting, the vfio_group is passed as an opaque pointer which type1 can use for a vfio_group_for_each_vfio_device() type call. Thanks, Alex ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[git pull] IOMMU Fixes for Linux v5.19-rc3
Hi Linus, The following changes since commit a111daf0c53ae91e71fd2bfe7497862d14132e3e: Linux 5.19-rc3 (2022-06-19 15:06:47 -0500) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git tags/iommu-fixes-v5.19-rc3 for you to fetch changes up to c242507c1b895646b4a25060df13b6214805759f: MAINTAINERS: Add new IOMMU development mailing list (2022-06-24 15:36:11 +0200) IOMMU Fixes for Linux v5.19-rc3 Including: - Add a new IOMMU mailing list to the MAINTAINERS file to prepare for the a list migration happening on July 5th. The old list needs to stay in place until the switch happens to guarantee seemless archiving of list email. - Fix compatible device-tree string for rcar-gen4 in Renesas IOMMU driver. Joerg Roedel (1): MAINTAINERS: Add new IOMMU development mailing list Yoshihiro Shimoda (1): iommu/ipmmu-vmsa: Fix compatible for rcar-gen4 MAINTAINERS| 11 +++ drivers/iommu/ipmmu-vmsa.c | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) Please pull. Thanks, Joerg signature.asc Description: Digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: iommu_sva_bind_device question
On Fri, Jun 24, 2022 at 09:43:30AM +0800, Baolu Lu wrote: > On 2022/6/24 09:14, Jerry Snitselaar wrote: > > On Fri, Jun 24, 2022 at 08:55:08AM +0800, Baolu Lu wrote: > > > On 2022/6/24 01:02, Jerry Snitselaar wrote: > > > > Hi Baolu & Dave, > > > > > > > > I noticed last night that on a Sapphire Rapids system if you boot > > > > without > > > > intel_iommu=on, the idxd driver will crash during probe in > > > > iommu_sva_bind_device(). > > > > Should there be a sanity check before calling dev_iommu_ops(), or is > > > > the expectation > > > > that the caller would verify it is safe to call? This seemed to be > > > > uncovered by > > > > the combination of 3f6634d997db ("iommu: Use right way to retrieve > > > > iommu_ops"), and > > > > 42a1b73852c4 ("dmaengine: idxd: Separate user and kernel pasid > > > > enabling"). > > > > > > > > [ 21.423729] BUG: kernel NULL pointer dereference, address: > > > > 0038 > > > > [ 21.445108] #PF: supervisor read access in kernel mode > > > > [ 21.450912] #PF: error_code(0x) - not-present page > > > > [ 21.456706] PGD 0 > > > > [ 21.459047] Oops: [#1] PREEMPT SMP NOPTI > > > > [ 21.464004] CPU: 0 PID: 1420 Comm: kworker/0:3 Not tainted > > > > 5.19.0-0.rc3.27.eln120.x86_64 #1 > > > > [ 21.464011] Hardware name: Intel Corporation > > > > EAGLESTREAM/EAGLESTREAM, BIOS EGSDCRB1.SYS.0067.D12.2110190954 > > > > 10/19/2021 > > > > [ 21.464015] Workqueue: events work_for_cpu_fn > > > > [ 21.464030] RIP: 0010:iommu_sva_bind_device+0x1d/0xe0 > > > > [ 21.464046] Code: c3 cc 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 > > > > 00 41 57 41 56 49 89 d6 41 55 41 54 55 53 48 83 ec 08 48 8b 87 d8 02 00 > > > > 00 <48> 8b 40 38 48 8b 50 10 48 83 7a 70 00 48 89 14 24 0f 84 91 00 00 > > > > [ 21.464050] RSP: 0018:ff7245d9096b7db8 EFLAGS: 00010296 > > > > [ 21.464054] RAX: RBX: ff1eadeec8a51000 RCX: > > > > > > > > [ 21.464058] RDX: ff7245d9096b7e24 RSI: RDI: > > > > ff1eadeec8a510d0 > > > > [ 21.464060] RBP: ff1eadeec8a51000 R08: b1a12300 R09: > > > > ff1eadffbfce25b4 > > > > [ 21.464062] R10: R11: 0038 R12: > > > > c09f8000 > > > > [ 21.464065] R13: ff1eadeec8a510d0 R14: ff7245d9096b7e24 R15: > > > > ff1eaddf54429000 > > > > [ 21.464067] FS: () GS:ff1eadee7f60() > > > > knlGS: > > > > [ 21.464070] CS: 0010 DS: ES: CR0: 80050033 > > > > [ 21.464072] CR2: 0038 CR3: 0008c0e10006 CR4: > > > > 00771ef0 > > > > [ 21.464074] DR0: DR1: DR2: > > > > > > > > [ 21.464076] DR3: DR6: fffe07f0 DR7: > > > > 0400 > > > > [ 21.464078] PKRU: 5554 > > > > [ 21.464079] Call Trace: > > > > [ 21.464083] > > > > [ 21.464092] idxd_pci_probe+0x259/0x1070 [idxd] > > > > [ 21.464121] local_pci_probe+0x3e/0x80 > > > > [ 21.464132] work_for_cpu_fn+0x13/0x20 > > > > [ 21.464136] process_one_work+0x1c4/0x380 > > > > [ 21.464143] worker_thread+0x1ab/0x380 > > > > [ 21.464147] ? _raw_spin_lock_irqsave+0x23/0x50 > > > > [ 21.464158] ? process_one_work+0x380/0x380 > > > > [ 21.464161] kthread+0xe6/0x110 > > > > [ 21.464168] ? kthread_complete_and_exit+0x20/0x20 > > > > [ 21.464172] ret_from_fork+0x1f/0x30 > > > > > > > > I figure either there needs to be a check in iommu_sva_bind_device, or > > > > idxd needs to check in idxd_enable_system_pasid that that > > > > idxd->pdev->dev.iommu is not null before it tries calling > > > > iommu_sva_bind_device. > > > > > > As documented around the iommu_sva_bind_device() interface: > > > > > > * iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) must be called > > > first, > > > to > > > * initialize the required SVA features. > > > > > > idxd->pdev->dev.iommu should be checked in there. > > > > > > Dave, any thoughts? > > > > > > Best regards, > > > baolu > > > > Duh, sorry I missed that in the comments. > > > > It calls iommu_dev_enable_feature(), but then goes into code that > > calls iommu_sva_bind_device whether or not iommu_dev_enable_feature() > > fails. > > > > You also will get the following warning if you don't have scalable > > mode enabled (either not enabled by default, or if enabled by default > > and passed intel_iommu=on,sm_off): > > If scalable mode is disabled, iommu_dev_enable_feature(IOMMU_SVA) will > return failure, hence driver should not call iommu_sva_bind_device(). > I guess below will disappear if above is fixed in the idxd driver. > > Best regards, > baolu > It looks like there was a recent maintainer change, and Fenghua is now the maintainer. Fenghua thoughts on this? With 42a1b73852c4 ("dmaengine: idxd: Separate user and kernel pasid enabling") the code no longer depends on iommu_dev_feature_enable succeeding. Testing with something like this works (
Re: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver
On Mon, Jun 20, 2022 at 05:49:13AM +, nobuhiro1.iwama...@toshiba.co.jp wrote: > Hi, > > Thanks for your review. > > > -Original Message- > > From: Jason Gunthorpe > > Sent: Thursday, May 26, 2022 3:27 AM > > To: Baolu Lu > > Cc: iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT) > > ; Joerg Roedel ; Will > > Deacon ; Rob Herring ; > > devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; > > iommu@lists.linux-foundation.org; ishikawa yuji(石川 悠司 ○RDC□AIT > > C○EA開) ; > > linux-arm-ker...@lists.infradead.org > > Subject: Re: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver > > > > On Wed, May 25, 2022 at 02:26:37PM +0800, Baolu Lu wrote: > > > On 2022/5/25 09:31, Nobuhiro Iwamatsu wrote: > > > > +static const struct iommu_ops visconti_atu_ops = { > > > > + .domain_alloc = visconti_atu_domain_alloc, > > > > + .probe_device = visconti_atu_probe_device, > > > > + .release_device = visconti_atu_release_device, > > > > + .device_group = generic_device_group, > > > > + .of_xlate = visconti_atu_of_xlate, > > > > + .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP, > > > > + .default_domain_ops = &(const struct iommu_domain_ops) { > > > > + .attach_dev = visconti_atu_attach_device, > > > > + .detach_dev = visconti_atu_detach_device, > > > > > > The detach_dev callback is about to be deprecated. The new drivers > > > should implement the default domain and blocking domain instead. > > > > Yes please, new drivers need to use default_domains. > > > > It is very strange that visconti_atu_detach_device() does nothing. It is > > not > > required that a domain is fully unmapped before being destructed, I think > > detach should set ATU_AT_EN to 0. > > I see, I rethink implementation. > > > > > What behavior does the HW have when ATU_AT_ENTRY_EN == 0? If DMA is > > rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and > > return that from ops->def_domain_type(). > > If ATU_AT_ENTRY_EN is 0, nothing happens. It does not work with IOMMU, > it works with the memory space set in device tree. So that would be an assignment to DOMAIN_IDENTITY > > I'm feeling like these "special" drivers need some kind of handshake with > > their > > only users because they don't work with things like VFIO.. > > Since the devices that utilize this IOMMU function are fixed, I do > not think that a special handshake is required. Could you you tell > me where you thought you needed a handshake? In this case the iommu driver is so limited that it will not work with VFIO - it is only ment to be used with the fixed drivers that are paired with it. Ideally we'd prevent VFIO from connecting and only allow drivers that know the limitations of the IOMMU to use the unmanaged domain. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/dma: Add config for PCI SAC address trick
On Thu, Jun 23, 2022 at 12:41:00PM +0100, Robin Murphy wrote: > On 2022-06-23 12:33, Joerg Roedel wrote: > > On Wed, Jun 22, 2022 at 02:12:39PM +0100, Robin Murphy wrote: > > > Thanks for your bravery! > > > > It already starts, with that patch I am getting: > > > > xhci_hcd :02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT > > domain=0x000f address=0xff00ffefe000 flags=0x] > > > > In my kernel log. The device is an AMD XHCI controller and seems to > > funciton normally after boot. The message disappears with > > iommu.forcedac=0. > > > > Need to look more into that... > > Given how amd_iommu_domain_alloc() sets the domain aperture, presumably the > DMA address allocated was 0xffefe000? Odd that it gets bits punched > out in the middle rather than simply truncated off the top as I would have > expected :/ So even more weird, as a workaround I changed the AMD IOMMU driver to allocate a 4-level page-table and limit the DMA aperture to 48 bits. I still get the same message. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] MAINTAINERS: Add new IOMMU development mailing list
On Fri, Jun 24, 2022 at 05:59:39AM -0700, Christoph Hellwig wrote: > Shouldn't this also remove the old list given it has only a few days > to live? No, the new list is not yet archived on lore. There will be a hard switch of the archive on July 5th, and after that the old list can be removed. Regards, -- Jörg Rödel jroe...@suse.de SUSE Software Solutions Germany GmbH Maxfeldstr. 5 90409 Nürnberg Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/5] vfio/iommu_type1: Remove the domain->ops comparison
On Wed, Jun 22, 2022 at 08:54:45AM +0100, Robin Murphy wrote: > On 2022-06-16 23:23, Nicolin Chen wrote: > > On Thu, Jun 16, 2022 at 06:40:14AM +, Tian, Kevin wrote: > > > > > > The domain->ops validation was added, as a precaution, for mixed-driver > > > > systems. However, at this moment only one iommu driver is possible. So > > > > remove it. > > > > > > It's true on a physical platform. But I'm not sure whether a virtual > > > platform > > > is allowed to include multiple e.g. one virtio-iommu alongside a virtual > > > VT-d > > > or a virtual smmu. It might be clearer to claim that (as Robin pointed > > > out) > > > there is plenty more significant problems than this to solve instead of > > > simply > > > saying that only one iommu driver is possible if we don't have explicit > > > code > > > to reject such configuration. 😊 > > > > Will edit this part. Thanks! > > Oh, physical platforms with mixed IOMMUs definitely exist already. The main > point is that while bus_set_iommu still exists, the core code effectively > *does* prevent multiple drivers from registering - even in emulated cases > like the example above, virtio-iommu and VT-d would both try to > bus_set_iommu(&pci_bus_type), and one of them will lose. The aspect which > might warrant clarification is that there's no combination of supported > drivers which claim non-overlapping buses *and* could appear in the same > system - even if you tried to contrive something by emulating, say, VT-d > (PCI) alongside rockchip-iommu (platform), you could still only describe one > or the other due to ACPI vs. Devicetree. Right, and that is still something we need to protect against with this ops check. VFIO is not checking that the bus's are the same before attempting to re-use a domain. So it is actually functional and does protect against systems with multiple iommu drivers on different busses. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] MAINTAINERS: Add new IOMMU development mailing list
On Fri, Jun 24, 2022 at 02:51:39PM +0200, Joerg Roedel wrote: > From: Joerg Roedel > > The IOMMU mailing list will move from lists.linux-foundation.org to > lists.linux.dev. The hard switch of the archive will happen on July > 5th, but add the new list now already so that people start using the > list when sending patches. After July 5th the old list will disappear. Shouldn't this also remove the old list given it has only a few days to live? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] MAINTAINERS: Add new IOMMU development mailing list
From: Joerg Roedel The IOMMU mailing list will move from lists.linux-foundation.org to lists.linux.dev. The hard switch of the archive will happen on July 5th, but add the new list now already so that people start using the list when sending patches. After July 5th the old list will disappear. Cc: sta...@vger.kernel.org Signed-off-by: Joerg Roedel --- MAINTAINERS | 11 +++ 1 file changed, 11 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3cf9842d9233..36d1bc999815 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -427,6 +427,7 @@ ACPI VIOT DRIVER M: Jean-Philippe Brucker L: linux-a...@vger.kernel.org L: iommu@lists.linux-foundation.org +L: io...@lists.linux.dev S: Maintained F: drivers/acpi/viot.c F: include/linux/acpi_viot.h @@ -960,6 +961,7 @@ AMD IOMMU (AMD-VI) M: Joerg Roedel R: Suravee Suthikulpanit L: iommu@lists.linux-foundation.org +L: io...@lists.linux.dev S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git F: drivers/iommu/amd/ @@ -5962,6 +5964,7 @@ M:Christoph Hellwig M: Marek Szyprowski R: Robin Murphy L: iommu@lists.linux-foundation.org +L: io...@lists.linux.dev S: Supported W: http://git.infradead.org/users/hch/dma-mapping.git T: git git://git.infradead.org/users/hch/dma-mapping.git @@ -5974,6 +5977,7 @@ F:kernel/dma/ DMA MAPPING BENCHMARK M: Xiang Chen L: iommu@lists.linux-foundation.org +L: io...@lists.linux.dev F: kernel/dma/map_benchmark.c F: tools/testing/selftests/dma/ @@ -7558,6 +7562,7 @@ F:drivers/gpu/drm/exynos/exynos_dp* EXYNOS SYSMMU (IOMMU) driver M: Marek Szyprowski L: iommu@lists.linux-foundation.org +L: io...@lists.linux.dev S: Maintained F: drivers/iommu/exynos-iommu.c @@ -9977,6 +9982,7 @@ INTEL IOMMU (VT-d) M: David Woodhouse M: Lu Baolu L: iommu@lists.linux-foundation.org +L: io...@lists.linux.dev S: Supported T: git git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git F: drivers/iommu/intel/ @@ -10356,6 +10362,7 @@ IOMMU DRIVERS M: Joerg Roedel M: Will Deacon L: iommu@lists.linux-foundation.org +L: io...@lists.linux.dev S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git F: Documentation/devicetree/bindings/iommu/ @@ -12504,6 +12511,7 @@ F: drivers/i2c/busses/i2c-mt65xx.c MEDIATEK IOMMU DRIVER M: Yong Wu L: iommu@lists.linux-foundation.org +L: io...@lists.linux.dev L: linux-media...@lists.infradead.org (moderated for non-subscribers) S: Supported F: Documentation/devicetree/bindings/iommu/mediatek* @@ -16545,6 +16553,7 @@ F: drivers/i2c/busses/i2c-qcom-cci.c QUALCOMM IOMMU M: Rob Clark L: iommu@lists.linux-foundation.org +L: io...@lists.linux.dev L: linux-arm-...@vger.kernel.org S: Maintained F: drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -19170,6 +19179,7 @@ F: arch/x86/boot/video* SWIOTLB SUBSYSTEM M: Christoph Hellwig L: iommu@lists.linux-foundation.org +L: io...@lists.linux.dev S: Supported W: http://git.infradead.org/users/hch/dma-mapping.git T: git git://git.infradead.org/users/hch/dma-mapping.git @@ -21844,6 +21854,7 @@ M: Juergen Gross M: Stefano Stabellini L: xen-de...@lists.xenproject.org (moderated for non-subscribers) L: iommu@lists.linux-foundation.org +L: io...@lists.linux.dev S: Supported F: arch/x86/xen/*swiotlb* F: drivers/xen/*swiotlb* -- 2.36.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 1/5] iommu: Return -EMEDIUMTYPE for incompatible domain and device/group
On Fri, 2022-06-24 at 06:16 +, Tian, Kevin wrote: > > From: Yong Wu > > Sent: Friday, June 24, 2022 1:39 PM > > > > On Thu, 2022-06-23 at 19:44 -0700, Nicolin Chen wrote: > > > On Fri, Jun 24, 2022 at 09:35:49AM +0800, Baolu Lu wrote: > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > On 2022/6/24 04:00, Nicolin Chen wrote: > > > > > diff --git a/drivers/iommu/mtk_iommu_v1.c > > > > > b/drivers/iommu/mtk_iommu_v1.c > > > > > index e1cb51b9866c..5386d889429d 100644 > > > > > --- a/drivers/iommu/mtk_iommu_v1.c > > > > > +++ b/drivers/iommu/mtk_iommu_v1.c > > > > > @@ -304,7 +304,7 @@ static int > > > > > mtk_iommu_v1_attach_device(struct > > > > > iommu_domain *domain, struct device > > > > > /* Only allow the domain created internally. */ > > > > > mtk_mapping = data->mapping; > > > > > if (mtk_mapping->domain != domain) > > > > > - return 0; > > > > > + return -EMEDIUMTYPE; > > > > > > > > > > if (!data->m4u_dom) { > > > > > data->m4u_dom = dom; > > > > > > > > This change looks odd. It turns the return value from success > > > > to > > > > failure. Is it a bug? If so, it should go through a separated > > > > fix > > > > patch. > > > > Thanks for the review:) > > > > > > > > Makes sense. > > > > > > I read the commit log of the original change: > > > > > > > https://lore.kernel.org/r/1589530123-30240-1-git-send-email- > > yong...@mediatek.com > > > > > > It doesn't seem to allow devices to get attached to different > > > domains other than the shared mapping->domain, created in the > > > in the mtk_iommu_probe_device(). So it looks like returning 0 > > > is intentional. Though I am still very confused by this return > > > value here, I doubt it has ever been used in a VFIO context. > > > > It's not used in VFIO context. "return 0" just satisfy the iommu > > framework to go ahead. and yes, here we only allow the shared > > "mapping- > > > domain" (All the devices share a domain created internally). > > > > thus I think we should still keep "return 0" here. > > > > What prevent this driver from being used in VFIO context? Nothing prevent this. Just I didn't test. mtk_iommu_v1.c only is used in mt2701 and there is no VFIO scenario. I'm not sure if it supports VFIO. (mtk_iommu.c support VFIO.) > and why would we want to go ahead when an obvious error occurs > i.e. when a device is attached to an unexpected domain? The iommu flow in this file always is a bit odd as we need share iommu domain in ARM32. As I tested before in the above link, "The iommu framework will create a iommu domain for each a device.", therefore we have to *workaround* in this file. And this was expected to be fixed by: https://lore.kernel.org/linux-iommu/cover.1597931875.git.robin.mur...@arm.com/ sorry, I don't know its current status. Thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] MAINTAINERS: Add new IOMMU development mailing list
On Thu, Jun 23, 2022 at 11:30:06PM -0700, Christoph Hellwig wrote: > iommu@lists.linux-foundation.org is also listed for various other > MAINTAINERS entries. Can you please send a list to update all of them > to Linus ASAP, including for 5.19 and -stable? Right, will do, thanks for the heads-up. -- Jörg Rödel jroe...@suse.de SUSE Software Solutions Germany GmbH Maxfeldstr. 5 90409 Nürnberg Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 3/3] iommu/mediatek: Add support for MT6795 Helio X10 M4Us
Add support for the M4Us found in the MT6795 Helio X10 SoC. Signed-off-by: AngeloGioacchino Del Regno Reviewed-by: Yong Wu --- drivers/iommu/mtk_iommu.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index 63df612cf2e0..82eba647c183 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -158,6 +158,7 @@ enum mtk_iommu_plat { M4U_MT2712, M4U_MT6779, + M4U_MT6795, M4U_MT8167, M4U_MT8173, M4U_MT8183, @@ -1419,6 +1420,19 @@ static const struct mtk_iommu_plat_data mt6779_data = { .larbid_remap = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}}, }; +static const struct mtk_iommu_plat_data mt6795_data = { + .m4u_plat = M4U_MT6795, + .flags= HAS_4GB_MODE | HAS_BCLK | RESET_AXI | + HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM | + TF_PORT_TO_ADDR_MT8173, + .inv_sel_reg = REG_MMU_INV_SEL_GEN1, + .banks_num= 1, + .banks_enable = {true}, + .iova_region = single_domain, + .iova_region_nr = ARRAY_SIZE(single_domain), + .larbid_remap = {{0}, {1}, {2}, {3}, {4}}, /* Linear mapping. */ +}; + static const struct mtk_iommu_plat_data mt8167_data = { .m4u_plat = M4U_MT8167, .flags= RESET_AXI | HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM, @@ -1531,6 +1545,7 @@ static const struct mtk_iommu_plat_data mt8195_data_vpp = { static const struct of_device_id mtk_iommu_of_ids[] = { { .compatible = "mediatek,mt2712-m4u", .data = &mt2712_data}, { .compatible = "mediatek,mt6779-m4u", .data = &mt6779_data}, + { .compatible = "mediatek,mt6795-m4u", .data = &mt6795_data}, { .compatible = "mediatek,mt8167-m4u", .data = &mt8167_data}, { .compatible = "mediatek,mt8173-m4u", .data = &mt8173_data}, { .compatible = "mediatek,mt8183-m4u", .data = &mt8183_data}, -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 0/3] MediaTek Helio X10 MT6795 - M4U/IOMMU Support
In an effort to give some love to the apparently forgotten MT6795 SoC, I am upstreaming more components that are necessary to support platforms powered by this one apart from a simple boot to serial console. This series introduces support for the IOMMUs found on this SoC. Tested on a MT6795 Sony Xperia M5 (codename "Holly") smartphone. Changes in v4: - Retitled mtk_iommu commits to iommu/mediatek as suggested by Yong Wu - Removed unused M4U_LARB5_ID definition - Rebased on next-20220624 and https://patchwork.kernel.org/project/linux-mediatek/list/?series=650969 Changes in v3: - Added new flag as suggested by Yong Wu - Rebased on top of https://patchwork.kernel.org/project/linux-mediatek/list/?series=648784 Changes in v2: - Rebased on top of https://patchwork.kernel.org/project/linux-mediatek/list/?series=642681 AngeloGioacchino Del Regno (3): dt-bindings: mediatek: Add bindings for MT6795 M4U iommu/mediatek: Introduce new flag TF_PORT_TO_ADDR_MT8173 iommu/mediatek: Add support for MT6795 Helio X10 M4Us .../bindings/iommu/mediatek,iommu.yaml| 4 + drivers/iommu/mtk_iommu.c | 21 +++- include/dt-bindings/memory/mt6795-larb-port.h | 95 +++ 3 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 include/dt-bindings/memory/mt6795-larb-port.h -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 2/3] iommu/mediatek: Introduce new flag TF_PORT_TO_ADDR_MT8173
In preparation for adding support for MT6795, add a new flag named TF_PORT_TO_ADDR_MT8173 and use that instead of checking for m4u_plat type in mtk_iommu_hw_init() to avoid seeing a long list of m4u_plat checks there in the future. Signed-off-by: AngeloGioacchino Del Regno Reviewed-by: Yong Wu --- drivers/iommu/mtk_iommu.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index b2ae84046249..63df612cf2e0 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -138,6 +138,7 @@ /* PM and clock always on. e.g. infra iommu */ #define PM_CLK_AO BIT(15) #define IFA_IOMMU_PCIE_SUPPORT BIT(16) +#define TF_PORT_TO_ADDR_MT8173 BIT(17) #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask) \ pdata)->flags) & (mask)) == (_x)) @@ -959,7 +960,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data, unsigned int ban * Global control settings are in bank0. May re-init these global registers * since no sure if there is bank0 consumers. */ - if (data->plat_data->m4u_plat == M4U_MT8173) { + if (MTK_IOMMU_HAS_FLAG(data->plat_data, TF_PORT_TO_ADDR_MT8173)) { regval = F_MMU_PREFETCH_RT_REPLACE_MOD | F_MMU_TF_PROT_TO_PROGRAM_ADDR_MT8173; } else { @@ -1432,7 +1433,8 @@ static const struct mtk_iommu_plat_data mt8167_data = { static const struct mtk_iommu_plat_data mt8173_data = { .m4u_plat = M4U_MT8173, .flags= HAS_4GB_MODE | HAS_BCLK | RESET_AXI | - HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM, + HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM | + TF_PORT_TO_ADDR_MT8173, .inv_sel_reg = REG_MMU_INV_SEL_GEN1, .banks_num= 1, .banks_enable = {true}, -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 1/3] dt-bindings: mediatek: Add bindings for MT6795 M4U
Add bindings for the MediaTek Helio X10 (MT6795) IOMMU/M4U. Signed-off-by: AngeloGioacchino Del Regno Acked-by: Rob Herring --- .../bindings/iommu/mediatek,iommu.yaml| 4 + include/dt-bindings/memory/mt6795-larb-port.h | 95 +++ 2 files changed, 99 insertions(+) create mode 100644 include/dt-bindings/memory/mt6795-larb-port.h diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml index fee0241b5098..839e3be0bf3c 100644 --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml @@ -73,6 +73,7 @@ properties: - mediatek,mt2701-m4u # generation one - mediatek,mt2712-m4u # generation two - mediatek,mt6779-m4u # generation two + - mediatek,mt6795-m4u # generation two - mediatek,mt8167-m4u # generation two - mediatek,mt8173-m4u # generation two - mediatek,mt8183-m4u # generation two @@ -124,6 +125,7 @@ properties: dt-binding/memory/mt2701-larb-port.h for mt2701 and mt7623, dt-binding/memory/mt2712-larb-port.h for mt2712, dt-binding/memory/mt6779-larb-port.h for mt6779, + dt-binding/memory/mt6795-larb-port.h for mt6795, dt-binding/memory/mt8167-larb-port.h for mt8167, dt-binding/memory/mt8173-larb-port.h for mt8173, dt-binding/memory/mt8183-larb-port.h for mt8183, @@ -148,6 +150,7 @@ allOf: enum: - mediatek,mt2701-m4u - mediatek,mt2712-m4u + - mediatek,mt6795-m4u - mediatek,mt8173-m4u - mediatek,mt8186-iommu-mm - mediatek,mt8192-m4u @@ -177,6 +180,7 @@ allOf: contains: enum: - mediatek,mt2712-m4u + - mediatek,mt6795-m4u - mediatek,mt8173-m4u then: diff --git a/include/dt-bindings/memory/mt6795-larb-port.h b/include/dt-bindings/memory/mt6795-larb-port.h new file mode 100644 index ..58cf6a6b6372 --- /dev/null +++ b/include/dt-bindings/memory/mt6795-larb-port.h @@ -0,0 +1,95 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ +/* + * Copyright (c) 2022 Collabora Ltd. + * Author: AngeloGioacchino Del Regno + */ + +#ifndef _DT_BINDINGS_MEMORY_MT6795_LARB_PORT_H_ +#define _DT_BINDINGS_MEMORY_MT6795_LARB_PORT_H_ + +#include + +#define M4U_LARB0_ID 0 +#define M4U_LARB1_ID 1 +#define M4U_LARB2_ID 2 +#define M4U_LARB3_ID 3 +#define M4U_LARB4_ID 4 + +/* larb0 */ +#define M4U_PORT_DISP_OVL0 MTK_M4U_ID(M4U_LARB0_ID, 0) +#define M4U_PORT_DISP_RDMA0MTK_M4U_ID(M4U_LARB0_ID, 1) +#define M4U_PORT_DISP_RDMA1MTK_M4U_ID(M4U_LARB0_ID, 2) +#define M4U_PORT_DISP_WDMA0MTK_M4U_ID(M4U_LARB0_ID, 3) +#define M4U_PORT_DISP_OVL1 MTK_M4U_ID(M4U_LARB0_ID, 4) +#define M4U_PORT_DISP_RDMA2MTK_M4U_ID(M4U_LARB0_ID, 5) +#define M4U_PORT_DISP_WDMA1MTK_M4U_ID(M4U_LARB0_ID, 6) +#define M4U_PORT_DISP_OD_R MTK_M4U_ID(M4U_LARB0_ID, 7) +#define M4U_PORT_DISP_OD_W MTK_M4U_ID(M4U_LARB0_ID, 8) +#define M4U_PORT_MDP_RDMA0 MTK_M4U_ID(M4U_LARB0_ID, 9) +#define M4U_PORT_MDP_RDMA1 MTK_M4U_ID(M4U_LARB0_ID, 10) +#define M4U_PORT_MDP_WDMA MTK_M4U_ID(M4U_LARB0_ID, 11) +#define M4U_PORT_MDP_WROT0 MTK_M4U_ID(M4U_LARB0_ID, 12) +#define M4U_PORT_MDP_WROT1 MTK_M4U_ID(M4U_LARB0_ID, 13) + +/* larb1 */ +#define M4U_PORT_VDEC_MC MTK_M4U_ID(M4U_LARB1_ID, 0) +#define M4U_PORT_VDEC_PP MTK_M4U_ID(M4U_LARB1_ID, 1) +#define M4U_PORT_VDEC_UFO MTK_M4U_ID(M4U_LARB1_ID, 2) +#define M4U_PORT_VDEC_VLD MTK_M4U_ID(M4U_LARB1_ID, 3) +#define M4U_PORT_VDEC_VLD2 MTK_M4U_ID(M4U_LARB1_ID, 4) +#define M4U_PORT_VDEC_AVC_MV MTK_M4U_ID(M4U_LARB1_ID, 5) +#define M4U_PORT_VDEC_PRED_RD MTK_M4U_ID(M4U_LARB1_ID, 6) +#define M4U_PORT_VDEC_PRED_WR MTK_M4U_ID(M4U_LARB1_ID, 7) +#define M4U_PORT_VDEC_PPWRAP MTK_M4U_ID(M4U_LARB1_ID, 8) + +/* larb2 */ +#define M4U_PORT_CAM_IMGO MTK_M4U_ID(M4U_LARB2_ID, 0) +#define M4U_PORT_CAM_RRZO MTK_M4U_ID(M4U_LARB2_ID, 1) +#define M4U_PORT_CAM_AAO MTK_M4U_ID(M4U_LARB2_ID, 2) +#define M4U_PORT_CAM_LCSO MTK_M4U_ID(M4U_LARB2_ID, 3) +#define M4U_PORT_CAM_ESFKO MTK_M4U_ID(M4U_LARB2_ID, 4) +#define M4U_PORT_CAM_IMGO_SMTK_M4U_ID(M4U_LARB2_ID, 5) +#define M4U_PORT_CAM_LSCI MTK_M4U_ID(M4U_LARB2_ID, 6) +#define M4U_PORT_CAM_LSCI_DMTK_M4U_ID(M4U_LARB2_ID, 7) +#define M4U_PORT_CAM_BPCI MTK_M4U_ID(M4U_LARB2_ID, 8) +#define M4U_PORT_CAM_BPCI_DMTK_M4U_ID(M4U_LARB2_ID, 9) +#define M4U_PORT_CAM_UFDI
Re: [PATCH v2 3/3] arch/*/: remove CONFIG_VIRT_TO_BUS
On Sat, Jun 18, 2022 at 3:06 AM Michael Schmitz wrote: > Am 18.06.2022 um 00:57 schrieb Arnd Bergmann: > > > > All architecture-independent users of virt_to_bus() and bus_to_virt() > > have been fixed to use the dma mapping interfaces or have been > > removed now. This means the definitions on most architectures, and the > > CONFIG_VIRT_TO_BUS symbol are now obsolete and can be removed. > > > > The only exceptions to this are a few network and scsi drivers for m68k > > Amiga and VME machines and ppc32 Macintosh. These drivers work correctly > > with the old interfaces and are probably not worth changing. > > The Amiga SCSI drivers are all old WD33C93 ones, and replacing > virt_to_bus by virt_to_phys in the dma_setup() function there would > cause no functional change at all. Ok, thanks for taking a look here. > drivers/vme/bridges/vme_ca91cx42.c hasn't been used at all on m68k (it > is a PCI-to-VME bridge chipset driver that would be needed on > architectures that natively use a PCI bus). I haven't found anything > that selects that driver, so not sure it is even still in use?? It's gone now, Greg has already taken my patches for this through the staging tree. > That would allow you to drop the remaining virt_to_bus define from > arch/m68k/include/asm/virtconvert.h. > > I could submit a patch to convert the Amiga SCSI drivers to use > virt_to_phys if Geert and the SCSI maintainers think it's worth the churn. I don't think using virt_to_phys() is an improvement here, as virt_to_bus() was originally meant as a better abstraction to replace the use of virt_to_phys() to make drivers portable, before it got replaced by the dma-mapping interface in turn. It looks like the Amiga SCSI drivers have an open-coded version of what dma_map_single() does, to do bounce buffering and cache management. The ideal solution would be to convert the drivers actually use the appropriate dma-mapping interfaces and remove this custom code. The same could be done for the two vme drivers (scsi/mvme147.c and ethernet/82596.c), which do the cache management but apparently don't need swiotlb bounce buffering. Rewriting the drivers to modern APIs is of course non-trivial, and if you want a shortcut here, I would suggest introducing platform specific helpers similar to isa_virt_to_bus() and call them amiga_virt_to_bus() and vme_virt_to_bus, respectively. Putting these into a platform specific header file at least helps clarify that both the helper functions and the drivers using them are non-portable. > 32bit powerpc is a different matter though. It's similar, but unrelated. The two apple ethernet drivers (bmac and mace) can again either get changed to use the dma-mapping interfaces, or get a custom pmac_virt_to_bus()/ pmac_bus_to_virt() helper. There is also drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c, which I think just needs a trivial change, but I'm not sure how to do it correctly. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 1/1] iommu/vt-d: Fix RID2PASID setup/teardown failure
在 2022/6/24 14:54, Baolu Lu 写道: On 2022/6/24 14:02, Ethan Zhao wrote: 在 2022/6/23 14:57, Lu Baolu 写道: The IOMMU driver shares the pasid table for PCI alias devices. When the RID2PASID entry of the shared pasid table has been filled by the first device, the subsequent device will encounter the "DMAR: Setup RID2PASID failed" failure as the pasid entry has already been marked as present. As the result, the IOMMU probing process will be aborted. On the contrary, when any alias device is hot-removed from the system, for example, by writing to /sys/bus/pci/devices/.../remove, the shared RID2PASID will be cleared without any notifications to other devices. As the result, any DMAs from those rest devices are blocked. Sharing pasid table among PCI alias devices could save two memory pages for devices underneath the PCIe-to-PCI bridges. Anyway, considering that those devices are rare on modern platforms that support VT-d in scalable mode and the saved memory is negligible, it's reasonable to remove this part of immature code to make the driver feasible and stable. In my understanding, thus cleanning will make the pasid table become per-dev datastructure whatever the dev is pci-alias or not, and the pasid_pte_is_present(pte)will only check against every pci-alias' own private pasid table,the setup stagewouldn't break, so does the detach/release path, and little value to code otherreference counter like complex implenmataion, looks good to me ! Thanks! Can I add a Reviewd-by from you? Sure ! Best regards, baolu -- "firm, enduring, strong, and long-lived" ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu