linux-kernel - Re: [Intel-gfx] [PATCH] drm/i915/display: Fix phys

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]

Date:   Tue, 28 Nov 2023 03:47:26 +0000
From:   Paz Zcharya <pazz@...omium.org>
To:     Andrzej Hajda <andrzej.hajda@...el.com>
Cc:     Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Subrata Banik <subratabanik@...gle.com>,
        Tvrtko Ursulin <tvrtko.ursulin@...el.com>,
        intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, Sean Paul <seanpaul@...omium.org>,
        matthew.auld@...el.com, Daniel Vetter <daniel@...ll.ch>,
        Marcin Wojtas <mwojtas@...omium.org>,
        Drew Davenport <ddavenport@...omium.org>,
        David Airlie <airlied@...il.com>,
        Nirmoy Das <nirmoy.das@...el.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Fix phys_base to be
 relative not absolute


On Mon, Nov 27, 2023 at 8:20 PM Paz Zcharya <pazz@...omium.org> wrote:
>
> On 21.11.2023 13:06, Andrzej Hajda wrote:
> > On 18.11.2023 00:01, Paz Zcharya wrote:
> > > On Tue, Nov 14, 2023 at 10:13:59PM -0500, Rodrigo Vivi wrote:
> > > > On Sun, Nov 05, 2023 at 05:27:03PM +0000, Paz Zcharya wrote:
> > >
> > > Hi Rodrigo, thanks for the great comments.
> > >
> > > Apologies for using a wrong/confusing terminology. I think 'phys_base'
> > > is supposed to be the offset in the GEM BO, where base (or
> > > "Surface Base Address") is supposed to be the GTT offset.
> >
> > Since base is taken from PLANE_SURF register it should be resolvable via
> > GGTT to physical address pointing to actual framebuffer.
> > I couldn't find anything in the specs.
>
> It was quite cryptic. I meant I have not found anything about assumption
> from commit history that for iGPU there should be 1:1 mapping, this is why
> there was an assignment "phys_base = base". Possibly the assumption is not
> valid anymore for MTL(?).
> Without the assumption we need to check GGTT to determine phys address.
>
> > The simplest approach would be then do the same as in case of DGFX:
> >          gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
> >          gen8_pte_t pte;
> >
> >          gte += base / I915_GTT_PAGE_SIZE;
> >
> >          pte = ioread64(gte);
> >          phys_base = pte & I915_GTT_PAGE_MASK;
> >
> > Regards
> > Andrzej

Hey Andrzej,

On a second thought, what do you think about something like

+               gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
+               gen8_pte_t pte;
+               gte += base / I915_GTT_PAGE_SIZE;
+               pte = ioread64(gte);
+               pte = pte & I915_GTT_PAGE_MASK;
+               phys_base = pte - i915->mm.stolen_region->region.start;

The only difference is the last line.

Based on what I wrote before, I think `phys_base` is named incorrectly and
that it does not reflect the physical address, but the start offset of
i915->mm.stolen_region. So if we offset the start value of the stolen
region, this code looks correct to me (and it also works on my
MeteorLake device).

What do you think?


Many thanks,
Paz