Re: [PATCH 02/11] xen/page_alloc: Remove `claim` from domain_set_outstanding_pages()

[Alejandro Vallejo <agarciav@amd.com>]

On Tue Jun 10, 2025 at 2:23 PM CEST, Jan Beulich wrote:
> On 05.06.2025 18:42, Roger Pau Monné wrote:
>> On Fri, Mar 14, 2025 at 05:24:53PM +0000, Alejandro Vallejo wrote:
>>> With a single global count for the claims it's easy to substract
>>> domain_tot_pages() from the claim so the number given in the hypercall
>>> is the real reservation of the domain. This is the current behaviour.
>>>
>>> However, a later patch introduces exact-node claims and those interact
>>> very poorly with such a scheme. Since accounting domain_tot_pages() in
>>> one case but not the other seems strictly worse than not accounting them
>>> at all (which is at least consistent), this patch stops substracting
>>> tot_pages from the claim and instead checks that claimed memory +
>>> allocated memory don't exceed max_mem.
>> 
>> Hm, while I don't have any specific interest in keeping the current
>> behavior, XENMEM_claim_pages is part of the stable ABI (it's not a
>> domctl), and hence should be stable.
>
> Is that true? It sits inside a
>
> #if defined(__XEN__) || defined(__XEN_TOOLS__)
>
> which generally de-marks unstable (tools-only) interfaces.
>
>>  Note also the comment above the
>> definition of XENMEM_claim_pages how it states the specific behavior
>> that you are trying to change (and which should have been adjusted as
>> part of this change).
>
> This is the more important part, imo.

Ugh. I missed that. Regardless, the current scheme is highly counterintuitive
and works only because toolstack colaborates in removing the claim after dom
creation.

>
>> I have no idea why this was made a xenmem rather than a domctl
>> hypercall, but if you want to change the semantics I think the only
>> option is introducing a new hypercall.
>
> It _is_ a memory operation, and it re-uses one of the interface structs
> there. (Yes, none of these would technically have prevented it from
> being a domctl.)
>
> Jan

As Jan mentions, my understanding was that __XEN_TOOLS__ effectively meant
unstable. It is quite hard to keep the previous semantics for one hypercall
while introducing a new one that behaves slightly differently.

Cheers,
Alejandro