On Sun, Feb 10, 2019 at 08:53:11PM +0100, Tristan Bastian wrote:
> Thierry, do you have any news on this?
>
> I don't think, that google is going to push an updated version of coreboot
> to each nyan-big device..
> So reverting this patch at least for the nyan devices would be the best I
> think..
Yes, I agree. I had a brief chat with Rob Herring about this and he too
agrees that we should revert it for now. I'll make it a manual revert
and add a comment to the device tree node that will hopefully avoid any
future janitorial "cleanups" of this sort.
> BTW: I'm now running u-boot natively and it seems like u-boot always has a
> problem with the memory..
> If u-boot is used chainloaded to coreboot it is only getting 2GB of memory
> and running u-boot natively also just gives me 2GB..
> I've tested that with a kernel with "ARM: tegra: Fix unit_address_vs_reg DTC
> warnings for /memory" reverted and also on a kernel, before this patch was
> applied..
It's possible that U-Boot doesn't support LPAE and therefore may not be
able to use more than the 2 GiB of memory. However, U-Boot should be
able to tell the kernel exactly how much memory the system has and pass
that on via device tree. That still does work, right?
Thierry
> Am 24.01.19 um 00:59 schrieb Tristan Bastian:
> >
> > Am 23.01.19 um 14:54 schrieb Thierry Reding:
> > > On Wed, Jan 23, 2019 at 02:51:56PM +0100, Thierry Reding wrote:
> > > > On Wed, Jan 23, 2019 at 12:19:13PM +0000, Tristan Bastian wrote:
> > > > > Thierry Reding – Wed, 23. January 2019 13:04
> > > > > > On Tue, Jan 22, 2019 at 07:34:39PM +0000, Tristan Bastian wrote:
> > > > > > >
> > > > > > >
> > > > > > > Thierry Reding – Tue, 22. January 2019 17:13
> > > > > > > > On Tue, Jan 22, 2019 at 03:23:51PM +0000, Tristan Bastian wrote:
> > > > > > > > > Hello,
> > > > > > > > >
> > > > > > > > > since mainline kernel 4.19 LPAE is no longer
> > > > > > > > > working for me and some
> > > > > > others
> > > > > > > > on tegra124-nyan-big.
> > > > > > > > > Is this a known problem with a fix already available?
> > > > > > > > >
> > > > > > > > > The defconfig to compile the kernel can be found here:
> > > > > > > > github.com/reey/PKGBUILDs/blob/master/core/linux-nyan/nyan-big_defconfig
> > > > > > > >
> > > > > > > > > Basically we are only getting 2 GB instead of 4 GB of memory.
> > > > > > > > > Kernel 4.18 still gives us 4 GB.
> > > > > > > > > Both kernels are configured with CONFIG_ARM_LPAE=y.
> > > > > > > > > All newer kernel versions also have this bug..
> > > > > > > > Looking at the git log, I can only find one
> > > > > > > > possibly suspicious change:
> > > > > > > >
> > > > > > > > commit 482997699ef038af7553399d49b7ba74c3301424
> > > > > > > > Author: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> > > > > > > > Date: Mon Jul 9 18:05:17 2018 +0200
> > > > > > > >
> > > > > > > > ARM: tegra: Fix unit_address_vs_reg DTC warnings for /memory
> > > > > > > >
> > > > > > > > Add a generic /memory node in each Tegra DTSI
> > > > > > > > (with empty reg property,
> > > > > > > > to be overidden by each DTS) and set proper unit
> > > > > > > > address for /memory
> > > > > > > > nodes to fix the DTC warnings:
> > > > > > > >
> > > > > > > > arch/arm/boot/dts/tegra20-harmony.dtb: Warning
> > > > > > > > (unit_address_vs_reg):
> > > > > > > > /memory: node has a reg or ranges property, but no unit name
> > > > > > > >
> > > > > > > > The DTB after the change is the same as before except adding
> > > > > > > > unit-address to /memory node.
> > > > > > > >
> > > > > > > > Signed-off-by: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> > > > > > > > Reviewed-by: Stefan Agner <stefan@xxxxxxxx>
> > > > > > > > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> > > > > > > >
> > > > > > > > I suppose this could result in the bootloader
> > > > > > > > failing to find the memory
> > > > > > > > node when trying to patch up the memory banks. It looks like U-Boot
> > > > > > > > handles this properly by ignoring the
> > > > > > > > unit-address when looking up nodes
> > > > > > > > by name (without unit-address), but I suspect
> > > > > > > > that coreboot may not be
> > > > > > > > doing this. I'm assuming that coreboot is what you're using as
> > > > > > > > bootloader?
> > > > > > > >
> > > > > > > > If coreboot behaves similarly to U-Boot, it'll
> > > > > > > > create a new node if it
> > > > > > > > can't find a matching one, so you may be able to
> > > > > > > > inspect the device tree
> > > > > > > > that was passed to the kernel (/sys/firmware/fdt
> > > > > > > > should have the binary
> > > > > > > > and /sys/firmware/devicetree should have a
> > > > > > > > directory tree representation
> > > > > > > > of the device tree).
> > > > > > > >
> > > > > > > > Thierry
> > > > > > > Hi Thierry,
> > > > > > >
> > > > > > > I'm using coreboot.
> > > > > > > But what I can also tell is, that when you use U-Boot chainloaded to
> > > > > > coreboot, LPAE is also not working with older kernel
> > > > > > revisions like 4.18.
> > > > > > > I only found a single memory entry in /sys/firmware/devicetree/
> > > > > > "memory@80000000"...
> > > > > > > BTW: @Thierry, I'm reverting this patch:
> > > > > > > patchwork.kernel.org/patch/9516105/
> > > > > > because otherwise I'm getting a blank screen on bootup.
> > > > > > Is there any fix for
> > > > > > that going upstream?
> > > > > >
> > > > > > Yes, there was some work to get that fixed, but it keeps
> > > > > > getting stuck.
> > > > > > I'll resume the old threads to see if we can gain some
> > > > > > traction. Can I
> > > > > > add you on Cc for testing any patch related to that?
> > > > > >
> > > > > > Thierry
> > > > > Sure, just add me.
> > > > > My email client somehow removed the subject of these emails..
> > > > > I've sent another mail with the original subject, where I wrote:
> > > > >
> > > > > > I have to correct myself.
> > > > > > I found two memory entries in /sys/firmware/devicetree/base/.
> > > > > > One called "memory@80000000" and the other one just "memory".
> > > > > Just making sure, that you've got that.
> > > > It looks as if your email client also breaks threading. I see these
> > > > emails from you appear randomly in my mailbox rather than threaded with
> > > > the earlier messages.
> > > >
> > > > > Should I try a kernel, which reverted "ARM: tegra: Fix
> > > > > unit_address_vs_reg DTC warnings for /memory" to make sure that this
> > > > > is causing the issue?
> > > > I replied to that email separately and suggested pretty much that. I'm
> > > > fairly confident that that will restore > 2 GiB for you, so we
> > > > just need
> > > > to figure out what to do about it. Just reverting the patch will
> > > > reintroduce the warning that it silenced.
> > > Just in case you don't have that email, here's the full reply:
> > >
> > > --- >8 ---
> > > Okay, that could explain why you're only seeing 2 GiB of memory. I
> > > suspect that the kernel will look at memory@80000000 which is only
> > > what's prepopulated in the upstream device tree and coreboot will
> > > update the memory node, so the kernel won't see the real values.
> > >
> > > I suspect that the issue with U-Boot chainloaded also being present
> > > might have to do with the fact that U-Boot checks for the substring
> > > at the beginning of a node name. So it looks up the "memory" node,
> > > which can match "memory@80000000", but in this case will likely
> > > only match that other "memory" node.
> > >
> > > A simple test would be to change the kernel DTS file and remove the
> > > unit-address from the memory node again using something like this:
> > >
> > > - memory@80000000 {
> > > + memory {
> > >
> > > That should restore functionality for you. If so, we'll have to
> > > figure out for a real fix. The problem with removing the unit-address
> > > is that it will reinstate the DTC warnings that the above commit was
> > > fixing. On the other hand the above technically broke DT ABI, so one
> > > solution would be to just revert it and add an exception to DT to
> > > prevent it from issuing the warning for the memory nodes.
> > >
> > > The better alternative, in my opinion, would be to modify coreboot to
> > > not look up the memory node by an exact string match, but to also allow
> > > an optional unit-address. Perhaps an even better way to look up the node
> > > would be by looking for a node with the device_type property set to
> > > "memory".
> > >
> > > Do you have the source code available so that coreboot could be fixed?
> > > If not, we'll probably have to resort to the first solution.
> > >
> > > Thierry
> >
> > So I had to remove all changes made by "ARM: tegra: Fix
> > unit_address_vs_reg DTC warnings for /memory" to the tegra124-nyan.dtsi
> > and also to the tegra124.dtsi.
> > Now LPAE is working again, thanks for that.
> > I don't think, that fixing this in coreboot would be great.
> > If we would do so, every user would be forced to replace the ChromeOS
> > coreboot with a self build one to get all 4GB of memory..
> >
> > Or would those changes also be merged into the official ChromeOS builds?
> >
> > The coreboot code should be this: https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/firmware-nyan-5771.B
> >
> > Thanks,
> >
> > Tristan
> >
Attachment:
signature.asc
Description: PGP signature
