Re: [PATCH 1/3] xen/ppc: Enable Boot Allocator

[Julien Grall <julien@xxxxxxx>]

Hi,

On 01/12/2023 22:10, tpearson@xxxxxxxxxxxxxxxxxxxxx wrote:
> > (+ Arm and RISC-V folks)
> >
> > Hi Shawn,
> >
> > On 01/12/2023 20:59, Shawn Anastasio wrote:
> > > Adapt arm's earlyfdt parsing code to ppc64 and enable Xen's early boot
> > > allocator. Routines for parsing arm-specific devicetree nodes (e.g.
> > > multiboot) were excluded, reducing the overall footprint of code that
> > > was copied.
> > I expect RISC-V to want similar code. I am not really thrilled in the
> > idea of having 3 similar copy of the parsing. So can we extract the
> > common bits (or harmonize it) so it can be shared?
> >
> > Maybe Oleksii has already a version doing that.
> Just my $0.02, but wouldn't it make more sense to have the RISC-V port
> handle the deduplication, seeing as the POWER support came first here?  We
> don't know if/when the RISC-V port will be ready for submission, so I'm
> not sure why we should be on the hook for this particular work.
That would have been a valid point if you were writing a brand new 
implementation. But this was *copied* from Arm.
Looking at the diff between arm/bootfdt.c and ppc/bootfdt.c, you seem to 
have:
   - As well copied some code from arm/setup.c
   - Re-order some statement (not clear why)
   - Remove some code which you say are Arm specific. Yet some is part 
of the Device-Tree spec and I would expect to be used in the future.
So my question here stands. Why are you mainly copying verbatimly the 
Arm code rather than consolidating in one place?
Cheers,

--
Julien Grall