Re: [PATCH v3 7/7] xen: New hypercall to claim memory using XEN_DOMCTL_claim_memory

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.09.2025 18:15, Bernhard Kaindl wrote:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -267,6 +267,35 @@ int get_domain_state(struct xen_domctl_get_domain_state 
> *info, struct domain *d,
>      return rc;
>  }
>  
> +/* XEN_DOMCTL_claim_memory: Claim an amount of memory for a domain */
> +int claim_memory(struct domain *d, const struct xen_domctl_claim_memory 
> *uinfo)
> +{
> +    memory_claim_t claim;
> +    int rc;
> +
> +    switch ( uinfo->nr_claims )
> +    {
> +        case 0:

Nit (style) Indentation (case labels ant indenting the same as the respective
opening figure brace).

Considering what follows I'm not quite sure though that using switch() is
appropriate here. Kind of depends on what the longer-term plans are.

> +            /* Cancel existing claim. */
> +            rc = domain_claim_pages(d, 0, 0);
> +            break;

This effect also wants mentioning in the public header. It's not what I would
expect (a no-op). Also, shouldn't NUMA_NO_NODE be passed here?

> +        case 1:
> +            /* Only single node claims supported at the moment. */

Isn't the comment misplaced? Comments usually describe what follows, and the
"nr_claims == 1" restriction starts already a line earlier. Maybe the comment
would best go on the same line as the case label.

> +            if ( copy_from_guest(&claim, uinfo->claims, 1) )
> +                return -EFAULT;
> +
> +            rc = domain_claim_pages(d, claim.node, claim.nr_pages);

claim.node needs bounds checking, for nodemask_test() to not overrun the bitmap.

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1276,6 +1276,21 @@ struct xen_domctl_get_domain_state {
>      uint64_t unique_id;      /* Unique domain identifier. */
>  };
>  
> +struct xen_memory_claim {
> +    unsigned int node;      /* NUMA node, XC_NUMA_NO_NODE for a host claim */
> +    unsigned long nr_pages; /* Number of pages to claim */

Fixed-width types need using here and ...

> +};
> +typedef struct xen_memory_claim memory_claim_t;
> +DEFINE_XEN_GUEST_HANDLE(memory_claim_t);
> +
> +/* XEN_DOMCTL_claim_memory: Claim an amount of memory for a domain */
> +struct xen_domctl_claim_memory {
> +    /* IN: array of memory claims */
> +    XEN_GUEST_HANDLE_64(memory_claim_t) claims;
> +    /* IN: number of claims */
> +    unsigned int nr_claims;

... here.

Jan