[U-Boot] [PATCH] boston: Ensure DDR address calcuations don't overflow

[Daniel Schwierzeck]

On 22.01.2018 19:54, Daniel Schwierzeck wrote:
> 
> 
> On 22.01.2018 19:01, Paul Burton wrote:
>> Hi Daniel,
>>
>> On Fri, Jan 19, 2018 at 12:31:25PM +0100, Daniel Schwierzeck wrote:
>>> On 18.01.2018 22:19, Paul Burton wrote:
>>>> When constraining the highest DDR address that U-Boot will use for its
>>>> data & relocated self, we need to handle the common case in which a 32
>>>> bit system with 2GB DDR will have a zero gd->ram_top, due to the
>>>> addition of 2GB (0x80000000) to the base address of kseg0 (also
>>>> 0x80000000) which overflows & wraps to 0.
>>>>
>>>> We originally had a check for this case, but it was lost in commit
>>>> 78edb25729ce ("boston: Provide physical CONFIG_SYS_SDRAM_BASE") causing
>>>> problems for the affected 32 bit systems.
>>>
>>> I think I did a wrong conflict resolution because the patch didn't apply
>>> anymore. I folded this patch into "boston: Provide physical
>>> CONFIG_SYS_SDRAM_BASE" to fix this. Actually I wanted to resend the
>>> updated patches. But if you are okay with the current state in
>>> u-boot-mips/next branch, I'll take them as they are.
>>>
>>> BTW: could you resend your series "boston: Ethernet support for MIPS
>>> Boston board"? I still have no Acks or Reviews on the generic DM parts.
>>> Thanks.
>>
>> When I last fetched from u-boot-mips.git I saw patches up to
>> 564cc3a11c45 ("mips: Remove virt_to_phys call on bi_memstart") in the
>> next branch, which I have then rebased my ethernet patches atop with the
>> result working fine on a real Boston board.
>>
>> I see that next now contains only 2 patches up to d2a4e3664150 ("mips:
>> bmips: increment SYS_MALLOC_F_LEN") and has dropped the patches
>> switching to a physical CONFIG_SYS_SDRAM_BASE. Would you like me to
>> rebase those plus the Boston ethernet support atop the current next
>> branch?
>>
> 
> I had to remove the patches because there is a failing test case in qemu
> pytest [1] which needs to be fixed. The test case fetches the RAM base
> address from the "bdinfo" output which is 0x0 due to
> CONFIG_SYS_SDRAM_BASE = 0. I'm not sure if we need to add a phys_to_virt
> mapping to the "md" command or if "bdinfo" should show the virtual
> address. What do you think? Actually other tools like "cp" are also
> affected.
> 

I think we need to implement "include/mapmem.h" for MIPS. But this won't work with the current phys_to_virt() implementation because there a lot of places using map_sysmem() and therefore expecting a physical address.

One hack would be:

diff --git a/arch/mips/config.mk b/arch/mips/config.mk
index cefdbe65e1..82fcd55f8c 100644
--- a/arch/mips/config.mk
+++ b/arch/mips/config.mk
@@ -39,6 +39,9 @@ PLATFORM_CPPFLAGS += -D__MIPS__
 PLATFORM_ELFENTRY = "__start"
 PLATFORM_ELFFLAGS += -B mips $(OBJCOPYFLAGS)
 
+# Force this until converted to a Kconfig symbol
+PLATFORM_CPPFLAGS += -DCONFIG_ARCH_MAP_SYSMEM
+
 #
 # From Linux arch/mips/Makefile
 #
diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
index 45d7ca0cc6..9173beda0b 100644
--- a/arch/mips/include/asm/io.h
+++ b/arch/mips/include/asm/io.h
@@ -559,6 +559,20 @@ BUILD_CLRSETBITS(q, le64, le64, u64)
 BUILD_CLRSETBITS(q, be64, be64, u64)
 BUILD_CLRSETBITS(q, 64, _, u64)
 
+static inline void *map_sysmem(phys_addr_t paddr, unsigned long len)
+{
+       return (void *)CKSEG0ADDR(paddr);
+}
+
+static inline void unmap_sysmem(const void *vaddr)
+{
+}
+
+static inline phys_addr_t map_to_sysmem(const void *ptr)
+{
+       return CKSEG0ADDR((uintptr_t)ptr);
+}
+
 #include <asm-generic/io.h>
 
 #endif /* _ASM_IO_H */


Test with qemu_mips:

qemu-mips # bdinfo 
boot_params = 0x87F488E8
memstart    = 0x00000000
memsize     = 0x08000000
flashstart  = 0xBFC00000
flashsize   = 0x00400000
flashoffset = 0x00030508
ethaddr     = 52:54:00:12:34:56
IP addr     = <NULL>
baudrate    = 115200 bps
relocaddr   = 0x87F90000
reloc off   = 0xC8390000
qemu-mips # md 0
00000000: 00000000 00000000 00000000 00000000    ................
00000010: 00000000 00000000 00000000 00000000    ................
00000020: 00000000 00000000 00000000 00000000    ................


There is another issue. If "struct bd_info" is supposed to hold physical addresses, 
we would need to fix bi_flashstart and CONFIG_SYS_FLASH_BASE too ;)

-- 
- Daniel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180122/6d7e719c/attachment.sig>