Re: [PATCH 18/26] mm/memblock: Add MEMBLOCK_INSPECT flag

[Eugen Hristev <eugen.hristev@linaro.org>]

On 12/29/25 08:56, Mike Rapoport wrote:
> Hi Eugen,
> 
> On Wed, Nov 19, 2025 at 05:44:19PM +0200, Eugen Hristev wrote:
>> This memblock flag indicates that a specific block is registered
>> into an inspection table.
>> The block can be marked for inspection using memblock_mark_inspect()
>> and cleared with memblock_clear_inspect()
> 
> Can you explain why memblock should treat memory registered for inspection
> differently?

It should not, at a first glance.

The purpose of the flag is to let memblock be aware of it.
The flag is there to have a "memblock way" of registering the memory,
which inside memblock , it can translate to a meminspect way of
registering the memory. It's just an extra layer on top of meminspect.
With this, it would be avoided to call meminspect all over the places it
would be required, but rather use the memblock API.
And further, inside memblock, it would be a single point where
meminspect can be disabled (while preserving a no-op memblock flag), or
easily changed to another API if needed.
Ofcourse, one can call here directly the meminspect API if this is desired.
Do you think it would be better to have it this way ?

Thanks for looking into it,
Eugen


> 
>> Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
>> ---
>>  include/linux/memblock.h |  7 +++++++
>>  mm/memblock.c            | 36 ++++++++++++++++++++++++++++++++++++
>>  2 files changed, 43 insertions(+)
>>
>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
>> index 221118b5a16e..c3e55a4475cf 100644
>> --- a/include/linux/memblock.h
>> +++ b/include/linux/memblock.h
>> @@ -51,6 +51,10 @@ extern unsigned long long max_possible_pfn;
>>   * memory reservations yet, so we get scratch memory from the previous
>>   * kernel that we know is good to use. It is the only memory that
>>   * allocations may happen from in this phase.
>> + * @MEMBLOCK_INSPECT: memory region is annotated in kernel memory inspection
>> + * table. This means a dedicated entry will be created for this region which
>> + * will contain the memory's address and size. This allows kernel inspectors
>> + * to retrieve the memory.
>>   */
>>  enum memblock_flags {
>>  	MEMBLOCK_NONE		= 0x0,	/* No special request */
>> @@ -61,6 +65,7 @@ enum memblock_flags {
>>  	MEMBLOCK_RSRV_NOINIT	= 0x10,	/* don't initialize struct pages */
>>  	MEMBLOCK_RSRV_KERN	= 0x20,	/* memory reserved for kernel use */
>>  	MEMBLOCK_KHO_SCRATCH	= 0x40,	/* scratch memory for kexec handover */
>> +	MEMBLOCK_INSPECT	= 0x80,	/* memory selected for kernel inspection */
>>  };
>>  
>>  /**
>> @@ -149,6 +154,8 @@ unsigned long memblock_addrs_overlap(phys_addr_t base1, phys_addr_t size1,
>>  bool memblock_overlaps_region(struct memblock_type *type,
>>  			      phys_addr_t base, phys_addr_t size);
>>  bool memblock_validate_numa_coverage(unsigned long threshold_bytes);
>> +int memblock_mark_inspect(phys_addr_t base, phys_addr_t size);
>> +int memblock_clear_inspect(phys_addr_t base, phys_addr_t size);
>>  int memblock_mark_hotplug(phys_addr_t base, phys_addr_t size);
>>  int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>>  int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index e23e16618e9b..a5df5ab286e5 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -17,6 +17,7 @@
>>  #include <linux/seq_file.h>
>>  #include <linux/memblock.h>
>>  #include <linux/mutex.h>
>> +#include <linux/meminspect.h>
>>  
>>  #ifdef CONFIG_KEXEC_HANDOVER
>>  #include <linux/libfdt.h>
>> @@ -1016,6 +1017,40 @@ static int __init_memblock memblock_setclr_flag(struct memblock_type *type,
>>  	return 0;
>>  }
>>  
>> +/**
>> + * memblock_mark_inspect - Mark inspectable memory with flag MEMBLOCK_INSPECT.
>> + * @base: the base phys addr of the region
>> + * @size: the size of the region
>> + *
>> + * Return: 0 on success, -errno on failure.
>> + */
>> +int __init_memblock memblock_mark_inspect(phys_addr_t base, phys_addr_t size)
>> +{
>> +	int ret;
>> +
>> +	ret = memblock_setclr_flag(&memblock.memory, base, size, 1, MEMBLOCK_INSPECT);
>> +	if (ret)
>> +		return ret;
>> +
>> +	meminspect_lock_register_pa(base, size);
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * memblock_clear_inspect - Clear flag MEMBLOCK_INSPECT for a specified region.
>> + * @base: the base phys addr of the region
>> + * @size: the size of the region
>> + *
>> + * Return: 0 on success, -errno on failure.
>> + */
>> +int __init_memblock memblock_clear_inspect(phys_addr_t base, phys_addr_t size)
>> +{
>> +	meminspect_lock_unregister_pa(base, size);
>> +
>> +	return memblock_setclr_flag(&memblock.memory, base, size, 0, MEMBLOCK_INSPECT);
>> +}
>> +
>>  /**
>>   * memblock_mark_hotplug - Mark hotpluggable memory with flag MEMBLOCK_HOTPLUG.
>>   * @base: the base phys addr of the region
>> @@ -2704,6 +2739,7 @@ static const char * const flagname[] = {
>>  	[ilog2(MEMBLOCK_RSRV_NOINIT)] = "RSV_NIT",
>>  	[ilog2(MEMBLOCK_RSRV_KERN)] = "RSV_KERN",
>>  	[ilog2(MEMBLOCK_KHO_SCRATCH)] = "KHO_SCRATCH",
>> +	[ilog2(MEMBLOCK_INSPECT)] = "INSPECT",
>>  };
>>  
>>  static int memblock_debug_show(struct seq_file *m, void *private)
>> -- 
>> 2.43.0
>>
>