all: prototype of RAM persistence across reset #1715

kenbell · 2021-03-14T15:31:58Z

See issue #1716 which proposes adding persistence regions to TinyGo

ardnew

This is really neat! Only a few questions/comments added

ardnew · 2021-03-14T20:19:26Z

src/runtime/persistence/persistence.go

+	// SubAllocation creates a new persistence object that
+	// represents a sub-allocation of the main persistence
+	// area
+	SubAllocation(offset int, len int) (p Region, err error)


What is the imagined use-case of this? Seems like it could be left out (for now) to simplify the interface until implementations have been fleshed out and tested.

It seems like more of a convenience that isn't truly essential for this initial eval.

I was thinking along the lines of slices in Go - being able to divide the space up, so parts can be handed off to different libraries without risk of bounds overflow.

ardnew · 2021-03-14T20:30:26Z

compiler/ramallocation.go

+
+func (b *builder) createRAMAllocation(instr *ssa.CallCommon) error {
+	// Get the size, which must be a compile-time constant.
+	size, ok := instr.Args[0].(*ssa.Const)


It could be really useful to be able to pass in a type and determine the size to allocate accordingly.

This would let users define persistent variables; when their type definition changes, the reserved region grows/shrinks accordingly, without having to redo any arithmetic and update the NewRAM call separately (or risk forgetting to do so).

Just an idea, which may be a bit more elaborate than necessary for now.

Example use case:

type Config struct {id int} func main() { p := persistence.NewRAM(Config) // would be cool to either re-use the `make` builtin somehow, or create an equivalent: // p := persist(Config) // q := persist([]rune, 1024) }

That may sound like a good idea, but has some issues in practice. For example: you can't put pointers in this area (and thus no slices, strings, etc.) because they live on the heap which is reinitialized on reset.

This issue details why this is a bad idea for a very similar use case on server hardware: golang/go#43810

Yeah - i think serialization needs to be explicit.

ardnew · 2021-03-14T20:48:45Z

targets/arm.ld

+_persist_start = _spersist;
+_persist_end = _epersist;


Can you not just define these above in place of _spersist and _epersist? There doesn't appear to be anything else using these symbols, so I suggest getting rid of them.

I was just aiming to follow the pattern in the file - i'm more than happy to just use _spersist, _epersist. It would work fine.

ardnew · 2021-03-14T20:54:53Z

targets/arm.ld

+        *(.persist)
+        *(.persist.*)


I've never understood the convention with these includes, are they both necessary? Wouldn't *(.persist*), or, in this case, simply *(.persist) be sufficient?

It's simply a glob. So .persist matches exactly the string .persist while .persist.* matches anything that starts with .persist.. This emulates somewhat the similar conventions around -ffunction-sections and -fdata-sections but may not be necessary in this case.

aykevl

While I think this is an interesting idea, I have a few concerns with this implementation.

I do not think we should be mixing RAM and flash here. While reading RAM and flash is usually similar (just read as any normal pointer), writing most definitely is not. For example: in flash, usually a whole page (1024 bytes, 4069 bytes, or even bigger) needs to be erased at a time.
I don't really see the purpose of the special Region type? Why not return a byte slice from NewRAM?
Why shouldn't libraries allocate persistent RAM?
How can an application detect whether the memory area has been initialized or not? Should it care?
What about compatibility with the Go toolchain?

Especially the last point is important to me. I think it is fairly simple to make this compatible with the Go toolchain by doing something similar to the new file embedding support in Go 1.16. I think the API could be simplified to the following:

//go:persistent
var somePersistentMemory [100]byte

The semantics could for example be: the memory area is reset to zero with a power-on reset but otherwise left as-is (for example, with a wake up from deep sleep). In the Go toolchain the variable is always zero initialized on startup.
That requires detecting a power-on reset on every supported chip. However, I think this feature is almost useless without a way to know whether some data has been written to memory or not: RAM is not necessarily all zeroes on startup, it's more likely to be random data.

Overall, I'm curious whether you have a use case in mind? You've mentioned a few examples where it could be useful, but few practical examples that currently apply to TinyGo. It really helps to know how it will be used to shape what the API looks like.

aykevl · 2021-03-14T21:56:30Z

compiler/ramallocation.go

+
+func (b *builder) createRAMAllocation(instr *ssa.CallCommon) error {
+	// Get the size, which must be a compile-time constant.
+	size, ok := instr.Args[0].(*ssa.Const)


That may sound like a good idea, but has some issues in practice. For example: you can't put pointers in this area (and thus no slices, strings, etc.) because they live on the heap which is reinitialized on reset.

This issue details why this is a bad idea for a very similar use case on server hardware: golang/go#43810

aykevl · 2021-03-14T21:59:25Z

compiler/ramallocation.go

+	// _persist$buf is the allocated space.
+	bufType := llvm.ArrayType(llvm.Int8Type(), int(size.Int64()))
+	buf := llvm.AddGlobal(b.mod, bufType, "_persist$buf")
+	buf.SetLinkage(llvm.ExternalLinkage)


Why external linkage? It should be internal linkage to help the optimizer.

I'll admit - i don't really know what the values do, I was bashing on values until one worked :)

I'll take another look.

aykevl · 2021-03-14T22:06:59Z

src/examples/persistence/main.go

+)
+
+func main() {
+	p := persistence.NewRAM(uint32(unsafe.Sizeof(byte(0)) * 100))


You can assume unsafe.Sizeof(byte) is always 1. Doing something different would break the Go language specification: https://golang.org/ref/spec#Size_and_alignment_guarantees

I have absolutely no plans of supporting architectures that do not support byte addressing. There is hardware that doesn't support byte addressing, but it's all very special purpose (DSPs etc.). And even if TinyGo would support this hardware, it would still follow the Go specification (whatever that might look like in practice).

Yeah - you're right, that example is excessively complex - should just be 'i need a region of X bytes'

kenbell · 2021-03-14T23:31:15Z

I do not think we should be mixing RAM and flash here.

Reading your thoughts, I think you're right.. There's enough different between them that trying to pretend they're the same will add a lot of complexity without much (if any) clear benefit.

I don't really see the purpose of the special Region type? Why not return a byte slice from NewRAM?

I wasn't sure how that would interact with the GC. It would be a lot cleaner if that was the interface. I'll investigate.

Why shouldn't libraries allocate persistent RAM?

There's an ordering constraint I'm not sure how to fix. I don't think the compiler / linker chain guarantees the order symbols are packed into sections - so on a rebuild the layout could vary. For RAM, this probably isn't a concern (good chance the RAM is wiped anyway on a re-flash) - but for Flash, would be good to make the ordering explicit, which is why I left it to the app to allocate the data to libraries. I'm hoping we can have it so that on a reasonable sub-set of devices, flashing a new app will leave the config intact.

This may be another good reason to separate RAM and Flash, since letting libraries allocate persistent RAM on the understanding it will not be guaranteed (or even likely) to be preserved across re-builds / re-flashing seems a good approach.

How can an application detect whether the memory area has been initialized or not? Should it care?

I'm thinking magic values guarding the memory region, or some kind of CRC - would be use-case specific. If the guard values are wrong, or the CRC is invalid then assume uninitialized and do the initialization. STM32 does the same for RTC - it has a few bytes that are explicitly there for detecting if the RTC has been initialized - they document to set to a magic value of the developer's choosing after initializing the RTC.

What about compatibility with the Go toolchain?

I really like the idea of using []byte and //go:persistent - feels a lot more elegant. I'll take a look at trying to do that.

kenbell · 2021-03-14T23:48:28Z

Overall, I'm curious whether you have a use case in mind? You've mentioned a few examples where it could be useful, but few practical examples that currently apply to TinyGo. It really helps to know how it will be used to shape what the API looks like.

I've put a sketch of an prototypical app in the issue / proposal #1716, here (#1716 (comment))

These pragmas weren't really tested anywhere, except that some code might break if they are not properly applied. These tests make it easy to see they work correctly and also provide a logical place to add new pragma tests. I've also made a slight change to how functions and globals are created: with the change they're also created in the IR even if they're not referenced. This makes testing easier.

This patch adds a new pragma for functions and globals to set the section name. This can be useful to place a function or global in a special device specific section, for example: * Functions may be placed in RAM to make them run faster, or in flash (if RAM is the default) to not let them take up RAM. * DMA memory may only be placed in a special memory area. * Some RAM may be faster than other RAM, and some globals may be performance critical thus placing them in this special RAM area can help. * Some (large) global variables may need to be placed in external RAM, which can be done by placing them in a special section. To use it, you have to place a function or global in a special section, for example: //go:section .externalram var externalRAMBuffer [1024]byte This can then be placed in a special section of the linker script, for example something like this: .bss.extram (NOLOAD) : { *(.externalram) } > ERAM

deadprogram · 2023-04-04T09:34:03Z

Just wondering if this is actually now better addressed via the recent flash interface changes?

aykevl · 2023-04-04T11:17:22Z

No, I don't think the flash API is appropriate for this. Flash is very different from RAM.

I do quite like the design here: #1716 (comment)
I don't think the magic seed in that example is really necessary though: practically all MCUs provide a way to read the reset reason and therefore can know whether the reset was due to a power-on reset (RAM is gone) or a software reset (RAM is still available). We just need to make this available in the machine package.

@kenbell any plans to continue working on this? Perhaps start by rebasing on the dev branch.

kenbell · 2023-04-04T21:17:45Z

Yeah - I'd kinda forgotten about this. I'll take a look in the next few days.

kenbell mentioned this pull request Mar 14, 2021

Proposal: Provide persistence in RAM and Flash across low-power and reset states #1716

Open

ardnew approved these changes Mar 14, 2021

View reviewed changes

aykevl reviewed Mar 14, 2021

View reviewed changes

kenbell force-pushed the low-power-persistence branch from b344fd3 to bddb925 Compare April 6, 2021 02:51

kenbell mentioned this pull request Apr 6, 2021

//go:section pragma #1759

Merged

aykevl and others added 10 commits April 7, 2021 22:37

all: prototype of RAM persistence across reset

4d872d2

Smoketest persistence prototype

53567b5

change to using //go:section pragma

2d96c27

Update example

723ed69

Use chip-specific RAM for low-power persistence

3a19c02

Fix typo

ffef2f0

Update based on section pragma

9df8125

stm32l031k6 added

3e0e27c

kenbell force-pushed the low-power-persistence branch from bddb925 to 3e0e27c Compare April 8, 2021 05:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

all: prototype of RAM persistence across reset #1715

all: prototype of RAM persistence across reset #1715

kenbell commented Mar 14, 2021 •

edited

Loading

ardnew left a comment

ardnew Mar 14, 2021

kenbell Mar 14, 2021

ardnew Mar 14, 2021

aykevl Mar 14, 2021

kenbell Mar 14, 2021

ardnew Mar 14, 2021

kenbell Mar 14, 2021

ardnew Mar 14, 2021

aykevl Mar 14, 2021 •

edited

Loading

aykevl left a comment •

edited

Loading

aykevl Mar 14, 2021

aykevl Mar 14, 2021

kenbell Mar 14, 2021

aykevl Mar 14, 2021 •

edited

Loading

kenbell Mar 14, 2021

kenbell commented Mar 14, 2021

kenbell commented Mar 14, 2021

deadprogram commented Apr 4, 2023

aykevl commented Apr 4, 2023

kenbell commented Apr 4, 2023

		_persist_start = _spersist;
		_persist_end = _epersist;

all: prototype of RAM persistence across reset #1715

Are you sure you want to change the base?

all: prototype of RAM persistence across reset #1715

Conversation

kenbell commented Mar 14, 2021 • edited Loading

ardnew left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aykevl Mar 14, 2021 • edited Loading

Choose a reason for hiding this comment

aykevl left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aykevl Mar 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenbell commented Mar 14, 2021

kenbell commented Mar 14, 2021

deadprogram commented Apr 4, 2023

aykevl commented Apr 4, 2023

kenbell commented Apr 4, 2023

kenbell commented Mar 14, 2021 •

edited

Loading

aykevl Mar 14, 2021 •

edited

Loading

aykevl left a comment •

edited

Loading

aykevl Mar 14, 2021 •

edited

Loading