linux-kernel - Re: exynos-drm: display manager fails to start without IOMMU problem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <58079E16.50302@math.uni-bielefeld.de>
Date:   Wed, 19 Oct 2016 18:23:50 +0200
From:   Tobias Jakobi <tjakobi@...h.uni-bielefeld.de>
To:     Shuah Khan <shuahkh@....samsung.com>,
        Inki Dae <inki.dae@...sung.com>, jy0922.shim@...sung.com,
        sw0312.kim@...sung.com, kyungmin.park@...sung.com,
        airlied@...ux.ie, kgene@...nel.org,
        Krzysztof Kozlowski <krzk@...nel.org>
Cc:     dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: exynos-drm: display manager fails to start without IOMMU problem

Hello Shuah,

just a short note that more misleading comments about default allocation
flags can be found in libdrm.

https://cgit.freedesktop.org/mesa/drm/tree/exynos/exynos_drm.c

See e.g. the comment for exynos_bo_create().

In my opinion, the whole approach to _set_ a bit to get non-contigious
memory is messed up. It would make more sense to me to set a bit to
request an additional property (here "being contiguous") of the memory.

Anyway, clearing up this situation is highly appreciated!

More comments below.

With best wishes,
Tobias



Shuah Khan wrote:
> Restarting the thread with a different subject line:
> 
> I haven't given up on this yet. I am still seeing the following failure:
> 
> Additional debug messages I added:
> [   15.287403] exynos_drm_gem_create_ioctl() 1
> [   15.287419] exynos_drm_gem_create() flags 1
> 
> [   15.311511] [drm:exynos_drm_framebuffer_init] *ERROR* Non-contiguous GEM memory is not supported.
> 
> Additional debug message I added:
> [   15.318981] [drm:exynos_user_fb_create] *ERROR* failed to initialize framebuffer
> 
> This is what happens:
> 
> 1. exynos_drm_gem_create_ioctl() gets called with EXYNOS_BO_NONCONTIG request
> 2. exynos_drm_gem_create(0 goes ahead and creates the GEM buffers
> 3. exynos_user_fb_create() tries to associate GEM to fb and fails during
>    check_fb_gem_memory_type()
> 
> At this point, there is no recovery and lightdm fails
> 
> xf86-video-armsoc/src/drmmode_exynos/drmmode_exynos.c assumes contiguous
> allocations are not supported in some exynos drm versions: The following
> commit introduced this change:
> 
> https://git.linaro.org/arm/xorg/driver/xf86-video-armsoc.git/commitdiff/3be1f6273441fe95dd442f44064387322e16b7e9
> 
> excerpts from the diff:-       if (create_gem->buf_type == ARMSOC_BO_SCANOUT)
> -               create_exynos.flags = EXYNOS_BO_CONTIG;
> -       else
> -               create_exynos.flags = EXYNOS_BO_NONCONTIG;
> +
> +       /* Contiguous allocations are not supported in some exynos drm versions.
> +        * When they are supported all allocations are effectively contiguous
> +        * anyway, so for simplicity we always request non contiguous buffers.
> +        */
> +       create_exynos.flags = EXYNOS_BO_NONCONTIG;
> 
> There might have been logic on exynos_drm that forced Contig when it coudn't
> support NONCONTIG. At least, that is what this comment suggests. This assumption
> doesn't appear to be a good one and not sure if this change was made to fix a bug.
> 
> After the IOMMU support, this assumption is no longer true. Hence, with IOMMU
> support, latest kernels have a mismatch with the installed xf86-video-armsoc
> 
> This is what I am running into. This leads to the following question:
> 
> 1. How do we ensure exynos_drm kernel changes don't break user-space
>    specifically xf86-video-armsoc
> 2. This seems to have gone undetected for a while. I see a change in
>    exynos_drm_gem_dumb_create() that is probably addressing this type
>    of breakage. Commit 122beea84bb90236b1ae545f08267af58591c21b adds
>    handling for IOMMU NONCONTIG case.
I don't think that this commit is related to the issue, since it is only
used for the generic dumb buffer ioctl, while armsoc is using an Exynos
specific ioctl.

So in particular you shouldn't see the issue with
xf86-video-modesetting. Might be worth trying that one out?


> 
> Anyway, I am interested in getting the exynos_drm kernel side code
> and xf86-video-armsoc in sync to resolve the issue.
> 
> Could you recommend a going forward plan?
> 
> I can submit a patch to xf86-video-armsoc. I am also looking ahead to
> see if we can avoid such breaks in the future by keeping kernel and
> xf86-video-armsoc in sync.
> 
> thanks,
> -- Shuah
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>