Re: [XEN v2 02/11] xen/arm: Use the correct format specifier

[Julien Grall <julien@xxxxxxx>]

On 20/01/2023 14:40, Michal Orzel wrote:
> Hello,
Hi,

> On 20/01/2023 10:32, Julien Grall wrote:
> > Hi,
> >
> > On 19/01/2023 22:54, Stefano Stabellini wrote:
> > > On Tue, 17 Jan 2023, Ayan Kumar Halder wrote:
> > > > 1. One should use 'PRIpaddr' to display 'paddr_t' variables.
> > > > 2. One should use 'PRIx64' to display 'u64' in hex format. The current
> > > > use of 'PRIpaddr' for printing PTE is buggy as this is not a physical
> > > > address.
> > > >
> > > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> > > Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> >
> > I have committed the patch.
> The CI test jobs (static-mem) failed on this patch:
> https://gitlab.com/xen-project/xen/-/pipelines/752911309
Thanks for the report.

> I took a look at it and this is because in the test script we
> try to find a node whose unit-address does not have leading zeroes.
> However, after this patch, we switched to PRIpaddr which is defined as 
> 016lx/016llx and
> we end up creating nodes like:
>
> memory@0000000050000000
>
> instead of:
>
> memory@60000000
>
> We could modify the script, 
TBH, I think it was a mistake for the script to rely on how Xen describe 
the memory banks in the Device-Tree.
For instance, from my understanding, it would be valid for Xen to create 
a single node for all the banks or even omitting the unit-address if 
there is only one bank.
> but do we really want to create nodes
> with leading zeroes? The dt spec does not mention it, although [1]
> specifies that the Linux convention is not to have leading zeroes.
Reading through the spec in [2], it suggested the current naming is 
fine. That said the example match the Linux convention (I guess that's 
not surprising...).
I am open to remove the leading. However, I think the CI also needs to 
be updated (see above why).
Cheers,

[2] https://www.devicetree.org/

--
Julien Grall