| Message ID | cover.1684154219.git.robin.murphy@arm.com |
|---|---|
| Headers | show |
| Series | iommu: tegra-gart cleanups | expand |
Le lun. 15 mai 2023 à 14:57, Robin Murphy <robin.murphy@arm.com> a écrit : > > Hi all, > > For the sake of discussion, here's my irrational pet project to bring > the tegra-gart driver right up to date as an example of a > properly-implemented IOMMU driver for a non-isolated address space. Part > of that irrationality is that I don't even own any hardware which uses > this driver, so it's only build-tested :) > > Thanks, > Robin. > > > Robin Murphy (4): > iommu/tegra-gart: Add default identity domain support > iommu/tegra-gart: Improve domain support > iommu/tegra-gart: Generalise domain support > iommu: Clean up force_aperture confusion > > drivers/iommu/dma-iommu.c | 19 ++-- > drivers/iommu/mtk_iommu_v1.c | 4 + > drivers/iommu/sprd-iommu.c | 1 + > drivers/iommu/tegra-gart.c | 162 +++++++++++++++++++---------------- > 4 files changed, 99 insertions(+), 87 deletions(-) For what it worth, I've tried to test this serie with "grate patches" (1) rebased on top on 6.4-rc2, that would make use of the tegra-gart. That was on PAZ00 (with only 512M of RAM and 96M CMA still allocated). Unfortunately, this lead to the following errors with display problems (no character displayed in lxt-terminal and etc) [ 888.691348] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use [ 888.698400] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* mapping failed 4294967274 262144 [ 888.707365] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* failed size 262144: -12 [ 888.716735] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use [ 888.723800] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* mapping failed 4294967274 262144 [ 888.733156] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* failed size 262144: -12 [ 889.055247] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use [ 889.062296] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* mapping failed 4294967274 262144 [ 889.071266] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* failed size 262144: -12 (1) https://github.com/grate-driver/linux
On 2023-05-16 10:53, Nicolas Chauvet wrote: > Le lun. 15 mai 2023 à 14:57, Robin Murphy <robin.murphy@arm.com> a écrit : >> >> Hi all, >> >> For the sake of discussion, here's my irrational pet project to bring >> the tegra-gart driver right up to date as an example of a >> properly-implemented IOMMU driver for a non-isolated address space. Part >> of that irrationality is that I don't even own any hardware which uses >> this driver, so it's only build-tested :) >> >> Thanks, >> Robin. >> >> >> Robin Murphy (4): >> iommu/tegra-gart: Add default identity domain support >> iommu/tegra-gart: Improve domain support >> iommu/tegra-gart: Generalise domain support >> iommu: Clean up force_aperture confusion >> >> drivers/iommu/dma-iommu.c | 19 ++-- >> drivers/iommu/mtk_iommu_v1.c | 4 + >> drivers/iommu/sprd-iommu.c | 1 + >> drivers/iommu/tegra-gart.c | 162 +++++++++++++++++++---------------- >> 4 files changed, 99 insertions(+), 87 deletions(-) > > > For what it worth, I've tried to test this serie with "grate patches" > (1) rebased on top on 6.4-rc2, that would make use of the tegra-gart. > That was on PAZ00 (with only 512M of RAM and 96M CMA still allocated). > Unfortunately, this lead to the following errors with display problems > (no character displayed in lxt-terminal and etc) Thanks for testing - it's quite possible I've made a silly mistake somewhere, but just to double-check, does the same happen with the existing driver if you boot with "tegra-gart.gart_debug=1" (or at least enable the parameter via sysfs before the DRM driver gets going)? Thanks, Robin. > [ 888.691348] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use > [ 888.698400] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > mapping failed 4294967274 262144 > [ 888.707365] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > failed size 262144: -12 > [ 888.716735] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use > [ 888.723800] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > mapping failed 4294967274 262144 > [ 888.733156] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > failed size 262144: -12 > [ 889.055247] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use > [ 889.062296] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > mapping failed 4294967274 262144 > [ 889.071266] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > failed size 262144: -12 > > (1) https://github.com/grate-driver/linux
Le mar. 16 mai 2023 à 13:45, Robin Murphy <robin.murphy@arm.com> a écrit : > > On 2023-05-16 10:53, Nicolas Chauvet wrote: ... > > For what it worth, I've tried to test this serie with "grate patches" > > (1) rebased on top on 6.4-rc2, that would make use of the tegra-gart. > > That was on PAZ00 (with only 512M of RAM and 96M CMA still allocated). > > Unfortunately, this lead to the following errors with display problems > > (no character displayed in lxt-terminal and etc) > > Thanks for testing - it's quite possible I've made a silly mistake > somewhere, but just to double-check, does the same happen with the > existing driver if you boot with "tegra-gart.gart_debug=1" (or at least > enable the parameter via sysfs before the DRM driver gets going)? Using echo 1 > /sys/module/tegra_gart/parameters/gart_debug shows the same messages and the same artefacts (missing refreshed window content). Using "echo 0 > /sys/module/tegra_gart/parameters/gart_debug" revert back to normal... [ 7661.026139] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use [ 7661.033189] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* mapping failed 4294967274 262144 [ 7661.042197] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* failed size 262144: -12 [ 7661.589552] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use [ 7661.596690] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* mapping failed 4294967274 262144 [ 7661.605865] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* failed size 262144: -12 [ 7662.078823] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use [ 7662.085987] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* mapping failed 4294967274 262144 [ 7662.095137] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* failed size 262144: -12 [ 7662.123677] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use [ 7662.130758] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* mapping failed 4294967274 262144 [ 7662.140100] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* failed size 262144: -12
On 2023-05-16 13:16, Nicolas Chauvet wrote: > Le mar. 16 mai 2023 à 13:45, Robin Murphy <robin.murphy@arm.com> a écrit : >> >> On 2023-05-16 10:53, Nicolas Chauvet wrote: > ... >>> For what it worth, I've tried to test this serie with "grate patches" >>> (1) rebased on top on 6.4-rc2, that would make use of the tegra-gart. >>> That was on PAZ00 (with only 512M of RAM and 96M CMA still allocated). >>> Unfortunately, this lead to the following errors with display problems >>> (no character displayed in lxt-terminal and etc) >> >> Thanks for testing - it's quite possible I've made a silly mistake >> somewhere, but just to double-check, does the same happen with the >> existing driver if you boot with "tegra-gart.gart_debug=1" (or at least >> enable the parameter via sysfs before the DRM driver gets going)? > > Using echo 1 > /sys/module/tegra_gart/parameters/gart_debug shows the > same messages and the same artefacts (missing refreshed window > content). > Using "echo 0 > /sys/module/tegra_gart/parameters/gart_debug" revert > back to normal... OK, cool, so it looks like a pre-existing bug in the caller, but I guess it does mean that unconditionally enabling the checks isn't ideal until that can be sorted out. Thanks, Robin. > [ 7661.026139] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use > [ 7661.033189] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > mapping failed 4294967274 262144 > [ 7661.042197] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > failed size 262144: -12 > [ 7661.589552] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use > [ 7661.596690] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > mapping failed 4294967274 262144 > [ 7661.605865] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > failed size 262144: -12 > [ 7662.078823] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use > [ 7662.085987] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > mapping failed 4294967274 262144 > [ 7662.095137] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > failed size 262144: -12 > [ 7662.123677] tegra-mc 7000f000.memory-controller: gart: Page entry is in-use > [ 7662.130758] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > mapping failed 4294967274 262144 > [ 7662.140100] [drm:tegra_bo_gart_map_locked [tegra_drm]] *ERROR* > failed size 262144: -12
Le mar. 16 mai 2023 à 14:31, Robin Murphy <robin.murphy@arm.com> a écrit : > > On 2023-05-16 13:16, Nicolas Chauvet wrote: > > Le mar. 16 mai 2023 à 13:45, Robin Murphy <robin.murphy@arm.com> a écrit : > >> > >> On 2023-05-16 10:53, Nicolas Chauvet wrote: > > ... > >>> For what it worth, I've tried to test this serie with "grate patches" > >>> (1) rebased on top on 6.4-rc2, that would make use of the tegra-gart. > >>> That was on PAZ00 (with only 512M of RAM and 96M CMA still allocated). > >>> Unfortunately, this lead to the following errors with display problems > >>> (no character displayed in lxt-terminal and etc) > >> > >> Thanks for testing - it's quite possible I've made a silly mistake > >> somewhere, but just to double-check, does the same happen with the > >> existing driver if you boot with "tegra-gart.gart_debug=1" (or at least > >> enable the parameter via sysfs before the DRM driver gets going)? > > > > Using echo 1 > /sys/module/tegra_gart/parameters/gart_debug shows the > > same messages and the same artefacts (missing refreshed window > > content). > > Using "echo 0 > /sys/module/tegra_gart/parameters/gart_debug" revert > > back to normal... > > OK, cool, so it looks like a pre-existing bug in the caller, but I guess > it does mean that unconditionally enabling the checks isn't ideal until > that can be sorted out. Seems like I had a non-default option with tegra-drm that was controlling the way tegra-gart is used: https://github.com/grate-driver/linux/blob/master/drivers/gpu/drm/grate/gart.c#L19-L29 With option 4, occurrences of failing allocation are experienced more often than the default 0 options when only "scattered BOs are mapped". Also with the default option, no noticeable failure is seen. Thanks.
On 2023-05-16 16:15, Nicolas Chauvet wrote: > Le mar. 16 mai 2023 à 14:31, Robin Murphy <robin.murphy@arm.com> a écrit : >> >> On 2023-05-16 13:16, Nicolas Chauvet wrote: >>> Le mar. 16 mai 2023 à 13:45, Robin Murphy <robin.murphy@arm.com> a écrit : >>>> >>>> On 2023-05-16 10:53, Nicolas Chauvet wrote: >>> ... >>>>> For what it worth, I've tried to test this serie with "grate patches" >>>>> (1) rebased on top on 6.4-rc2, that would make use of the tegra-gart. >>>>> That was on PAZ00 (with only 512M of RAM and 96M CMA still allocated). >>>>> Unfortunately, this lead to the following errors with display problems >>>>> (no character displayed in lxt-terminal and etc) >>>> >>>> Thanks for testing - it's quite possible I've made a silly mistake >>>> somewhere, but just to double-check, does the same happen with the >>>> existing driver if you boot with "tegra-gart.gart_debug=1" (or at least >>>> enable the parameter via sysfs before the DRM driver gets going)? >>> >>> Using echo 1 > /sys/module/tegra_gart/parameters/gart_debug shows the >>> same messages and the same artefacts (missing refreshed window >>> content). >>> Using "echo 0 > /sys/module/tegra_gart/parameters/gart_debug" revert >>> back to normal... >> >> OK, cool, so it looks like a pre-existing bug in the caller, but I guess >> it does mean that unconditionally enabling the checks isn't ideal until >> that can be sorted out. > > Seems like I had a non-default option with tegra-drm that was > controlling the way tegra-gart is used: > https://github.com/grate-driver/linux/blob/master/drivers/gpu/drm/grate/gart.c#L19-L29 > > With option 4, occurrences of failing allocation are experienced more > often than the default 0 options when only "scattered BOs are mapped". > Also with the default option, no noticeable failure is seen. Oh, I see it now - the logic around tegra_bo_mm_evict_something() actually depends on being able to map new PTEs over the top of existing ones without an unmap :/ The map/unmap overhead that that's trying to avoid could probably be reduced quite significantly anyway by converting GART to the new map_pages/unmap_pages callbacks. Thanks, Robin.