Re: [PATCH RFC v2 13/27] arm64: mte: Make tag storage depend on ARCH_KEEP_MEMBLOCK

From: David Hildenbrand
Date: Tue Nov 28 2023 - 12:06:49 EST


On 27.11.23 16:04, Alexandru Elisei wrote:
Hi,

On Fri, Nov 24, 2023 at 08:51:38PM +0100, David Hildenbrand wrote:
On 19.11.23 17:57, Alexandru Elisei wrote:
Tag storage memory requires that the tag storage pages used for data are
always migratable when they need to be repurposed to store tags.

If ARCH_KEEP_MEMBLOCK is enabled, kexec will scan all non-reserved
memblocks to find a suitable location for copying the kernel image. The
kernel image, once loaded, cannot be moved to another location in physical
memory. The initialization code for the tag storage reserves the memblocks
for the tag storage pages, which means kexec will not use them, and the tag
storage pages can be migrated at any time, which is the desired behaviour.

However, if ARCH_KEEP_MEMBLOCK is not selected, kexec will not skip a
region unless the memory resource has the IORESOURCE_SYSRAM_DRIVER_MANAGED
flag, which isn't currently set by the tag storage initialization code.

Make ARM64_MTE_TAG_STORAGE depend on ARCH_KEEP_MEMBLOCK to make it explicit
that that the Kconfig option required for it to work correctly.

Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
---
arch/arm64/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 047487046e8f..efa5b7958169 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -2065,6 +2065,7 @@ config ARM64_MTE
if ARM64_MTE
config ARM64_MTE_TAG_STORAGE
bool "Dynamic MTE tag storage management"
+ depends on ARCH_KEEP_MEMBLOCK
select CONFIG_CMA
help
Adds support for dynamic management of the memory used by the hardware

Doesn't arm64 select that unconditionally? Why is this required then?

I've added this patch to make the dependancy explicit. If, in the future, arm64
stops selecting ARCH_KEEP_MEMBLOCK, I thinkg it would be very easy to miss the
fact that tag storage depends on it. So this patch is not required per-se, it's
there to document the dependancy.

I see. Could you add some static_assert / BUILD_BUG_ON instead?

I suspect there are plenty other (undocumented) reasons why ARCH_KEEP_MEMBLOCK has to be enabled for now, and none sets ARCH_KEEP_MEMBLOCK, I suspect/

--
Cheers,

David / dhildenb