[Toybox] [PATCH] devmem: Fix 8 byte wide writes

[Michael Shavit]

On Fri, Sep 1, 2023 at 5:13 AM Rob Landley <rob at landley.net> wrote:
>
> On 8/31/23 07:26, Michael Shavit wrote:
> > On Thu, Aug 31, 2023 at 6:57 PM Rob Landley <rob at landley.net> wrote:
> >> Could you send me the test case you hit that motivated this change? (I haven't
> >> got a tests/devmem.test yet and am not 100% sure how I'd even set that up in
> >> mkroot under qemu, but I should work out something...)
> >
> > I ran something like this:
> > # devmem 0x000000008b300000 8 0x8FFFFFFFFFFFFFFF
> > # devmem 0x000000008b300000 8
> > 0x8fffffffffffffff
> >
> > # devmem 0x000000008b300000 8 500k
> > # devmem 0x000000008b300000 8
> > 0x0000000007d000
> >
> > # devmem 0x000000008b300000 8 0xFFFFFFFFFFFFFFFF
> > # devmem 0x000000008b300000 8
> > 0xffffffffffffffff
>
> That would be the one that failed then?
>
> > # devmem 0x000000008b300000 4 0xFFFFFFFF
> > # devmem 0x000000008b300000 4
> > 0xffffffff
> >
> > # devmem 0x000000008b300000  4 0x1FFFFFFFF
> > devmem: data: 8589934591 exceeds write width: 4
>
> Which is true...

Oh yeah, the outputs here are with the fix. All 8 byte wide writes
fail without the fix because atolx_range is called with a low and high
value of 0.

>
> > I don't, but problem is I don't know if other users of devmem have
> > come to expect and depend on this behavior. Are we ok potentially
> > breaking compatibility for a bug fix?
> >
> >> Is sizeof(unsigned long long) ever _not_ 8?
> >
> > IIUC, technically the C spec only guarantees that it's at least 8.
>
> Toybox uses LP64: https://landley.net/toybox/design.html#bits
>
> Which technically only guarantees that long long is at _least_ 8 bytes, but I
> have yet to see a system where it's bigger and somebody (the musl-libc
> maintainer?) brought up several existing things changing it would break when I
> asked him....
>
> > But
> > anyways, the intent of using sizeof() was to make sure any updates of
> > the data/max_value types would propagate to the check.
> >
> >>
> >> I take it you're still ok with the decimal output being signed instead of
> >> unsigned?
> >
> > What do you mean, isn't %llx unsigned?
>
> Line 72:
>
>     printf((!strchr(*toys.optargs, 'x')) ? "%0*lld\n" : "0x%0*llx\n",
>       bytes*2, data);
>
> Probably fine, but now input is unsigned and output is signed so I thought I'd
> ask somebody with domain experience using this command. :)

ohhh I didn't notice that....I always use hex inputs so ¯\_(ツ)_/¯ .
Gut feeling is that we should be symmetric with inputs since it'd be
weird for devmem to return a different value when reading the same
address we just wrote to.