Hi Alex, On 21 June 2018 at 04:10, Alexander Graf <[email protected]> wrote: > On 06/21/2018 04:01 AM, Simon Glass wrote: >> >> 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 :-) > > > https://port70.net/~nsz/c/c11/n1570.html#6.5.8 > > Point 5. Since we're not in an array or a struct member, we fall into the > "In all other cases, the behavior is undefined" case. Our compiler people > basically recommend to convert to uintptr_t first and compare then. That way > behavior is always defined.
There are two pointers being compared: (const uint8_t *)ptr and (const uint8_t *)gd->arch.ram_buf + gd->ram_size; which is just a const uint_8 *, call it 'x' if you like. The bahaviour is clearly defined in the spec you pointed to: "both operands are pointers to qualified or unqualified versions of compatible object types." Regards, Simon _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
