From: Matthew Wilcox <willy@infradead.org>
To: Alexandre Ghiti <alexghiti@rivosinc.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>,
Ryan Roberts <ryan.roberts@arm.com>,
Mark Rutland <mark.rutland@arm.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Andrew Morton <akpm@linux-foundation.org>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
linux-mm@kvack.org
Subject: Re: [PATCH v4 2/9] riscv: Restore the pfn in a NAPOT pte when manipulated by core mm code
Date: Mon, 27 Jan 2025 13:51:34 +0000 [thread overview]
Message-ID: <Z5ePZt61CM84Hb36@casper.infradead.org> (raw)
In-Reply-To: <20250127093530.19548-3-alexghiti@rivosinc.com>
On Mon, Jan 27, 2025 at 10:35:23AM +0100, Alexandre Ghiti wrote:
> +#ifdef CONFIG_RISCV_ISA_SVNAPOT
> +static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
> + pte_t *ptep, pte_t pteval, unsigned int nr)
> +{
> + if (unlikely(pte_valid_napot(pteval))) {
> + unsigned int order = ilog2(nr);
> +
> + if (!is_napot_order(order)) {
> + /*
> + * Something's weird, we are given a NAPOT pte but the
No, nothing is weird. This can happen under a lot of different
circumstances. For example, one might mmap() part of a file and the
folio containing the data is only partially mapped. The filesystem /
page cache might choose to use a folio order that isn't one of your
magic hardware orders.
> + * size of the mapping is not a known NAPOT mapping
> + * size, so clear the NAPOT bit and map this without
> + * NAPOT support: core mm only manipulates pte with the
> + * real pfn so we know the pte is valid without the N
> + * bit.
> + */
> + pr_err("Incorrect NAPOT mapping, resetting.\n");
> + pteval = pte_clear_napot(pteval);
> + } else {
> + /*
> + * NAPOT ptes that arrive here only have the N bit set
> + * and their pfn does not contain the mapping size, so
> + * set that here.
> + */
> + pteval = pte_mknapot(pteval, order);
You're assuming that pteval is aligned to the order that you've
calculated, and again that's not true. For example, the user may have
called mmap() on range 0x21000-0x40000 of a file which is covered by
a 128kB folio. You'll be called with a pteval pointing to 0x21000 and
calculate that you can put a 64kB entry there ... no.
I'd suggest you do some testing with fstests and xfs as your underlying
filesystem. It should catch these kinds of mistakes.
next prev parent reply other threads:[~2025-01-27 13:51 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-27 9:35 [PATCH v4 0/9] Merge arm64/riscv hugetlbfs contpte support Alexandre Ghiti
2025-01-27 9:35 ` [PATCH v4 1/9] riscv: Safely remove huge_pte_offset() when manipulating NAPOT ptes Alexandre Ghiti
2025-01-27 9:35 ` [PATCH v4 2/9] riscv: Restore the pfn in a NAPOT pte when manipulated by core mm code Alexandre Ghiti
2025-01-27 13:51 ` Matthew Wilcox [this message]
2025-02-14 12:39 ` Alexandre Ghiti
2025-02-24 14:34 ` Alexandre Ghiti
2025-01-27 9:35 ` [PATCH v4 3/9] mm: Use common huge_ptep_get() function for riscv/arm64 Alexandre Ghiti
2025-01-27 9:35 ` [PATCH v4 4/9] mm: Use common set_huge_pte_at() " Alexandre Ghiti
2025-01-27 9:35 ` [PATCH v4 5/9] mm: Use common huge_pte_clear() " Alexandre Ghiti
2025-01-27 9:35 ` [PATCH v4 6/9] mm: Use common huge_ptep_get_and_clear() " Alexandre Ghiti
2025-01-27 9:35 ` [PATCH v4 7/9] mm: Use common huge_ptep_set_access_flags() " Alexandre Ghiti
2025-01-27 9:35 ` [PATCH v4 8/9] mm: Use common huge_ptep_set_wrprotect() " Alexandre Ghiti
2025-01-27 9:35 ` [PATCH v4 9/9] mm: Use common huge_ptep_clear_flush() " Alexandre Ghiti
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=Z5ePZt61CM84Hb36@casper.infradead.org \
--to=willy@infradead.org \
--cc=akpm@linux-foundation.org \
--cc=alexghiti@rivosinc.com \
--cc=aou@eecs.berkeley.edu \
--cc=catalin.marinas@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-riscv@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=ryan.roberts@arm.com \
--cc=will@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).