RE: [PATCH] loader/i386/linux: Add device tree support

[Mislav Stublić]

Hi,

There is a lot of duplication as I have used existing implementation as a 
reference (arm, aarch64 and fdt). But refactoring existing implementation atop 
of implementing x86 didn’t seem realistic as those modules are quite 
different... at least when I first looked at this. My biggest question is how 
to unify existing memory allocation procedures as those are different, at least 
 in a sense x86 has relocator and the setup around it (or can/should this 
unification be avoided). Also, it seems there is a lot of divergence in UEFI 
implementations for which I'm not sure it can be avoided without bigger 
refactor of both UEFI and x86 code. But I would need to think some more on your 
suggestion, maybe it's not so complicated.

Mislav

> -----Original Message-----
> From: Daniel Kiper [mailto:dkiper@net-space.pl]
> Sent: Wednesday, October 06, 2021 2:29 PM
> To: Mislav Stublić
> Cc: grub-devel@gnu.org
> Subject: Re: [PATCH] loader/i386/linux: Add device tree support
>
> Hi Mislav,
>
> On Tue, Sep 21, 2021 at 03:37:45PM +0000, Mislav Stublić wrote:
> > Hi,
> >
> > This implements device tree support for x86. Unfortunately I haven't
> > been able to test this on latest master but I have tested against 2.04
> > and it works fine. I will test on master later but I wanted to get
> > some initial comments if this is going in the right direction. What I
> > haven't tested is firmware DTB loading support which only calls
> > grub_efi_get_firmware_fdt from kern/efi implementation as I don't have
> > hardware to test this scenario.
>
> I skimmed through your patch and it seems to me you are duplicating at
> least some code from grub-core/loader/arm/linux.c. I would do this a bit
> differently. First of all I would factor out non-UEFI device tree code
> from grub-core/loader/arm/linux.c to a separate module. This should be
> a separate patch. Then I would try to unify the interface for UEFI and
> non-UEFI device tree code (again, separate patch). Of course if needed.
> Then the last patch should add device tree support to the x86. If you
> have any questions drop me a line.
>
> Anyway, thank you for taking a stab at this.
>
> Daniel


----- Disclaimer -----
This e-mail message and its attachments may contain privileged and/or 
confidential information. Please do not read the message if You are not its 
designated recipient. If You have received this message by mistake, please 
inform its sender and destroy the original message and its attachments without 
reading or storing of any kind. Any unauthorized use, distribution, 
reproduction or publication of this message is forbidden.

----- Pravne napomene -----
Ova elektronicka poruka i njeni prilozi mogu sadrzavati povlastene informacije 
i/ili povjerljive informacije. Molimo Vas da poruku ne citate ako niste njen 
naznaceni primatelj. Ako ste ovu poruku primili greskom, molimo Vas da o tome 
obavijestite posiljatelja i da izvornu poruku i njene privitke unistite bez 
citanja ili bilo kakvog pohranjivanja. Svaka neovlastena upotreba, 
distribucija, reprodukcija ili priopcavanje ove poruke zabranjena je.