| Message ID | 1406835607-14879-1-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
On 07/31/2014 01:40 PM, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > If a 32-bit system has 2GB of RAM, and the base address of that RAM is > 2GB, then start+size will overflow a 32-bit value (to a value of 0). > > __lmb_alloc_base is affected by this; it calculates the minimum of > (start+size of RAM) and max_addr. However, when start+size is 0, it > is always less than max_addr, which causes the value of max_addr not > to be taken into account when restricting the allocation's location. > > Fix this by calculating start+size separately, and if that calculation > underflows, using -1 (interpreted as the max unsigned value) as the > value instead, and then taking the min of that and max_addr. Now that > start+size doesn't overflow, it's typically large, and max_addr > dominates the min() call, and is taken into account. > > The user-visible symptom of this bug is that CONFIG_BOOTMAP_SZ is ignored > on Tegra124 systems with 2GB of RAM, which in turn causes the DT to be > relocated at the very end of RAM, which the ARM Linux kernel doesn't map > during early boot, and which causes boot failures. With this fix, > CONFIG_BOOTMAP_SZ correctly restricts the relocated DT to a much lower > address, and everything works. TomR, Does this patch look OK to you? I imagine that TomW is holding off applying "ARM: tegra: Use mem size from MC rather than ODMDATA" since it depends on this patch, which isn't applied yet. Thanks.
On Thu, Jul 31, 2014 at 01:40:07PM -0600, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > If a 32-bit system has 2GB of RAM, and the base address of that RAM is > 2GB, then start+size will overflow a 32-bit value (to a value of 0). > > __lmb_alloc_base is affected by this; it calculates the minimum of > (start+size of RAM) and max_addr. However, when start+size is 0, it > is always less than max_addr, which causes the value of max_addr not > to be taken into account when restricting the allocation's location. > > Fix this by calculating start+size separately, and if that calculation > underflows, using -1 (interpreted as the max unsigned value) as the > value instead, and then taking the min of that and max_addr. Now that > start+size doesn't overflow, it's typically large, and max_addr > dominates the min() call, and is taken into account. > > The user-visible symptom of this bug is that CONFIG_BOOTMAP_SZ is ignored > on Tegra124 systems with 2GB of RAM, which in turn causes the DT to be > relocated at the very end of RAM, which the ARM Linux kernel doesn't map > during early boot, and which causes boot failures. With this fix, > CONFIG_BOOTMAP_SZ correctly restricts the relocated DT to a much lower > address, and everything works. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> Applied to u-boot/master, thanks!
diff --git a/lib/lmb.c b/lib/lmb.c index 49a3c9e01e59..41a2be463565 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -295,7 +295,10 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align, phy if (max_addr == LMB_ALLOC_ANYWHERE) base = lmb_align_down(lmbbase + lmbsize - size, align); else if (lmbbase < max_addr) { - base = min(lmbbase + lmbsize, max_addr); + base = lmbbase + lmbsize; + if (base < lmbbase) + base = -1; + base = min(base, max_addr); base = lmb_align_down(base - size, align); } else continue;