Re: [RFC PATCH v0 0/6] x86/AMD: Userspace address tagging

From: Bharata B Rao <bharata@amd.com>
To: Dave Hansen <dave.hansen@intel.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
Subject: Re: [RFC PATCH v0 0/6] x86/AMD: Userspace address tagging
Date: Mon, 14 Mar 2022 10:30:09 +0530	[thread overview]
Message-ID: <57338b2a-291c-c7e7-57d5-86a8468b619f@amd.com> (raw)
In-Reply-To: <81b6f618-05bc-f7d0-5461-4c3f0ca42d3f@intel.com>

On 3/10/2022 8:46 PM, Dave Hansen wrote:
> On 3/10/22 03:15, Bharata B Rao wrote:>
>> This patchset builds on that prctl() extension and adds support
>> for AMD UAI. AMD implementation is kept separate as equivalent
>> Intel LAM implementation is likely to be different due to different
>> bit positions and tag width.
> 
> Please don't keep the implementations separate.
> 
> We'll have one x86 implementation of address bit masking.  Both the
> Intel and AMD implementations will feed into a shared implementation.
> Something _like_ the cc_set_mask() interface where both implementations
> do their detection and then call into common code to say how many bits
> are being ignored.

The difference isn't limited to the number of ignored bits, see below...

> 
> A good litmus test for this is how many vendor-specific checks there are
> in common code.  If there are a lot of them, it's not a good sign for
> the design.

That is generally true but considering the below differences, I felt it
was much cleaner to go for separate implementations for enablement and
untagging while still building on the common prctl() extension in the
RFC version.

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.
   - 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.

However, let me take a detailed look once again and explore further
code sharing possibilities.

Regards,
Bharata.