Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Tock Attributes #695

Closed
bradjc opened this issue Nov 27, 2017 · 13 comments
Closed

[RFC] Tock Attributes #695

bradjc opened this issue Nov 27, 2017 · 13 comments
Assignees
Labels
last-call Final review period for a pull request. rfc Issue designed for discussion and to solicit feedback.

Comments

@bradjc
Copy link
Contributor

bradjc commented Nov 27, 2017

Vision

High level goal: all binaries in a Tock system (bootloader, kernel, apps) have metadata stored at or near the beginning of their flash region. The metadata should be coupled with the binary it is relevant to.

Contents at each layer

  • bootloader: The bootloader metadata should contain two types of data:

    1. State that is immutable about the board. This includes its name, serial number, MCU, etc.
    2. State that the bootloader needs to know. This would include the location in flash of the kernel.
  • kernel: The kernel metadata holds certain parameters of the kernel that could change over the lifetime of the device and that the kernel needs to know after a power cycle. In particular, this would include the address where apps are located in flash. We use _sapps now, but a future system may want to place new apps later in flash and change which apps are active while not losing an older copy of the apps. Another attribute may be the git commit hash or the version of the compiled kernel.

  • apps: This is the content in the TBF header. It specifies the size of the app and where the first instruction is, among other things.

Advantages to this approach

  • There is clear parallelism where metadata is stored with each binary.
  • Because metadata is stored with the binary, it gets updated when the binary itself gets updated (e.g. we generate a new TBF header when we compile an app, and it gets loaded with the app).
  • Any future metadata has a clear place to go.
  • It is obvious how Tockloader gets access to any state that it needs.

Disadvantages to this approach

  • Compiling metadata in with the binary is tricky (ugly in C, really ugly in rust/Cargo).
  • We have an entirely different tool (elf2tbf) that generates the metadata for apps.
  • Might be clumsy for binaries to have to pull the metadata out of a header in its flash rather than having it in the code somehow.
  • Binaries may wish to store their metadata elsewhere rather than having to read/write the MCU flash.

Implementations

We currently have implementations for the bootloader and apps.

Bootloader

All metadata is stored in two flash pages after the vector table for the bootloader. Attributes are stored as key-value pairs, where the key can be up to 8 bytes and the value can be up to 55 bytes (one byte is for the length of the value). There are up to 16 supported attributes.

The attributes are created by a special C struct hand-crafted in the bootloader source.

Apps

All metadata for apps is stored at the beginning of their flash blob. The header is composed of TLV elements, and more documentation is here.

The TBF header is generated by the elf2tbf tool after the app is compiled.

Proposed Implementations

Kernel

The kernel would get attributes that match the attributes in the bootloader. Two pages of flash after the vector table would be set aside for attributes in the same key-value format. The linker would ensure those pages are available.

Implementation option 1

The attributes are created by the build.rs file as a part of the compilation step. This works by creating the structs that are hand crafted in the bootloader automatically and including the generated source file in the build. See this pull request for how that might look.

Implementation option 2

The space for the attributes is reserved by the cargo build process in the kernel binary, but the actual population is done by tockloader. A new command is added to tockloader (maybe flash-kernel) that would populate the attributes as it flashes the kernel to the board.

This is easier to implement, but doesn't quite work if a platform doesn't use tockloader.

Alternatives

We already have bootloader and app metadata structures. We just leave those, don't add kernel metadata, and figure out what to do on a case-by-case basis as things come up. For example, the imix kernel moved where apps go, and tockloader needs to know that. To address this, tockloader was run with the --app-address flag, and we have now added a bootloader attribute that specifies where the apps go that tockloader will use if it exists.

@bradjc bradjc added the rfc Issue designed for discussion and to solicit feedback. label Nov 28, 2017
@ppannuto
Copy link
Member

Binaries may wish to store their metadata elsewhere rather than having to read/write the MCU flash.

Given that the stated goal is to tie metadata to binaries, does it make sense for binaries to be able to edit their attributes? I think I would expect attributes to be exclusively read-only.


Existing Implementations

One of the listed advantages is parallelism about how attributes are stored. However, my reading of this section is that the bootloader and apps currently employ different on-disk storage of attributes? Is this a desired intention on an anachronism of current implementations?


Bootloader implementation

Currently, there is a whole page dedicated to relatively little information (the first page). Given that TOCK_BOOTLOADER\0...VERSION is smaller than one attribute, should we collapse the first two pages, eliminating the 490 reserved bytes, and reduce the number of total allowed attributes by one?

The space-saving becomes more relevant if we choose to use the same layout for attributes in all cases.


Kernel attributes implementation

I am inclined to lean towards option 1, or something akin to it. Tying correctness to the tool used to get bits on the board (a) perhaps ties Tock too strongly to tockloader, but more importantly (b) loses the property that all relevant implementation is stored in the binary image -- a kernel that runs correctly when flashed with some tockloader flags but then fails if a flag is forgotten by someone else feels dangerously fragile.

Some specific thoughts for how to implement this, the cargo/rust build process could simply allocate an empty array in the kernel binary. The attribute location is by definition at a fixed offset and known size, so any tool could write bits to the right place in the binary image. This approach is somewhat analogous to an elf2tbf-like tool for the kernel. There is an argument that objcopy is an elf2bin tool, so we're already running such a tool in our build process, although a custom tool certainly does increase complexity.

Concrete questions that I think inform the implementation: What attributes should/need to be attached to the kernel, and where is that information available?

  • Application memory location
    -- Currently implemented as a static array in the kernel with a symbol
  • Kernel build environment (version/git hash; toolchain; etc)
    -- Available to the compilation environment. May change without any source code change
  • Compiled in modules/capsules/capabilities
    -- It may be sensible for tockloader to be able to validate board features when loading apps. Attributes seem like the right mechanism for this

@bradjc
Copy link
Contributor Author

bradjc commented Nov 29, 2017

I think I would expect attributes to be exclusively read-only.

I don't think that has to be the case. The apps location could move, an encryption key could change, the kernel location could move, etc. Tying to the binary is more of a spatial relationship than thinking of an application binary as immutable.

However, my reading of this section is that the bootloader and apps currently employ different on-disk storage of attributes? Is this a desired intention on an anachronism of current implementations?

Well we stabilized app binaries so that is fixed. We could in theory use TLVs for the other attributes, but that would either mean specifying the format of a lot of attributes, or using a generic key-value TLV but with more overhead for the type field.

should we collapse the first two pages, eliminating the 490 reserved bytes, and reduce the number of total allowed attributes by one? The space-saving becomes more relevant if we choose to use the same layout for attributes in all cases.

We could, but we should probably start with the reserved flash after the bootloader and before the kernel. Also we can't change apps now.

Compiled in modules/capsules/capabilities
-- It may be sensible for tockloader to be able to validate board features when loading apps.

This is interesting. We currently have a basically unused feature in TABs where a TAB can specify compatible boards (where the board is identified by a bootloader attribute). Extending this idea to required syscall interfaces could be interested. Although we would get benefit immediately if we actually used the supported board feature (e.g. so you dont run the wrong BLE apps for your board).

@ppannuto
Copy link
Member

ppannuto commented Nov 29, 2017

read-only attributes

Fair points. The other thought I'd had is that in some cases one might imagine that attributes are written to immutable storage of some kind, so baking in an assumption that they could be writable felt dangerous. On the flip side, however baking in the restriction that they can't be written is probably a premature restriction. I think leaving them as possibly writable make sense. ✅


TLV vs Key-Value for bootloader/kernel attributes

TLV

  • + Consistent attribute format with apps
  • + Permits arbitrary-length attributes
  • - Not easily "human-readable"
  • - Requires external knowledge across tools for meanings of types

KV

  • + Key expresses meaning, rather than external lookup of type
  • - Space-inefficient
  • - Arbitrary restriction on attribute size

We could, but we should probably start with the reserved flash after the bootloader and before the kernel.

Putting it after the kernel is fine, but I think we should collapse the page. The current scheme feels needlessly wasteful. Any of the extensibility we want from flags I think we can realize with attributes (or a much smaller number of flags)


Capabilities

Indeed. I've mixed up nrf52 vs hail BLE apps more than once :(. Think we're agreed here.


Do we have other examples of things that make sense as kernel attributes? I'm trying to gauge if anything that we'd want to include as an attribute should push the implementation one way or another.

@ppannuto
Copy link
Member

[Meta-comment on RFC process]:

I understand now why the Rust model is a PR with a text file with the body that's in Brad's top-level comment. As we start to reach consensus on some ideas, it'd be nice to update the document to realize a desired final form, but that feels a bit awkward if I just edit the top-level comment.

@bradjc
Copy link
Contributor Author

bradjc commented Nov 29, 2017

The "pre-page" I purposefully left out because that was more of a hack than anything, and really is just there so that Tockloader knows if there is a bootloader or not. But even with that, it's only 6 pages of flash, or 3072 bytes, or 0.6% of the SAM4L flash. We allocate 128 pages of flash to the bootloader, but only use about 70 of them. I guess I just don't think that flash space matters in this case. Also we have to be needlessly wasteful to reserve space for future attributes.

@alevy
Copy link
Member

alevy commented Nov 30, 2017

The kernel metadata holds certain parameters of the kernel that could change over the lifetime of the device and that the kernel needs to know after a power cycle.

I think app location in particular is not a great motivating example since we're liking to change who loads applications and how in the future.

Maybe better motivating example (which I think fits your model mostly) are things like the device address for, e.g., Bluetooth or 802.15.4. These are particular good examples because it makes sense to tie them to the kernel and it makes sense to let external tools modify them --- consider provisioning a fleet of devices by flashing the same kernel binary and also updating the address attribute with a unique address for each.

@alevy
Copy link
Member

alevy commented Nov 30, 2017

RE: TLV vs Key-Value, a few points

  1. I don't think we need to be that concerned about syncing attribute types across the App/Bootloader/Kernel types. It's nice for the format to be similar, so we can share parsing code, but they are logically for different things anyway.

  2. TLV is less extensible, which is a bummer, because there does need to be some global agreement about key types (e.g. what if two capsules written by different people end up both choosing the same "type" for different purposes)

  3. We need to be a bit careful about packing meta-data if it appears before or in the middle of the kernel binary. Unlike processes, the kernel is not compiled with PIC (and we probably don't want to require that). That means if changing an attribute would change the total size of the attributes section, and thus change the offset of some of the kernel code, we would need to recompile the kernel---which I think we don't want.

@alevy
Copy link
Member

alevy commented Jan 5, 2018

Revisiting this, I'm in favor of going with:

  • Key-value pairs for the bootloader and kernel

  • Leaving setting keys and values to an external tool, e.g. tockloader. In some cases, of course, the kernel might set them as well, but that's a choice it can make depending on the platform etc.

Btw, it's perfectly reasonable to, for example, generate something like csv file when building the kernel with default values that tockloader can consume.

@bradjc
Copy link
Contributor Author

bradjc commented Jul 5, 2018

If we have no intention of having the kernel setting its own attributes, then it doesn't seem to make sense to have the kernel have attributes. However, for boards without a bootloader, it can still be useful to have a section for attributes.

So I propose:

  • There is only one attributes section at address 0x400 (lasting for 1536 bytes) that is the same as what we are currently using with the bootloader.
  • If there is a bootloader on the board then it is part of the bootloader's flash. If there isn't, then it is part of the kernel's flash. In either case, these are board-level attributes, and aren't conceptually attached to either the kernel or the bootloader.

This makes it easier on the tools which only need to look in one spot for all boards. Also, there are 16 key-value slots, and we are only using I think 5 at the most currently, so we should have enough space for all of the attributes a board could need.

This is I think entirely consistent with what Amit proposed above.

@ppannuto
Copy link
Member

ppannuto commented Jul 5, 2018

If there is a bootloader on the board then it is part of the bootloader's flash. If there isn't, then it is part of the kernel's flash.

I think I'm broadly in agreement here, but I'm not quite sure how this should be handled by the tooling. If the attributes can be part of "either" flash, what tool is responsible for writing them (both?). How do the tools find the attributes to set? What happens if the bootloader's view of the attributes to write conflict with the kernel's?

Should the attributes perhaps instead be part of neither flash, and written independently (possibly by tooling as one of the make flash steps from one or both of the bootloader or kernel, but not part of a single image).

@bradjc
Copy link
Contributor Author

bradjc commented Jul 5, 2018

I don't think it matters what writes the attributes, just that they are always in the same spot on every board. I think it makes practical sense to have the board bootloader bake in attributes that are board specific, and then tockloader can write more if needed.

@ppannuto
Copy link
Member

Appending board attributes feels fine, my only concern was getting into a situation where the underspecification caused tools to inadvertently fight and overwrite attributes.

I think we're at consensus here. Perhaps we should start last-call tagging RFCs as well? In which case I think we can call this RFC at last-call.

@ppannuto ppannuto added the last-call Final review period for a pull request. label Jul 11, 2018
@alevy
Copy link
Member

alevy commented Jul 13, 2018

Agree on consensus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
last-call Final review period for a pull request. rfc Issue designed for discussion and to solicit feedback.
Projects
None yet
Development

No branches or pull requests

3 participants