From ae6c15d3d604eff335d2c0c1f7531eac7f4fc4c5 Mon Sep 17 00:00:00 2001 From: Andreas Galauner Date: Wed, 15 Jul 2026 17:51:09 +0200 Subject: [PATCH] feat: add AVR target with configuration change protection support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an `avr` target to svd2rust, whose main purpose is support for the configuration change protection (CCP) mechanism of modern (xmega based) AVR cores: certain registers only accept writes within four instructions after a magic value has been written to an unlock register (`CPU.CCP`). The support consists of two parts: First, a new generic file (`generic_avr_ccp.rs`, included for the `avr` target) provides the `UnlockRegister` and `Protected` marker traits and, for protected registers, `write_protected`/`modify_protected` methods. These mirror `write`/`modify` but perform the unlock sequence in inline assembly, so the protected write is guaranteed to hit the four instruction window regardless of optimization level. Second, since SVD files carry no information about CCP (neither which register unlocks protected writes, nor which registers are protected), the protected register list is declared in the `--settings` YAML file, mirroring the RISC-V approach: avr_config: ccp: unlock_register: "CPU.CCP" protected_registers: - { register: "NVMCTRL.CTRLA", magic: 0x9D } - { register: "WDT.CTRLA", magic: 0xD8 } For every listed register, an `impl Protected` is emitted at the end of the generated device module. Design notes: - The asm block contains only the two instructions that must be adjacent — `out CCP, magic` and the protected store. The magic is passed as an `in(reg_upper)` operand (the `ldi`-capable r16..=r31 class, avr-libc's `"d"` constraint in `ccp_write_io`) instead of being loaded inside the block, so the compiler can hoist the load out of loops and reuse a register that already holds the value across consecutive same-magic writes. - The protected store is `st X` with the runtime register pointer rather than a compile-time `sts`: the pointer comes from the register reference the method is called on, which keeps writes correct for `derivedFrom` peripherals whose instances share one set of register spec types at different base addresses. The `sts` form would save one word and the X clobber per call site, but requires per-register address consts plus a codegen guard against aliased peripherals. - No tiny/xmega split (avr-libc has one): the split only optimizes compile-time-constant addresses; the pointer variant we use is identical on the xmega family and reduced-core tinys, and avoids `sts`, the one instruction that differs (7-bit form on reduced cores). - `UnlockRegister::ADDR` is generated as `::ADDRESS as u8 + ` — anchored on the peripheral's existing address const instead of duplicating a literal. Codegen rejects unlock registers at addresses >= 0x40, which `out` cannot reach. - The magic is a raw byte rather than a named signature (SPM/IOREG) to keep svd2rust agnostic of particular AVR families. - Registers referenced in the settings file are resolved against the SVD and generation fails on unknown paths, so typos surface as build errors instead of silently missing impls. - The impls use module-relative paths (`cpu::ccp::CCP_SPEC`), so they are correct regardless of where the device module is mounted in the PAC crate (e.g. `avr-device` places it under `crate::`). - A new `Settings::from_yaml` constructor lets PACs that drive svd2rust as a library (like `avr-device`) parse settings files without depending on `serde_yaml` themselves; the CLI uses it too. --- CHANGELOG.md | 5 + src/config.rs | 19 +++- src/config/avr.rs | 36 +++++++ src/generate/avr.rs | 160 +++++++++++++++++++++++++++++++ src/generate/device.rs | 19 +++- src/generate/generic_avr_ccp.rs | 145 ++++++++++++++++++++++++++++ src/generate/interrupt.rs | 2 + src/generate/mod.rs | 1 + src/lib.rs | 48 +++++++++- src/main.rs | 7 +- svd2rust-regress/src/svd_test.rs | 6 +- 11 files changed, 438 insertions(+), 10 deletions(-) create mode 100644 src/config/avr.rs create mode 100644 src/generate/avr.rs create mode 100644 src/generate/generic_avr_ccp.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 24056472..b0c63bd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/). the `cortex-m::interrupt::InterruptNumber` trait. A new `add-cortex-m-int-num` option which allows adding the generation of the old trait. - Bump MSRV of generated code to 1.81 +- Add an `avr` target which includes generic code to access configuration + change protected (CCP) registers in a more convenient way +- Add AVR specific settings (`avr_config`) to the `--settings` file, declaring + which registers are CCP protected; `Protected` trait implementations are + generated from it ## [v0.37.1] - 2025-10-17 diff --git a/src/config.rs b/src/config.rs index d275615a..06b732d9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -82,6 +82,8 @@ pub enum Target { XtensaLX, #[cfg_attr(feature = "serde", serde(rename = "mips"))] Mips, + #[cfg_attr(feature = "serde", serde(rename = "avr"))] + Avr, #[cfg_attr(feature = "serde", serde(rename = "none"))] None, } @@ -94,6 +96,7 @@ impl std::fmt::Display for Target { Target::RISCV => "riscv", Target::XtensaLX => "xtensa-lx", Target::Mips => "mips", + Target::Avr => "avr", Target::None => "none", }) } @@ -107,6 +110,7 @@ impl Target { "riscv" => Target::RISCV, "xtensa-lx" => Target::XtensaLX, "mips" => Target::Mips, + "avr" => Target::Avr, "none" => Target::None, _ => bail!("unknown target {}", s), }) @@ -114,7 +118,7 @@ impl Target { pub const fn all() -> &'static [Target] { use self::Target::*; - &[CortexM, Msp430, RISCV, XtensaLX, Mips] + &[CortexM, Msp430, RISCV, XtensaLX, Mips, Avr] } } @@ -353,9 +357,18 @@ pub struct Settings { pub crate_path: Option, /// RISC-V specific settings pub riscv_config: Option, + /// AVR specific settings + pub avr_config: Option, } impl Settings { + /// Parse chip-specific settings from their YAML representation, e.g. the + /// contents of the file passed with `--settings`. + #[cfg(all(feature = "serde", feature = "yaml"))] + pub fn from_yaml(yaml: &str) -> Result { + serde_yaml::from_str(yaml).map_err(Into::into) + } + pub fn update_from(&mut self, source: Self) { if source.html_url.is_some() { self.html_url = source.html_url; @@ -366,6 +379,9 @@ impl Settings { if source.riscv_config.is_some() { self.riscv_config = source.riscv_config; } + if source.avr_config.is_some() { + self.avr_config = source.avr_config; + } } pub fn extra_build(&self) -> Option { @@ -405,4 +421,5 @@ impl FromStr for CratePath { } } +pub mod avr; pub mod riscv; diff --git a/src/config/avr.rs b/src/config/avr.rs new file mode 100644 index 00000000..af389ae6 --- /dev/null +++ b/src/config/avr.rs @@ -0,0 +1,36 @@ +/// AVR specific configuration +/// +/// The SVD files for AVR devices do not contain any information about the +/// configuration change protection (CCP) mechanism found on modern (xmega +/// based) AVR cores: neither which register unlocks protected writes, nor +/// which registers are protected by it. This configuration fills that gap. +#[cfg_attr(feature = "serde", derive(serde::Deserialize), serde(default))] +#[derive(Clone, PartialEq, Eq, Debug, Default)] +#[non_exhaustive] +pub struct AvrConfig { + /// Configuration change protection (CCP) description for this device + pub ccp: Option, +} + +#[cfg_attr(feature = "serde", derive(serde::Deserialize), serde(default))] +#[derive(Clone, PartialEq, Eq, Debug, Default)] +#[non_exhaustive] +pub struct CcpConfig { + /// `PERIPHERAL.REGISTER` path of the register that unlocks protected + /// writes when the magic value is written to it, e.g. `CPU.CCP` + pub unlock_register: String, + /// All registers of the device that are configuration change protected + pub protected_registers: Vec, +} + +#[cfg_attr(feature = "serde", derive(serde::Deserialize), serde(default))] +#[derive(Clone, PartialEq, Eq, Debug, Default)] +#[non_exhaustive] +pub struct CcpProtectedRegister { + /// `PERIPHERAL.REGISTER` path of the protected register, e.g. + /// `NVMCTRL.CTRLA` + pub register: String, + /// Magic value that must be written to the unlock register to allow + /// writing this register, e.g. `0x9D` (SPM) or `0xD8` (IOREG) + pub magic: u8, +} diff --git a/src/generate/avr.rs b/src/generate/avr.rs new file mode 100644 index 00000000..2f1d083c --- /dev/null +++ b/src/generate/avr.rs @@ -0,0 +1,160 @@ +use crate::{svd::Peripheral, util, Config}; +use anyhow::{anyhow, Context, Result}; +use log::debug; +use proc_macro2::{Span, TokenStream}; +use quote::quote; +use svd_parser::svd::{Access, Register}; + +/// Whole AVR-specific generation. +/// +/// SVD files for AVR devices carry no information about the configuration +/// change protection (CCP) mechanism of modern (xmega based) AVR cores, so the +/// list of protected registers and their unlock magic comes from the settings +/// file ([`crate::config::avr::AvrConfig`]). Here we translate that list into +/// `UnlockRegister` and `Protected` trait implementations for the generated +/// register types, so that protected registers can be written with +/// `write_protected`/`modify_protected` from `generic_avr_ccp.rs`. +pub fn render(peripherals: &[Peripheral], config: &Config) -> Result { + let mut mod_items = TokenStream::new(); + + let Some(ccp) = config + .settings + .avr_config + .as_ref() + .and_then(|avr| avr.ccp.as_ref()) + else { + return Ok(mod_items); + }; + + debug!("Rendering AVR configuration change protection impls"); + + // The unlock register itself is not listed as protected in the SVD either; + // its address is all the unlock sequence needs, and we can derive that + // from the SVD instead of asking for it in the settings file. + let (unlock_periph, unlock_reg, unlock_addr) = + resolve_register(peripherals, &ccp.unlock_register) + .context("can't resolve CCP unlock register")?; + + // The unlock sequence writes the magic with `out`, which can only reach + // I/O addresses below 0x40. On every CCP-bearing core those are identical + // to the data-space addresses the SVD describes; reject anything else so + // a bad settings entry fails here instead of in the assembler. + if unlock_addr >= 0x40 { + return Err(anyhow!( + "CCP unlock register {} is at address {:#x}, but `out` can only \ + reach I/O addresses below 0x40", + ccp.unlock_register, + unlock_addr + )); + } + + // Anchor the address on the peripheral's `PeripheralSpec::ADDRESS` const + // instead of duplicating it as a literal; only the register's byte offset + // is emitted directly. + let unlock_spec = register_spec_path(unlock_periph, unlock_reg, config); + let unlock_periph_spec = util::ident( + &unlock_periph.name, + config, + "peripheral_spec", + Span::call_site(), + ); + let unlock_offset = util::hex(unlock_reg.address_offset as u64); + + mod_items.extend(quote! { + impl crate::UnlockRegister for #unlock_spec { + const ADDR: u8 = + <#unlock_periph_spec as crate::PeripheralSpec>::ADDRESS as u8 + #unlock_offset; + } + }); + + for protected in &ccp.protected_registers { + let (periph, reg, _) = resolve_register(peripherals, &protected.register) + .with_context(|| format!("can't resolve protected register {}", protected.register))?; + + // The unlock sequence in `generic_avr_ccp.rs` only supports 8-bit + // writable registers (`RegisterSpec + Writable`); catch + // mismatches here instead of failing later in the PAC build. Size and + // access are usually inherited defaults on AVR SVDs, so only reject + // explicit contradictions. + if let Some(size) = reg.properties.size { + if size != 8 { + return Err(anyhow!( + "protected register {} is {} bits wide; only 8-bit registers are supported", + protected.register, + size + )); + } + } + if reg.properties.access == Some(Access::ReadOnly) { + return Err(anyhow!( + "protected register {} is read-only", + protected.register + )); + } + + let reg_spec = register_spec_path(periph, reg, config); + // Emit the magic as a hex literal so the generated code matches the + // datasheet notation (0x9D/0xD8) instead of decimal. + let magic = util::hex(protected.magic as u64); + + mod_items.extend(quote! { + impl crate::Protected for #reg_spec { + const MAGIC: u8 = #magic; + type CcpReg = #unlock_spec; + } + }); + } + + Ok(mod_items) +} + +/// Look up a `PERIPHERAL.REGISTER` path from the settings file in the SVD and +/// return the peripheral, the register and the register's data-space address. +/// +/// Registers of derived peripherals are found on the base peripheral, but the +/// address is computed from the derived peripheral's own base address. +fn resolve_register<'a>( + peripherals: &'a [Peripheral], + path: &str, +) -> Result<(&'a Peripheral, &'a Register, u64)> { + let (periph_name, reg_name) = path.split_once('.').ok_or_else(|| { + anyhow!("register path `{path}` must have the form `PERIPHERAL.REGISTER`") + })?; + + let periph = peripherals + .iter() + .find(|p| p.name == periph_name) + .ok_or_else(|| anyhow!("no peripheral named `{periph_name}` in the SVD"))?; + + // Follow `derivedFrom` (once, as mandated by the SVD spec) in case the + // peripheral inherits its registers from another one. + let base = match periph.derived_from.as_deref() { + Some(base_name) => peripherals + .iter() + .find(|p| p.name == base_name) + .ok_or_else(|| { + anyhow!("peripheral `{periph_name}` is derived from unknown `{base_name}`") + })?, + None => periph, + }; + + let reg = base + .registers() + .find(|r| r.name == reg_name) + .ok_or_else(|| anyhow!("no register named `{reg_name}` in peripheral `{periph_name}`"))?; + + let address = periph.base_address + reg.address_offset as u64; + Ok((periph, reg, address)) +} + +/// Build the module-relative path to a register's spec type, e.g. +/// `cpu::ccp::CCP_SPEC`. The impls are emitted at the top level of the device +/// module where all peripheral modules are siblings, so no crate-level prefix +/// is needed. +fn register_spec_path(periph: &Peripheral, reg: &Register, config: &Config) -> TokenStream { + let span = Span::call_site(); + let periph_mod = util::ident(&periph.name, config, "peripheral_mod", span); + let reg_mod = util::ident(®.name, config, "register_mod", span); + let reg_spec = util::ident(®.name, config, "register_spec", span); + quote! { #periph_mod::#reg_mod::#reg_spec } +} diff --git a/src/generate/device.rs b/src/generate/device.rs index c16a8eea..4c23b0e4 100644 --- a/src/generate/device.rs +++ b/src/generate/device.rs @@ -11,7 +11,7 @@ use crate::config::{Config, RustEdition, Target}; use crate::util::{self, ident}; use anyhow::{Context, Result}; -use crate::generate::{interrupt, peripheral, riscv}; +use crate::generate::{avr, interrupt, peripheral, riscv}; /// Whole device generation pub fn render(d: &Device, config: &Config, device_x: &mut String) -> Result { @@ -139,6 +139,7 @@ pub fn render(d: &Device, config: &Config, device_x: &mut String) -> Result Result Result Result(ptr: *mut u8, val: u8) +where + REG: RegisterSpec + Writable + Protected, +{ + core::arch::asm!( + // Write the CCP register with the magic to open the unlock window + "out {ccpreg}, {magic}", + + // Then immediately write the protected register + "st X, {perval}", + + ccpreg = const REG::CcpReg::ADDR, + magic = in(reg_upper) REG::MAGIC, + + in("X") ptr, + perval = in(reg) val, + ); +} + +/// Trait implemented by [`Writable`] and [`Protected`] registers which +/// allows writing to the protected register by first writing a magic value to +/// the CCP register. +pub trait ProtectedWritable +where + REG: Writable + Protected +{ + /// Write to a CCP protected register by unlocking it first. + /// + /// Refer to [`Reg::write`] for usage. + fn write_protected(&self, f: F) + where + F: FnOnce(&mut W) -> &mut W; +} + +impl ProtectedWritable for Reg +where + REG: RegisterSpec + Writable + Resettable + Protected +{ + /// Unlocks and then writes bits to a `Writable` register. + /// + /// Refer to [`Reg::write`] for usage. + #[inline(always)] + fn write_protected(&self, f: F) + where + F: FnOnce(&mut W) -> &mut W + { + let val = f(&mut W::::from(W { + bits: REG::RESET_VALUE & !REG::ONE_TO_MODIFY_FIELDS_BITMAP + | REG::ZERO_TO_MODIFY_FIELDS_BITMAP, + _reg: marker::PhantomData, + })).bits; + + unsafe { ccp_protected_write::(self.register.as_ptr(), val) } + } +} + +impl + Readable + Writable + Protected> Reg { + /// Modifies the contents of a protected register by reading and then + /// unlocking and writing it. + /// + /// Refer to [`Reg::modify`] for usage. + #[inline(always)] + pub fn modify_protected(&self, f: F) + where + for<'w> F: FnOnce(&R, &'w mut W) -> &'w mut W, + { + let bits = self.register.get(); + let val = f( + &R::::from(R { + bits, + _reg: marker::PhantomData, + }), + &mut W::::from(W { + bits: bits & !REG::ONE_TO_MODIFY_FIELDS_BITMAP + | REG::ZERO_TO_MODIFY_FIELDS_BITMAP, + _reg: marker::PhantomData, + }), + ) + .bits; + + unsafe { ccp_protected_write::(self.register.as_ptr(), val) } + } +} diff --git a/src/generate/interrupt.rs b/src/generate/interrupt.rs index 3888ee46..225998c7 100644 --- a/src/generate/interrupt.rs +++ b/src/generate/interrupt.rs @@ -269,6 +269,7 @@ pub fn render( }); } Target::Mips => {} + Target::Avr => {} Target::None => {} } @@ -409,6 +410,7 @@ pub fn render( && target != Target::Msp430 && target != Target::XtensaLX && target != Target::Mips + && target != Target::Avr { mod_items.extend(quote! { #[cfg(feature = "rt")] diff --git a/src/generate/mod.rs b/src/generate/mod.rs index 16d5bb75..6dbf3a61 100644 --- a/src/generate/mod.rs +++ b/src/generate/mod.rs @@ -1,3 +1,4 @@ +pub mod avr; pub mod device; pub mod interrupt; pub mod peripheral; diff --git a/src/lib.rs b/src/lib.rs index 934059b6..8e00cf57 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -280,6 +280,48 @@ //! core_interrupt: "MachineExternal" //! ``` //! +//! ## AVR specific settings +//! +//! Modern (xmega based) AVR cores protect some of their registers with the +//! configuration change protection (CCP) mechanism: a magic value has to be written +//! to an unlock register right before the protected register can be written. +//! SVD files carry no information about CCP, so it can be supplied with a settings +//! file in YAML format passed with the `--settings` flag: +//! +//! ```text +//! $ svd2rust -g --target avr --settings my_device.yaml -i my_device.svd +//! ``` +//! +//! For registers listed in the settings file, `svd2rust` generates implementations of +//! the `Protected` trait, which enables the `write_protected` and `modify_protected` +//! methods that perform the unlock sequence. The settings file expects an `avr_config` +//! map with a `ccp` field holding the following entries: +//! +//! - `unlock_register` (mandatory): The `PERIPHERAL.REGISTER` path of the register that +//! unlocks protected writes, as named in the SVD file (usually `CPU.CCP`). Its address +//! is derived from the SVD file. +//! +//! - `protected_registers` (mandatory): The list of registers that are configuration +//! change protected. Each entry is specified with the following fields: +//! - `register`: The `PERIPHERAL.REGISTER` path of the protected register, as named +//! in the SVD file. +//! - `magic`: The magic value that must be written to the unlock register to allow +//! writing this register. Refer to the device datasheet; most devices use `0x9D` +//! (SPM signature) for NVM self-programming registers and `0xD8` (IOREG signature) +//! for protected I/O registers. +//! +//! A settings file will look like this: +//! +//! ```yaml +//! avr_config: +//! ccp: +//! unlock_register: "CPU.CCP" +//! protected_registers: +//! - { register: "NVMCTRL.CTRLA", magic: 0x9D } +//! - { register: "NVMCTRL.CTRLB", magic: 0xD8 } +//! - { register: "CLKCTRL.MCLKCTRLA", magic: 0xD8 } +//! ``` +//! //! ## Rust editions //! //! Default rust edition for generated code is 2021. Pass `--edition=2024` if you want to @@ -764,9 +806,9 @@ pub fn generate(input: &str, config: &Config) -> Result { #[cfg(feature = "yaml")] Some(settings) => { let file = std::fs::read_to_string(settings).context("could not read settings file")?; - config - .settings - .update_from(serde_yaml::from_str(&file).context("could not parse settings file")?) + config.settings.update_from( + config::Settings::from_yaml(&file).context("could not parse settings file")?, + ) } #[cfg(not(feature = "yaml"))] Some(_) => { diff --git a/src/main.rs b/src/main.rs index 9c4c6565..fcee4cc7 100755 --- a/src/main.rs +++ b/src/main.rs @@ -349,9 +349,10 @@ Ignore this option if you are not building your own FPGA based soft-cores."), #[cfg(feature = "yaml")] Some(settings) => { let file = std::fs::read_to_string(settings).context("could not read settings file")?; - config - .settings - .update_from(serde_yaml::from_str(&file).context("could not parse settings file")?) + config.settings.update_from( + svd2rust::config::Settings::from_yaml(&file) + .context("could not parse settings file")?, + ) } #[cfg(not(feature = "yaml"))] Some(_) => { diff --git a/svd2rust-regress/src/svd_test.rs b/svd2rust-regress/src/svd_test.rs index ba214004..699d4447 100644 --- a/svd2rust-regress/src/svd_test.rs +++ b/svd2rust-regress/src/svd_test.rs @@ -313,7 +313,7 @@ impl TestCase { )?; process_stderr_paths.push(svd2rust_err_file); match self.arch { - Target::CortexM | Target::Mips | Target::Msp430 | Target::XtensaLX => { + Target::CortexM | Target::Mips | Target::Msp430 | Target::XtensaLX | Target::Avr => { // TODO: Give error the path to stderr fs::rename(path_helper_base(&chip_dir, &["lib.rs"]), &lib_rs_file) .with_context(|| "While moving lib.rs file")?; @@ -428,6 +428,7 @@ impl TestCase { Target::Mips => CRATES_MIPS.iter(), Target::Msp430 => CRATES_MSP430.iter(), Target::XtensaLX => [].iter(), + Target::Avr => [].iter(), Target::None => unreachable!(), }) .chain(opts.as_ref().map_or(Vec::new().into_iter(), |opts| { @@ -488,6 +489,7 @@ impl TestCase { Target::Mips => "mips", Target::RISCV => "riscv", Target::XtensaLX => "xtensa-lx", + Target::Avr => "avr", Target::None => unreachable!(), }; let mut svd2rust_bin = Command::new(svd2rust_bin_path); @@ -507,7 +509,7 @@ impl TestCase { Some(lib_rs_file).filter(|_| { !matches!( self.arch, - Target::CortexM | Target::Msp430 | Target::XtensaLX + Target::CortexM | Target::Msp430 | Target::XtensaLX | Target::Avr ) }), Some(svd2rust_err_file),