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:dki...@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. _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
