| Message ID | 1317082255-24247-20-git-send-email-sjg@chromium.org |
|---|---|
| State | New, archived |
| Headers | show |
On Monday, September 26, 2011 20:10:53 Simon Glass wrote: > --- a/common/cmd_mem.c > +++ b/common/cmd_mem.c > > printf ("\nMem error @ 0x%08X: " > "found %08lX, expected %08lX\n", > - (uint)addr, readback, val); > + (uint)(uintptr_t)addr, readback, val); could you change the printf format to %#p instead (and drop the casts completely for "addr") ? -mike
Hi Mike, On Mon, Oct 3, 2011 at 11:57 AM, Mike Frysinger <vapier@gentoo.org> wrote: > On Monday, September 26, 2011 20:10:53 Simon Glass wrote: >> --- a/common/cmd_mem.c >> +++ b/common/cmd_mem.c >> >> printf ("\nMem error @ 0x%08X: " >> "found %08lX, expected %08lX\n", >> - (uint)addr, readback, val); >> + (uint)(uintptr_t)addr, readback, val); > > could you change the printf format to %#p instead (and drop the casts > completely for "addr") ? > -mike > Well it's not complete clear where we want to go with this. My current thinking is that the emulated memory will be small (in 2011 terms) - maybe 128MB. The real memory of the machine is not accessible since it doesn't make sense. So perhaps just because the host happens to be a 64-bit machine we don't suddenly want to be printing 64-bit addresses. So the answer is 'yes' but for now I'm not sure that's the right thing to do. I have another patch which enables 'md', etc. but of course only within the 128MB 'emulated' memory area. It actually brings up a question - does U-Boot run on 64-machines? Regards, Simon
On Tuesday 04 October 2011 01:24:56 Simon Glass wrote: > On Mon, Oct 3, 2011 at 11:57 AM, Mike Frysinger wrote: > > On Monday, September 26, 2011 20:10:53 Simon Glass wrote: > >> --- a/common/cmd_mem.c > >> +++ b/common/cmd_mem.c > >> > >> printf ("\nMem error @ 0x%08X: " > >> "found %08lX, expected %08lX\n", > >> - (uint)addr, readback, val); > >> + (uint)(uintptr_t)addr, readback, > >> val); > > > > could you change the printf format to %#p instead (and drop the casts > > completely for "addr") ? > > Well it's not complete clear where we want to go with this. My current > thinking is that the emulated memory will be small (in 2011 terms) - > maybe 128MB. The real memory of the machine is not accessible since it > doesn't make sense. So perhaps just because the host happens to be a > 64-bit machine we don't suddenly want to be printing 64-bit addresses. > > So the answer is 'yes' but for now I'm not sure that's the right thing > to do. I have another patch which enables 'md', etc. but of course > only within the 128MB 'emulated' memory area. > > It actually brings up a question - does U-Boot run on 64-machines? i don't believe u-boot runs on any 64bit system (or at least, in 64bit mode). there's quite a lot of random assumptions all over the tree where pointers are stored in 32bits, so for making sandbox work on 64bit stuff, i would focus on making stuff work with minimal intrusion for 32bit systems. although thinking about this a bit more, %p will not zero pad its output (i think even if you specify like "08" it'll get ignored). but maybe people don't care as normally this output shouldn't occur ? -mike
Hi Mike, On Sun, Oct 9, 2011 at 7:54 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Tuesday 04 October 2011 01:24:56 Simon Glass wrote: >> On Mon, Oct 3, 2011 at 11:57 AM, Mike Frysinger wrote: >> > On Monday, September 26, 2011 20:10:53 Simon Glass wrote: >> >> --- a/common/cmd_mem.c >> >> +++ b/common/cmd_mem.c >> >> >> >> printf ("\nMem error @ 0x%08X: " >> >> "found %08lX, expected %08lX\n", >> >> - (uint)addr, readback, val); >> >> + (uint)(uintptr_t)addr, readback, >> >> val); >> > >> > could you change the printf format to %#p instead (and drop the casts >> > completely for "addr") ? >> >> Well it's not complete clear where we want to go with this. My current >> thinking is that the emulated memory will be small (in 2011 terms) - >> maybe 128MB. The real memory of the machine is not accessible since it >> doesn't make sense. So perhaps just because the host happens to be a >> 64-bit machine we don't suddenly want to be printing 64-bit addresses. >> >> So the answer is 'yes' but for now I'm not sure that's the right thing >> to do. I have another patch which enables 'md', etc. but of course >> only within the 128MB 'emulated' memory area. >> >> It actually brings up a question - does U-Boot run on 64-machines? > > i don't believe u-boot runs on any 64bit system (or at least, in 64bit mode). > there's quite a lot of random assumptions all over the tree where pointers are > stored in 32bits, so for making sandbox work on 64bit stuff, i would focus on > making stuff work with minimal intrusion for 32bit systems. Yes - I don't seem a lot of use in having massive amounts of RAM for testing U-Boot, so 32-bit addresses should be fine for now. > > although thinking about this a bit more, %p will not zero pad its output (i > think even if you specify like "08" it'll get ignored). but maybe people > don't care as normally this output shouldn't occur ? > -mike > Yes %p is not quite what we want, but as you say this is an error condition. Regards, Simon
Our OCTEON platform is a 64-bit SOC and we run it in the MIPS N32 ABI mode (64-bit registers, 32-bit address space). In our case we use virtual memory so we can move U-Boot to the top of memory which is often above 4GB. The only changes we had to make to U-Boot was that drivers need to use the proper mapping functions and support 64-bit physical addresses. Generally the hardware devices support this and the driver changes have been minimal. Some of our boards have over 8GB of RAM installed and 4GB is common for us. With the hole we create for our boot bus (flash, etc.) that means that in many cases U-Boot is loaded above the 32-bit boundary. By using a couple of TLB entries on MIPS we always map U-Boot to 0xC0000000 no matter where it is executing from. While a 64-bit U-Boot would be nice, we don't really need it since the TLB solves all of our issues. I would rather see drivers better make use of the proper mapping functions and support 64-bit physical addresses as well as use wrappers for accessing the PCI register space. (in our case all of our SOC registers require 64-bit addresses). -Aaron On Sunday, October 09, 2011 07:54:50 PM Mike Frysinger wrote: > On Tuesday 04 October 2011 01:24:56 Simon Glass wrote: > > On Mon, Oct 3, 2011 at 11:57 AM, Mike Frysinger wrote: > > > On Monday, September 26, 2011 20:10:53 Simon Glass wrote: > > >> --- a/common/cmd_mem.c > > >> +++ b/common/cmd_mem.c > > >> > > >> printf ("\nMem error @ 0x%08X: " > > >> > > >> "found %08lX, expected %08lX\n", > > >> > > >> - (uint)addr, readback, val); > > >> + (uint)(uintptr_t)addr, readback, > > >> val); > > > > > > could you change the printf format to %#p instead (and drop the casts > > > completely for "addr") ? > > > > Well it's not complete clear where we want to go with this. My current > > thinking is that the emulated memory will be small (in 2011 terms) - > > maybe 128MB. The real memory of the machine is not accessible since it > > doesn't make sense. So perhaps just because the host happens to be a > > 64-bit machine we don't suddenly want to be printing 64-bit addresses. > > > > So the answer is 'yes' but for now I'm not sure that's the right thing > > to do. I have another patch which enables 'md', etc. but of course > > only within the 128MB 'emulated' memory area. > > > > It actually brings up a question - does U-Boot run on 64-machines? > > i don't believe u-boot runs on any 64bit system (or at least, in 64bit > mode). there's quite a lot of random assumptions all over the tree where > pointers are stored in 32bits, so for making sandbox work on 64bit stuff, > i would focus on making stuff work with minimal intrusion for 32bit > systems. > > although thinking about this a bit more, %p will not zero pad its output (i > think even if you specify like "08" it'll get ignored). but maybe people > don't care as normally this output shouldn't occur ? > -mike
On Monday 10 October 2011 22:46:10 Aaron Williams wrote: > Our OCTEON platform is a 64-bit SOC and we run it in the MIPS N32 ABI mode > (64-bit registers, 32-bit address space). right, n32 == 32bit pointers, so i don't consider that a port with 64bit pointer issues > rather see drivers better make use of the proper mapping functions and > support 64-bit physical addresses as well as use wrappers for accessing > the PCI register space. (in our case all of our SOC registers require > 64-bit addresses). what exactly are "proper mapping functions" ? you mean things like the kernel's ioremap() ? also, please don't top post -mike
On Monday, October 10, 2011 09:52:59 PM Mike Frysinger wrote: > On Monday 10 October 2011 22:46:10 Aaron Williams wrote: > > Our OCTEON platform is a 64-bit SOC and we run it in the MIPS N32 ABI > > mode (64-bit registers, 32-bit address space). > > right, n32 == 32bit pointers, so i don't consider that a port with 64bit > pointer issues > > > rather see drivers better make use of the proper mapping functions and > > support 64-bit physical addresses as well as use wrappers for accessing > > the PCI register space. (in our case all of our SOC registers require > > 64-bit addresses). > > what exactly are "proper mapping functions" ? you mean things like the > kernel's ioremap() ? > > also, please don't top post > -mike I mean using functions like pci_virt_to_mem(). I have fixed a couple of drivers such as the USB EHCI driver and Intel E1000 driver to make use of this on our platform. We also can optionally perform I/O remapping so that PCI devices that only support 32-bit addressing can still access the memory where U-Boot is located. I have found that numerous drivers assume that an address in a pointer can be used directly for DMA which is not the case on MIPS since U-Boot typically runs in KSEG0 (0x8000000) which is aliased to physical memory address 0. The other area I have seen issues is where drivers do not use readl/writel/etc. to access the registers. It is even easier if they use a macro per-driver since in our case we might use different functions to access the underlying hardware registers depending on if they're part of our SOC or on the PCIe bus since they map to different 64-bit I/O areas. Now none of these actually require 64-bit pointers, just 64-bit physical addresses. We even support 64-bit ELF executables loaded from U-Boot as well as the 64-bit Linux kernel (we don't support a 32-bit kernel any longer) and basically make use of a 64-bit version of memcpy. 64-bit support would be useful for the various memory functions, i.e. md, mw, mm, md5sum, etc. It can also be useful for loading images to higher addresses. It's not essential though and currently we added separate commands for accessing 64-bit addresses. One of these daya I need to work on getting some of our patches back upstream since some of them are quite useful for other platforms as well. Even though we use virtual memory for U-Boot the number of changes to U-Boot has been very minimal except for drivers and even then the changes can be portable (i.e. using pci_virt_to_mem(). It actually simplifies things in that we don't have to deal with remapping. Some drivers like the proposed one for the Silicon Image SATA controller work as-is. The virtual memory allows the same U-Boot image to run from anywhere in memory. We use a single image whether we're booting out of NOR flash or over PCI. We also support multiple flash images (standard and failsafe). We perform the mapping before any C code is executed so we don't have to worry about relocation when U-Boot copies itself to the top of RAM and begins executing there. We do have a very different lib/board.c however, in order to deal with our SOC. I think moving U-Boot to be fully 64-bit will be a lot more difficult compared to what we are doing since a lot of code assumes addresses are 32-bit. Our VM change allows U-Boot to remain 32-bit while still being able to support a 64- bit platform. -Aaron
On Tuesday 11 October 2011 01:41:07 Aaron Williams wrote: > One of these daya I need to work on getting some of our patches back > upstream since some of them are quite useful for other platforms as well. yeah, for things like changing pci drivers to use pci_virt_to_mem() or changing pointer derefs to asm/io.h helpers, that sounds like things we all want. -mike
diff --git a/common/cmd_mem.c b/common/cmd_mem.c index e84cc4e..28476d7 100644 --- a/common/cmd_mem.c +++ b/common/cmd_mem.c @@ -937,7 +937,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (readback != val) { printf ("\nMem error @ 0x%08X: " "found %08lX, expected %08lX\n", - (uint)addr, readback, val); + (uint)(uintptr_t)addr, readback, val); errs++; if (ctrlc()) { putc ('\n'); diff --git a/common/fdt_support.c b/common/fdt_support.c index 19b2ef6..9bed3ce 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -495,7 +495,7 @@ int fdt_resize(void *blob) total = fdt_num_mem_rsv(blob); for (i = 0; i < total; i++) { fdt_get_mem_rsv(blob, i, &addr, &size); - if (addr == (uint64_t)(u32)blob) { + if (addr == (uintptr_t)blob) { fdt_del_mem_rsv(blob, i); break; } @@ -511,14 +511,14 @@ int fdt_resize(void *blob) fdt_size_dt_strings(blob) + 5 * sizeof(struct fdt_reserve_entry); /* Make it so the fdt ends on a page boundary */ - actualsize = ALIGN(actualsize + ((uint)blob & 0xfff), 0x1000); - actualsize = actualsize - ((uint)blob & 0xfff); + actualsize = ALIGN(actualsize + ((uintptr_t)blob & 0xfff), 0x1000); + actualsize = actualsize - ((uintptr_t)blob & 0xfff); /* Change the fdt header to reflect the correct size */ fdt_set_totalsize(blob, actualsize); /* Add the new reservation */ - ret = fdt_add_mem_rsv(blob, (uint)blob, actualsize); + ret = fdt_add_mem_rsv(blob, (uintptr_t)blob, actualsize); if (ret < 0) return ret;
This fixes a few problems when building on 64-bit machines. Signed-off-by: Simon Glass <sjg@chromium.org> --- common/cmd_mem.c | 2 +- common/fdt_support.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)