[U-Boot] [RFC] ARM: U-boot and 2 GiB of ram with get_ram_size only being long
Hey all,
I just yesterday received my CubieTruck (cubieboard3) with 2 GiB of Ram and added support for it to the sunxi-u-boot branch. While I know this isn't merged into the main u-boot tree (yet), I ran into the following problem.
At the end of the dram init code, it is customary to call get_ram_size() and return its value. This is then used to print the DRAM size and also is passed to the Linux kernel.
However the return size of get_ram_size() is a long. While I don't understand why not unsigned long or even u64 was chosen, this causes get_dram_size to overflow when having a ramsize of 2 GiB. While only printing of the value isn't hugely important, this does indicate u-boot seems to be somewhat artificially limited to 2 GiB of Ram? This only seems to affect the SPL as, if I understood correctly, there it stores the ram_size into the gd struct which I think is unsigned long.
I've started working on a patch to convert common/memsize.c's get_ram_size(), to be completely unsigned long, however there seems to be quite a lot of code that calls this. So my question is now before going over all drivers and change that and submit a big patch-set, did I overlook anything and are my conclusions correct, get_ram_size should return unsigned long.
Finally, a long is 32 bits on x86 and armv7, but how will that relate to 64bits armv8? As I understood, Windows treats long's as 4 bytes no matter if it's 32 bit or 64 bit. Linux is better and a long is 4 bytes on 32 bits, and 8 bytes on 64 bits versions. So how will u-boot work on armv8? Will the long datatype work well, or should I consider changing things more future proof? (u32 and u64 come to mind).
Thank you for any input regarding that issue.
Oliver
Hey all,
Having not received any feed back at all, I went ahead and did the changes anyway. Everything seems to run and work fine for sunxi and prints proper sizes.
For the other boards, I tried to run a MAKEALL but there where so many random other warnings I can't say for 100% certainty there where no mistakes that crept in. Boards where strange size conversion where done or it was assumed that the return value of get_ram_size() was long are typecast to have the same (erroneous) behavior. Boards that had simple fixes where fixed.
Ideally, every maintainer should check if their board requires deeper review and fixing due to this change.
Oliver
On 03-10-13 23:15, Oliver Schinagl wrote:
Hey all,
I just yesterday received my CubieTruck (cubieboard3) with 2 GiB of Ram and added support for it to the sunxi-u-boot branch. While I know this isn't merged into the main u-boot tree (yet), I ran into the following problem.
At the end of the dram init code, it is customary to call get_ram_size() and return its value. This is then used to print the DRAM size and also is passed to the Linux kernel.
However the return size of get_ram_size() is a long. While I don't understand why not unsigned long or even u64 was chosen, this causes get_dram_size to overflow when having a ramsize of 2 GiB. While only printing of the value isn't hugely important, this does indicate u-boot seems to be somewhat artificially limited to 2 GiB of Ram? This only seems to affect the SPL as, if I understood correctly, there it stores the ram_size into the gd struct which I think is unsigned long.
I've started working on a patch to convert common/memsize.c's get_ram_size(), to be completely unsigned long, however there seems to be quite a lot of code that calls this. So my question is now before going over all drivers and change that and submit a big patch-set, did I overlook anything and are my conclusions correct, get_ram_size should return unsigned long.
Finally, a long is 32 bits on x86 and armv7, but how will that relate to 64bits armv8? As I understood, Windows treats long's as 4 bytes no matter if it's 32 bit or 64 bit. Linux is better and a long is 4 bytes on 32 bits, and 8 bytes on 64 bits versions. So how will u-boot work on armv8? Will the long datatype work well, or should I consider changing things more future proof? (u32 and u64 come to mind).
Thank you for any input regarding that issue.
Oliver _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Oliver,
On Mon, 07 Oct 2013 04:41:31 +0200, Oliver Schinagl oliver+list@schinagl.nl wrote:
Hey all,
Having not received any feed back at all, I went ahead and did the changes anyway. Everything seems to run and work fine for sunxi and prints proper sizes.
For the other boards, I tried to run a MAKEALL but there where so many random other warnings I can't say for 100% certainty there where no mistakes that crept in.
There cannot possibly be a single warning if you're working from an official U-Boot repo, as warnings are considered failures and thus no patch reaches u-boot/master if it causes a warning. Thus, any warning stems from the change you're introducing, and they are probably due to a few common causes.
Can you provide a rough analysis/classification of the warnings?
Amicalement,
On Tue, 2013-10-15 at 09:12 +0200, Albert ARIBAUD wrote:
Hi Oliver,
On Mon, 07 Oct 2013 04:41:31 +0200, Oliver Schinagl oliver+list@schinagl.nl wrote:
Hey all,
Having not received any feed back at all, I went ahead and did the changes anyway. Everything seems to run and work fine for sunxi and prints proper sizes.
For the other boards, I tried to run a MAKEALL but there where so many random other warnings I can't say for 100% certainty there where no mistakes that crept in.
There cannot possibly be a single warning if you're working from an official U-Boot repo, as warnings are considered failures and thus no patch reaches u-boot/master if it causes a warning.
That might be the theory, but in practice this is simply false. Different toolchains produce different warnings, and not all patches always get test-built on every target (especially on obscure architectures). And since it's false that no warnings exist, that means sometimes even when a patch is test-built, some newly introduced warnings get missed (I got an e-mail pointing out such an occurance just today).
-Scott
Hi Scott,
On Tue, 15 Oct 2013 12:57:33 -0500, Scott Wood scottwood@freescale.com wrote:
On Tue, 2013-10-15 at 09:12 +0200, Albert ARIBAUD wrote:
Hi Oliver,
On Mon, 07 Oct 2013 04:41:31 +0200, Oliver Schinagl oliver+list@schinagl.nl wrote:
Hey all,
Having not received any feed back at all, I went ahead and did the changes anyway. Everything seems to run and work fine for sunxi and prints proper sizes.
For the other boards, I tried to run a MAKEALL but there where so many random other warnings I can't say for 100% certainty there where no mistakes that crept in.
There cannot possibly be a single warning if you're working from an official U-Boot repo, as warnings are considered failures and thus no patch reaches u-boot/master if it causes a warning.
That might be the theory, but in practice this is simply false. Different toolchains produce different warnings, and not all patches always get test-built on every target (especially on obscure architectures). And since it's false that no warnings exist, that means sometimes even when a patch is test-built, some newly introduced warnings get missed (I got an e-mail pointing out such an occurance just today).
You are correct that the same code may or may not emit warnings depending on the toolchain, and that U-Boot's build system won't stop building because of warnings.
However, when a new toolchain version causes such warnings, but they are not 'random' in any case; they may be numerous though, if in some source code used in a lot of boards.
In any case, if Oliver gets warnings, chances are we'll get them to when applying his code, in which case it'll be rejected, or we'll see them happening later if he's unsing a common toolchain in a new version, or he's using an unusual toolchain.
-Scott
Amicalement,
On 17-10-13 08:27, Albert ARIBAUD wrote:
Hi Scott,
On Tue, 15 Oct 2013 12:57:33 -0500, Scott Wood scottwood@freescale.com wrote:
On Tue, 2013-10-15 at 09:12 +0200, Albert ARIBAUD wrote:
Hi Oliver,
On Mon, 07 Oct 2013 04:41:31 +0200, Oliver Schinagl oliver+list@schinagl.nl wrote:
Hey all,
Having not received any feed back at all, I went ahead and did the changes anyway. Everything seems to run and work fine for sunxi and prints proper sizes.
For the other boards, I tried to run a MAKEALL but there where so many random other warnings I can't say for 100% certainty there where no mistakes that crept in.
There cannot possibly be a single warning if you're working from an official U-Boot repo, as warnings are considered failures and thus no patch reaches u-boot/master if it causes a warning.
That might be the theory, but in practice this is simply false. Different toolchains produce different warnings, and not all patches always get test-built on every target (especially on obscure architectures). And since it's false that no warnings exist, that means sometimes even when a patch is test-built, some newly introduced warnings get missed (I got an e-mail pointing out such an occurance just today).
You are correct that the same code may or may not emit warnings depending on the toolchain, and that U-Boot's build system won't stop building because of warnings.
However, when a new toolchain version causes such warnings, but they are not 'random' in any case; they may be numerous though, if in some source code used in a lot of boards.
In any case, if Oliver gets warnings, chances are we'll get them to when applying his code, in which case it'll be rejected, or we'll see them happening later if he's unsing a common toolchain in a new version, or he's using an unusual toolchain.
I wasn't getting warnings or errors even remotely related to my patches and while it's a pitty we are discussing peanuts without even looking at the patch, cest la vie.
A few of the warnings are from my toolchain (missing bits) a few others from random stuff, here a few one liners to give an indication where boards break:
mxc_gpio.c:105:9: error: dereferencing pointer to incomplete type
at91rm9200_devices.c:64:20: error: ‘AT91_PIO_PORTA’ undeclared (first use in this function)
mini2440.c:70:24: error: ‘GPH8’ undeclared (first use in this function
/silo/build/sunxi-bsp/u-boot-sunxi/include/config.h:7:0: warning: "CONFIG_SYS_SOC" redefined [enabled by default] /silo/build/sunxi-bsp/build/u-boot-all/include/config.h:9:0: note: this is the location of the previous definition /silo/build/sunxi-bsp/u-boot-sunxi/include/config.h:8:0: warning: "CONFIG_BOARDDIR" redefined [enabled by default] / tons of errors on this one for the atmel at91sam configs
at91sam9260_devices.c:34:20: error: ‘AT91_PIO_PORTB’ undeclared (first use in this function)
da8xx_gpio.c:388:1: error: dereferencing pointer to incomplete type
dm355leopard.c:35:2: error: ‘DAVINCI_GPIO_BINTEN’ undeclared (first use in this function) dm355leopard.c:38:2: error: dereferencing pointer to incomplete type
I must admit however, quite a few boards built cleanly, so I may have overstated things?
I'm building using gcc-4.6.3 on gentoo (with gcc build natively via cross-dev) The command I used was: CROSS_COMPILE=arm-pc-linux-gnueabi- BUILD_DIR=/silo/build/sunxi-bsp/build/u-build-all ./MAKEALL -a arm
So while a few of these errors might be long fixed, we merge the u-boot patches on a monthly or so basis, I can't imagine all these errors being from the wrong toolchain?
So now that that's settled, anything fundamentally wrong with my patch? :)
oliver
-Scott
Amicalement,
On Fri, 2013-10-18 at 02:04 +0200, Oliver Schinagl wrote:
So now that that's settled, anything fundamentally wrong with my patch? :)
Did you see my other mail in this thread? This patch is sort of OK for raising the get_ram_size() limit from 1 GiB to 2 GiB (with an increased risk of false positives from I/O), but it can't go beyond that on 32-bit. A better approach would be to get the RAM size from the memory controller, which is what we do on many Freescale PPC boards.
-Scott
Dear Scott Wood,
In message 1382114601.7979.843.camel@snotra.buserror.net you wrote:
Did you see my other mail in this thread? This patch is sort of OK for raising the get_ram_size() limit from 1 GiB to 2 GiB (with an increased risk of false positives from I/O), but it can't go beyond that on 32-bit. A better approach would be to get the RAM size from the memory controller, which is what we do on many Freescale PPC boards.
This is NOT a better approach. Reading the memory controller just tells you what is supposed to be there, i. e. what you programmed into the controller. get_ram_size() shows you what is _actually_ there, which may be a totally different thing, for example when different RAM chips can be fit on the board, or when the working area of the RAM is not the same as the actual chip size, for example due to hardware errors (shorts or interruptions on the address lines, etc.).
get_ram_size() is a very efficient memory test that detects 95% or more of all RAM related hardware issues.
Best regards,
Wolfgang Denk
On Fri, 2013-10-18 at 22:26 +0200, Wolfgang Denk wrote:
Dear Scott Wood,
In message 1382114601.7979.843.camel@snotra.buserror.net you wrote:
Did you see my other mail in this thread? This patch is sort of OK for raising the get_ram_size() limit from 1 GiB to 2 GiB (with an increased risk of false positives from I/O), but it can't go beyond that on 32-bit. A better approach would be to get the RAM size from the memory controller, which is what we do on many Freescale PPC boards.
This is NOT a better approach. Reading the memory controller just tells you what is supposed to be there, i. e. what you programmed into the controller. get_ram_size() shows you what is _actually_ there,
That may be useful with simpler memory controllers where there isn't much to get wrong other than size, but on modern DDR controllers you're pretty screwed anyway if you don't know what's actually there. If you don't get the size right, what are the odds you got the timing right? We hard code a lot of other things in U-Boot such as the address of various I/O...
And if RAM is socketed (or the board designer was nice enough to include it with non-socketed RAM), the information should be coming from SPD EEPROMs, not hardcoded in the U-Boot image.
which may be a totally different thing, for example when different RAM chips can be fit on the board, or when the working area of the RAM is not the same as the actual chip size, for example due to hardware errors (shorts or interruptions on the address lines, etc.).
get_ram_size() is a very efficient memory test that detects 95% or more of all RAM related hardware issues.
So call it something like test_ram_simple() and bound it by the maximum amount of RAM that you expect to be there, so you don't accidentally touch I/O. For RAM beyond 2G, either leave it untested or use the MMU to test it, but don't tell the rest of the system that RAM beyond that doesn't exist. For RAM that ends on a non-power-of-2 (below 2G), do one final test at the end of the supplied size.
And if the test finds missing/bad RAM below the supplied limit, the caller should generally make noise that there's a problem rather than just use less RAM.
-Scott
Dear Scott,
In message 1382130258.7979.896.camel@snotra.buserror.net you wrote:
This is NOT a better approach. Reading the memory controller just tells you what is supposed to be there, i. e. what you programmed into the controller. get_ram_size() shows you what is _actually_ there,
That may be useful with simpler memory controllers where there isn't much to get wrong other than size, but on modern DDR controllers you're pretty screwed anyway if you don't know what's actually there. If you
The standard approach here is to go through all expected memory configurations...
don't get the size right, what are the odds you got the timing right? We hard code a lot of other things in U-Boot such as the address of various I/O...
You sound as if you were proud of that? I don't think this is any special kind of "acchievement".
And if RAM is socketed (or the board designer was nice enough to include it with non-socketed RAM), the information should be coming from SPD EEPROMs, not hardcoded in the U-Boot image.
Agreed - but then, still, get_ram_size() is a pretty efficient test if your memory is actually working as expected. Even DIMMs with SPD EEPROMs tend to fail every now and then. And when you know you have fit a 1 GB module, and U-Boot recognizes only 128 MB, you might start to think...
get_ram_size() is a very efficient memory test that detects 95% or more of all RAM related hardware issues.
So call it something like test_ram_simple() and bound it by the maximum
Actually the _primary_ function is the memory sizing. The testimg is just a pleasant side effect.
amount of RAM that you expect to be there, so you don't accidentally touch I/O. For RAM beyond 2G, either leave it untested or use the MMU to test it, but don't tell the rest of the system that RAM beyond that doesn't exist. For RAM that ends on a non-power-of-2 (below 2G), do one final test at the end of the supplied size.
Patches welcoem :-)
And if the test finds missing/bad RAM below the supplied limit, the caller should generally make noise that there's a problem rather than just use less RAM.
Well, it reports the working RAM size. You can check that, if you like.
Best regards,
Wolfgang Denk
On 10/18/13 22:26, Wolfgang Denk wrote:
Dear Scott Wood,
In message 1382114601.7979.843.camel@snotra.buserror.net you wrote:
Did you see my other mail in this thread? This patch is sort of OK for raising the get_ram_size() limit from 1 GiB to 2 GiB (with an increased risk of false positives from I/O), but it can't go beyond that on 32-bit. A better approach would be to get the RAM size from the memory controller, which is what we do on many Freescale PPC boards.
This is NOT a better approach. Reading the memory controller just tells you what is supposed to be there, i. e. what you programmed into the controller. get_ram_size() shows you what is _actually_ there, which may be a totally different thing, for example when different RAM chips can be fit on the board, or when the working area of the RAM is not the same as the actual chip size, for example due to hardware errors (shorts or interruptions on the address lines, etc.).
get_ram_size() is a very efficient memory test that detects 95% or more of all RAM related hardware issues.
But is my patch acceptable and does it fix the phys_size_t = long vs phys_size_t = unsigned long 'issue'. Does this patch fix that or make it worse? And if so, how would that needed to be fixed.
Oliver
Best regards,
Wolfgang Denk
On 10/18/13 18:43, Scott Wood wrote:
On Fri, 2013-10-18 at 02:04 +0200, Oliver Schinagl wrote:
So now that that's settled, anything fundamentally wrong with my patch? :)
Did you see my other mail in this thread? This patch is sort of OK for
Sorry I did and I got distracted from it.
raising the get_ram_size() limit from 1 GiB to 2 GiB (with an increased risk of false positives from I/O), but it can't go beyond that on
I'd ask 'how so' but I'm not sure I'd understand anyway ;)
32-bit. A better approach would be to get the RAM size from the memory controller, which is what we do on many Freescale PPC boards.
Not possible for us at this moment. The memory controller is programed with hard-coded values on a per board basis. I think we could technically obtain values via/from the memory controller, but have no knowledge at this moment. Allwinner has a tool, livesuit, which is used to flash full disk images to a device. We currently guesstimate that livesuit can somehow detect the memory parameters and injects it into the stock bootloader. But we really have no clue if that really happens or how it's done. So we rely on extracting the information from a running stock android/linux and hardcode it into u-boot.
Oliver
-Scott
On Sat, 2013-10-19 at 01:07 +0200, Oliver Schinagl wrote:
On 10/18/13 18:43, Scott Wood wrote:
On Fri, 2013-10-18 at 02:04 +0200, Oliver Schinagl wrote:
So now that that's settled, anything fundamentally wrong with my patch? :)
Did you see my other mail in this thread? This patch is sort of OK for
Sorry I did and I got distracted from it.
raising the get_ram_size() limit from 1 GiB to 2 GiB (with an increased risk of false positives from I/O), but it can't go beyond that on
I'd ask 'how so' but I'm not sure I'd understand anyway ;)
Do you mean why it can't go beyond 2 GiB? The next address to probe after 0x8000_0000 would be 0x1_0000_0000 which is beyond what can be addressed in a 32-bit environment. I suppose you could return 4 GiB if 0x8000_0000 tests OK, but nothing beyond that. You'd need a larger datatype than "unsigned long" if you want to return 4 GiB, though.
And the one 64-bit environment that we're about to have in U-Boot (armv8) has discontiguous memory, which is another case where get_ram_size() won't work.
32-bit. A better approach would be to get the RAM size from the memory controller, which is what we do on many Freescale PPC boards.
Not possible for us at this moment. The memory controller is programed with hard-coded values on a per board basis. I think we could technically obtain values via/from the memory controller, but have no knowledge at this moment. Allwinner has a tool, livesuit, which is used to flash full disk images to a device. We currently guesstimate that livesuit can somehow detect the memory parameters and injects it into the stock bootloader. But we really have no clue if that really happens or how it's done. So we rely on extracting the information from a running stock android/linux and hardcode it into u-boot.
So the issue is that you don't have documentation on what the values you program into the memory controller mean? Can you extract the memory size as well from a running stock image?
BTW, shouldn't get_ram_size restore the original data in the final "return (maxsize)" case? I know, patches welcome. :-)
-Scott
On Fri, 2013-10-18 at 18:25 -0500, Scott Wood wrote:
On Sat, 2013-10-19 at 01:07 +0200, Oliver Schinagl wrote:
On 10/18/13 18:43, Scott Wood wrote:
On Fri, 2013-10-18 at 02:04 +0200, Oliver Schinagl wrote:
So now that that's settled, anything fundamentally wrong with my patch? :)
Did you see my other mail in this thread? This patch is sort of OK for
Sorry I did and I got distracted from it.
raising the get_ram_size() limit from 1 GiB to 2 GiB (with an increased risk of false positives from I/O), but it can't go beyond that on
I'd ask 'how so' but I'm not sure I'd understand anyway ;)
Do you mean why it can't go beyond 2 GiB? The next address to probe after 0x8000_0000 would be 0x1_0000_0000 which is beyond what can be addressed in a 32-bit environment. I suppose you could return 4 GiB if 0x8000_0000 tests OK, but nothing beyond that. You'd need a larger datatype than "unsigned long" if you want to return 4 GiB, though.
Oh, and if you actually had 4 GiB of RAM mapped in a 32-bit environment, where would I/O go?
-Scott
On 10/19/13 01:25, Scott Wood wrote:
On Fri, 2013-10-18 at 18:25 -0500, Scott Wood wrote:
On Sat, 2013-10-19 at 01:07 +0200, Oliver Schinagl wrote:
On 10/18/13 18:43, Scott Wood wrote:
On Fri, 2013-10-18 at 02:04 +0200, Oliver Schinagl wrote:
So now that that's settled, anything fundamentally wrong with my patch? :)
Did you see my other mail in this thread? This patch is sort of OK for
Sorry I did and I got distracted from it.
raising the get_ram_size() limit from 1 GiB to 2 GiB (with an increased risk of false positives from I/O), but it can't go beyond that on
I'd ask 'how so' but I'm not sure I'd understand anyway ;)
Do you mean why it can't go beyond 2 GiB? The next address to probe after 0x8000_0000 would be 0x1_0000_0000 which is beyond what can be addressed in a 32-bit environment. I suppose you could return 4 GiB if 0x8000_0000 tests OK, but nothing beyond that. You'd need a larger datatype than "unsigned long" if you want to return 4 GiB, though.
Oh, and if you actually had 4 GiB of RAM mapped in a 32-bit environment, where would I/O go?
Well you need 4 GiB of address-space don't you? If you have 2 GiB of ram, the other 2 GiB is used as register-space isn't it. So to support 2 GiB of ram you need 4 GiB of address sapce. Granted having more then 2 GiB of ram is highly unlikly as you probably can't get ramsizes that would fit. But the bug exists with 2 GiB, get_ram_size in its current form, which triggered me into fixing it. get_ram_size reports negative ram on our 2 GiB board. Obviously incidentally it doesn't hugely matter because ramsize = get_ram_size(); you put a long inside an unsigned long and it gets automagically fixed.
My point was simply get_ram_size is bugged, it cramps signed long into an unsigned long, and I tried to fix that. A side effect is that upto 4 GiB of address space is fixed. :)
Oliver
-Scott
On 10/19/13 01:25, Scott Wood wrote:
On Sat, 2013-10-19 at 01:07 +0200, Oliver Schinagl wrote:
On 10/18/13 18:43, Scott Wood wrote:
On Fri, 2013-10-18 at 02:04 +0200, Oliver Schinagl wrote:
So now that that's settled, anything fundamentally wrong with my patch? :)
Did you see my other mail in this thread? This patch is sort of OK for
Sorry I did and I got distracted from it.
raising the get_ram_size() limit from 1 GiB to 2 GiB (with an increased risk of false positives from I/O), but it can't go beyond that on
I'd ask 'how so' but I'm not sure I'd understand anyway ;)
Do you mean why it can't go beyond 2 GiB? The next address to probe after 0x8000_0000 would be 0x1_0000_0000 which is beyond what can be
Yeah why can't it go beyond 2 GiB. It should be an unsigned long, so it should be able to go beyond 2 GiB, as you state, upto 4 GiB on a 32-bit environment.
What my patch fixes is, that u-boot passes ram parameters (ramsize) via the global data -> ramsize. Ramsize is a phys_size_t which is defined as unsigned long (yay for typedef :p). Nearly all platforms pass the memory size to the linux kernel as gd->ramsize = get_ram_size();
And here is the bug, (unsigned long)ramsize = (signed long)get_ram_size();
addressed in a 32-bit environment. I suppose you could return 4 GiB > if 0x8000_0000 tests OK, but nothing beyond that. You'd need a larger datatype than "unsigned long" if you want to return 4 GiB, though.
As for returning exactly 4 GiB, assuming that register space is 0 bytes (impossible but lets just say for arguments sake) we now have 0x8000_0000 addresses available, so exactly 4 GiB. If for whatever reason it ends up being only 4 GiB -1 byte, I don't think anybody will care/notice (but it would have to be taken into account I suppose?
And the one 64-bit environment that we're about to have in U-Boot (armv8) has discontiguous memory, which is another case where get_ram_size() won't work.
So get_ram_size() needs a brother, get_discont_ram_size? :)
32-bit. A better approach would be to get the RAM size from the memory controller, which is what we do on many Freescale PPC boards.
Not possible for us at this moment. The memory controller is programed with hard-coded values on a per board basis. I think we could technically obtain values via/from the memory controller, but have no knowledge at this moment. Allwinner has a tool, livesuit, which is used to flash full disk images to a device. We currently guesstimate that livesuit can somehow detect the memory parameters and injects it into the stock bootloader. But we really have no clue if that really happens or how it's done. So we rely on extracting the information from a running stock android/linux and hardcode it into u-boot.
So the issue is that you don't have documentation on what the values you program into the memory controller mean? Can you extract the memory size as well from a running stock image?
Sort of, we bus-width, io-size and chip density, from those values we determine the chip-size/ram-size we can't exctract the actual number.
That said, io-size I think (or was it bus-width?) while programmed into the ram controller, isn't even highly important, it is expected that it is used for drive strength, 2 chips vs 4 chips, but physical examination of the tablet/board helps here.
Oliver
BTW, shouldn't get_ram_size restore the original data in the final "return (maxsize)" case? I know, patches welcome. :-)
-Scott
On Sat, Oct 19, 2013 at 11:07:54AM +0200, Oliver Schinagl wrote:
[snip]
What my patch fixes is, that u-boot passes ram parameters (ramsize) via the global data -> ramsize. Ramsize is a phys_size_t which is defined as unsigned long (yay for typedef :p). Nearly all platforms pass the memory size to the linux kernel as gd->ramsize = get_ram_size();
And here is the bug, (unsigned long)ramsize = (signed long)get_ram_size();
Well, here's where we have a few related problems. We need to: a) Know how much memory U-Boot has available to it, to work with (for relocation, image loading, etc, etc) b) Tell an OS how much memory exists via ATAGs or Device Tree.
(b) is what gets complicated because the rule of thumb U-Boot has had forever is to update the device tree to have "correct" memory information. A number of PowerPC boards have dtb files with 0x0 for size, for auto-detect in U-Boot. A number of ARM boards just have 128MiB size and then work well in Linux as we fixup that incorrect amount to the larger real value.
We need a way to add in support for case (c) of "Device Tree has correct memory information, DO NOT TOUCH". This has been talked about before but what I said in (b) complicates things. We may way to see if for example the DT indicates a size beyond what we can determine and if so, just go with it (and perhaps a debug() print about not touching).
Dear Scott Wood,
In message 1382138723.7979.928.camel@snotra.buserror.net you wrote:
And the one 64-bit environment that we're about to have in U-Boot (armv8) has discontiguous memory, which is another case where get_ram_size() won't work.
get_ram_size() is supposed to be run per memory bank. If you have discontiguous memory, then you probably have several memory banks that can be sized separately?
BTW, shouldn't get_ram_size restore the original data in the final "return (maxsize)" case? I know, patches welcome. :-)
Yes, get_ram_size() is non-destructive (at least in the no-error case; otherwise things like PRAM would not work).
Best regards,
Wolfgang Denk
Hey all,
*ping*
On 10/21/2013 09:44 PM, Wolfgang Denk wrote:
Dear Scott Wood,
In message 1382138723.7979.928.camel@snotra.buserror.net you wrote:
And the one 64-bit environment that we're about to have in U-Boot (armv8) has discontiguous memory, which is another case where get_ram_size() won't work.
get_ram_size() is supposed to be run per memory bank. If you have discontiguous memory, then you probably have several memory banks that can be sized separately?
BTW, shouldn't get_ram_size restore the original data in the final "return (maxsize)" case? I know, patches welcome. :-)
Yes, get_ram_size() is non-destructive (at least in the no-error case; otherwise things like PRAM would not work).
Is there anything else that needs to be cleaned up in the patch I submitted back then (other then re-basing it I suppose).
With all the sunxi stuff slowly being cleaned up, this patch came to mind again and I was just wondering if anything needs to be done to get it merged. There was quite a big discussion about get_ram_size() in general, but nobody ever said if the signed long -> unsigned long was a good fix.
Olliver
Best regards,
Wolfgang Denk
On Thu, 2013-10-03 at 23:15 +0200, Oliver Schinagl wrote:
Hey all,
I just yesterday received my CubieTruck (cubieboard3) with 2 GiB of Ram and added support for it to the sunxi-u-boot branch. While I know this isn't merged into the main u-boot tree (yet), I ran into the following problem.
At the end of the dram init code, it is customary to call get_ram_size() and return its value. This is then used to print the DRAM size and also is passed to the Linux kernel.
However the return size of get_ram_size() is a long. While I don't understand why not unsigned long or even u64 was chosen, this causes get_dram_size to overflow when having a ramsize of 2 GiB. While only printing of the value isn't hugely important, this does indicate u-boot seems to be somewhat artificially limited to 2 GiB of Ram? This only seems to affect the SPL as, if I understood correctly, there it stores the ram_size into the gd struct which I think is unsigned long.
I've started working on a patch to convert common/memsize.c's get_ram_size(), to be completely unsigned long, however there seems to be quite a lot of code that calls this. So my question is now before going over all drivers and change that and submit a big patch-set, did I overlook anything and are my conclusions correct, get_ram_size should return unsigned long.
get_ram_size() is inherently incapable of dealing with memory that is too large to be mapped all at once. It also would have a hard time with 2GiB RAM, because it sizes by powers of two, so the next address to be checked would be at 4GiB which cannot be mapped on 32-bit (plus, at such large RAM sizes you run a significant risk of a false positive by hitting I/O).
Can't you get the RAM size from the memory controller config instead?
Finally, a long is 32 bits on x86 and armv7, but how will that relate to 64bits armv8? As I understood, Windows treats long's as 4 bytes no matter if it's 32 bit or 64 bit. Linux is better and a long is 4 bytes on 32 bits, and 8 bytes on 64 bits versions. So how will u-boot work on armv8? Will the long datatype work well, or should I consider changing things more future proof? (u32 and u64 come to mind).
U-Boot on armv8 follows an LP64 model just like Linux.
-Scott
participants (6)
-
Albert ARIBAUD -
Oliver Schinagl -
Olliver Schinagl
-
Scott Wood -
Tom Rini -
Wolfgang Denk