[U-Boot] [RFC PATCH] itest: allow map_physmem to return 0 in limited cases
From: Stephen Warren swarren@nvidia.com
On some systems, RAM starts at address 0. If the user executes itest against address 0 on such a system, it will call map_physmem(0, ...) which will return 0 back; mapping only changes the address on sandbox. This causes itest to believe map_physmem() has failed, and hence fails the comparison.
Fix itest so that it allows map_physmem() to return 0 /if/ the orignal address passed to it was also 0.
This fixes "tegra-uboot-flasher flash" on Tegra20.
This has the disadvantage that on sandbox, failed mapping attempts for address 0 are not detected. Instead, should the code only call map_physmem() on sandbox? Or, should map_physmem() return its error status some other way. Or, should the special case only be allowed on systems where the base of RAM is 0 somehow?
Fixes: 7861204c9af7 ("itest: make memory access work under sandbox") Signed-off-by: Stephen Warren swarren@nvidia.com --- Another approach would be to fix the scripts generated by t-u-f to use some other address, e.g. address 4. However, I think itest should work on any legal address in RAM?
common/cmd_itest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/cmd_itest.c b/common/cmd_itest.c index 91ae5c2704c8..fb4d797e43d7 100644 --- a/common/cmd_itest.c +++ b/common/cmd_itest.c @@ -59,7 +59,7 @@ static long evalexp(char *s, int w) if (s[0] == '*') { addr = simple_strtoul(&s[1], NULL, 16); buf = map_physmem(addr, w, MAP_WRBACK); - if (!buf) { + if (!buf && addr) { puts("Failed to map physical memory\n"); return 0; }
On 29 January 2016 at 16:10, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
On some systems, RAM starts at address 0. If the user executes itest against address 0 on such a system, it will call map_physmem(0, ...) which will return 0 back; mapping only changes the address on sandbox. This causes itest to believe map_physmem() has failed, and hence fails the comparison.
Fix itest so that it allows map_physmem() to return 0 /if/ the orignal address passed to it was also 0.
This fixes "tegra-uboot-flasher flash" on Tegra20.
This has the disadvantage that on sandbox, failed mapping attempts for address 0 are not detected. Instead, should the code only call map_physmem() on sandbox? Or, should map_physmem() return its error status some other way. Or, should the special case only be allowed on systems where the base of RAM is 0 somehow?
Fixes: 7861204c9af7 ("itest: make memory access work under sandbox") Signed-off-by: Stephen Warren swarren@nvidia.com
Another approach would be to fix the scripts generated by t-u-f to use some other address, e.g. address 4. However, I think itest should work on any legal address in RAM?
common/cmd_itest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Simon Glass sjg@chromium.org
This seems OK. I don't think we care about the case where sandbox can't get to its memory. But if you want to improve this, I'd favour a new function that returns an error code and puts the return pointer in a parameter, like
int map_physmem_ptr(phys_addr_t paddr, ulong len, ulong flags, void **ptrp)
diff --git a/common/cmd_itest.c b/common/cmd_itest.c index 91ae5c2704c8..fb4d797e43d7 100644 --- a/common/cmd_itest.c +++ b/common/cmd_itest.c @@ -59,7 +59,7 @@ static long evalexp(char *s, int w) if (s[0] == '*') { addr = simple_strtoul(&s[1], NULL, 16); buf = map_physmem(addr, w, MAP_WRBACK);
if (!buf) {
if (!buf && addr) { puts("Failed to map physical memory\n"); return 0; }-- 2.7.0
Regards, Simon
On 6 February 2016 at 13:29, Simon Glass sjg@chromium.org wrote:
On 29 January 2016 at 16:10, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
On some systems, RAM starts at address 0. If the user executes itest against address 0 on such a system, it will call map_physmem(0, ...) which will return 0 back; mapping only changes the address on sandbox. This causes itest to believe map_physmem() has failed, and hence fails the comparison.
Fix itest so that it allows map_physmem() to return 0 /if/ the orignal address passed to it was also 0.
This fixes "tegra-uboot-flasher flash" on Tegra20.
This has the disadvantage that on sandbox, failed mapping attempts for address 0 are not detected. Instead, should the code only call map_physmem() on sandbox? Or, should map_physmem() return its error status some other way. Or, should the special case only be allowed on systems where the base of RAM is 0 somehow?
Fixes: 7861204c9af7 ("itest: make memory access work under sandbox") Signed-off-by: Stephen Warren swarren@nvidia.com
Another approach would be to fix the scripts generated by t-u-f to use some other address, e.g. address 4. However, I think itest should work on any legal address in RAM?
common/cmd_itest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Simon Glass sjg@chromium.org
This seems OK. I don't think we care about the case where sandbox can't get to its memory. But if you want to improve this, I'd favour a new function that returns an error code and puts the return pointer in a parameter, like
int map_physmem_ptr(phys_addr_t paddr, ulong len, ulong flags, void **ptrp)
Applied to u-boot-dm, thanks!
diff --git a/common/cmd_itest.c b/common/cmd_itest.c index 91ae5c2704c8..fb4d797e43d7 100644 --- a/common/cmd_itest.c +++ b/common/cmd_itest.c @@ -59,7 +59,7 @@ static long evalexp(char *s, int w) if (s[0] == '*') { addr = simple_strtoul(&s[1], NULL, 16); buf = map_physmem(addr, w, MAP_WRBACK);
if (!buf) {
if (!buf && addr) { puts("Failed to map physical memory\n"); return 0; }-- 2.7.0
Regards, Simon
participants (2)
-
Simon Glass -
Stephen Warren