On Tue, Jan 23, 2024 at 11:15:08AM -0400, Jason Gunthorpe wrote:
> On Tue, Jan 23, 2024 at 02:33:15PM +0000, diogo.ivo@xxxxxxxxxxxxxxxxxx wrote:
> > Commit c8cc2655cc6c in the recent IOMMU changes breaks Tegra fbdev
> > at least on the Pixel C with the following error message reporting
> > a failed buffer allocation:
> >
> > [ 1.857660] drm drm: failed to allocate buffer of size 18432000
> >
> > This error message is printed from tegra_bo_alloc() which is called by
> > tegra_bo_create() in tegra_fbdev_probe(), which may indicate that other
> > allocations would fail as well, not just the framebuffer.
>
> Presumably this is because iommu_map_sgtable() under
> tegra_bo_iommu_map() fails?
>
> Which I suspect is because of the logic in
> host1x_client_iommu_attach().
>
> After c8cc2655cc6c iommu_get_domain_for_dev() will never return NULL.
>
> So this:
>
> if (domain && domain != tegra->domain)
> return 0;
>
> Will happen and the domain will be left at IDENTITY, while I suppose
> the tegra_bo_iommu_map() assumes the domain was switched to tegra->domain.
>
> Does this solve your issue?
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index ff36171c8fb700..15c7910b2e1c76 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -960,7 +960,7 @@ int host1x_client_iommu_attach(struct host1x_client *client)
> * not the shared IOMMU domain, don't try to attach it to a different
> * domain. This allows using the IOMMU-backed DMA API.
> */
> - if (domain && domain != tegra->domain)
> + if (domain && domain->type != IOMMU_DOMAIN_IDENTITY && domain != tegra->domain)
> return 0;
>
> if (tegra->domain) {
>
> > This may be connected with an error in of_iommu_configure() that
> > became visible after commit 6ff6e184f1f4d:
> >
> > [ 1.200004] host1x drm: iommu configuration for device failed with -ENOENT
>
> Hmmm
>
> This is a new logging, so it doesn't necessarily mean something has
> changed in the code flow.
>
> It seems the issue is something in there is returning ENOENT when it
> probably should be ENODEV, but I haven't been able to guess where it
> comes from.
>
> Can you do some tracing and figure out where under
> of_iommu_configure() this ENOENT return code is from?
>
> Jason
I did the tracing and found that the ENOENT is coming from
sysfs_do_create_link_sd() in the following function call chain:
of_iommu_configure() -> iommu_probe_device() -> __iommu_probe_device() ->
iommu_init_device() -> iommu_device_link() -> sysfs_add_link_to_group() ->
sysfs_create_link_sd() -> sysfs_do_create_link_sd()
The particular block that emits the error is
static int sysfs_do_create_link_sd()
{
...
if (target_kobj->sd) {
target = target_kobj->sd;
kernfs_get(target);
}
if (!target)
return -ENOENT;
...
}
Best regargs,
Diogo
