From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tony Lindgren Subject: Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry Date: Mon, 13 Jul 2015 06:19:02 -0700 Message-ID: <20150713131902.GH26485@atomide.com> References: <1436214373-12969-1-git-send-email-pali.rohar@gmail.com> <1436214373-12969-6-git-send-email-pali.rohar@gmail.com> <20150707113213.GC7576@n2100.arm.linux.org.uk> <20150707115819.GF12087@pali> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <20150707115819.GF12087@pali> Sender: linux-kernel-owner@vger.kernel.org To: Pali =?utf-8?B?Um9ow6Fy?= Cc: Russell King - ARM Linux , Laura Abbott , Grant Likely , Rob Herring , Will Deacon , Ivaylo Dimitrov , Sebastian Reichel , Pavel Machek , Andreas =?utf-8?Q?F=C3=A4rber?= , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org List-Id: linux-omap@vger.kernel.org * Pali Roh=C3=A1r [150707 05:00]: > On Tuesday 07 July 2015 12:32:13 Russell King - ARM Linux wrote: > > On Mon, Jul 06, 2015 at 10:26:13PM +0200, Pali Roh=C3=A1r wrote: > > > Legacy bootloaders can pass additional information for kernel or = legacy > > > userspace applications. When booting DT kernel then ATAGs structu= re is not > > > more visible after running kernel uncompress code. This patch sto= res full > > > ATAGs structure into DT "/chosen/linux,atags" entry, so kernel ca= n later > > > reuse it and export via /proc/atags to userspace. > >=20 > > I think you need to go through your commit messages and improve the= m, > > especially the ones with "TODO" in them. As long as there's still = things > > to be done, they're obviously not ready for merging. > >=20 >=20 > I know, in cover letter email I wrote that documentation is not ready= =2E.. > I send patches for review and comments (like yours). I think it is st= ill > better to send something and mark it as incomplete. It could prevent = to > work on something which will be again rewritten... >=20 > > Moreover, exporting the ATAGS is questionable, even _if_ there are = non- > > kexec programs making use of this. The ATAGs have _never_ been exp= orted > > to userspace when kexec disabled is the kernel - it was introduced = for > > kexec, and has always had this: > >=20 > > config ATAGS_PROC > > bool "Export atags in procfs" > > depends on ATAGS && KEXEC > > default y > >=20 > > Now, the fact that someone decided to start using it is pretty sad, > > because it means that if you disable KEXEC, userspace breaks. That= 's > > not a kernel regression in any shape or form, because /proc/atags h= as > > never been there without KEXEC enabled. That's a userspace bug, pl= ain > > and simple. > >=20 > > Given that, I'm in two minds about whether to accept the last two > > patches which make this more than just "for KEXEC use to enable a K= EXEC > > kernel to be booted." > >=20 > > Had it been provided without the KEXEC conditional, then I don't ha= ve > > a problem with these two patches. > >=20 >=20 > I understand it. Nokia originally invented their own entries in /proc= / > which export needed ATAGs from kernel in human-readable form, but all > those entries were non-standard and specific for Nokia's kernels. >=20 > Do you have some other idea how to provide ATAGs information created > dynamically by legacy closed proprietary bootloader to userspace from= DT > booted kernel? >=20 > Anyway, for supporting kexec (with passing ATAGs) it is needed to hav= e > working /proc/atags file, right? Yeah I think that since we already have it in /proc, we should just support it. And keep it behind CONFIG_KEXEC and CONFIG_ARM_APPENDED_DTB and hope we don't find other users for it.. Then reconsider the Kconfig dependencies if we do find other users. =20 > > It also sets a precedent: by adding this into DT, it is creating a = new > > DT ABI as well, and we'll end up seeing dts files with an ATAG bloc= k > > patched into them. > >=20 > > Are the ATAGs at a fixed address on the N900? >=20 > Yes, in board-rx51.c is: >=20 > .atag_offset =3D 0x100 >=20 > and Nokia Bootloader (proprietary) store them to that address. > > > Can that be handled in > > some kind of legacy file for the N900 which calls save_atags() on i= t, so > > we don't end up introducing yet more stuff that we have to maintain= into > > the distant future? If not, what about copying a known working ata= g > > structure into a legacy file for the N900? > > I already asked question if it is possible to read ATAGs from DT boot= ed > kernel. And somebody (do not remember who) wrote to ML, that it is no= t > possible and it can be done in that uncompress code. I guess the other option would be to keep the raw ATAG area reserved, and only initialize /proc/atags from a board specific initcall. But I think that would complicate the already fragile uncompress relocation code even further? Regards, Tony