Re: [PATCH 0/4] arm64: Support the TSO memory model

From: Zayd Qumsieh <zayd_qumsieh@apple.com>
To: marcan@marcan.st
Cc: catalin.marinas@arm.com, will@kernel.org, maz@kernel.org,
	mark.rutland@arm.com, zayd_qumsieh@apple.com,
	ih_justin@apple.com, Houdek.Ryan@fex-emu.org, broonie@kernel.org,
	ardb@kernel.org, mjguzik@gmail.com, anshuman.khandual@arm.com,
	oliver.upton@linux.dev, miguel.luis@oracle.com,
	joey.gouly@arm.com, cpaasch@apple.com, keescook@chromium.org,
	samitolvanen@google.com, bhe@redhat.com, j.granados@samsung.com,
	dawei.li@shingroup.cn, akpm@linux-foundation.org,
	revest@chromium.org, david@redhat.com, shr@devkernel.io,
	andy.chiu@sifive.com, josh@joshtriplett.org, oleg@redhat.com,
	deller@gmx.de, zev@bewilderbeest.net, omosnace@redhat.com,
	ojeda@kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, asahi@lists.linux.dev
Subject: Re: [PATCH 0/4] arm64: Support the TSO memory model
Date: Mon, 15 Apr 2024 19:11:00 -0700	[thread overview]
Message-ID: <20240416021100.87652-1-zayd_qumsieh@apple.com> (raw)
In-Reply-To: Hector Martin <20240411-tso-v1-0-754f11abfbff@marcan.st>

The patch looks great! :) I have one minor suggestion, though:

>static __always_inline bool system_has_actlr_state(void)
>{
>	return IS_ENABLED(CONFIG_ARM64_ACTLR_STATE) &&
>		alternative_has_cap_unlikely(ARM64_HAS_TSO_APPLE);
>}

ACTLR_EL1.TSO is not exposed for writing on Virtual Machines on all
versions of MacOS. However, AIDR_EL1 may still advertise TSO, whether
or not ACTLR_EL1.TSO is writable. Could you modify the patch such that
we check the writability of ACTLR_EL1.TSO in system_has_actlr_state
(or once on startup, and cache it, since reading from AIDR_EL1 causes
a trap to Hypervisor.fwk)?

Thanks,
Zayd