dc740 has quit [Remote host closed the connection]
<re_irc>
<@dirbaio:matrix.org> : how're you triggering busfault on out-of-stack? flip-link or MPU?
<re_irc>
<@mathias_koch:matrix.org> fliplink
<re_irc>
<@dirbaio:matrix.org> ah
<re_irc>
<@dirbaio:matrix.org> with MPU if you place the stack at top of RAM, and a protected page of say, 1k, at the bottom of the stack
<re_irc>
<@dirbaio:matrix.org> whe the stack hits the protected page, it goes to hardfault
<re_irc>
<@dirbaio:matrix.org> and the MPU is not active for code running at hardfault priority, so the stack can continue growing into that 1k
<re_irc>
<@dirbaio:matrix.org> so there's some stack left for actually handling the hardfault
<re_irc>
<@dirbaio:matrix.org> without having to manipulate SP
<re_irc>
<@mathias_koch:matrix.org> Ahh.. that does sound convenient..
<re_irc>
Can i do that on both STM's and RP2040?
<re_irc>
<@dirbaio:matrix.org> I guess if they have an MPU yes
<re_irc>
<@dirbaio:matrix.org> the rp2040 does, probably all stm32 do too?
<re_irc>
<@dirbaio:matrix.org> there are downsides though
<re_irc>
<@dirbaio:matrix.org> you need to "waste" 1k of RAM (or smaller if you make the guard page smaller)
<re_irc>
<@dirbaio:matrix.org> and detection is not guaranteed, a big stack allocation can "jump" over the guard page and start corrupting statics anyway
<re_irc>
<@dirbaio:matrix.org> and the probability of that gets bigger as you make the guard page smaller
<re_irc>
<@dirbaio:matrix.org> :S
<re_irc>
<@dirbaio:matrix.org> stack probes fix this in theory but afaik llvm doesn't support them on cortex-m
<re_irc>
<@rmja:matrix.org> Is there an example around that does this? Sounds really useful!
<re_irc>
<@dirbaio:matrix.org> I have it setup this way because I wanted the build to fail if I add too many statics and the stack gets smaller than 0x7fa0
<re_irc>
<@dirbaio:matrix.org> you can also do a more conventional setup where RAM has both statics+stack, and place _stack_guard_start/_stack_guard_end after all the statics, so stack automatically gets all free RAM
<re_irc>
<@dirbaio:matrix.org> keep in mind these have to be aligned though. if the guard page is 1k, they have to be aligned to 1k
<re_irc>
<@dirbaio:matrix.org> now, if only we could get cargo-call-stack to work nicely 🥲
<re_irc>
<@rmja:matrix.org> someone should create a "stack-guard" create, so something like this would be easy accessible.
<re_irc>
<@juliand:fehler-in-der-matrix.de> : This would certainly avoid some headache
<re_irc>
<@mathias_koch:matrix.org> That is kinda what flip-link is though? An attempt to make this easy across all cortex-m devices, without having to deal with each MPU?
<re_irc>
<@mathias_koch:matrix.org> but yeah, the call-stack would be nice!
<re_irc>
<@dirbaio:matrix.org> flip-link doesn't work with the softdevice, links 2x as slow (it has to link twice), and as you've seen once you run out of stack you can't actually handle it...
<re_irc>
<@mathias_koch:matrix.org> With the obvious tradeoff that generic usually is not a optimal as specific
<re_irc>
<@mathias_koch:matrix.org> * as
<re_irc>
<@mathias_koch:matrix.org> Of course there will be tradeoffs to doing it generically
<re_irc>
<@dirbaio:matrix.org> also thumbv8 has MSPLIM/PSPLIM which is suuuuuuper nice
<re_irc>
<@ryan-summers:matrix.org> : There's also stack-probing that the Rust compiler + LLVM can do
<re_irc>
<@ryan-summers:matrix.org> But it probes the stack on every increment of the stack pointer by some frame size and is mainly designed for systems with virtual memory paging
<re_irc>
<@ryan-summers:matrix.org> I did a ton of investigation into that a few years ago
emerent has quit [Ping timeout: 240 seconds]
<re_irc>
<@ryan-summers:matrix.org> But if your embedded "page size" is only 1 byte, that means that you have to probe the stack on every function call, which adds a massive amount of code overhead and slows your app down
<re_irc>
<@dirbaio:matrix.org> if you make the guard page 1k you only have to do stack probes in fns with a stack frame of 1k or bigger
<re_irc>
<@ryan-summers:matrix.org> Yeah that's definitely an approach
<re_irc>
<@dirbaio:matrix.org> the issue is probetack is not supported in cortex-m
<re_irc>
<@ryan-summers:matrix.org> But doesn't save you _all_ the time, just on big allocs
<re_irc>
<@ryan-summers:matrix.org> I went about poking in LLVM a few years back and it didn't seem hard to add
<re_irc>
<@ryan-summers:matrix.org> Probably still have the patch laying around. Got lazy after I wrote it up and realized all the code instruction overhead it would entail
<re_irc>
<@dirbaio:matrix.org> : no, it's guaranteed to catch all stack overflows
<re_irc>
<@dirbaio:matrix.org> frames small than 1k WILL hit the 1k guard page for sure, without having to probe
<re_irc>
<@ryan-summers:matrix.org> Ah I misunderstood you, didn't realize you'd have a frame
<re_irc>
<@ryan-summers:matrix.org> +guard
<re_irc>
<@dirbaio:matrix.org> frames bigger than 1k might or might not, the stack probing is needed to make them hit it guaranteed
<re_irc>
<@ryan-summers:matrix.org> My impl was guard frame-less originally, but that requires a probe on every fn call
<re_irc>
<@dirbaio:matrix.org> what does the probe do?
<re_irc>
<@ryan-summers:matrix.org> It's a function stub that you provide
<re_irc>
<@ryan-summers:matrix.org> But essentially, it's a pointer comparison
<re_irc>
<@ryan-summers:matrix.org> I.e. you get the current stack pointer and the total size of allocations for the current function. The intent of the stackprobe is to do memory accesses every N bytes to trigger a memory access fault
<re_irc>
<@ryan-summers:matrix.org> But you can coopt it to do whatever you want on fn calls from what I remember
<re_irc>
<@rmja:matrix.org> Previously, before in my C firmware code, I did something like: "Write a known pattern on the top of the stack". Then schedule on a regular interval a "monitoring task" that validates that this pattern is still present, if it is, then clear the watchdog. This does not protect against an overflow, but allows for it to be detected during development.
<re_irc>
<@ryan-summers:matrix.org> So could i.e. just check if the proposed maximum SP would exceed memory and trigger a fault
<re_irc>
<@dirbaio:matrix.org> : but to trigger the memory access fault you need a guard page
<re_irc>
<@ryan-summers:matrix.org> Not if you just do something like a SWI
<re_irc>
<@ryan-summers:matrix.org> : I do this for a medical implant as well, it's a pretty solid method
<re_irc>
<@ryan-summers:matrix.org> Or something approximately similar
<re_irc>
<@dirbaio:matrix.org> SWI how?
<re_irc>
<@ryan-summers:matrix.org> With probestack, it literally calls a function "__rust_probestack"whenever the stack alloc is over N bytes. So you could write a "__rust_probestack" function that takes the current alloc size + current SP, and compare to end of memory
<re_irc>
<@ryan-summers:matrix.org> Then call the "SWI" from __rust_probestack if that happened
<re_irc>
<@ryan-summers:matrix.org> This is _not_ what "probestack"is intended for, but it's intended for virtual paging, not bare metal
<re_irc>
<@rmja:matrix.org> : Have you done it as an integrated part of the app, or do you use a crate?
<re_irc>
<@dirbaio:matrix.org> ah okay, so literally compare the SP pointer value. I see
<re_irc>
<@ryan-summers:matrix.org> It's in C, so not part of a crate sadly
<re_irc>
<@ryan-summers:matrix.org> Yeah, could be better ways to literally do a true stackprobe, but that would essentially just be flip-link
<re_irc>
<@dirbaio:matrix.org> you confused me with "do memory accesses every N bytes" :D
<re_irc>
<@ryan-summers:matrix.org> And that method isn't valid unless you do the memory reorg that flip-link does, otherwise you're probing into const / global memory
<re_irc>
<@ryan-summers:matrix.org> It's because the "stackprobe" function is intended for i.e. linux, where you get a virtual stack and need to ensure that someone remains within it. There were exploits where you could alloc a specially sized object and absolutely obliterate the stack, but get yourself into another proc's memory space
<re_irc>
<@dirbaio:matrix.org> sure sure
<re_irc>
<@ryan-summers:matrix.org> So the "probestack" function enforced an access at N bytes to force functions to hit the guard memory between process stacks to prevent that
<re_irc>
<@ryan-summers:matrix.org> We obviously don't have MPUs and fancy memory coprocessors on baremetal to do that for us unfortunately
<re_irc>
<@dirbaio:matrix.org> but you either
<re_irc>
- setup a guard page of N bytes, make stackprobe do dummy writes every N bytes
<re_irc>
- make stackprobe check the SP pointer value
<re_irc>
or
<re_irc>
<@ryan-summers:matrix.org> Yeah, correct
<re_irc>
<@ryan-summers:matrix.org> I think it's just an access, not necessarily a write
<re_irc>
<@dirbaio:matrix.org> : most cortex-m's _do_ have an MPU
<re_irc>
<@ryan-summers:matrix.org> Yeah a simple single-section MPU would be sufficient for this
<re_irc>
<@ryan-summers:matrix.org> Didn't think about that
<re_irc>
<@ryan-summers:matrix.org> So maybe probestack _could_ be very useful for embedded
<re_irc>
<@dirbaio:matrix.org> it would, the only missing piece is adding support for probestack on thumb
<re_irc>
<@dirbaio:matrix.org> 🥲
<re_irc>
<@dirbaio:matrix.org> and make N configurable... on desktop x86/arm it just hardcodes the 4k page size, but on embedded you likely want a smaller guard page to not waste so much RAM...
<re_irc>
<@ryan-summers:matrix.org> I can take a look at my LLVM patch again, I didn't realize it wasn't supported
<re_irc>
<@ryan-summers:matrix.org> It's not too much from what I recall
<re_irc>
<@dirbaio:matrix.org> 👀
<re_irc>
<@ryan-summers:matrix.org> Probably don't have time today, but will look tomorrow, feel free to ping me a reminder
<re_irc>
I didn't find anything similar for thumb.
<re_irc>
<@jannic:matrix.org> Depends on 4 not-yet-reviewed LLVM patches.
<re_irc>
<@dirbaio:matrix.org> ...patches from 2021 💀
<re_irc>
<@ryan-summers:matrix.org> Yeah LLVM's process is one of the reasons I never submitted my patch
<re_irc>
<@ryan-summers:matrix.org> And then you've got to wait for LLVM to get released, and then Rust to start using that version
<re_irc>
<@ryan-summers:matrix.org> So it's a long flow to actually get it landed in Rust
sauce has quit [Remote host closed the connection]
sauce has joined #rust-embedded
<re_irc>
<@adamgreig:matrix.org> hi @room, meeting time again! agenda is https://hackmd.io/YLUVfAunTZSG2RdS7MoK9Q, please add anything you'd like to discuss/announce and we'll start in a few minutes
<re_irc>
<@adamgreig:matrix.org> that's all the announcements i had, did anyone have anything else?
<re_irc>
<@burrbull:matrix.org> russians destroyed Kahovka dum on Dnipro river
<re_irc>
<@tempoanon:matrix.org> manga > anime
<cr1901>
It's primary day in NJ
<cr1901>
So I may not be here for the whole meeting
<re_irc>
<@thejpster:matrix.org> A quick one on the Leadership Council - it's forming, slowly. Trying to schedule our first meeting atm. I will reach out to all the leads of my sub-teams soon.
<re_irc>
<@jannic:matrix.org> Ticket sale for CCCamp starts immedately after the meeting.
<re_irc>
<@thejpster:matrix.org> (drawing a big diagram trying to make sense of the project at the moment)
<re_irc>
<@adamgreig:matrix.org> thanks
<re_irc>
<@adamgreig:matrix.org> first up on the agenda is a brief note about bors, svd and svd2rust repos have also moved over to ghmq and seem fine so far, I guess we'll keep chugging away at that
<re_irc>
<@adamgreig:matrix.org> if anyone has anything else to discuss about that we can talk about it briefly now, otherwise continue on the issue thread
<re_irc>
<@adamgreig:matrix.org> OK, next up was the cortex-m-rt hardfault trampoline, https://github.com/rust-embedded/cortex-m/pull/476 by , we discussed last week making it an argument to the exception macro instead of a cargo feature, and I think that's still the intended direction
<re_irc>
<@diondokter:matrix.org> Yeah I made a comment on the PR with the discussed direction. Seems pretty doable. So if everybody agrees, I'll try to find some time to change the PR
<re_irc>
<@adamgreig:matrix.org> sounds good, thanks! it's a shame the cargo feature doesn't work as it is very simple but it does seem annoyingly compelling that it's not a good approach
<re_irc>
<@adamgreig:matrix.org> we discussed the PR to svd2rust adding per-peripheral steal() methods, the pr was closed and re-opened as https://github.com/rust-embedded/svd2rust/pull/723, but there wasn't much support for it last week
<cr1901>
I don't want to commit to per-peripheral steal
<cr1901>
And there seems to be misgivings about steal period
<re_irc>
<@adamgreig:matrix.org> indeed
<re_irc>
<@adamgreig:matrix.org> since we do already have "Peripherals::steal()", a per-peripheral "steal()" doesn't really change the picture in terms of what's supported, makes stealing one peripheral a bit more convenient at the cost of encouraging doing it a bit more
<re_irc>
<@adamgreig:matrix.org> so I dunno on balance, but it doesn't seem very compelling
<re_irc>
<@burrbull:matrix.org> He wants to "steal" UART twice for RX and TX. Is this UB?
<re_irc>
<@newam:matrix.org> I think the more compelling part of it is that it is zero-cost (I think the "Peripherals::steal" sets the static variable still)
<re_irc>
<@adamgreig:matrix.org> I don't think stealing twice is _necessarily_ UB, especially if you only read/write different registers on each half (and possibly even if you didn't)
<re_irc>
<@adamgreig:matrix.org> the object it returns Derefs to &RegisterBlock, which is a shared reference so OK to have multiple of them
<re_irc>
<@thejpster:matrix.org> I made the point in the issue that you can just conjure up a pointer, and it's basically the same thing, and as you are breaking the ownership system for peripherals, what you are doing shouldn't be easy.
<re_irc>
<@adamgreig:matrix.org> the not setting the static variable is a functional difference to Peripherals::steal() but it's not clear if this method should also set it really (or why Peripherals::steal doesn't...)
<re_irc>
<@adamgreig:matrix.org> you can just conjure up the peripheral struct itself too if you want, you can transmute a unit tuple
<re_irc>
<@adamgreig:matrix.org> some people need/want a struct instead of the RegisterBlock pointer though
<cr1901>
The fact that the per peripheral steal doesn't set the static variable is one reason I'm against it; harder to reason about
<re_irc>
<@adamgreig:matrix.org> true
<re_irc>
<@adamgreig:matrix.org> I don't know why Peripherals::steal() sets the flag either though :P
<re_irc>
<@thejpster:matrix.org> so you're saying the boolean flag should be a "taint" flag, as in, "someone bypassed ownership so all bets are off"
<re_irc>
<@adamgreig:matrix.org> that's sort-of-kind-of what it is now, if you call steal() it gets set so you can't later take() them
IlPalazzo-ojiisa has quit [Quit: Leaving.]
<re_irc>
<@adamgreig:matrix.org> but of course you could just transmute a Peripherals out of thin air and the flag wouldn't get set, and it's the same "unsafe" either way
<cr1901>
adamgreig: I've relied on steal setting the flag in C/Rust interop
<re_irc>
<@thejpster:matrix.org> and you can also steal them after they've been taken
<cr1901>
But I don't have a concrete example handy immediately
<re_irc>
<@dirbaio:matrix.org> you can always steal with "mem::transmute(())"
<re_irc>
<@jannic:matrix.org> : That can also be a liability. Eg if the steal() happens in an interrupt (not so unusual) that may get triggered in rare circumstances before the take() in main.
<cr1901>
adamgreig: Choosing between whether I opt into take-based abstractions or an alternate API meant for C interop
<re_irc>
<@adamgreig:matrix.org> steal() is already unsafe, and once someone's using unsafe code they can also use transmute to steal without setting the flag, so I dunno how reliable it is anyway
<re_irc>
<@adamgreig:matrix.org> ah, so you check if the flag is already taken at runtime and then behave differently?
<cr1901>
yes
<re_irc>
<@dirbaio:matrix.org> cr1901: but the user can do take(), and _later_ steal
<re_irc>
<@adamgreig:matrix.org> how do you check, by trying to take() and see if you got some/none?
<cr1901>
>cr1901> but if you've take() successfully, you can also create abstractions with can steal w/o unsoundness
<cr1901>
if the take() doesn't succeed, you've already stolen
<cr1901>
adamgreig: Yes
<re_irc>
<@adamgreig:matrix.org> what difference does it make if the stealing happened before or after?
<cr1901>
You can pass the take()n Peripherals in as a token that you've took them safely
<cr1901>
You can pass the take()n Peripherals in as a token to a function with steals- i.e. proof that you've took them safely*
<cr1901>
Or opted into unsafety and are upholding the variants some other way
<cr1901>
Hmmm, maybe this falls apart
<re_irc>
<@thejpster:matrix.org> I guess the question is, is "Peripherals::steal().UART0" better than "Uart0::steal()"?
<cr1901>
Hmmm, I guess I could rework relying on Peripherals::steal() setting a global
<re_irc>
<@thejpster:matrix.org> Or should you be forced to "struct MyUnsafeUart(*mut uart0::RegisterBlock)" or some ZST version of that?
<cr1901>
adamgreig: Sorry, I don't remember all the gory details, other than "yes I've relied on the internal details of steal() before".
<cr1901>
Also keep in mind that take() is written in terms of steal() :D
<re_irc>
<@adamgreig:matrix.org> : right, given most uses of Peripherals::steal() are just for later getting a single peripheral out of it, should we make that a bit easier and perhaps less overhead
<re_irc>
<@adamgreig:matrix.org> cr1901: interesting, ok
<re_irc>
<@dirbaio:matrix.org> I'm for making stealing easy
<re_irc>
<@dirbaio:matrix.org> people are stealing so much because the ownership model is not fulfilling people's needs
<re_irc>
<@dirbaio:matrix.org> so make stealing easy
<re_irc>
<@thejpster:matrix.org> I was initially for making stealing hard, but given how much stealing is going on, I may need to concede on this
<re_irc>
<@dirbaio:matrix.org> or remove the ownership model
<re_irc>
<@adamgreig:matrix.org> I'm not sure that steal not setting the flag would change your API though, if the user has passed in a Peripheral token either they got it through take() once, or they unsafely stole it, either way they have it now
<re_irc>
<@dirbaio:matrix.org> literally every single HAL is stealing peripherals
<re_irc>
<@thejpster:matrix.org> Are Peripherals Send?
<re_irc>
<@adamgreig:matrix.org> yea, I think moving ownership out of PACs is better long term
<cr1901>
yes Peripherals are Send
<cr1901>
japaric designed the take-based API. Can anyone get in contact w/ them at all?
<cr1901>
And ask if they remember the rationale?
<re_irc>
<@thejpster:matrix.org> So if you conjure up (better than stealing) two UARTs, you can totally break read-modify-write registers?
<re_irc>
<@thejpster:matrix.org> I work with them
<re_irc>
<@thejpster:matrix.org> I can ask
<cr1901>
(Has _anyone_ had contact w/ japaric?)
<cr1901>
Ahhh
<re_irc>
<@dirbaio:matrix.org> also, I'm pretty sure RMW races on volatile registers are _not_ UB
<re_irc>
<@thejpster:matrix.org> We keep japaric busy doing lots of non-embedded stuff I'm afraid.
<cr1901>
:(
<re_irc>
<@adamgreig:matrix.org> you could have unsynchronised rmw accesses to registers, yea
<re_irc>
<@dirbaio:matrix.org> * UB, contrary to popular wisdom
<re_irc>
<@thejpster:matrix.org> : maybe not, but they can also cause surprising breakage
<re_irc>
<@dirbaio:matrix.org> yeah
<re_irc>
<@dirbaio:matrix.org> but HALs are already stealing e.g GPIO regs and doing RMWs on them
<re_irc>
<@adamgreig:matrix.org> but that's why it's behind "unsafe", you could do that anyway with "ptr::write_volatile"
<re_irc>
<@dirbaio:matrix.org> using critical section or similar to avoid races
<re_irc>
<@thejpster:matrix.org> and the point of the ownership system for peripherals is, I think, to avoid surprising breakage
<cr1901>
As long as the register isn't bigger than word width, it's not UB in the sense of a tearing read/write
<cr1901>
But still probably undesirable
<re_irc>
<@thejpster:matrix.org> > but HALs are already stealing e.g GPIO regs and doing RMWs on them
<re_irc>
I really hope not. Surely the peripheral has either bit-banding or one register per pin?
<cr1901>
(But also, is two threads incrementing a <= word width register without synchronization UB?)
<re_irc>
<@thejpster:matrix.org> 😭
<re_irc>
<@dirbaio:matrix.org> somtimes they don't. Many stm32's don't
<re_irc>
<@adamgreig:matrix.org> but no, it's extremely common for them not to
<cr1901>
(That is _technically_ a data race, just not tearing)
<re_irc>
<@adamgreig:matrix.org> cr1901: I'm not sure any more. I think it depends on how the memory model gets defined in the end. "data races" are UB
<re_irc>
<@dirbaio:matrix.org> the ownership system forces HALs to either have very unergonomic APIs (e.g. take reg refs like "&mut moder, &mut otyper" every time you want to configure a GPIO), or use steal+CS
<re_irc>
<@adamgreig:matrix.org> but simultaneous writes to word-sized memory are atomic on all the platforms we're interested in so can't end up with arbitrary values, it's only the first value or one of the two values that got written to it, sooo is it UB? not sure
<re_irc>
<@dirbaio:matrix.org> it's not
<cr1901>
adamgreig: Just worth keeping in mind, IMO. In my view, it's a data race, but not tearing. So can it lead to seeing "illegal" state, rather than just "undesirable"?
<re_irc>
<@adamgreig:matrix.org> if you call it a data race then it's UB by definition, but I think the argument is that it's not really a data race
<re_irc>
<@thejpster:matrix.org> : But now I worry HALs are using steal without a CS
<re_irc>
<@adamgreig:matrix.org> you won't see values that weren't written to the register at some point though
<re_irc>
<@adamgreig:matrix.org> : that's allowed too if they know there's no simultaneous access e.g. by using atomic write-only registers, or a register per pin, or bitbanding, or something else
<re_irc>
<@adamgreig:matrix.org> but it's up to the hal to do something correct
<re_irc>
<@dirbaio:matrix.org> "volatile write" is specified to compile to a STR
<re_irc>
"volatile read" is specified to compile to a LDR
<re_irc>
so "volatile read, modify, volatile write" can give a wrong result if it races, but not UB
<re_irc>
<@dirbaio:matrix.org> yeah, there's tons of other ways HALs can screw up register access anyway
<cr1901>
Yes, "wrong result" is what I mean by "undesirable"
<re_irc>
<@dirbaio:matrix.org> also there's legit applications of doing RMWs without CSs
<re_irc>
<@dirbaio:matrix.org> for example, main thread can do a RMW on one register without CS, while interrupt accesses a disjoint set of regs
<re_irc>
<@dirbaio:matrix.org> or interrupt accesses atomic "write 1 to clear" interrupt flag regs
<re_irc>
<@dirbaio:matrix.org> forcing the HAL to do CSs is unnecessary overhead
<re_irc>
<@adamgreig:matrix.org> yea, all things the HAL can know about and the PAC cannot
<re_irc>
<@dirbaio:matrix.org> embassy does RMWs like that in a lot of places
<re_irc>
<@thejpster:matrix.org> OK, so if the world's leading expert on Rust PACs and HALs can easily blow an hour discussing the details, I posit that maybe we don't have enough docs to help people using raw PACs and/or writing their own HALs
<re_irc>
<@thejpster:matrix.org> * experts
<re_irc>
<@thejpster:matrix.org> I also have to run for another call
<re_irc>
<@dirbaio:matrix.org> I don't think anybody knows what's _the_ best way to write a HAL yet
<cr1901>
I would say make an issue w/ the unsafe working group re: "are undesirable read/writes considered data races"
<re_irc>
<@dirbaio:matrix.org> :D
<re_irc>
<@thejpster:matrix.org> but we know a lot of ways to do it badly
<re_irc>
<@thejpster:matrix.org> we just have to run out of them 🤷
<re_irc>
<@dirbaio:matrix.org> lol!!
<cr1901>
where "undesirable" is defined as "two threads write to the same <= word width register without synchronization"
<re_irc>
<@adamgreig:matrix.org> alas, creativity is boundless :P
<re_irc>
<@adamgreig:matrix.org> cr1901: I'll need to dig it up but I think it's been discussed there before
<cr1901>
Would appreciate a link if you can find it, immensely
<re_irc>
<@adamgreig:matrix.org> anyway, we're out of time for this week, but I feel like consensus has come around to probably having per-peripheral steal() that doesn't set the static?
<re_irc>
<@dirbaio:matrix.org> and actually i've found some of the rust-embedded "best practice" docs we have today are somewhat harmful
<re_irc>
<@dirbaio:matrix.org> like "GPIO types must have typestates"...
<cr1901>
I'm still against per-peripheral steal
<re_irc>
<@dirbaio:matrix.org> so i'd be wary to document more "best practices"
<re_irc>
<@dirbaio:matrix.org> because these change fast
<re_irc>
<@dirbaio:matrix.org> so i'd be wary of documenting more "best practices"
<re_irc>
<@adamgreig:matrix.org> indeed. the docs for hal authors is "don't have bugs" basically, huh?
<re_irc>
<@adamgreig:matrix.org> you can directly access peripherals all over, and have to, and what you have to do is use whatever your specific chip provides to ensure correctness
<re_irc>
<@adamgreig:matrix.org> but the specific means are so different on different platforms
<cr1901>
"which may be more guarantees than Rust the lang provides :)"
<re_irc>
<@adamgreig:matrix.org> almost certainly they will be, because it's almost all stuff outside the rust model
<cr1901>
adamgreig: I have to go to, but my final decision is "I'm against committing to per-peripheral steal right now, in case steal setting a global _is_ there for a good reason and we've collectively forgotten (bad!)"
<cr1901>
So it'll have to be potentially removed b/c of unsoundness/impossibility to use correctly
<cr1901>
I'd feel better if we'd ask japaric for their rationale. Hopefully they remember
<cr1901>
(Disagreement is healthy :P)
<re_irc>
<@adamgreig:matrix.org> OK, thanks, let's see if we can hear from I guess
<re_irc>
<@dirbaio:matrix.org> I don't think there's any strong reason...
<re_irc>
<@adamgreig:matrix.org> my suspicion is there's no longer a good reason for it, yea
<cr1901>
Well, just humor me. If they say there was no good reason, then fine. Add per-peripheral steal and let the community figure out whether the API is sound or not. They'll figure it out :).
<cr1901>
(And if it's unsound, chances are it won't be horrific breakage. I'm just risk-averse here.)
<re_irc>
<@adamgreig:matrix.org> rust says a data race is ub and is whenever one thread writes while another thread performs any unsynchronised access, but as usual the devil is in the details
<re_irc>
<@adamgreig:matrix.org> I think in practice because racing rmw access to mmio does not impact memory safety, only program correctness, it's probably not going to enter ub territory and it's just wrong (and racey and annoying)
<re_irc>
<@adamgreig:matrix.org> which I guess is 's understanding
<re_irc>
<@adamgreig:matrix.org> but it's somewhat immaterial to whether we can have steal() in pacs, because that's an unsafe function and the user has to hold it right
<re_irc>
<@adamgreig:matrix.org> having steal() alone doesn't cause ub, and per-peripheral steal is no different to all-peripheral steal in that case
<re_irc>
<@grantm11235:matrix.org> : btw, do you have a citation for that?
<re_irc>
<@dirbaio:matrix.org> no
<re_irc>
<@dirbaio:matrix.org> but if that's not true then EVERYTHING is broken
<re_irc>
<@adamgreig:matrix.org> because it's not specified, right :P
<re_irc>
<@dirbaio:matrix.org> for example if Rust could compile "u32 volatile write" to STRH+STRH
<re_irc>
<@dirbaio:matrix.org> many peripherals wouldn't like it
<re_irc>
<@dirbaio:matrix.org> because they require "single 32bit read"
<re_irc>
<@dirbaio:matrix.org> yeah I don't think it's actually specified
<re_irc>
<@grantm11235:matrix.org> "everyone needs it to work like this, and there is no reason to change it" is the next best thing to a specification though
<re_irc>
<@dirbaio:matrix.org> C11:
<re_irc>
6.7.3.6: [...] If an attempt is made to refer to an object defined with a volatile-qualified type through use of an lvalue with non-volatile-qualified type, the behavior is undefined.133)
<re_irc>
6.7.3.7: An object that has volatile-qualified type may be modified in ways unknown to the implementation or have other unknown side effects. Therefore any expression referring to such an object shall be evaluated strictly according to the rules of the abstract machine, as described in 5.1.2.3. Furthermore, at every sequence point the value last stored in the object shall agree with that prescribed by the abstract machine, except as modified by...
<re_irc>
... the unknown factors mentioned previously.134) What constitutes an access to an object that has volatile-qualified type is implementation-defined.
<re_irc>
<@dirbaio:matrix.org> I think it's "implementation defined"
Foxyloxy has joined #rust-embedded
<re_irc>
<@dirbaio:matrix.org> volatile accesses are indeed not required to be atomic
<re_irc>
<@dirbaio:matrix.org> but in practice the implementations emit
<re_irc>
LDRH/STRH for 16bit volatile access
<re_irc>
LDR/STR for 32bit volatile access
<re_irc>
LDRB/STRB for 8bit volatile access
<re_irc>
<@dirbaio:matrix.org> so these _are_ atomic
<re_irc>
<@dirbaio:matrix.org> now, where's the document from the implementation (llvm?) promising that implementation-defined behavior is guaranteed to be like that forever, I dunno
<re_irc>
<@dirbaio:matrix.org> but it's extremely unlikely to change. If it did, all embedded code would break
<re_irc>
<@dirbaio:matrix.org> I think a lot of the "volatile doesn't avoid data races" fear comes from the fact that volatile doesn't establish happens-before relationships between threads
<re_irc>
<@dirbaio:matrix.org> ie if thread A volatile-writes 42 to some memory location, then thread B volatile-reads 42 from that location. that doesn't guarantee thread B now sees all the writes thread A did before
<re_irc>
<@dirbaio:matrix.org> vs atomic load/store with the right orderings would
<re_irc>
<@dirbaio:matrix.org> ie you can't use volatile to build a sound Mutex
<re_irc>
<@dirbaio:matrix.org> ie if thread A volatile-writes 42 to some memory location, then thread B volatile-reads 42 from that location. that doesn't guarantee thread B now sees all the _other_ memory writes thread A did before
<re_irc>
<@dirbaio:matrix.org> it would be UB if you did
<re_irc>
<@dirbaio:matrix.org> but that doesn't mean a volatile RMW is UB _by itself_, ie you're not using it to synchronize other memory locations
<re_irc>
<@dirbaio:matrix.org> +if
<re_irc>
<@dirbaio:matrix.org> and given this, it's not
<re_irc>
<@dirbaio:matrix.org> that's my interpretation at least
<re_irc>
<@dirbaio:matrix.org> > using this pattern with raw pointers is fine, but *with references it is not currently fine*
<re_irc>
<@d3zd3z:matrix.org> This isn't exactly just an embedded question, but since one of my possible solutions involves allocation, I thought I'd ask here. I have a struct "Foo" that is used to process/iterate a data structure in Storage. The read method on "ReadStorage" needs a "&mut Self" argument. Right now, I'm not storing that in my "Foo", and just passing in an extra argument. But, if I want to implement a real "Iterator", I would somehow need...
<re_irc>
... to capture. I could make my iterator type general, and store a "&mut R" in the struct. This has the disadvantage that it would make the storage device inaccessible for the lifetime of the iterator. Other thoughts would be "Rc<Cell<R>>", but that starts to get messy. Any suggestions from people that have encountered this before?
<re_irc>
<@dirbaio:matrix.org> you can often get away with just RefCell, not Rc<RefCell<T>>. Making the iterator own a "&RefCell".
<re_irc>
<@dirbaio:matrix.org> other than that... yes, there's not much magic you can do. either require exclusive access, or use refcell/mutex
<re_irc>
<@dirbaio:matrix.org> requiring exclusive access can be seen as a _feature_ though
<re_irc>
<@dirbaio:matrix.org> what if some other code modifies that data structure while you're iterating it? can the iterator break?
<re_irc>
<@dirbaio:matrix.org> requiring exclusive access prevents that
dc740 has quit [Remote host closed the connection]
<re_irc>
<@d3zd3z:matrix.org> Well, these images are not really mutable. A separate part of the program might replace it with something entirely. But, maybe it is fine to require exclusive access. My current "iterator" is a function that takes a closure that is called on each entry, and allowing the user to at least have control would be better than that.
dc740 has joined #rust-embedded
<re_irc>
<@d3zd3z:matrix.org> But, I think I'll just pass in either the &RefCell or the &mut R.
<re_irc>
<@larunite:matrix.org> Maybe I'm misunderstanding the question but if the underlying data isn't mutable, why does the iterator require mutable access to it?
<re_irc>
<@dirbaio:matrix.org> "ReadStorage" might be reading from e.g. an SPI flash
<re_irc>
<@dirbaio:matrix.org> so it requires &mut access to the SPI bus to do the transfers on it to carry out the read
<re_irc>
<@dirbaio:matrix.org> which is faster because you do the read 1 time, not N
<re_irc>
<@dirbaio:matrix.org> but note it introduces a "speculative read": the optimized code will _always_ read val, vs the previous code would only read it if n != 0
<re_irc>
<@dirbaio:matrix.org> this is valid thanks to rust's "references must always be valid" guarantee
<re_irc>
<@dirbaio:matrix.org> but it means you're gonna have a bad time if you use references to point to MMIO registers, like "&VolatileCell<u32>"
<re_irc>
<@dirbaio:matrix.org> because the compiler can insert reads anytime it wants
<re_irc>
<@dirbaio:matrix.org> which is bad because reading MMIO regs sometimes has side effects, like popping a value from a FIFO
<re_irc>
<@dirbaio:matrix.org> so the question in that issue is "do we add some exception to the memory model to forbid speculative reads in some cases, to support VolatileCell?"
<re_irc>
<@dirbaio:matrix.org> and my view is "no, we don't need VolatileCell"
<re_irc>
<@dirbaio:matrix.org> * VolatileCell, so don't bother"
<cr1901>
Ahhh, interesting. Thanks for the simple example. Will need to think about it
<cr1901>
My experience w/ Rust is that I try to avoid pointers, b/c Rust really wants to work w/ refs in the safe APIs b/c of reference's guarantees. And I find it too stupidly easy to create multiple &mut by accident when dealing w/ pointers.
<re_irc>
<@d3zd3z:matrix.org> I got it working with "struct Foo<R> { ..., flash: RefCell<R> }", I then added an "into_storage" method to pull the storage cell out of it when done using it.
<re_irc>
<@grantm11235:matrix.org> : Your example demonstrates another downside of the volatilecell approach, it's too easy to forget "#[repr(C)]" 😬
<re_irc>
<@dirbaio:matrix.org> lol!!!
<re_irc>
<@dirbaio:matrix.org> indeed
<cr1901>
why is repr(C) needed? field reordering?
<re_irc>
<@grantm11235:matrix.org> yup
<cr1901>
Guess I never felt the need to access a register using something that wasn't an unsigned int or something wrapped w/ #[repr(transparent)]
<re_irc>
<@grantm11235:matrix.org> If I recall correctly, there was an instance of that bug in the wild recently
dc740 has quit [Remote host closed the connection]