RFC: Persistent run-time configs

[crawfxrd]

• Feature name: persistent-configs
• Start date: 2021-11-12
• Status: Rejected

Summary

Implement a data format and API for persistent run-time configuration data.

Motivation

Allow users to set certain values at run-time so they do not have to recompile
and flash the EC firmware when they want to change them. Such values include:

• Battery charging thresholds (currently configurable, but not persistent)
• Fn lock
• Keyboard color (for RGB LED KBs)
• Keyboard brightness (for LED KBs)
• Webcam power
• Fan points

Guide-level explanation

The EC can be configured to some extent during run-time using ectool, without
the need to recompile and flash new firmware.

A possible interface for ectool could be something like:

$ ectool config -h
Query or set EC run-time configuration data.

USAGE:
    ectool config [SUBCOMMAND]

OPTIONS:
    -h, --help      Prints help information

SUBCOMMANDS
    get             Get the data for a config
    set             Set the data for a config
    reset           Reset run-time configs and erase config flash space

$ ectool config get
01: charging-thresholds
    Values at while the battery will start and stop changing
    Start: 40
    End: 70

02: fn-lock
    Use alternative functions when pressing F1-F12 by default
    FnLock: OFF

03: kb-color
    The color of the keyboard backlight
    Color: FF00FF

04: kb-brightness
    The brightness of the keyboard backlight
    Brightness: 75%

05: webcam-power
    Set webcam power on boot
    Power: OFF

06: fan-points
    Fan curve
    Temp  Duty
    55    25
    65    30
    70    40
    75    60
    80    75
    85    90
    90    100

$ ectool config set charging-thresholds 50 80
$ ectool config set kb-color 00FFFF
$ ectool config set webcam-power off

Because ectool is a user-space tool and can be easily and rapidly iterated on,
only minimal consideration has been given to the UI. What is important for this
proposal are the commands used to interface with the EC. Bikeshed the UI
somewhere else.

Reference-level explanation

Considerations

Architecture

The EC is based on the Intel 8051, a 40 year old architecture with quite a few
quirks. Along with certain design decisions made for the chips, these impose
some constraints that should first be considered. Some constraints for this
feature are:

• Mechanism: It has a special mechanism for accessing the flash,
  EC-Indirect Memory Access, which is executed from Scratch ROM.
• Erase size: It can erase in 1 page blocks
  • Depending on EC, page size may be either 1024 or 4096 bytes.
• Granularity: It can read/write 1 byte at a time.

Compatibility

The dynamic keyboard layout is saved in the top 1k of flash (0x1FC00). The
config block cannot use this space.

No mechanism exists for saving any other configuration data, so no
consideration is made for them.

Flash layout

The config block will occupy a 1024 byte region just below the keyboard layout.

Start	End	Size
0x1F800	0x1FBFF	1024

Header

The config block begins with an 8 byte header, with the following format:

Offset	Type	Name	Default value
0	[u8; 4]	Signature	'$CFG'
4	u8	Version	1
5	[u8; 3]	Reserved	0x00

• signature: The 4 byte ASCII string '$CFG'.
• version: The version of the data format.
• reserved: These bytes are reserved and must be 0.

The signature makes the config block easily identifiable in binary dumps, and
indicates that the block is used.

The version is used to handle any potential changes in the data layout. It
prevents the EC from reading potentially invalid data if a format change
occurs in the future.

0 is used for reserved bytes to distinguish them from the erase byte (0xFF).

If the header is not valid, the EC should erase the block and write out the
default header. In the case of the version changing, the EC may be able to
migrate data from a previous format version.

Record format

Offset	Type	Name
0	u8	Tag
1	u8	Size
2	[u8]	Data

• tag: Integer that identifies which config this record is for.
  • 0x00: Invalid
  • 0x01-0xFE: Valid
  • 0xFF: Invalid
• size: The size of the record data in bytes.
  • Valid values are 1-254, inclusive
• data: size bytes of data.

The use of a 1 byte tag allows up to 253 valid IDs, with 0 and 0xFF being
invalid. These two values are considered invalid to avoid any confusion about
a config being erased or unused.

The use of a 1 byte size allows for a config to be up to 254 bytes large,
with 0 and 255 being invalid. There is no inherent structure or size for the
config data. It is up to the caller to use a valid size and structure when
accessing config values.

The offset of each record in flash is predefined and known at compile time.
The offset is determined when the config and its tag are initially added to the
code. It should begin immediately after the data of the previous config.

API

Tags are identified by an enum. New tags are added sequentially without gaps.

enum config_tag {
    CFG_INVALID = 0x00,
    ...,
    CFG_NR_TAGS = _,
};

CFG_NR_TAGS is to be set to the last config value. It is used to identify
the number of known tags at compile time.

Get config

int8_t config_get(enum config_tag tag, uint8_t size, uint8_t *const data);

Reads size bytes from flash into data for tag.

• On success, 0 is returned and data contains the new value.
• If the tag does not exist, data is not modified and an error is returned.
• If the size does not match, data is not modified and an error is returned.

Set config

int8_t config_set(enum config_tag tag, uint8_t size, uint8_t const *const data);

Writes size bytes of data to flash for tag.

• On success, 0 is returned and the value of tag is set to data.
• If the tag does not exist, a new tag is written out at the offset for tag.
• If the size does not match, the value is not modified and an error
  is returned.

Reset configs

int8_t config_reset(void);

This function currently exists, but will be changed to fit this proposal.

Resets all config values to their defaults. Erases the config flash space and
writes out the default header.

Commands

The following commands will be added to expose the functionality to ectool:

• CMD_CONFIG_GET
• CMD_CONFIG_SET
• CMD_CONFIG_RESET

Drawbacks

• Unusable holes if tags are deprecated.
• No revision field on tags means a new tag would be needed if the semantics
  of a config change.

Rationale and alternatives

• It's small
  • The minimum size of a config is 3 bytes.
  • Can fit the entire config block into a single 1024 byte page.
• It's fast
  • Tags and data sizes use 1 byte, the width of the data bus.
  • Using predefined offsets into the config block gives O(1) lookup.
  • Operates from the Scratch ROM for a minimal amount of time, reducing skew
    of the global tick (interrupts are disabled during flash access).

Position-independent records

Instead of using hard-coded offsets for each config, the EC could linearly scan
the config block. This would incur a penalty on access, but could save offsets
in a lookup table for future accesses.

static uint16_t config_tag_offsets[CFG_NR_TAGS] = {0};

if (config_tag_offsets[<tag>] == 0) {
    // On first access, cache the config's offset
    config_tag_offsets[<tag>] = <offset>;
}

Configs could be written at any offset (in the sense that their position is not
predefined) and in any order.

The 0xFF tag, currently invalid, would then be used as an "End tag" to indicate
that there are no more records.

enum config_tag {
    CFG_INVALID = 0x00,
    ...,
    CFG_NR_TAGS = _,
    CFG_END = 0xFF,
};

Proposal in #63

#63 was not considered for these reasons:

• Each config includes English-only strings for names and a description. This
  will cause an excessive amount of flash space to be used and cannot be
  localized. This information should instead exist in user-space tools.
• It uses a 4 byte ASCII identifier. The rationale was to make it easily to
  identify in binary dumps. However, a 1 or 2 byte integer ID would be
  sufficient, with user-space tools being responsible for decoding it.
• Data is limited to a 32-bit integer. It does not handle data type that cannot
  be represented by a single u32. It must also specify a minimum and maximum
  value, even if such values would not make sense for the config.

Configure from system flash

As stated in #63, the UEFI payload could be responsible for configuring the EC
and storing values.

This may not work, as the EC must be able to read some values from the store
before the SBIOS starts.

Load/store from CMOS

CMOS NVRAM could be used as persistent storage for EC configs. The EC would
only ever read from the CMOS NVRAM. coreboot or a user-space tool (such as
nvramtool) would be responsible for writing values.

CMOS config space is limited to 72 bytes, some of which is already used.
Changing a config would required updating both the CMOS and EC run-time value.

Prior art

• coreboot tables
• Multiboot2

Unresolved questions

• Should the keyboard layout have a tag and be treated (internal to the config
  module) as a special config? Or should it remain separate with its own API?
  • Given the size of the data, and that there are 2 (custom and default), it
    would be better to keep in a separate region with its own header/format.
• Should a 1 byte checksum field be added for each config to ensure that the
  data is valid when read on init?
  • With the constraints of the 8051, this should not be done. If the header
    is valid, it should be assumed all entries are valid.
• Should a 1 byte version field be added for each config to handle changes in
  semantics for a config?
  • If the semantics of a config changes significantly enough, it should use a
    new tag. If enough change, or the tags are exhausted, we can increment the
    version in the header and prune all deprecated tags.
• This was originally for ITE ECs. Should the ENE KB9548G used on HP Dev One
  be considered as well (4 KiB pages)?

Future possibilities

• Fault tolerance mechanism (such as page flipping) so that if the config block
  becomes invalid it can reset to the previous init's config instead of erasing
  the block and resetting all values to defaults.

[ids1024]

Hm, don't recall seeing this issue when it was created.

If flash space is very limited, and each tag has a known fixed address, using 2 bytes to store the signature and size seems potentially unnecessary. config_get could just take an offset in the config space and length, and ectool is responsible for knowing what is stored at that offset, and knowing that the size is correct.

Though that doesn't allow distinguishing per-tag whether or not it is set to anything.

The size of the config region could also be more flexible if it grew downward like a stack (when new tags are added), but I guess a single 1024 byte page is probably simpler to deal with.

Should the keyboard layout have a tag and be treated (internal to the config module) as a special config? Or should it remain separate with its own API?

Not sure how that fits here, but it would be beneficial to be able to read or write the whole keymap in one operation.

[MilesBHuff]

Just want to add that the goal of user-configurable fan curves can be achieved via a different method, which may be preferable: pop-os/system76-acpi-dkms#9.
Advantages of this are that the fan curve can be changed without a reboot and there's no need to mess with limited NVRAM.
Downsides are that the fan curve would only take effect once the system has booted, and it would be OS-specific (It also wouldn't work for Windows.).

[crawfxrd]

Also may want to change the terminology to match coreboot. (Thinking about this since another idea is importing and using coreboot's Kbuild/Kconfig fork.)

• config: Value that is set at build-time (config values in Kconfig)
• option: Value that is configurable at run-time (e.g., CMOS options)

[crawfxrd]

Should the keyboard layout have a tag and be treated (internal to the config module) as a special config? Or should it remain separate with its own API?

Given the size of the data, and that there are 2 (custom and default), I think it would be better to keep in a separate region with its own header/format.

---

Should a 1 byte checksum field be added for each config to ensure that the data is valid when read on init?

I think with the constraints of the 8051, this should not be done. If the header is valid, it should be assumed all entries are valid.

---

Should a 1 byte version field be added for each config to handle changes in semantics for a config?

This should also not be done. If the semantics of a config changes significantly enough, it should use a new tag. If enough change, or the tags are exhausted, we can increment the version in the header and prune all deprecated tags.

---

Newly added:

This was originally for ITE ECs. Should the ENE KB9548G used on HP Dev One be considered as well (4 KiB pages)?

I think the main difference (for this RFC) between the ITE and ENE ECs is the page size (1 KiB vs 4 KiB). This would affect what we use for the config block. Should it be 1024 bytes (meaning the CFG block may share the page with something else on the KB9548G) or should it be 1 page (erase size)?

[crawfxrd]

Configure from system flash

As stated in #63, the UEFI payload could be responsible for configuring the EC and storing values.

This may not work, as the EC must be able to read some values from the store before the SBIOS starts.

I wonder if it's possible to have the EC read configs from the system SPI flash. Configs could be stored in a RAW section of an EFI binary. I don't know how locating it would work.

[ilikenwf]

Whether it's defaulting webcam to off or just remembering the last set value, I'd love to see even just that change. The WIP pull request for backlight color/brightness works somewhat - it does remember if I have them on or off, however it usually resets the color to white if I remember correctly.

[crawfxrd]

If it's possible to communicate with CMOS from the EC, that would be the best bet. RFC will have to be redone for it. Basically: Use coreboot CMOS option table, and offload as much as possible to system firmware.

Not sure how to do that though. BRAM is sure as hell is not what's in CMOS RAM, so using it directly or accessing "CMOS" through EC2I won't work.

Also need to evaluate what configs would actually be required. A lot of the examples could be managed and saved in system firmware rather than the EC. They only really matter when the system is in S0. Battery thresholds is really the only thing that sticks out to me, since it needs to be managed while the system is not in S0.

[crawfxrd]

Closing this in favor of pursuing system76/firmware-open#571.

May be revisited once EC design improves for supporting configs that must be persistent without dependence on system firmware, such as battery charging thresholds

[digitalcircuit]

Should we have a separate issue open for persistent battery charging thresholds?

I understand it not being a priority, just to keep track of it. At the moment, I guide family members to "never shut down the laptop, just put it in suspend" to not lose the charging thresholds.

[crawfxrd]

There is #128 for charging thresholds.