Re: [PATCH v2 16/17] xen/riscv: implement mfn_valid() and page reference, ownership handling helpers

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.07.2025 16:49, Oleksii Kurochko wrote:
> On 7/2/25 12:09 PM, Jan Beulich wrote:
>> On 10.06.2025 15:05, Oleksii Kurochko wrote:
>>> Implement the mfn_valid() macro to verify whether a given MFN is valid by
>>> checking that it falls within the range [start_page, max_page).
>>> These bounds are initialized based on the start and end addresses of RAM.
>>>
>>> As part of this patch, start_page is introduced and initialized with the
>>> PFN of the first RAM page.
>>>
>>> Also, after providing a non-stub implementation of the mfn_valid() macro,
>>> the following compilation errors started to occur:
>>>    riscv64-linux-gnu-ld: prelink.o: in function `__next_node':
>>>    /build/xen/./include/xen/nodemask.h:202: undefined reference to 
>>> `page_is_ram_type'
>>>    riscv64-linux-gnu-ld: prelink.o: in function `get_free_buddy':
>>>    /build/xen/common/page_alloc.c:881: undefined reference to 
>>> `page_is_ram_type'
>>>    riscv64-linux-gnu-ld: prelink.o: in function `alloc_heap_pages':
>>>    /build/xen/common/page_alloc.c:1043: undefined reference to 
>>> `page_get_owner_and_reference'
>>>    riscv64-linux-gnu-ld: /build/xen/common/page_alloc.c:1098: undefined 
>>> reference to `page_is_ram_type'
>>>    riscv64-linux-gnu-ld: prelink.o: in function `ns16550_interrupt':
>>>    /build/xen/drivers/char/ns16550.c:205: undefined reference to `get_page'
>>>    riscv64-linux-gnu-ld: ./.xen-syms.0: hidden symbol 
>>> `page_get_owner_and_reference' isn't defined
>>>    riscv64-linux-gnu-ld: final link failed: bad value
>>>    make[2]: *** [arch/riscv/Makefile:35: xen-syms] Error 1
>>> To resolve these errors, the following functions have also been introduced,
>>> based on their Arm counterparts:
>>> - page_get_owner_and_reference() and its variant to safely acquire a
>>>    reference to a page and retrieve its owner.
>>> - put_page() and put_page_nr() to release page references and free the page
>>>    when the count drops to zero.
>>>    For put_page_nr(), code related to static memory configuration is wrapped
>>>    with CONFIG_STATIC_MEMORY, as this configuration has not yet been moved 
>>> to
>>>    common code. Therefore, PGC_static and free_domstatic_page() are not
>>>    introduced for RISC-V. However, since this configuration could be useful
>>>    in the future, the relevant code is retained and conditionally compiled.
>>> - A stub for page_is_ram_type() that currently always returns 0 and asserts
>>>    unreachable, as RAM type checking is not yet implemented.
>> How does this end up working when common code references the function?
> 
> Based on the following commit message:
>      Callers are VT-d (so x86 specific) and various bits of page offlining
>      support, which although it looks generic (and is in xen/common) does
>      things like diving into page_info->count_info which is not generic.
>      
>      In any case on this is only reachable via XEN_SYSCTL_page_offline_op,
>      which clearly shouldn't be called on ARM just yet.

Assuming this is from an old commit, then I have to question this justification.
I see nothing preventing XEN_SYSCTL_page_offline_op to be invoked on an Arm
system. Hence (unless I'm overlooking somthing) ASSERT_UNREACHABLE() is simply
inappropriate (and wants fixing). Luckily it being sysctl-s only, there's no
need for an XSA. In no case should known flawed code be copied into another
port.

Jan