Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <1a8ffb1a-d948-0a02-33d5-c332d3e2c228@intel.com>
Date:   Mon, 14 Mar 2022 00:03:57 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Content-Language: en-US
To:     Bharata B Rao <bharata@amd.com>, linux-kernel@vger.kernel.org
Cc:     linux-mm@kvack.org, x86@kernel.org,
        kirill.shutemov@linux.intel.com, tglx@linutronix.de,
        mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
        catalin.marinas@arm.com, will@kernel.org, shuah@kernel.org,
        oleg@redhat.com, ananth.narayan@amd.com
References: <20220310111545.10852-1-bharata@amd.com>
 <81b6f618-05bc-f7d0-5461-4c3f0ca42d3f@intel.com>
 <57338b2a-291c-c7e7-57d5-86a8468b619f@amd.com>
From:   Dave Hansen <dave.hansen@intel.com>
Subject: Re: [RFC PATCH v0 0/6] x86/AMD: Userspace address tagging
In-Reply-To: <57338b2a-291c-c7e7-57d5-86a8468b619f@amd.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/13/22 22:00, Bharata B Rao wrote:
> On 3/10/2022 8:46 PM, Dave Hansen wrote:> 1. While Intel LAM provides two LAM widths (15 and 6 bits wide), AMD UAI
> has a fixed tag width of 7 bits. This results in following differences
> in the implementation:
>    - Two threadinfo flags (TIF_LAM_57 and TIF_LAM_48) in Intel LAM vs
>      single flag TIF_TAGGED_ADDR(like in ARM64) in AMD UAI.
>    - Untagging needs to be aware of 2 widths in Intel LAM as against
>      a single width in AMD UAI.

I'd be perfectly happy with an initial version of this stuff that only
comprehends UAI and LAM_57.  As long as the ABI can be used for all
three cases, adding the two most similar ones initially would make a lot
of sense.

>    - Requirement of making LAM_U48 and mappings above 47bits mutually
>      exclusive is required for Intel LAM only.
> 
> 2. The enablement bit is part of CR3 in Intel LAM which brings in
> additional complexity of updating the CR3 with right LAM mode on every
> MM switch and associated tlbstate changes. In AMD UAI, enablement bit
> is part of EFER MSR and it is a single time enablement with no MM switch
> time changes.

Hold on a sec.  The context-switching is a *policy*.  A _kernel_ policy.
If we wanted the LAM settings to be static and system wide, we'd just
have a single:

	if (cpu_feature_enabled(LAM))
		cr3 |= LAM_MASK;

in build_cr3().  That's not exactly complicated.

You know what's a heck of a lot more complicated than that?  Adding
context-switching for EFER.

I can't imagine a world where we want this tagging to be system-wide.
There *ARE* going to be apps that break with this pointer tagging stuff.
 Normal, user apps.  Apps in containers.  With a system-wide setting,
they have zero recourse when things break.  All a user can do is reboot
the kernel.

As-is, it seems like it would be awful to even develop apps that use
tagging.  You always want to test them with tagging on and off.  With
this, you need to reboot to test either way.

Also, the prctl() in Kirill's version actually enables and disables the
hardware feature in addition to the in-kernel tag/untag operations.
This series takes the same ABI and doesn't change the hardware feature
state.  That will need to be rectified at some point.

Speaking of which, it's also really worrying that kernel behavior is
affected by _EFER_UAI.  When tagging is "disabled" in the prctl(),
_EFER_UAI is still set.  The kernel can go merrily dereferencing both
kernel and userspace pointers with no canonical checks.  That seems
scary.  LAM's supervisor separation makes things a lot less scary.