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

David Hildenbrand
2023-11-28T17:05:55Z

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@arm.com>
>>> ---
>>> 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