Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.112.34 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.112.34; helo=mail.nvidia.com;
From:   Alistair Popple <apopple@nvidia.com>
To:     Peter Xu <peterx@redhat.com>
CC:     Hugh Dickins <hughd@google.com>, <linux-kernel@vger.kernel.org>,
        <linux-mm@kvack.org>, Axel Rasmussen <axelrasmussen@google.com>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Andrew Morton <akpm@linux-foundation.org>,
        Miaohe Lin <linmiaohe@huawei.com>,
        Mike Rapoport <rppt@linux.vnet.ibm.com>,
        Jerome Glisse <jglisse@redhat.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Nadav Amit <nadav.amit@gmail.com>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Jason Gunthorpe <jgg@ziepe.ca>,
        Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH v3 04/27] mm/userfaultfd: Introduce special pte for unmapped file-backed mem
Date:   Tue, 8 Jun 2021 23:18:25 +1000
Message-ID: <3035789.C3dLqLxEWu@nvdebian>
In-Reply-To: <YLpOdxWnRbWx9CL+@t490s>
References: <20210527201927.29586-1-peterx@redhat.com> <2408831.NcqaVN92ti@nvdebian> <YLpOdxWnRbWx9CL+@t490s>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jun 2021 13:18:32.3469
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 52ff4874-7e42-4511-43a2-08d92a7fe9fc
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.112.34];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: DM6NAM11FT023.eop-nam11.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR12MB1281
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Saturday, 5 June 2021 2:01:59 AM AEST Peter Xu wrote:
> On Fri, Jun 04, 2021 at 04:16:30PM +1000, Alistair Popple wrote:
> > > My understanding is that it does *not* use an additional arch-dependent
> > > bit, but puts the _PAGE_UFFD_WP bit (already set aside by any architecture
> > > implementing UFFD WP) to an additional use.  That's why I called this
> > > design (from Andrea) more elegant than mine (swap type business).
> >
> > Oh my bad, I had somehow missed this was reusing an *existing* arch-dependent
> > swap bit (_PAGE_SWP_UFFD_WP, although the same argument could apply) even
> > though it's in the commit message. Obviously I should have read that more
> > carefully, apologies for the noise but thanks for the clarification.
> 
> Right, as Hugh mentioned what this series wanted to use is one explicit pte
> that no one should ever be using, so ideally that should be the most saving way
> per address-space pov.
> 
> Meanwhile I think that pte can actually be not related to _PAGE_UFFD_WP at all,
> as long as it's a specific pte value then it will service the same goal (even
> if to reuse a new swp type, I'll probably only use one pte for it and leave the
> rest for other use; but who knows who will start to use the rest!).
> 
> I kept using it because that's suggested by Andrea (it actually has
> type==off==0 as Hugh suggested too - so it keeps a suggestion of both!) and
> it's a good idea to use it since (1) it's never used by anyone before, and (2)
> it is _somehow_ related to uffd-wp itself already by having that specific bit
> set in the special pte, while that's also the only bit set for the u64 field.
> 
> It looks very nice too when debug, because when I dump the ptes it reads 0x4 on
> x86.. so the pte value is even easy to read as a number. :)
> 
> However I can see that it is less easy to follow than the swap type solution.
> In all cases it's still something worth thinking about before using up the swap
> types - it's not so rich there, and we keep shrinking MAX_SWAPFILES.. so let's
> see whether uffd-wp could be the 1st one to open a new field for unused
> "invalid/swap pte" address space.

Agreed, that matches with what I was thinking as well. If we do end up having
more swap types such as this which don't need to store much information in
the swap pte itself we could define a special swap type (eg. this bit) for
that.

> Meanwhile, I did have a look at ARM on supporting uffd-wp in general, starting
> from anonymous pages.  I doubt whether it can be done for old arms (uffd-wp not
> even supported on 32bit x86 after all), but for ARM64 I see it has:
> 
> For normal ptes:
> 
> /*
>  * Level 3 descriptor (PTE).
>  */
> #define PTE_VALID               (_AT(pteval_t, 1) << 0)
> #define PTE_TYPE_MASK           (_AT(pteval_t, 3) << 0)
> #define PTE_TYPE_PAGE           (_AT(pteval_t, 3) << 0)
> #define PTE_TABLE_BIT           (_AT(pteval_t, 1) << 1)
> #define PTE_USER                (_AT(pteval_t, 1) << 6)         /* AP[1] */
> #define PTE_RDONLY              (_AT(pteval_t, 1) << 7)         /* AP[2] */
> #define PTE_SHARED              (_AT(pteval_t, 3) << 8)         /* SH[1:0], inner shareable */
> #define PTE_AF                  (_AT(pteval_t, 1) << 10)        /* Access Flag */
> #define PTE_NG                  (_AT(pteval_t, 1) << 11)        /* nG */
> #define PTE_GP                  (_AT(pteval_t, 1) << 50)        /* BTI guarded */
> #define PTE_DBM                 (_AT(pteval_t, 1) << 51)        /* Dirty Bit Management */
> #define PTE_CONT                (_AT(pteval_t, 1) << 52)        /* Contiguous range */
> #define PTE_PXN                 (_AT(pteval_t, 1) << 53)        /* Privileged XN */
> #define PTE_UXN                 (_AT(pteval_t, 1) << 54)        /* User XN */
> 
> For swap ptes:
> 
> /*
>  * Encode and decode a swap entry:
>  *      bits 0-1:       present (must be zero)
>  *      bits 2-7:       swap type
>  *      bits 8-57:      swap offset
>  *      bit  58:        PTE_PROT_NONE (must be zero)
>  */
> 
> So I feel like we still have chance there at least for 64bit ARM? As both
> normal/swap ptes have some bits free (bits 2-5,9 for normal ptes; bits 59-63
> for swap ptes).  But as I know little on ARM64, I hope I looked at the right
> things..

I don't claim to be an expert there either. Given there's already a bit
defined for x86 anyway (which is what I missed) I now think the special
swap idea is ok, although I still need to look at the rest of the series.

> Thanks,
> 
> --
> Peter Xu
>