Add PICO_MINIMAL_VECTOR_TABLE compile option #2233

will-v-pi · 2025-02-04T13:34:08Z

This option will create a vector table which only contains NMI and HardFault vectors, and a single bkpt instruction for them to point to. I believe that this is the smallest size vector table that can be used safely with RP2350, as the NMI and HardFault interrupts cannot be disabled, and by default BusFaults etc all escalate to HardFaults so don't require separate vectors.

When this is enabled you cannot use any interrupts - I haven't disabled them as I wasn't sure of the best way to do that, but I've mentioned that in the PICO_CONFIG comment.

This supercedes #2200

Creates vector tables which only contains NMI and HardFault vectors, and a single bkpt instruction for them to point to

lurch · 2025-02-20T17:06:54Z

When this is enabled you cannot use any interrupts - I haven't disabled them as I wasn't sure of the best way to do that

Would the disable_interrupts() that got added in #2276 be useful here?

will-v-pi · 2025-02-24T14:30:50Z

When this is enabled you cannot use any interrupts - I haven't disabled them as I wasn't sure of the best way to do that

Would the disable_interrupts() that got added in #2276 be useful here?

Yes, although I think @kilograham would need to suggest the best place to put a call to that function?

This will now fail CI checks until raspberrypi/pico-sdk#2233 is merged

* There was a pre-existing (undocumented) variable PICO_NO_STORED_VECTOR_TABLE that did something similar (have replaced that with the new var) * Renamed PICO_MINIMAL_VECTOR_TABLE to PICO_MINIMAL_STORED_VECTOR_TABLE to contrast PICO_NO_RAM_VECTOR_TABLE * Explicitly mentioned that PICO_MINIMAL_STORED_VECTOR_TABLE has no effect on RISC-V atm * Added a new documented variable PICO_NUM_VTABLE_IRQs to indicate how many IRQs the user would like to store in the vtable (either stored or RAM) - this is the limit of what any IRQ api can touch, or indeed what isr_irqN you can override. This is a better alternative to changing PICO_RAM_VECTOR_TABLE_SIZE (which wasn't documented), but did allow you (unsafely) to limit the RAM vector table size (but not the stored size) TLDR: 1. if you set PICO_MINIMAL_STORED_VECTOR_TABLE=1, you get a 4 word VTABLE on Arm.. you still get a RAM vector table with up to PICO_NUM_VTABLE_IRQs (defaults to NUM_IRQs) but everything - including things like `isr_pendsv` point to `__unhandled_user_irq`. 2. PICO_NUM_VTABLE_IRQs (default NUM_IRQS) sets the number of IRQs that have handlers in the VTABLE (this affects the space reserved, and what the irq APIs let you touch - at least as far as invalid_params_if!) 3. If you set PICO_MINIMAL_STORED_VECTOR_TABLE=1 for a no flash binary, then PICO_NUM_VTABLE_IRQs is forced to 0 as there is no reserved space for the handlers.

Wren6991 · 2025-03-22T18:03:42Z

I don't think it's necessary to disable IRQs. Any reset that restores the old value of VTOR will also clear all the per-IRQ enables in the NVIC (or Xh3irq controller). The fact that we leave crt0 with the global interrupt enable set is also something we need to maintain for compatibility. It would be a bug to enable an IRQ without first installing a vector table, but it wouldn't be our bug.

Wren6991 · 2025-03-22T18:08:07Z

src/rp2_common/pico_crt0/crt0.S

 __VECTOR_TABLE:
 __vectors:
 .word __StackTop
 .word _reset_handler
-.word isr_nmi


This seems to have removed the ability to hook NMI and HardFault without installing a RAM vector table.

oops; goot catch... somehow the two paths looked the same to me!

Wren6991 · 2025-03-22T18:11:09Z

src/rp2_common/pico_crt0/crt0.S

@@ -11,6 +11,7 @@

 #include "hardware/regs/addressmap.h"
 #include "hardware/regs/sio.h"
+#include "hardware/irq.h"


Looks like a new dependency on the hardware_irq library, which is not reflected in the CMake.

Ideally I'd like to be able to pull in the crt0 without the rest of the SDK. hardware_irq pulls in pico_sync etc

yeah, actually it was a bit of a circular nightmare because now the defines in hardware_irq.h are dependent on the crt0 vars... I'll actually go to what i was half way thru doing (which is to make hardware_irq dependent on the crt0.S headers - which I had avoided since it is hardware->pico - but i hadn't realized that already dependended on pico headers.

i have created a new pico_platfrom_common since we didn't have a good place to put common code between different pico_platforms (and this header is a good place to put these common defines)

^ note that this has deduplicated some code that was in both rp2350/pico_platform and rp2040/pico_platform

…cate some other platform code

will-v-pi · 2025-03-24T11:36:45Z

This rework has added 8 bytes to the size of the vector table, by replacing

20080010 <isr_invalid>:
20080010:     be00            bkpt    0x0000
20080012:     bf00            nop

with

20080010 <isr_invalid>:
20080010:     be00            bkpt    0x0000

20080012 <__unhandled_user_irq>:
20080012:     f3ef 8005       mrs     r0, IPSR
20080016:     3810            subs    r0, #16

20080018 <unhandled_user_irq_num_in_r0>:
20080018:     be00            bkpt    0x0000
2008001a:     bf00            nop

The encrypted bootloader can be modified to accomodate this extra size, but would it be possible to remove those unhandled IRQ functions, as they are not needed when both PICO_NO_FLASH and PICO_MINIMAL_STORED_VECTOR_TABLE are set?

…e e.g. isr_irq11 and there aren't 11 vector table entries - omit guts of __unhandler_user_irq if not needed, but keep label and separate breakpoint (since there is an alignment hole of 2 bytes anyway)

kilograham · 2025-03-24T19:01:06Z

good catch; see if that is better

src/rp2350/pico_platform/include/pico/platform.h

lurch · 2025-03-24T19:26:36Z

src/rp2_common/pico_platform_common/common.c

+
+#if !PICO_NO_SIM_CHECK
+bool __attribute__((weak)) running_in_sim(void) {
+    return (*(io_ro_32 *)TBMAN_BASE) & TBMAN_PLATFORM_HDLSIM_BITS;


The RP2040 version of pico_platform had running_on_fpga but not running_in_sim. Does that mean that if you set PICO_NO_SIM_CHECK to 0 and tried building for RP2040, this would give you a compilation error? (I suspect yes, because https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2040/hardware_regs/include/hardware/regs/tbman.h doesn't declare TBMAN_PLATFORM_HDLSIM_BITS ?)
Although whether this matters or not is of course a different question... 😉

lurch · 2025-03-24T19:27:56Z

Something is making the Bazel CI build fail with src/rp2_common/pico_platform_common/common.c:7:10: fatal error: pico.h: No such file or directory

armandomontanez · 2025-03-24T21:59:03Z

Something is making the Bazel CI build fail with src/rp2_common/pico_platform_common/common.c:7:10: fatal error: pico.h: No such file or directory

Ah, ye ol' "missing dep into a dependency cycle." This should fix it:

diff --git a/src/common/pico_base_headers/BUILD.bazel b/src/common/pico_base_headers/BUILD.bazel
index 5525c30..c22f9ae 100644
--- a/src/common/pico_base_headers/BUILD.bazel
+++ b/src/common/pico_base_headers/BUILD.bazel
@@ -112,6 +112,7 @@ cc_library(
         "//src/rp2_common/hardware_watchdog:__pkg__",
         "//src/rp2_common/hardware_xosc:__pkg__",
         "//src/rp2_common/pico_crt0:__pkg__",
+        "//src/rp2_common/pico_platform_common:__pkg__",
         "//src/rp2_common/pico_printf:__pkg__",
         "//src/rp2_common/pico_runtime:__pkg__",
         "//src/rp2_common/pico_runtime_init:__pkg__",
diff --git a/src/rp2040/pico_platform/BUILD.bazel b/src/rp2040/pico_platform/BUILD.bazel
index b2d98c8..47b90e4 100644
--- a/src/rp2040/pico_platform/BUILD.bazel
+++ b/src/rp2040/pico_platform/BUILD.bazel
@@ -27,7 +27,7 @@ cc_library(
     deps = [
         "//src/rp2040/hardware_regs",
         "//src/rp2040/hardware_regs:platform_defs",
-        "//src/rp2_common/pico_platform_common",
+        "//src/rp2_common/pico_platform_common:pico_platform_common_headers",
         "//src/rp2_common/pico_platform_compiler",
         "//src/rp2_common/pico_platform_panic:pico_platform_panic_headers",
         "//src/rp2_common/pico_platform_sections",
diff --git a/src/rp2350/pico_platform/BUILD.bazel b/src/rp2350/pico_platform/BUILD.bazel
index 002dbd3..b24291a 100644
--- a/src/rp2350/pico_platform/BUILD.bazel
+++ b/src/rp2350/pico_platform/BUILD.bazel
@@ -27,7 +27,7 @@ cc_library(
     deps = [
         "//src/rp2350/hardware_regs",
         "//src/rp2350/hardware_regs:platform_defs",
-        "//src/rp2_common/pico_platform_common",
+        "//src/rp2_common/pico_platform_common:pico_platform_common_headers",
         "//src/rp2_common/pico_platform_compiler",
         "//src/rp2_common/pico_platform_panic:pico_platform_panic_headers",
         "//src/rp2_common/pico_platform_sections",
diff --git a/src/rp2_common/pico_platform_common/BUILD.bazel b/src/rp2_common/pico_platform_common/BUILD.bazel
index 5241271..18a373a 100644
--- a/src/rp2_common/pico_platform_common/BUILD.bazel
+++ b/src/rp2_common/pico_platform_common/BUILD.bazel
@@ -3,12 +3,23 @@ load("//bazel:defs.bzl", "compatible_with_rp2")
 package(default_visibility = ["//visibility:public"])

 cc_library(
-    name = "pico_platform_common",
+    name = "pico_platform_common_headers",
     hdrs = ["include/pico/platform/common.h"],
-    srcs = ["common.c"],
     includes = ["include"],
+    visibility = [
+        "//src/rp2040/pico_platform:__pkg__",
+        "//src/rp2350/pico_platform:__pkg__",
+    ],
+)
+
+cc_library(
+    name = "pico_platform_common",
+    srcs = ["common.c"],
     target_compatible_with = compatible_with_rp2(),
     deps = [
+        ":pico_platform_common_headers",
         "//src/rp2_common:platform_defs",
+        "//src/rp2_common/hardware_base",
+        "//src/common/pico_base_headers",
     ],
 )

kilograham · 2025-03-24T22:36:44Z

When this is enabled you cannot use any interrupts - I haven't disabled them as I wasn't sure of the best way to do that

Would the disable_interrupts() that got added in #2276 be useful here?

Yes, although I think @kilograham would need to suggest the best place to put a call to that function?

No need to do anything here now - if you enable an IRQ you don't have, that's on you (you would get a param assertion if that was enabled)

kilograham · 2025-03-24T22:38:14Z

Ah, ye ol' "missing dep into a dependency cycle." This should fix it:

ooh thanks!

will-v-pi · 2025-03-25T09:31:27Z

good catch; see if that is better

It's still 8 bytes bigger, but with isr_invalid and the unhandled user stuff the other way round

20080010 <__unhandled_user_irq>:
20080010:	f3ef 8005 	mrs	r0, IPSR
20080014:	3810      	subs	r0, #16

20080016 <unhandled_user_irq_num_in_r0>:
20080016:	be00      	bkpt	0x0000

20080018 <isr_invalid>:
20080018:	be00      	bkpt	0x0000

2008001a <__default_isrs_end>:
2008001a:	bf00      	nop

You can check it by compiling the encrypted-shares branch of picotool with -DUSE_PRECOMPILED=0 - the compilation will fail if it's the larger size

kilograham · 2025-03-25T14:46:58Z

I'll try later, i thought i had added PICO_NO_RAM_VECTOR_TABLE=1 which i hadn't, which will probably help - should also save some RAM

kilograham · 2025-03-25T18:15:04Z

I'll try later, i thought i had added PICO_NO_RAM_VECTOR_TABLE=1 which i hadn't, which will probably help - should also save some RAM

This fixed it, so i think this is ready to merge

will-v-pi · 2025-03-26T10:51:11Z

I agree - happy to merge this now too

will-v-pi · 2025-03-31T10:40:33Z

@kilograham looks like this has introduced a compilation warning in pico-examples (which then causes an error for pico-examples CI runs https://github.com/raspberrypi/pico-examples/actions/runs/14169572195/job/39690172401)

pico-sdk/src/rp2_common/pico_runtime_init/runtime_init.c:219:29: error: conversion to 'uint32_t' {aka 'long unsigned int'} from 'int' may change the sign of the result [-Werror=sign-conversion]
  219 |     uint32_t stored_words = &__vectors_end - &__vectors;
      |                             ^
compilation terminated due to -Wfatal-errors.
cc1: all warnings being treated as errors

This will now fail CI checks until raspberrypi/pico-sdk#2233 is merged

Add PICO_MINIMAL_VECTOR_TABLE option

1412f8a

Creates vector tables which only contains NMI and HardFault vectors, and a single bkpt instruction for them to point to

will-v-pi added this to the 2.1.2 milestone Feb 4, 2025

will-v-pi mentioned this pull request Feb 4, 2025

Add ability to override number of IRQ vectors #2200

Closed

will-v-pi requested a review from kilograham February 24, 2025 14:31

will-v-pi added a commit to raspberrypi/picotool that referenced this pull request Feb 26, 2025

Remove pico-sdk branch handling

92ffec2

This will now fail CI checks until raspberrypi/pico-sdk#2233 is merged

will-v-pi mentioned this pull request Feb 26, 2025

Add support for creating self-decrypting binaries, and use 4-way AES key shares instead of just the AES key raspberrypi/picotool#207

Open

kilograham requested a review from Wren6991 March 22, 2025 17:41

fix RP2040 build

b653c83

Wren6991 reviewed Mar 22, 2025

View reviewed changes

move interdependent vars into new pico/platform/common.h, and dedupli…

cf56ddd

…cate some other platform code

kilograham added 2 commits March 24, 2025 13:53

- make non overridable symbols .global to cause link error if you hav…

d271b86

…e e.g. isr_irq11 and there aren't 11 vector table entries - omit guts of __unhandler_user_irq if not needed, but keep label and separate breakpoint (since there is an alignment hole of 2 bytes anyway)

Merge develop into min-vtor

8e58012

lurch reviewed Mar 24, 2025

View reviewed changes

src/rp2350/pico_platform/include/pico/platform.h Outdated Show resolved Hide resolved

lurch reviewed Mar 24, 2025

View reviewed changes

fix compile warnig, and add C++ support to new common header

d530ebb

bazel fix patch

da04544

kilograham requested a review from peterharperuk March 25, 2025 18:15

kilograham requested a review from Wren6991 March 25, 2025 18:15

kilograham added 2 commits March 26, 2025 10:46

fix PICO_CONFIG description

9655939

collapse 2 asserts to 1

339a3aa

kilograham approved these changes Mar 26, 2025

View reviewed changes

kilograham merged commit a5ba689 into raspberrypi:develop Mar 26, 2025
4 checks passed

will-v-pi added a commit to raspberrypi/picotool that referenced this pull request Apr 1, 2025

Remove pico-sdk branch handling

a8a8cd7

This will now fail CI checks until raspberrypi/pico-sdk#2233 is merged

will-v-pi deleted the min-vtor branch April 2, 2025 14:54

will-v-pi added a commit to will-v-pi/pico-sdk that referenced this pull request Apr 2, 2025

Fix sign conversion error introduced by raspberrypi#2233

8ff6c7a

will-v-pi mentioned this pull request Apr 2, 2025

Fix sign conversion error introduced by #2233 #2392

Merged

kilograham pushed a commit that referenced this pull request Apr 2, 2025

Fix sign conversion error introduced by #2233 (#2392)

472c781

kilograham mentioned this pull request Apr 4, 2025

#define PICO_RP2350A 0 to mean RP2350B is confusing #2364

Closed

earlephilhower mentioned this pull request Apr 7, 2025

Inconsistent of PICO_RP2350B earlephilhower/arduino-pico#2878

Closed

will-v-pi added a commit to raspberrypi/picotool that referenced this pull request Apr 15, 2025

Remove pico-sdk branch handling

d725474

This will now fail CI checks until raspberrypi/pico-sdk#2233 is merged

will-v-pi added a commit to raspberrypi/picotool that referenced this pull request Apr 22, 2025

Remove pico-sdk branch handling

abb4e63

This will now fail CI checks until raspberrypi/pico-sdk#2233 is merged

hanshuebner mentioned this pull request May 18, 2025

Set PICO_RP2350A to 1 for Pimoroni PGA2350 #2474

Closed

Add PICO_MINIMAL_VECTOR_TABLE compile option #2233

Add PICO_MINIMAL_VECTOR_TABLE compile option #2233

Uh oh!

Conversation

will-v-pi commented Feb 4, 2025

Uh oh!

lurch commented Feb 20, 2025

Uh oh!

will-v-pi commented Feb 24, 2025

Uh oh!

Wren6991 commented Mar 22, 2025

Uh oh!

Wren6991 Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

kilograham Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

Wren6991 Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

kilograham Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

kilograham Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

kilograham Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

will-v-pi commented Mar 24, 2025

Uh oh!

kilograham commented Mar 24, 2025

Uh oh!

Uh oh!

lurch Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

lurch commented Mar 24, 2025

Uh oh!

armandomontanez commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kilograham commented Mar 24, 2025

Uh oh!

kilograham commented Mar 24, 2025

Uh oh!

will-v-pi commented Mar 25, 2025

Uh oh!

kilograham commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kilograham commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

will-v-pi commented Mar 26, 2025

Uh oh!

Uh oh!

will-v-pi commented Mar 31, 2025

Uh oh!

Uh oh!

armandomontanez commented Mar 24, 2025 •

edited

Loading

kilograham commented Mar 25, 2025 •

edited

Loading

kilograham commented Mar 25, 2025 •

edited

Loading