The classification is pretty E820-centric anyway, and all uses of the
function are now in x86-only code.

Switch the boolean return type to properly use bool while at it.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.

--- 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);
+/* Returns the page type(s). */
+unsigned int page_get_ram_type(mfn_t mfn);
+
 /******************************************************************************
  * With shadow pagetables, the different kinds of address start
  * to get get confusing.
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -385,7 +385,7 @@ void __init arch_init_memory(void)
     ASM_CONSTANT(FIXADDR_X_SIZE, FIXADDR_X_SIZE);
 }
 
-int page_is_ram_type(unsigned long mfn, unsigned long mem_type)
+bool page_is_ram_type(unsigned long mfn, unsigned long mem_type)
 {
     uint64_t maddr = pfn_to_paddr(mfn);
     int i;
@@ -419,10 +419,10 @@ int page_is_ram_type(unsigned long mfn,
         /* Test the range. */
         if ( (e820.map[i].addr <= maddr) &&
              ((e820.map[i].addr + e820.map[i].size) >= (maddr + PAGE_SIZE)) )
-            return 1;
+            return true;
     }
 
-    return 0;
+    return false;
 }
 
 bool page_is_offlinable(mfn_t mfn)
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -565,15 +565,6 @@ int __must_check guest_remove_page(struc
 int __must_check steal_page(struct domain *d, struct page_info *page,
                             unsigned int memflags);
 
-#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. */
-int page_is_ram_type(unsigned long mfn, unsigned long mem_type);
-/* Returns the page type(s). */
-unsigned int page_get_ram_type(mfn_t mfn);
 /* Check if a range falls into a hole in the memory map. */
 bool is_memory_hole(mfn_t start, mfn_t end);

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.


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.

~Andrew

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

On 2025-08-18 03:57, Jan Beulich wrote:
> The classification is pretty E820-centric anyway, and all uses of the
> function are now in x86-only code.
> 
> Switch the boolean return type to properly use bool while at it.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>