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

Use named constants in the switcher #339

Merged
merged 4 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions sdk/core/loader/boot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,14 @@ namespace
FirstDynamicSoftware = 0x1000000,
};

// The switcher assembly includes the types of import table entries and
// trusted stacks. This enumeration and the assembly must be kept in sync.
// This will fail if the enumeration value changes.
static_assert(int(SealedImportTableEntries) == 9,
"If this fails, update switcher/entry.S to the new value");
static_assert(int(SealedTrustedStacks) == 10,
"If this fails, update switcher/entry.S to the new value");

// The allocator and static sealing types must be contiguous so that the
// token library can hold a permit-unseal capability for both.
static_assert(int(Allocator) + 1 == int(StaticToken),
Expand All @@ -162,6 +170,18 @@ namespace
static_assert(magic_enum::enum_count<SealingType>() <= 12,
"Too many sealing types reserved for a 3-bit otype field");

} // namespace

/*
* Unusually late, include this where we have access to the above enum
* SealingType, but early enough that the constants defined herein are available
* to the rest of the code.
*/
#include "../switcher/misc-assembly.h"

namespace
{

constexpr auto StoreLPerm = Root::Permissions<Root::Type::RWStoreL>;
/// PCC permissions for the switcher.
constexpr auto SwitcherPccPermissions =
Expand Down
25 changes: 13 additions & 12 deletions sdk/core/switcher/entry.S
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "export-table-assembly.h"
#include "trusted-stack-assembly.h"
#include "misc-assembly.h"
#include <errno.h>

.include "assembly-helpers.s"
Expand Down Expand Up @@ -256,7 +257,7 @@ __Z26compartment_switcher_entryz:

// Fetch the sealing key
LoadCapPCC cs0, compartment_switcher_sealing_key
li gp, 9
li gp, SEAL_TYPE_SealedImportTableEntries
csetaddr cs0, cs0, gp
// The target capability is in ct1. Unseal, check tag and load the entry point offset.
cunseal ct1, ct1, cs0
Expand Down Expand Up @@ -489,7 +490,7 @@ exception_entry_asm:
// If we hit one of the exception conditions that we should let
// compartments handle then deliver it to the compartment.
// CHERI exception code.
li a0, 0x1c
li a0, MCAUSE_CHERI
beq a0, t1, .Lhandle_error
// Misaligned instruction, instruction access, illegal instruction,
// breakpoint, misaligned load, load fault, misaligned store, and store
Expand Down Expand Up @@ -534,7 +535,7 @@ exception_entry_asm:

// Switch onto the new thread's trusted stack
LoadCapPCC ct0, compartment_switcher_sealing_key
li gp, 10
li gp, SEAL_TYPE_SealedTrustedStacks
csetaddr ct0, ct0, gp
cunseal csp, ca0, ct0
clw t0, TrustedStack_offset_mcause(csp)
Expand All @@ -548,11 +549,11 @@ exception_entry_asm:
// mret, so reentrancy is no longer a concern.
cspecialw mtdc, csp

// If mcause is 25, then we will jump into the error handler: another
// thread has signalled that this thread should be interrupted. 25 is a
// reserved exception number that we repurpose to indicate explicit
// interruption.
li t1, 25
// If mcause is MCAUSE_THREAD_INTERRUPT, then we will jump into the error
// handler: another thread has signalled that this thread should be
// interrupted. MCAUSE_THREAD_INTERRUPT is a reserved exception number that
// we repurpose to indicate explicit interruption.
li t1, MCAUSE_THREAD_INTERRUPT
beq t0, t1, .Lhandle_injected_error

// Environment call from M-mode is exception code 11.
Expand Down Expand Up @@ -858,7 +859,7 @@ exception_entry_asm:

// Value 24 is reserved for custom use.
.Lset_mcause_and_exit_thread:
csrw mcause, 24
csrw mcause, MCAUSE_THREAD_EXIT
// The thread exit code expects the trusted stack pointer to be in csp and
// the stack pointer to be in mtdc. After thread exit, we don't need the
// stack pointer so just put zero there.
Expand Down Expand Up @@ -964,7 +965,7 @@ __Z25switcher_interrupt_threadPv:
// Load the unsealing key into a register that we will clobber two
// instructions later.
LoadCapPCC ca1, compartment_switcher_sealing_key
li a2, 10
li a2, SEAL_TYPE_SealedTrustedStacks
csetaddr ca1, ca1, a2
// The target capability is in ct1. Unseal, check tag and load the entry point offset.
cunseal ca1, ca0, ca1
Expand Down Expand Up @@ -1010,7 +1011,7 @@ __Z25switcher_interrupt_threadPv:

// Mark the thread as interrupted.
// Store a magic value in mcause
li a2, 25
li a2, MCAUSE_THREAD_INTERRUPT
csw a2, TrustedStack_offset_mcause(ca1)
// Return success
li a0, 1
Expand All @@ -1023,7 +1024,7 @@ __Z25switcher_interrupt_threadPv:
.type __Z23switcher_current_threadv,@function
__Z23switcher_current_threadv:
LoadCapPCC ca0, compartment_switcher_sealing_key
li a1, 10
li a1, SEAL_TYPE_SealedTrustedStacks
csetaddr ca0, ca0, a1
cspecialr ca1, mtdc
cseal ca0, ca1, ca0
Expand Down
43 changes: 43 additions & 0 deletions sdk/core/switcher/misc-assembly.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright CHERIoT Contributors.
// SPDX-License-Identifier: MIT

#pragma once
#include <assembly-helpers.h>

/*
* Constant to represent the raw permissions of the compartment CSP. We use
* this in the switcher, to verify the permissions of the CSP that comes from
* the compartment are exactly what we expect.
*/
EXPORT_ASSEMBLY_EXPRESSION(COMPARTMENT_STACK_PERMISSIONS,
(CHERI::PermissionSet{
CHERI::Permission::Load,
CHERI::Permission::Store,
CHERI::Permission::LoadStoreCapability,
CHERI::Permission::LoadMutable,
CHERI::Permission::StoreLocal,
CHERI::Permission::LoadGlobal}
.as_raw()),
0x7e)

/**
* Space reserved at the top of a stack on entry to the compartment.
*
* This *must* be a multiple of 16, which is the stack alignment.
*/
#define STACK_ENTRY_RESERVED_SPACE 16

#ifdef __cplusplus
using namespace priv;
#endif

EXPORT_ASSEMBLY_NAME(MCAUSE_THREAD_EXIT, 24)
EXPORT_ASSEMBLY_NAME(MCAUSE_THREAD_INTERRUPT, 25)
EXPORT_ASSEMBLY_NAME(MCAUSE_CHERI, 28)

EXPORT_ASSEMBLY_EXPRESSION(SEAL_TYPE_SealedImportTableEntries,
SealingType::SealedImportTableEntries,
9)
EXPORT_ASSEMBLY_EXPRESSION(SEAL_TYPE_SealedTrustedStacks,
SealingType::SealedTrustedStacks,
10)
15 changes: 0 additions & 15 deletions sdk/core/switcher/trusted-stack-assembly.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,3 @@ EXPORT_ASSEMBLY_SIZE(TrustedStackFrame, (8 * 3))

#define TSTACKOFFSET_FIRSTFRAME \
(TrustedStack_offset_frameoffset + TSTACK_HEADER_SZ)

/* Constant to represent the raw permissions of the compartment CSP.
* We use this in the switcher, to verify the CSP comes from the
* compartment is exactly what we expect.
* This represents the following permissions:
* Load, Store, LoadStoreCapability, LoadMutable StoreLocal and LoadGlobal
*/
#define COMPARTMENT_STACK_PERMISSIONS 0x7e

/**
* Space reserved at the top of a stack on entry to the compartment.
*
* This *must* be a multiple of 16, which is the stack alignment.
*/
#define STACK_ENTRY_RESERVED_SPACE 16
10 changes: 0 additions & 10 deletions sdk/core/switcher/tstack.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,3 @@ struct TrustedStackGeneric
using TrustedStack = TrustedStackGeneric<0>;

#include "trusted-stack-assembly.h"

static_assert(
CheckSize<COMPARTMENT_STACK_PERMISSIONS,
CHERI::PermissionSet{CHERI::Permission::Load,
CHERI::Permission::Store,
CHERI::Permission::LoadStoreCapability,
CHERI::Permission::LoadMutable,
CHERI::Permission::StoreLocal,
CHERI::Permission::LoadGlobal}
.as_raw()>::value);
27 changes: 26 additions & 1 deletion sdk/include/assembly-helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,25 @@ struct CheckSize
static constexpr bool value = Real == Expected;
};

/**
* Export a macro into assembly named `name` with value `value`. In C++, this
* macro will report an error if the provided value does not equal the constexpr
* evaluation of `expression`.
*/
# define EXPORT_ASSEMBLY_NAME(name, val) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

It took me a minute to work out why we need both of these macros. I guess this one is just the same as EXPORT_ASSEMBLY_EXPRESSION(name, name, val) but without a name collision? That makes me think maybe we should use {} around the definition below?

Copy link
Member Author

Choose a reason for hiding this comment

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

We might want the names available in C++, maybe, and the invoking source could, in principle, choose to put things in scope to hide them if desired, but because these are otherwise at top-level, just using {} doesn't do the trick, eliciting "expected unqualified-id" errors from the compiler.

static_assert(CheckSize<name, val>::value, \
"Value provided for assembly is incorrect");

/**
* Export a macro into assembly named `name` with value `value`. In C++, this
* macro will report an error if the provided value does not equal the constexpr
* evaluation of `expression`.
*/
# define EXPORT_ASSEMBLY_EXPRESSION(name, expression, val) \
static constexpr size_t name = expression; \
static_assert(CheckSize<name, val>::value, \
"Value provided for assembly is incorrect");

/**
* Export a macro into assembly of the form `{structure}_offset_{field}`. The
* value of this macro will be `value`. In C++, this macro will report an error
Expand Down Expand Up @@ -61,12 +80,18 @@ struct CheckSize
static_assert(CheckSize<sizeof(structure), val>::value, \
"Size provided for assembly is incorrect");
#elif defined(__ASSEMBLER__)
# define EXPORT_ASSEMBLY_NAME(name, value) \
.set name, value
# define EXPORT_ASSEMBLY_EXPRESSION(name, expression, value) \
.set name, value
# define EXPORT_ASSEMBLY_OFFSET_NAMED(structure, field, value, name) \
.set name value
.set name, value
# define EXPORT_ASSEMBLY_OFFSET(structure, field, value) \
.set structure##_offset_##field, value
# define EXPORT_ASSEMBLY_SIZE(structure, value) .set structure##_size, value
#else
# define EXPORT_ASSEMBLY_NAME(name, value)
# define EXPORT_ASSEMBLY_EXPRESSION(name, expression, value)
# define EXPORT_ASSEMBLY_OFFSET(structure, field, name, value)
# define EXPORT_ASSEMBLY_SIZE(structure, name, value)
# define EXPORT_ASSEMBLY_OFFSET_NAMED(structure, field, value, name)
Expand Down
1 change: 1 addition & 0 deletions sdk/include/priv/riscv.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ namespace priv
constexpr size_t MCAUSE_LOAD_PAGE_FAULT = 13;
constexpr size_t MCAUSE_STORE_PAGE_FAULT = 15;
constexpr size_t MCAUSE_THREAD_EXIT = 24;
constexpr size_t MCAUSE_THREAD_INTERRUPT = 25;
constexpr size_t MCAUSE_CHERI = 28;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be a separate PR but these constants are needed in error handlers so we should make them more publicly available in the SDK.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a public heading (and this is why I don't like abbreviations in names of things: priv is short for 'privileged spec' not 'private').

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess there's nothing stopping users from include priv/riscv.h it just seems a bit weird. At any rate we should do this in relevant places in the examples/tests.


constexpr size_t MSTATUS_UIE = (1 << 0);
Expand Down
Loading