Re: [PATCH v3 14/16] x86/hyperlaunch: add memory parsing to domain config

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.04.2025 18:07, Alejandro Vallejo wrote:
> @@ -212,6 +213,39 @@ static int __init process_domain_node(
>              else
>                  printk("PV\n");
>          }
> +        else if ( strncmp(prop_name, "memory", name_len) == 0 )
> +        {
> +            uint64_t kb;
> +            if ( fdt_prop_as_u64(prop, &kb) != 0 )

Nit (you know what I have to say here, and again below.)

> +            {
> +                printk("  failed processing memory for domain %s\n", name);
> +                return -EINVAL;

Any reason to override fdt_prop_as_u64()'s return value here?

> +            }
> +            bd->mem_pages = PFN_DOWN(kb * SZ_1K);
> +            printk("  memory: %ld kb\n", kb);
> +        }
> +        else if ( strncmp(prop_name, "mem-min", name_len) == 0 )
> +        {
> +            uint64_t kb;
> +            if ( fdt_prop_as_u64(prop, &kb) != 0 )
> +            {
> +                printk("  failed processing memory for domain %s\n", name);
> +                return -EINVAL;
> +            }
> +            bd->min_pages = PFN_DOWN(kb * SZ_1K);
> +            printk("  min memory: %ld kb\n", kb);
> +        }
> +        else if ( strncmp(prop_name, "mem-max", name_len) == 0 )
> +        {
> +            uint64_t kb;
> +            if ( fdt_prop_as_u64(prop, &kb) != 0 )
> +            {
> +                printk("  failed processing memory for domain %s\n", name);

All three error messages being identical doesn't help diagnosing issues.

> --- a/xen/include/xen/libfdt/libfdt-xen.h
> +++ b/xen/include/xen/libfdt/libfdt-xen.h
> @@ -34,6 +34,16 @@ static inline int __init fdt_prop_as_u32(
>      return 0;
>  }
>  
> +static inline int __init fdt_prop_as_u64(
> +    const struct fdt_property *prop, uint64_t *val)
> +{
> +    if ( !prop || fdt32_to_cpu(prop->len) < sizeof(u64) )
> +        return -EINVAL;
> +
> +    *val = fdt_cell_as_u64((fdt32_t *)prop->data);

Please avoid casting away const. Looks like I overlooked this in
fdt_prop_as_u32() that was introduced by an earlier patch.

Jan