Skip to content
Open
90 changes: 83 additions & 7 deletions core/src/cpus/cortex_m.zig
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,25 @@ pub const InterruptOptions = microzig.utilities.GenerateInterruptOptions(&.{
.{ .InterruptEnum = Interrupt, .HandlerFn = Handler },
});

/// Allowable `platform` options for microzig.options.
pub const CPU_Options = core.CPU_Options;
/// Allowable `cpu` options for microzig.options.
pub const CPU_Options = struct {
/// If true, interrupt vectors are moved to RAM so handlers can be set at runtime.
///
/// NOTE: Not supported on cortex_m0.
ram_vector_table: bool = false,

/// If true, the Cortex-M interrupts will be initialized with a more verbose variant
/// of the interrupt handlers which print the interrupt name.
///
/// NOTE: This option is enabled in debug builds by default.
verbose_unhandled_irq: bool = (builtin.mode == .Debug),

/// If true, the FPU will be enabled in the startup code.
///
/// NOTE: This option is enabled by default if hard float is enabled.
/// NOTE: Not supported on cortex_m0, cortex_m0plus and cortex_m3.
enable_fpu: bool = builtin.abi.float() == .hard,
};
Comment on lines +37 to +54
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made common CPU variant between cortex m flavours to be easier to add options. I also added compilation errors if for instance you try to use a ram vector table on cortex m0 or if you try to enable the FPU on the cortex ms that don't support it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know what you think of this


/// External Interrupts
/// These are the interrupts generated by the NVIC.
Expand Down Expand Up @@ -643,6 +660,32 @@ pub const atomic = struct {
}
};

/// Enables the FPU.
///
/// NOTE: This function is automatically called on cpu startup if the cpu has
/// an fpu and hard float is enabled. HAL's also call this in the startup of
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit HALs

/// other cores.
pub inline fn enable_fpu() void {
switch (cortex_m) {
inline .cortex_m0,
.cortex_m0plus,
.cortex_m3,
=> |flavour| @compileError("FPU not supported on " ++ @tagName(flavour)),
else => {},
}

// Taken from the rust crate cortex-m-rt.
asm volatile (
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've implemented this using raw assembly since we might be missing some register definitions for FPU in some cortex-m variants and also it allows being embedded in naked functions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no clobbers? I get there's a dsb and isb, but couldn't the compile still use cached registers, especially since this is inline?

\\ldr r0, =0xE000ED88
\\ldr r1, =(0b1111 << 20)
\\ldr r2, [r0]
\\orr r2, r2, r1
\\str r2, [r0]
\\dsb
\\isb
);
}

/// The RAM vector table used. You can swap interrupt handlers at runtime here.
/// Available when using a RAM vector table or a RAM image.
pub var ram_vector_table: VectorTable align(256) = if (using_ram_vector_table or is_ram_image)
Expand All @@ -654,7 +697,7 @@ else
pub const startup_logic = struct {
extern fn microzig_main() noreturn;

pub fn ram_image_start() linksection("microzig_ram_start") callconv(.naked) void {
pub fn ram_image_start() linksection("microzig_ram_start") callconv(.naked) noreturn {
const eos = comptime microzig.utilities.get_end_of_stack();
asm volatile (
\\
Expand All @@ -672,6 +715,11 @@ pub const startup_logic = struct {
microzig.utilities.initialize_system_memories(.auto);

if (using_ram_vector_table or is_ram_image) {
switch (cortex_m) {
.cortex_m0 => @compileError("RAM image and RAM vector table are not supported on cortex_m0"),
else => {},
}

asm volatile (
\\
// Set VTOR to point to ram table
Expand All @@ -684,6 +732,25 @@ pub const startup_logic = struct {
: .{ .memory = true, .r0 = true, .r1 = true });
}

if (fpu_present and microzig.options.cpu.enable_fpu) {
enable_fpu();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that this change makes microzig have more "batteries included" by default, but let's allow the user to disable the FPU at build time, with a flag named disable_fpu.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add this but isn't it a footgun? You could achieve the same by switching to soft float in the rp2350_arm target in build.zig and you don't risk crashing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the option

} else if (!fpu_present and microzig.options.cpu.enable_fpu) {
@compileError(
\\FPU enable requested though the chip doesn't appear to have an
\\FPU. If your chip does have an FPU please add the `fpu_present`
\\equal to `true` property to your chip file, either manually or via
\\patches. If you want to use patches, you can use something like
\\this:
\\```
\\.{ .set_device_property = .{
\\ .device_name = "CHIP_NAME",
\\ .key = "fpu_present",
\\ .value = "true"
\\} },
\\```
);
}

if (@hasField(types.peripherals.SystemControlBlock, "SHCSR")) {
// Enable distinction between MemFault, BusFault and UsageFault:
peripherals.scb.SHCSR.modify(.{
Expand All @@ -694,7 +761,9 @@ pub const startup_logic = struct {
enable_fault_irq();
}

microzig_main();
// If the compiler gets too aggressive with inlining we might get some
// floating point operations before the FPU is enabled.
@call(.never_inline, microzig_main, .{});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NP: instead can we use the noinline from the microzig.zig ? it won't be that big of an issue for other chip ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo it's ok that calls inside microzig_main get inlined because the FPU is already enabled when execution gets there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would bubble up the hf store to the startup function and you end up with the same issue IIUC.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't understand what you are referring to. Can you elaborate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry I think I was not clear. I though of flagging the microzig_main as noinline for every CPU not only arm. Just to simplify this @call other arch could suffer the same issue. It might also deserve a small explanation why we don't want microzig_main not to be inlined ? Feel free to discard this comment if you don't think it worth the effort.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely worth a comment to explain.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though of flagging the microzig_main as noinline for every CPU not only arm.

How can this be achieved? Idk of a way to make calls to microzig_main never inline (since it's always referenced as extern). But I will mark this PR as draft for now. I want to do a bit of refactoring.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the comment. Is it ok?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great comment, I was unsure why this was an issue but that makes sense to me!

}

// Validate that the VectorTable type has all the fault handlers that the CPU expects
Expand Down Expand Up @@ -970,10 +1039,17 @@ const scb_base = scs_base + core.scb_base_offset;
const mpu_base = scs_base + 0x0D90;
const fpu_base = scs_base + 0x0F34;

const properties = microzig.chip.properties;
// TODO: will have to standardize this with regz code generation
const mpu_present = @hasDecl(properties, "cpu.mpuPresent") and std.mem.eql(u8, properties.@"cpu.mpuPresent", "true");
const fpu_present = @hasDecl(properties, "cpu.fpuPresent") and std.mem.eql(u8, properties.@"cpu.fpuPresent", "true");
const mpu_present = @hasDecl(microzig.chip, "properties") and
@hasDecl(microzig.chip.properties, "mpu_present") and
is_property_true(microzig.chip.properties.mpu_present);
const fpu_present = @hasDecl(microzig.chip, "properties") and
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes code generated by regz doesn't include a properties declaration => if you used in the past fpu_present or mpu_present you got a compilation error.

@hasDecl(microzig.chip.properties, "fpu_present") and
is_property_true(microzig.chip.properties.fpu_present);

fn is_property_true(value: []const u8) bool {
return std.mem.eql(u8, value, "true") or std.mem.eql(u8, value, "1");
}

const core = blk: {
break :blk switch (cortex_m) {
Expand Down
8 changes: 0 additions & 8 deletions core/src/cpus/cortex_m/m0.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,6 @@ const builtin = @import("builtin");
const mmio = microzig.mmio;
const shared = @import("shared_types.zig");

pub const CPU_Options = struct {
/// If true, the Cortex-M interrupts will be initialized with a more verbose variant
/// of the interrupt handlers which print the interrupt name.
///
/// NOTE: This option is enabled in debug builds by default.
verbose_unhandled_irq: bool = (builtin.mode == .Debug),
};

pub const scb_base_offset = 0x0d00;

pub const SystemControlBlock = extern struct {
Expand Down
2 changes: 0 additions & 2 deletions core/src/cpus/cortex_m/m0plus.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ const mmio = microzig.mmio;

const shared = @import("shared_types.zig");

pub const CPU_Options = shared.options.Ram_Vector_Options;

pub const scb_base_offset = 0x0d00;

pub const SystemControlBlock = extern struct {
Expand Down
2 changes: 0 additions & 2 deletions core/src/cpus/cortex_m/m3.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ const mmio = microzig.mmio;

const shared = @import("shared_types.zig");

pub const CPU_Options = shared.options.Ram_Vector_Options;

pub const scb_base_offset = 0x0d00;

pub const SystemControlBlock = extern struct {
Expand Down
2 changes: 0 additions & 2 deletions core/src/cpus/cortex_m/m33.zig
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ pub const cpu_flags: shared.CpuFlags = .{
.has_usage_fault = true,
};

pub const CPU_Options = shared.options.Ram_Vector_Options;

pub const scb_base_offset = 0x0cfc;

pub const SystemControlBlock = extern struct {
Expand Down
2 changes: 0 additions & 2 deletions core/src/cpus/cortex_m/m4.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ const mmio = microzig.mmio;

const shared = @import("shared_types.zig");

pub const CPU_Options = shared.options.Ram_Vector_Options;

pub const scb_base_offset = 0x0d00;

pub const SystemControlBlock = extern struct {
Expand Down
2 changes: 0 additions & 2 deletions core/src/cpus/cortex_m/m55.zig
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ const mmio = microzig.mmio;

const shared = @import("shared_types.zig");

pub const CPU_Options = shared.options.Ram_Vector_Options;

pub const scb_base_offset = 0x0cfc;

pub const SystemControlBlock = extern struct {
Expand Down
2 changes: 0 additions & 2 deletions core/src/cpus/cortex_m/m7.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ const mmio = microzig.mmio;

const shared = @import("shared_types.zig");

pub const CPU_Options = shared.options.Ram_Vector_Options;

pub const scb_base_offset = 0x0d00;

pub const SystemControlBlock = extern struct {
Expand Down
13 changes: 0 additions & 13 deletions core/src/cpus/cortex_m/shared_types.zig
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,6 @@ pub const CpuFlags = struct {
has_mem_manage_fault: bool,
};

pub const options = struct {
pub const Ram_Vector_Options = struct {
/// If true, interrupt vectors are moved to RAM so handlers can be set at runtime.
ram_vector_table: bool = false,

/// If true, the Cortex-M interrupts will be initialized with a more verbose variant
/// of the interrupt handlers which print the interrupt name.
///
/// NOTE: This option is enabled in debug builds by default.
verbose_unhandled_irq: bool = (builtin.mode == .Debug),
};
};

pub const scb = struct {
pub const SHCSR = packed struct(u32) {
/// [0]
Expand Down
62 changes: 19 additions & 43 deletions port/raspberrypi/rp2xxx/src/hal.zig
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ comptime {
// HACK: tests can't access microzig. maybe there's a better way to do this.
if (!builtin.is_test and compatibility.chip == .RP2350) {
_ = bootmeta;

if (compatibility.arch == .arm and microzig.options.hal.use_dcp) {
_ = dcp;
}
}

// On the RP2040, we need to import the `atomic.zig` file to export some global
Expand All @@ -69,12 +65,6 @@ pub const HAL_Options = switch (compatibility.chip) {
next_block: ?*const anyopaque = null,
} = .{},

/// Enable the FPU and lazy state preservation. Leads to much faster
/// single precision floating point arithmetic. If you want a custom
/// setup set this to false and configure the fpu yourself. Ignored on
/// riscv.
enable_fpu: bool = is_fpu_used,

/// Enable the DCP and export intrinsics. Leads to faster double
/// precision floating point arithmetic. Ignored on riscv.
use_dcp: bool = true,
Expand All @@ -94,12 +84,17 @@ pub inline fn init() void {
init_sequence(clock_config);
}

const is_fpu_used: bool = builtin.abi.float() == .hard;

/// Allows user to easily swap in their own clock config while still
/// using the recommended initialization sequence
pub fn init_sequence(comptime clock_cfg: clocks.config.Global) void {
maybe_enable_fpu_and_dcp();
if (compatibility.chip == .RP2350 and compatibility.arch == .arm and
microzig.options.hal.use_dcp)
{
// Export double floating point intrinsics
_ = dcp;

enable_dcp();
}

// Disable the watchdog as a soft reset doesn't disable the WD automatically!
watchdog.disable();
Expand Down Expand Up @@ -129,38 +124,19 @@ pub fn init_sequence(comptime clock_cfg: clocks.config.Global) void {
resets.unreset_block_wait(resets.masks.all);
}

/// Enables fpu and/or dcp on RP2350 arm if requested in HAL options. On RP2350
/// riscv and RP2040 this is a noop. Called in init_sequence and on core1
/// Enables dcp on RP2350 arm.
///
/// NOTE: Called automatically in the hal startup sequence and in core1
/// startup.
pub fn maybe_enable_fpu_and_dcp() void {
if (compatibility.chip == .RP2350 and
compatibility.arch == .arm)
{
if (microzig.options.hal.enable_fpu) {
if (is_fpu_used) {
// enable lazy state preservation
microzig.cpu.peripherals.fpu.FPCCR.modify(.{
.ASPEN = 1,
.LSPEN = 1,
});

// enable the FPU for the current core
microzig.cpu.peripherals.scb.CPACR.modify(.{
.CP10 = .full_access,
.CP11 = .full_access,
});
} else {
@compileError("target doesn't have FPU features enabled");
}
}

if (microzig.options.hal.use_dcp) {
// enable the DCP for the current core
microzig.cpu.peripherals.scb.CPACR.modify(.{
.CP4 = .full_access,
});
}
pub inline fn enable_dcp() void {
if (!(compatibility.chip == .RP2350 and compatibility.arch == .arm)) {
@compileError("Enable DCP is only available on RP2350 arm");
}

// enable the DCP for the current core
microzig.cpu.peripherals.scb.CPACR.modify(.{
.CP4 = .full_access,
});
}

pub fn get_cpu_id() u32 {
Expand Down
11 changes: 10 additions & 1 deletion port/raspberrypi/rp2xxx/src/hal/multicore.zig
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const builtin = @import("builtin");
const std = @import("std");
const assert = std.debug.assert;

Expand Down Expand Up @@ -77,7 +78,15 @@ pub fn launch_core1_with_stack(entrypoint: *const fn () void, stack: []u32) void
fn _wrapper(_: u32, _: u32, _: u32, _: u32, entry: u32, stack_base: [*]u32) callconv(.c) void {
// TODO: protect stack using MPU
_ = stack_base;
microzig.hal.maybe_enable_fpu_and_dcp();
if (microzig.hal.compatibility.chip == .RP2350) {
if (microzig.options.cpu.enable_fpu) {
microzig.cpu.enable_fpu();
}

if (microzig.options.hal.use_dcp) {
microzig.hal.enable_dcp();
}
}
@as(*const fn () void, @ptrFromInt(entry))();
}
}._wrapper;
Expand Down
1 change: 0 additions & 1 deletion port/stmicro/stm32/src/boards/STM32F3DISCOVERY.zig
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ pub const pin_map = .{
};

pub fn init() void {
hal.enable_fpu();
rcc.enable_hse(8_000_000);
rcc.enable_pll(.HSE, .Div1, .Mul5) catch {
@panic("PLL faile to enable");
Expand Down
11 changes: 0 additions & 11 deletions port/stmicro/stm32/src/hals/STM32F303.zig
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,6 @@ const I2C1 = microzig.peripherals.I2C1;

pub const cpu = @import("cpu");

pub fn enable_fpu() void {
microzig.cpu.peripherals.scb.CPACR.modify(.{
.CP10 = .full_access,
.CP11 = .full_access,
});
microzig.cpu.peripherals.fpu.FPCCR.modify(.{
.ASPEN = 1,
.LSPEN = 1,
});
}

pub fn parse_pin(comptime spec: []const u8) type {
const invalid_format_msg = "The given pin '" ++ spec ++ "' has an invalid format. Pins must follow the format \"P{Port}{Pin}\" scheme.";

Expand Down
Loading
Loading