Re: [PATCH v2 3/5] xen/arm: Move make_resv_memory_node()

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 7/9/25 03:58, Koichiro Den wrote:
> On Tue, Jul 08, 2025 at 04:12:50PM -0400, Stewart Hildebrand wrote:
>> On 7/5/25 10:27, Koichiro Den wrote:
>>> The /reserved-memory node is inherently not specific to static-shmem.
>>> Move it to a more generic domain build context. While at it, add an
>>> empty kernel_info_get_shm_mem_const() for the CONFIG_STATIC_SHM=n case,
>>> as it can now be invoked in such case.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
>>> ---
>>>  xen/arch/arm/domain_build.c           | 40 +++++++++++++++++++++++++++
>>>  xen/common/device-tree/static-shmem.c | 40 ---------------------------
>>>  xen/include/xen/fdt-domain-build.h    |  2 ++
>>>  xen/include/xen/static-shmem.h        | 15 ++++------
>>>  4 files changed, 48 insertions(+), 49 deletions(-)
>>
>> make_resv_memory_node() was recently moved from arch/arm to common in:
>>
>> 72c5fa220804 ("device-tree: Move Arm's static-shmem feature to common")
>>
>> Is there any rationale for moving it back?
> 
> I overlooked that commit. So to preserve its intent, and make it usable
> outside of the static-shmem purpose, which is my original intention, I
> think I should place it in xen/common/device-tree/dom0less-build.c. What do
> you think?

Hm, if only considering its current callers, yes, because STATIC_SHM
depends on STATIC_MEMORY depends on DOM0LESS_BOOT. However, whether to
put it in the common domain-build.c or dom0less-build.c would really
depend on how you intend to use it, but I don't see any new calls to
make_resv_memory_node() on the hypervisor side in the remainder of the
series. Given that you do introduce an equivalent call on the toolstack
side, I wonder if that's an oversight on the hypervisor side?

Lastly, in the context of the series, your intent looks to be to add the
pv time region to /reserved-memory, but is that actually necessary?