From: "Christian König" <christian.koenig@amd.com>
To: David Hildenbrand <david@redhat.com>,
intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
x86@kernel.org
Cc: airlied@gmail.com, thomas.hellstrom@linux.intel.com,
matthew.brost@intel.com, dave.hansen@linux.intel.com,
luto@kernel.org, peterz@infradead.org,
Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Subject: Re:
Date: Thu, 21 Aug 2025 10:10:17 +0200 [thread overview]
Message-ID: <4e5f4ef0-53f1-417e-8f3b-76fd7c64cd23@amd.com> (raw)
In-Reply-To: <edf4aee5-54eb-4fad-aa89-4913d44371fe@redhat.com>
On 20.08.25 17:23, David Hildenbrand wrote:
> CCing Lorenzo
>
> On 20.08.25 16:33, Christian König wrote:
>> Hi everyone,
>>
>> sorry for CCing so many people, but that rabbit hole turned out to be
>> deeper than originally thought.
>>
>> TTM always had problems with UC/WC mappings on 32bit systems and drivers
>> often had to revert to hacks like using GFP_DMA32 to get things working
>> while having no rational explanation why that helped (see the TTM AGP,
>> radeon and nouveau driver code for that).
>>
>> It turned out that the PAT implementation we use on x86 not only enforces
>> the same caching attributes for pages in the linear kernel mapping, but
>> also for highmem pages through a separate R/B tree.
>>
>> That was unexpected and TTM never updated that R/B tree for highmem pages,
>> so the function pgprot_set_cachemode() just overwrote the caching
>> attributes drivers passed in to vmf_insert_pfn_prot() and that essentially
>> caused all kind of random trouble.
>>
>> An R/B tree is potentially not a good data structure to hold thousands if
>> not millions of different attributes for each page, so updating that is
>> probably not the way to solve this issue.
>>
>> Thomas pointed out that the i915 driver is using apply_page_range()
>> instead of vmf_insert_pfn_prot() to circumvent the PAT implementation and
>> just fill in the page tables with what the driver things is the right
>> caching attribute.
>
> I assume you mean apply_to_page_range() -- same issue in patch subjects.
Oh yes, of course. Sorry.
> Oh this sounds horrible. Why oh why do we have these hacks in core-mm and have drivers abuse them :(
Yeah I was also a bit hesitated to use that, but the performance advantage is so high that we probably can't avoid the general approach.
> Honestly, apply_to_pte_range() is just the entry in doing all kinds of weird crap to page tables because "you know better".
Exactly that's the problem I'm pointing out, drivers *do* know it better. The core memory management has applied incorrect values which caused all kind of the trouble.
The problem is not a bug in PAT nor TTM/drivers but rather how they interact with each other.
What I don't understand is why do we have the PAT in the first place? No other architecture does it this way.
Is that because of the of x86 CPUs which have problems when different page tables contain different caching attributes for the same physical memory?
> All the sanity checks from vmf_insert_pfn(), gone.
>
> Can we please fix the underlying issue properly?
I'm happy to implement anything advised, my question is what should we solve this issue?
Regards,
Christian.
next prev parent reply other threads:[~2025-08-21 8:10 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20250820143739.3422-1-christian.koenig@amd.com>
2025-08-20 14:33 ` [PATCH 1/3] drm/ttm: use apply_page_range instead of vmf_insert_pfn_prot Christian König
2025-08-20 14:33 ` [PATCH 2/3] drm/ttm: reapply increase ttm pre-fault value to PMD size" Christian König
2025-08-20 14:33 ` [PATCH 3/3] drm/ttm: disable changing the global caching flags on newer AMD CPUs v2 Christian König
2025-08-20 15:12 ` Borislav Petkov
2025-08-20 15:23 ` David Hildenbrand
2025-08-21 8:10 ` Christian König [this message]
2025-08-25 19:10 ` Re: David Hildenbrand
2025-08-26 8:38 ` Re: Christian König
2025-08-26 8:46 ` Re: David Hildenbrand
2025-08-26 9:00 ` Re: Christian König
2025-08-26 9:17 ` Re: David Hildenbrand
2025-08-26 9:56 ` Re: Christian König
2025-08-26 12:07 ` Re: David Hildenbrand
2025-08-26 16:09 ` Re: Christian König
2025-08-27 9:13 ` [PATCH 0/3] drm/ttm: Michel Dänzer
2025-08-28 21:18 ` stupid and complicated PAT :) David Hildenbrand
2025-08-28 21:28 ` David Hildenbrand
2025-08-28 21:32 ` David Hildenbrand
2025-08-29 10:50 ` Christian König
2025-08-29 19:52 ` David Hildenbrand
2025-08-29 19:58 ` David Hildenbrand
2025-08-26 14:27 ` Thomas Hellström
2025-08-28 21:01 ` stupid PAT :) David Hildenbrand
2025-08-26 12:37 ` David Hildenbrand
2025-08-21 9:16 ` your mail Lorenzo Stoakes
2025-08-21 9:30 ` David Hildenbrand
2025-08-21 10:05 ` Lorenzo Stoakes
2025-08-21 10:16 ` David Hildenbrand
2025-08-25 18:35 ` Christian König
2025-08-25 19:20 ` David Hildenbrand
2025-08-21 8:19 ` ✗ i915.CI.BAT: failure for series starting with [1/3] drm/ttm: use apply_page_range instead of vmf_insert_pfn_prot Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4e5f4ef0-53f1-417e-8f3b-76fd7c64cd23@amd.com \
--to=christian.koenig@amd.com \
--cc=airlied@gmail.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=dave.hansen@linux.intel.com \
--cc=david@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=lorenzo.stoakes@oracle.com \
--cc=luto@kernel.org \
--cc=matthew.brost@intel.com \
--cc=peterz@infradead.org \
--cc=thomas.hellstrom@linux.intel.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).