linux-kernel - Re: [PATCH] drm/gma500: Fixup fbdev stolen size usage evaluation

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: <CAMeQTsYG+YvXqQqvJvsxT1h0z5zZJbdCtc5wPjUossvwidV=cA@mail.gmail.com>
Date:   Tue, 12 Nov 2019 11:20:27 +0100
From:   Patrik Jakobsson <patrik.r.jakobsson@...il.com>
To:     Paul Kocialkowski <paul.kocialkowski@...tlin.com>
Cc:     dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        James Hilliard <james.hilliard1@...il.com>
Subject: Re: [PATCH] drm/gma500: Fixup fbdev stolen size usage evaluation

On Thu, Nov 7, 2019 at 4:30 PM Paul Kocialkowski
<paul.kocialkowski@...tlin.com> wrote:
>
> psbfb_probe performs an evaluation of the required size from the stolen
> GTT memory, but gets it wrong in two distinct ways:
> - The resulting size must be page-size-aligned;
> - The size to allocate is derived from the surface dimensions, not the fb
>   dimensions.
>
> When two connectors are connected with different modes, the smallest will
> be stored in the fb dimensions, but the size that needs to be allocated must
> match the largest (surface) dimensions. This is what is used in the actual
> allocation code.
>
> Fix this by correcting the evaluation to conform to the two points above.
> It allows correctly switching to 16bpp when one connector is e.g. 1920x1080
> and the other is 1024x768.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
> ---
>  drivers/gpu/drm/gma500/framebuffer.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
> index 218f3bb15276..90237abee088 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -462,6 +462,7 @@ static int psbfb_probe(struct drm_fb_helper *helper,
>                 container_of(helper, struct psb_fbdev, psb_fb_helper);
>         struct drm_device *dev = psb_fbdev->psb_fb_helper.dev;
>         struct drm_psb_private *dev_priv = dev->dev_private;
> +       unsigned int fb_size;
>         int bytespp;
>
>         bytespp = sizes->surface_bpp / 8;
> @@ -471,8 +472,11 @@ static int psbfb_probe(struct drm_fb_helper *helper,
>         /* If the mode will not fit in 32bit then switch to 16bit to get
>            a console on full resolution. The X mode setting server will
>            allocate its own 32bit GEM framebuffer */
> -       if (ALIGN(sizes->fb_width * bytespp, 64) * sizes->fb_height >
> -                       dev_priv->vram_stolen_size) {
> +       fb_size = ALIGN(sizes->surface_width * bytespp, 64) *
> +                 sizes->surface_height;
> +       fb_size = ALIGN(fb_size, PAGE_SIZE);
> +
> +       if (fb_size > dev_priv->vram_stolen_size) {

psb_gtt_alloc_range() already aligns by PAGE_SIZE for us. Looks like
we align a couple of times extra for luck. This needs cleaning up
instead of adding even more aligns.

Your size calculation looks correct and indeed makes my 1024x600 +
1920x1080 setup actually display something, but for some reason I get
an incorrect panning on the smaller screen and stale data on the
surface only visible by the larger CRTC. Any idea what's going on?

>                  sizes->surface_bpp = 16;
>                  sizes->surface_depth = 16;
>          }
> --
> 2.23.0
>