Re: [PATCH v6 3/3] arm64: pac: Optimize kernel entry/exit key installation code paths

From: Peter Collingbourne <pcc@google.com>
To: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Evgenii Stepanov <eugenis@google.com>,
	Kostya Serebryany <kcc@google.com>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Dave Martin <Dave.Martin@arm.com>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>,
	Florian Weimer <fw@deneb.enyo.de>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Kevin Brodsky <kevin.brodsky@arm.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Linux API <linux-api@vger.kernel.org>,
	libc-alpha@sourceware.org
Subject: Re: [PATCH v6 3/3] arm64: pac: Optimize kernel entry/exit key installation code paths
Date: Thu, 11 Feb 2021 21:01:00 -0800	[thread overview]
Message-ID: <CAMn1gO4OGsYYXBAWk=OiauZoyHoPFR9znSeLfXV0rLoZ+H7j1A@mail.gmail.com> (raw)
In-Reply-To: <20210126130947.GD29702@willie-the-truck>

On Tue, Jan 26, 2021 at 5:09 AM Will Deacon <will@kernel.org> wrote:
>
> On Tue, Dec 29, 2020 at 10:59:15PM -0800, Peter Collingbourne wrote:
> > The kernel does not use any keys besides IA so we don't need to
> > install IB/DA/DB/GA on kernel exit if we arrange to install them
> > on task switch instead, which we can expect to happen an order of
> > magnitude less often.
> >
> > Furthermore we can avoid installing the user IA in the case where the
> > user task has IA disabled and just leave the kernel IA installed. This
> > also lets us avoid needing to install IA on kernel entry.
>
> I've got to be honest, this makes me nervous in case there is a way for
> userspace to recover the kernel key even though EnIA is clear. Currently,
> EnIA doesn't affect XPAC* and PACGA instructions, and the architecture

For GA I would expect it to be controlled by a hypothetical EnGA, not
by EnIA (and I'm a bit surprised that there isn't an EnGA; doesn't it
mean that a userspace program running under an unaware kernel or
hypervisor may sign things using the GA from potentially another
hypervisor guest?)

> clearly expects us to be switching these things:
>
>   | Note
>   | Keys are not banked by Exception level. Arm expects software to switch the
>   | keys between Exception levels, typically by swapping the values with zero
>   | so that the current key values are not present in memo
>
> But then:
>
> > On an Apple M1 under a hypervisor, the overhead of kernel entry/exit
> > has been measured to be reduced by 15.6ns in the case where IA is
> > enabled, and 31.9ns in the case where IA is disabled.
>
> That's a good improvement, so this feels like its worth doing. I suppose all we
> can do is keep an eye on the architecture in case any future extensions mean
> the approach taken here is dangerous.

Right. I think it makes sense for any future extensions not to expose
a key in any way if the corresponding key enable bit is clear (to
avoid the potential problem that I highlighted with GA above) unless
the operating system has explicitly opted into such behavior, e.g. by
setting a separate bit in SCTLR_EL1.

Peter