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

[Timothy Pearson <tpearson@xxxxxxxxxxxxxxxxxxxxx>]

----- Original Message -----
> From: "Julien Grall" <julien@xxxxxxx>
> To: "Timothy Pearson" <tpearson@xxxxxxxxxxxxxxxxxxxxx>
> Cc: "Shawn Anastasio" <sanastasio@xxxxxxxxxxxxxxxxxxxxx>, "xen-devel" 
> <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "Jan Beulich"
> <jbeulich@xxxxxxxx>, "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>, 
> "Stefano Stabellini" <sstabellini@xxxxxxxxxx>,
> "Bertrand Marquis" <bertrand.marquis@xxxxxxx>, "Michal Orzel" 
> <michal.orzel@xxxxxxx>, "Oleksii"
> <oleksii.kurochko@xxxxxxxxx>
> Sent: Friday, December 1, 2023 4:35:55 PM
> Subject: Re: [PATCH 1/3] xen/ppc: Enable Boot Allocator

> 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?

That's fair, with the future RISC-V port removed from the discussion and good 
reasons still being put forth it makes more sense to deduplicate now.  Thank 
you for clarifying the objection! :)