Re: [PATCH v7 5/6] tools: Allow building xen-hptool without CONFIG_MIGRATE

[Mykyta Poturai <Mykyta_Poturai@xxxxxxxx>]

On 4/29/26 17:33, Anthony PERARD wrote:
> On Thu, Apr 16, 2026 at 08:22:32AM +0000, Mykyta Poturai wrote:
>> On 4/16/26 09:49, Jan Beulich wrote:
>>> On 15.04.2026 16:51, Mykyta Poturai wrote:
>>>> On 3/30/26 15:32, Jan Beulich wrote:
>>>>> This looks wrong to me. There are x86-specifics in that file, which 
>>>>> shouldn't
>>>>> be built on Arm. And the name of the file also doesn't indicate any 
>>>>> relation
>>>>> to CPU management.
>>>>
>>>> xen-hptool requires xg_offline_page as it has both CPU and memory
>>>> hotplug commands. Without building xg_offline_page it fails with
>>>>
>>>> xen-hptool: symbol lookup error: xen-hptool: undefined symbol:
>>>> xc_mark_page_offline, version libxenguest_4.22.0
>>>>
>>>> when trying to do memory ops.
>>>>
>>>> Is it an acceptable behavior?
>>>
>>> I don't think so, no. The tool wouldn't, aiui, load at all then if built 
>>> with
>>> "bindnow" enabled.
>>>
>>>> If so I guess we can build xg_offline page only on x86.
>>>
>>> We still need to, imo. But the tool still needs to be usable no matter how
>>> specifically it is built. It should avoid referencing xg_offline_page.c
>>> functions when built for non-x86.
>>
>> As I understand, the usage of arch-specific compile time checks is
>> heavily discouraged in tools. So I don’t think it would be approved by
>> tools maintainers. Do we really need to omit this file if memory ops are
>> already getting blocked by Xen on Arm anyway?
> 
> So you are trying to modify a library and introduced untested
> functionality just to be able to build a different tool? I don't think
> that a good idea especially in this case where it's more than just glue
> code between a binary and xen.
> 
> We could change the library to provide the missing symbols, but it is
> probably best to keep it that way for now.
> 
> So, how about changing `xen-hptool` to have reduced functionality on
> other platform, and keep the 'mem-*' command on x86 only? You could move
> the function that implement the 'mem-*' command into a separate file,
> that compile only on x86 (or more specifically when CONFIG_MIGRATE is
> set) and just have a "#if defined(__i386__) || defined(__x86_64__)" in
> the `main_options` array.
> 
> They are compile-time arch-specific check everywhere in tools. Arch
> specific are often implemented in separated source file, this mean we
> can limit the #ifdefs to a minimum and keep the code readable.
> 
> Thanks,
> 
> 

Should I also do the same thing for SMT operations?

-- 
Mykyta