Re: [PATCHv4 14/17] arm64: uaccess: remove set_fs()

From: Catalin Marinas <catalin.marinas@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: james.morse@arm.com, will@kernel.org, hch@lst.de,
	linux-arm-kernel@lists.infradead.org, robin.murphy@arm.com
Subject: Re: [PATCHv4 14/17] arm64: uaccess: remove set_fs()
Date: Tue, 17 Nov 2020 11:02:20 +0000	[thread overview]
Message-ID: <X7OtvKJA1Rm92p2b@trantor> (raw)
In-Reply-To: <20201117105740.GB73209@C02TD0UTHF1T.local>

On Tue, Nov 17, 2020 at 10:57:40AM +0000, Mark Rutland wrote:
> On Tue, Nov 17, 2020 at 10:54:18AM +0000, Catalin Marinas wrote:
> > On Tue, Nov 17, 2020 at 10:44:54AM +0000, Mark Rutland wrote:
> > > On Mon, Nov 16, 2020 at 05:40:48PM +0000, Catalin Marinas wrote:
> > > > On Fri, Nov 13, 2020 at 12:49:34PM +0000, Mark Rutland wrote:
> > > > > We no longer need to flip UAO to access kernel memory under KERNEL_DS,
> > > > > and head.S unconditionally clears UAO for all kernel configurations via
> > > > > an ERET in init_kernel_el. Thus, we don't need to dynamically flip UAO,
> > > > > nor do we need to context-switch it. However, we do need to clear UAO
> > > > > (and set PAN) during SDEI entry.
> > > > 
> > > > If the kernel never sets the UAO bit, why do we need to clear it during
> > > > SDEI entry?
> > > 
> > > The fear was taking an SDEI event from a VM which had UAO set, with the
> > > SDEI FW not clearing UAO.
> > > 
> > > That might not happen in practice because while the spec implies that
> > > could happen, TF-A currently generates a new PSTATE from scratch, and
> > > going forward the spec will be aligned with regular exception entry
> > > rules for PSTATE (so UAO will be cleared automatically).
> > 
> > Does this requirement apply retrospectively or it only for the future
> > SDEI specs?
> 
> It applies from the current spec, but note that TF-A has always
> generated a new PSTATE (with UAO clear).

And we can assume that TF-A is the only implementation generating SDEI.

> > > So we can probably drop the clearing of UAO if you prefer.
> > 
> > I don't like clearing UAO specifically. There may be other PSTATE bits
> > in the future we don't know or care about and that are left set by
> > firmware. If we don't trust firmware to give a clean PSTATE, can we
> > reset it with an ERET?
> 
> In future we should get the new behaviour, so I think we should be fine
> there. If we can't trust PSTATE at all, then we're not necessarily able
> to perform an ERET anyway.
> 
> ... which I think means I'm arguing in favour of deleting the clearing
> of UAO, if you're happy with that?

Yes, I'm fine with that. That means that you can also get rid of
cpu_has_uao().

Could we apply the same logic to SDEI+PAN?

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel