Hi Alex, On 18 June 2018 at 08:50, Alexander Graf <[email protected]> wrote: > On 06/18/2018 04:08 PM, Simon Glass wrote: >> >> At present map_sysmem() maps an address into the sandbox RAM buffer, >> return a pointer, while map_to_sysmem() goes the other way. >> >> The mapping is currently just 1:1 since a case was not found where a more >> flexible mapping was needed. PCI does have a separate and more complex >> mapping, but uses its own mechanism. >> >> However this arrange cannot handle one important case, which is where a >> test declares a stack variable and passes a pointer to it into a U-Boot >> function which uses map_to_sysmem() to turn it into a address. Since the >> pointer is not inside emulated DRAM, this will fail. >> >> Add a mapping feature which can handle any such pointer, mapping it to a >> simple tag value which can be passed around in U-Boot as an address. >> >> Signed-off-by: Simon Glass <[email protected]> >> --- >> >> Changes in v8: None >> Changes in v7: None >> Changes in v6: None >> Changes in v5: None >> Changes in v4: None >> Changes in v3: None >> Changes in v2: None >> >> arch/sandbox/cpu/cpu.c | 141 +++++++++++++++++++++++++++++-- >> arch/sandbox/cpu/state.c | 8 ++ >> arch/sandbox/include/asm/state.h | 21 +++++ >> 3 files changed, 161 insertions(+), 9 deletions(-) >> >> diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c >> index cde0b055a6..fd77603ebf 100644 >> --- a/arch/sandbox/cpu/cpu.c >> +++ b/arch/sandbox/cpu/cpu.c >> @@ -57,14 +57,104 @@ int cleanup_before_linux_select(int flags) >> return 0; >> } >> +/** >> + * is_in_sandbox_mem() - Checks if a pointer is within sandbox's emulated >> DRAM >> + * >> + * This provides a way to check if a pointer is owned by sandbox (and is >> within >> + * its RAM) or not. Sometimes pointers come from a test which >> conceptually runs >> + * output sandbox, potentially with direct access to the C-library >> malloc() >> + * function, or the sandbox stack (which is not actually within the >> emulated >> + * DRAM. >> + * >> + * Such pointers obviously cannot be mapped into sandbox's DRAM, so we >> must >> + * detect them an process them separately, by recording a mapping to a >> tag, >> + * which we can use to map back to the pointer later. >> + * >> + * @ptr: Pointer to check >> + * @return true if this is within sandbox emulated DRAM, false if not >> + */ >> +static bool is_in_sandbox_mem(const void *ptr) >> +{ >> + return (const uint8_t *)ptr >= gd->arch.ram_buf && >> + (const uint8_t *)ptr < gd->arch.ram_buf + gd->ram_size; > > > Greater/Less comparisons on pointers are invalid (albeit used) C IIRC. You > should cast to uintptr_t instead and compare those.
Reference? That is news to me :-) > >> +} >> + >> +/** >> + * phys_to_virt() - Converts a sandbox RAM address to a pointer >> + * >> + * Sandbox uses U-Boot addresses from 0 to the size of DRAM. These index >> into >> + * the emulated DRAM buffer used by sandbox. This function converts such >> an >> + * address to a pointer into thi sbuffer, which can be used to access the > > > this OK fixed, ta. > > ... > > Thinking about this, wouldn't it be easier to just allocate the stack inside > U-Boot RAM? We could change the tests so that instead of using local variables they call a function to allocate sandbox RAM. But I'm not sure it is easier. This implementation of sandbox mapping is what I envisaged ages ago when writing the original implementation. I just never got around to it since for most cases the simple code works. For PCI I added a hack... So I think it is a good time to add it. [..] Regards, Simon _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
