linux-kernel - Re: [PATCH v4 35/36] mm: Convert do_set_pte() to set_pte

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: <ZBNXcmOrrOS4Rydg@casper.infradead.org>
Date:   Thu, 16 Mar 2023 17:52:50 +0000
From:   Matthew Wilcox <willy@...radead.org>
To:     Ryan Roberts <ryan.roberts@....com>
Cc:     "Yin, Fengwei" <fengwei.yin@...el.com>, linux-arch@...r.kernel.org,
        will@...nel.org, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 35/36] mm: Convert do_set_pte() to set_pte_range()

On Thu, Mar 16, 2023 at 04:38:58PM +0000, Ryan Roberts wrote:
> On 16/03/2023 16:23, Yin, Fengwei wrote:
> >> I think you are changing behavior here - is this intentional? Previously this
> >> would be evaluated per page, now its evaluated once for the whole range. The
> >> intention below is that directly faulted pages are mapped young and prefaulted
> >> pages are mapped old. But now a whole range will be mapped the same.
> > 
> > Yes. You are right here.
> > 
> > Look at the prefault and cpu_has_hw_af for ARM64, it looks like we
> > can avoid to handle vmf->address == addr specially. It's OK to 
> > drop prefault and change the logic here a little bit to:
> >   if (arch_wants_old_prefaulted_pte())
> >       entry = pte_mkold(entry);
> >   else
> >       entry = pte_sw_mkyong(entry);
> > 
> > It's not necessary to use pte_sw_mkyong for vmf->address == addr
> > because HW will set the ACCESS bit in page table entry.
> > 
> > Add Will Deacon in case I missed something here. Thanks.
> 
> I'll defer to Will's response, but not all arm HW supports HW access flag
> management. In that case it's done by SW, so I would imagine that by setting
> this to old initially, we will get a second fault to set the access bit, which
> will slow things down. I wonder if you will need to split this into (up to) 3
> calls to set_ptes()?

I don't think we should do that.  The limited information I have from
various microarchitectures is that the PTEs must differ only in their
PFN bits in order to use larger TLB entries.  That includes the Accessed
bit (or equivalent).  So we should mkyoung all the PTEs in the same
folio, at least initially.

That said, we should still do this conditionally.  We'll prefault some
other folios too.  So I think this should be:

        bool prefault = (addr > vmf->address) || ((addr + nr) < vmf->address);