Re: [PATCH v2 2/2] common: make page_{is,get}_ram_type() x86-only

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.08.2025 20:45, Andrew Cooper wrote:
> On 18/08/2025 8:57 am, Jan Beulich wrote:
>> --- a/xen/arch/x86/include/asm/mm.h
>> +++ b/xen/arch/x86/include/asm/mm.h
>> @@ -464,6 +464,16 @@ extern bool opt_pv_linear_pt;
>>      ASSERT(((_p)->count_info & PGC_count_mask) != 0);          \
>>      ASSERT(page_get_owner(_p) == (_d))
>>  
>> +#define RAM_TYPE_CONVENTIONAL 0x00000001
>> +#define RAM_TYPE_RESERVED     0x00000002
>> +#define RAM_TYPE_UNUSABLE     0x00000004
>> +#define RAM_TYPE_ACPI         0x00000008
>> +#define RAM_TYPE_UNKNOWN      0x00000010
>> +/* TRUE if the whole page at @mfn is of the requested RAM type(s) above. */
>> +bool page_is_ram_type(unsigned long mfn, unsigned long mem_type);
> 
> Making it x86-only is fine, but if you're changing the return type,
> mem_type can become shorter too.

Oh, indeed - done. Won't re-post just for this, though.

> Also, I'm struggling to convince myself that page_is_ram_type() is
> correct.  Even if it is correct, it is horribly inefficient, and we run
> this algorithm lots on boot.
> 
> Checking the type before the range is the backwards way of doing this,
> and it can be a binary search because we go out of our way to fix
> unsorted e820 maps.
> 
> Finally, acpi_memory_banned() shows that this really isn't the greatest
> API in the first place.  It's explicitly designed to take multiple
> inputs, but does the wrong thing for its' single caller wanting that
> behaviour.  I can't help but feel we'd be in a better position by
> removing this entirely and using plain E820 queries.

All valid remarks, but all to be taken care of separately imo.

Jan