On Thu Apr 10, 2025 at 1:03 PM BST, Jan Beulich wrote:
> 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.)
Ack
>
>> + {
>> + printk(" failed processing memory for domain %s\n", name);
>> + return -EINVAL;
>
> Any reason to override fdt_prop_as_u64()'s return value here?
Mostly to avoid needing to recover the error code. I'll just do it.
>
>> + }
>> + 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.
Indeed. Will add the prop that trigger each.
>
>> --- 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.
As part of v4 I moved this and fdt_prop_as_u32() earlier to patch8 and
already adjusted accordingly.
Cheers,
Alejandro
