Re: [PATCH 1/2] docs, xen/arm: Introduce reserved heap memory

[Michal Orzel <michal.orzel@xxxxxxx>]

On 30/08/2022 08:29, Michal Orzel wrote:
> Hi Henry,
> 
> On 30/08/2022 02:58, Henry Wang wrote:
>>
>> Hi Stefano and Michal,
>>
>>> -----Original Message-----
>>> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>> Sent: Tuesday, August 30, 2022 8:47 AM
>>> To: Henry Wang <Henry.Wang@xxxxxxx>
>>> Cc: Michal Orzel <michal.orzel@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
>>> Stefano Stabellini <sstabellini@xxxxxxxxxx>; Julien Grall <julien@xxxxxxx>;
>>> Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; Wei Chen
>>> <Wei.Chen@xxxxxxx>; Volodymyr Babchuk
>>> <Volodymyr_Babchuk@xxxxxxxx>; Penny Zheng <Penny.Zheng@xxxxxxx>
>>> Subject: RE: [PATCH 1/2] docs, xen/arm: Introduce reserved heap memory
>>>
>>> On Thu, 25 Aug 2022, Henry Wang wrote:
>>>>>>                                         const char *name,
>>>>>>                                         u32 address_cells, u32 
>>>>>> size_cells)
>>>>>>  {
>>>>>> @@ -301,16 +303,40 @@ static void __init process_chosen_node(const
>>>>> void *fdt, int node,
>>>>>>      paddr_t start, end;
>>>>>>      int len;
>>>>>>
>>>>>> +    if ( fdt_get_property(fdt, node, "xen,static-mem", NULL) )
>>>>>> +    {
>>>>>> +        u32 address_cells = device_tree_get_u32(fdt, node,
>>>>>> +                                                
>>>>>> "#xen,static-mem-address-cells",
>>>>>> +                                                0);
>>>>>> +        u32 size_cells = device_tree_get_u32(fdt, node,
>>>>>> +                                             
>>>>>> "#xen,static-mem-size-cells", 0);
>>>>>> +        int rc;
>>>>>> +
>>>>>> +        printk("Checking for reserved heap in /chosen\n");
>>>>>> +        if ( address_cells < 1 || size_cells < 1 )
>>>>> address_cells and size_cells cannot be negative so you could just check if
>>>>> there are 0.
>>>>
>>>> In bootfdt.c function device_tree_get_meminfo(), the address and size cells
>>>> are checked using <1 instead of =0. I agree they cannot be negative, but I
>>> am
>>>> not very sure if there were other reasons to do the "<1" check in
>>>> device_tree_get_meminfo(). Are you fine with we don't keep the
>>> consistency
>>>> here?
>>>
>>> I would keep the < 1 check but it doesn't make much difference either
>>> way
>>
>> I also would prefer to keep these two places consistent and I agree Michal is
>> making a good point.
> I'm ok with that so let's keep the consistency.
Actually, why do we want to duplicate exactly the same check in 
process_chosen_node that is already
present in device_tree_get_meminfo? There is no need for that so just remove it 
from process_chosen_node.

> 
>>
>> Kind regards,
>> Henry
>>
> 
> ~Michal