opentmk: libify opentmk core

[wanghenry-msft]

This makes the opentmk core code to be a library to prepare for another opentmk-based image that also uses the same core stuff. This moves all the testing specific code out into another binary crate that builds on top of opentmk lib.

Also makes a few minor API changes:

• Adds a context trait that is an abstracted hypercall for a platform, while also allowing to pass platform-specific parameters that may or may not be honored by the underlying platform.
• Makes in/out instructions unsafe to be consistent with how it can be used (i.e. not all instances of operating raw I/O would be considered safe)
• Add APIs to read from serial as well

[github-actions]

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

[wanghenry-msft]

This PR was made in communications with Mayank and his team

[Copilot]

Pull request overview

This PR restructures opentmk by moving the core implementation into a library crate so multiple UEFI images/binaries can share the same framework code, and it adds a separate opentmk_tests binary crate for test execution.

Changes:

• Converts opentmk into a library (opentmk/src/lib.rs) and moves the UEFI test entrypoint into a new opentmk_tests binary.
• Widens visibility for selected APIs (UEFI allocator module, Hyper-V hypercall helpers, assert formatting helpers) to support reuse.
• Makes raw port I/O APIs unsafe and updates call sites accordingly; adds additional serial I/O helpers.

Reviewed changes

Copilot reviewed 21 out of 24 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
opentmk/src/uefi/mod.rs	Exposes UEFI allocator module publicly; removes in-crate UEFI test entrypoint.
opentmk/src/uefi/alloc.rs	Adds mmap helper and adjusts allocator helpers.
opentmk/src/tmk_assert.rs	Makes selected assert/print helpers public for downstream crates.
opentmk/src/platform/hyperv/arch/hypercall.rs	Temporarily makes hypercall page + dispatch/input accessors public.
opentmk/src/lib.rs	Introduces opentmk library crate root and module exports.
opentmk/src/arch/x86_64/tpm.rs	Wraps port I/O in unsafe blocks after I/O APIs become unsafe.
opentmk/src/arch/x86_64/serial.rs	Updates UART init sequence; makes write_byte public and adds read/drain helpers.
opentmk/src/arch/x86_64/rtc.rs	Wraps CMOS port I/O in unsafe blocks after I/O APIs become unsafe.
opentmk/src/arch/x86_64/mod.rs	Makes io module public for external callers.
opentmk/src/arch/x86_64/io.rs	Marks port I/O functions unsafe and adds 16-bit in/out helpers.
opentmk/opentmk_tests/src/tests/mod.rs	Switches test imports to use the opentmk library.
opentmk/opentmk_tests/src/tests/hyperv/test_helpers.rs	Adds macro helper for asm wrappers around test functions.
opentmk/opentmk_tests/src/tests/hyperv/mod.rs	Introduces Hyper-V test module organization in the new binary crate.
opentmk/opentmk_tests/src/tests/hyperv/hv_tpm_write_cvm.rs	Updates imports to use the opentmk library.
opentmk/opentmk_tests/src/tests/hyperv/hv_tpm_read_cvm.rs	Updates imports to use the opentmk library.
opentmk/opentmk_tests/src/tests/hyperv/hv_register_intercept.rs	Updates imports to use the opentmk library.
opentmk/opentmk_tests/src/tests/hyperv/hv_processor.rs	Updates imports to use the opentmk library.
opentmk/opentmk_tests/src/tests/hyperv/hv_memory_protect_write.rs	Updates imports to use the opentmk library.
opentmk/opentmk_tests/src/tests/hyperv/hv_memory_protect_read.rs	Updates imports to use the opentmk library.
opentmk/opentmk_tests/src/tests/hyperv/hv_error_vp_start.rs	Updates imports and error enum path to use the opentmk library.
opentmk/opentmk_tests/src/main.rs	Adds new UEFI entrypoint binary that initializes opentmk + runs tests.
opentmk/opentmk_tests/Cargo.toml	Adds new opentmk_tests package and dependencies.
Cargo.toml	Adds opentmk_tests as a workspace member and adds opentmk to workspace dependencies.
Cargo.lock	Records the new opentmk_tests workspace package.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 24 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

opentmk/src/arch/x86_64/serial.rs:101

• The FIFO control value written to base + 2 is 0x07, but the comment still claims a 14-byte threshold. 0x07 enables FIFO + clears TX/RX, but does not set the trigger level bits (those are in 0xC0). Either update the comment or restore the previous value if a 14-byte trigger is intended.

            self.io.outb(self.serial_port.value() + 1, 0); // High byte divisor
            self.io.outb(self.serial_port.value() + 3, 0x03); // 8 bits, 1 stop bit, no parity
            self.io.outb(self.serial_port.value() + 2, 0x07); // Enable FIFO, clear them, with 14-byte threshold
        }

[Copilot]

Pull request overview

Copilot reviewed 21 out of 24 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

opentmk/src/platform/hyperv/arch/hypercall.rs:108

• HvCall tracks an initialized flag, but dispatch_hvcall doesn’t enforce it. Now that dispatch_hvcall is pub, callers can invoke hypercalls before initialize(), which can lead to hard-to-debug failures/crashes depending on hypercall-page setup. Add a fast fail check (or make the API unsafe) to encode the precondition.

    pub fn dispatch_hvcall(
        &mut self,
        code: hvdef::HypercallCode,
        rep_count: Option<usize>,
    ) -> hvdef::hypercall::HypercallOutput {

[Copilot]

Pull request overview

Copilot reviewed 21 out of 24 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (5)

opentmk/src/lib.rs:11

• This crate uses #![allow(missing_docs)], but the repo convention for suppressing missing-docs warnings is #![expect(missing_docs)] (e.g. openvmm_vhost/vhost_user_backend/src/lib.rs:14). Using expect keeps the lint intentionally tracked instead of permanently disabling it.
  opentmk/src/lib.rs:13
• pub extern crate alloc unnecessarily re-exports the external alloc crate as part of opentmk’s public API. Other no_std crates in this repo keep it private (extern crate alloc;) and explicitly use alloc::... internally; downstream users can also depend on alloc directly.
  opentmk/src/arch/x86_64/io.rs:60
• The 16-bit port I/O write helper is named outh, but the surrounding naming uses the x86 width suffixes (outb/outl). outw is the conventional 16-bit name and pairs naturally with inw.

/// Write a word to a port.
///
/// # Safety
/// Caller should assume that the port being written to is safe to do so
pub unsafe fn outh(port: u16, data: u16) {
    // SAFETY: The caller has assured us this is safe.

opentmk/src/arch/x86_64/serial.rs:125

• drain also uses a tight busy-wait loop. Adding core::hint::spin_loop() reduces CPU burn when draining large bursts or when hardware keeps reporting data-ready.

            while self.io.inb(self.serial_port.value() + 5) & 1 != 0 {
                self.io.inb(self.serial_port.value());
            }

opentmk/opentmk_tests/src/tests/hyperv/hv_error_vp_start.rs:12

• Typo in the doc comment: “negitive” → “negative”.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 24 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

opentmk/src/arch/x86_64/io.rs:43

• The new 16-bit port I/O helpers are named inh/outh, but their docs call them “Read/Write a word”. For consistency with inb/inl (byte/long), consider naming these inw/outw (word) to make the width obvious and match common x86 terminology.

/// Read a word from a port.
///
/// # Safety
/// Caller should assume that the port being read from is safe to do so
pub unsafe fn inh(port: u16) -> u16 {
    let mut data;

opentmk/src/lib.rs:11

• #![allow(missing_docs)] is the only crate-wide missing-docs suppression in the repo; other crates consistently use #![expect(missing_docs)] (e.g. vmm_core/src/lib.rs:7) to keep lint intent explicit. Using allow here makes it easy to accidentally expand the undocumented public API surface without noticing.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 24 changed files in this pull request and generated 3 comments.

[github-actions]

1 Petri tests failed (1 unstable)

[Copilot]

Pull request overview

Copilot reviewed 21 out of 24 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

Cargo.toml:60

• opentmk is no longer a workspace member, but opentmk/Cargo.toml still inherits workspace fields (edition.workspace = true, rust-version.workspace = true, [lints] workspace = true, and multiple *.workspace = true deps). Cargo only allows workspace inheritance for workspace members, so building opentmk_tests (which depends on opentmk) will fail manifest parsing for opentmk unless opentmk is re-added to workspace.members (or opentmk/Cargo.toml stops using workspace inheritance).

  # opentmk
  "opentmk/opentmk_tests",

opentmk/src/arch/x86_64/io.rs:12

• The Safety docs say port I/O is unsafe because it may "cause UB". Port I/O can definitely have dangerous side effects, but it doesn't inherently cause Rust undefined behavior; the safety contract here should describe the required platform/device invariants (and avoid implying Rust UB from an arbitrary port value).

///
/// # Safety
/// Caller guarantees that writing to the given port at this time does not cause UB
pub unsafe fn outb(port: u16, data: u8) {

[Copilot]

Pull request overview

Copilot reviewed 23 out of 26 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

Cargo.toml:60

• opentmk was removed from [workspace] members, but opentmk/Cargo.toml still uses workspace-inherited keys like edition.workspace = true / rust-version.workspace = true and workspace = true deps. Because opentmk lives under the workspace root and is not in members or exclude, building opentmk_tests (which depends on opentmk) is likely to fail with a Cargo workspace-membership error. Add opentmk back to the workspace members list (or explicitly exclude it and stop using workspace inheritance in opentmk/Cargo.toml).

  # opentmk
  "opentmk/opentmk_tests",

opentmk/src/platform/hyperv/ctx.rs:175

• Fast hypercalls read 16 bytes from input_page.buffer (see dispatch_hvcall_fast), but this implementation only overwrites 0..inp_len. When inp_len < 16, the remaining bytes come from the previous hypercall, so fast1/fast2 can incorporate stale data and invoke the wrong hypercall arguments. Clearing the input page (or at least 0..16) before copying avoids this.

        let inp_len = input.len().min(self.hvcall.input_page.buffer.len());
        let out_len = output.len().min(self.hvcall.output_page.buffer.len());

        // Write to input page
        self.hvcall.input_page.buffer[0..inp_len].copy_from_slice(&input[0..inp_len]);

        let result = if out_len == 0 && inp_len <= 16 && cfg.pass_by_register_hint {
            // Do a fast pass-by-register call
            self.hvcall.dispatch_hvcall_fast(code)

[Copilot]

Pull request overview

Copilot reviewed 23 out of 26 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (7)

opentmk/src/arch/x86_64/serial.rs:101

• Serial::init no longer programs the modem control register (MCR, +4). In this repo’s other x86_64 serial init (openhcl/minimal_rt/src/arch/x86_64/serial.rs:88-91), init sets FIFO to 0xC7 and MCR to 0x0F; omitting MCR can leave the UART in an unexpected state on some platforms. Consider restoring the MCR write (and using 0xC7) for consistency and robustness.

    pub fn init(&self) {
        // SAFETY: Initializing the serial port is safe.
        unsafe {
            self.io.outb(self.serial_port.value() + 1, 0x00); // Disable all interrupts
            self.io.outb(self.serial_port.value() + 3, 0x80); // Enable DLAB
            self.io.outb(self.serial_port.value(), 1); // Low byte divisor
            self.io.outb(self.serial_port.value() + 1, 0); // High byte divisor
            self.io.outb(self.serial_port.value() + 3, 0x03); // 8 bits, 1 stop bit, no parity
            self.io.outb(self.serial_port.value() + 2, 0x07); // Enable FIFO, clear them
        }

opentmk/src/tmk_assert.rs:57

• format_assert_json_string is now public, but its implementation panics on serialization failure (via serde_json::to_string(...).expect(...)). As a library API this can unexpectedly abort callers; prefer returning a best-effort fallback string (or making the API fallible) instead of panicking.

/// Format an assertion result as a JSON log line.
///
/// If `terminate_new_line` is true, a trailing newline is appended so the
/// output can be streamed directly to a line-oriented sink.
pub fn format_assert_json_string<T>(
    s: &str,
    terminate_new_line: bool,
    line: String,

opentmk/src/tmk_assert.rs:76

• write_str is now part of the public API but it discards the underlying writer result. Returning core::fmt::Result makes it possible for callers to detect/log write failures (and keeps the assertion macro able to explicitly ignore the result).

/// Write a string directly to the TMK logger's underlying writer.
pub fn write_str(s: &str) {
    _ = crate::tmk_logger::LOGGER.get_writer().write_str(s);
}

opentmk/src/arch/x86_64/serial.rs:131

• read_byte performs port I/O without taking the same mutex used for writes/fmt::Write. If multiple VPs/threads can access a Serial, reads can interleave with writes and make register polling/reads racy. Consider taking the mutex here as well for consistent, serialized access to the UART registers.

    /// Read a single byte from the serial port, blocking until one is
    /// available.
    pub fn read_byte(&self) -> u8 {
        // SAFETY: Reading and writing text to the serial device is safe.
        unsafe {
            while self.io.inb(self.serial_port.value() + 5) & 1 == 0 {
                core::hint::spin_loop();
            }
            self.io.inb(self.serial_port.value())
        }

opentmk/src/arch/x86_64/serial.rs:142

• drain performs repeated port reads without holding the serial mutex. For consistency with write_byte/fmt::Write (and to avoid concurrent register accesses), consider taking the mutex while draining as well.

    /// Drain any bytes currently pending in the receive FIFO.
    pub fn drain(&self) {
        unsafe {
            // SAFETY: reading text to the serial device is safe
            while self.io.inb(self.serial_port.value() + 5) & 1 != 0 {
                self.io.inb(self.serial_port.value());
                core::hint::spin_loop();
            }
        }

opentmk/src/platform/hyperv/ctx.rs:168

• The code conversion uses try_into().ok().ok_or(...), which is hard to read and loses the original conversion error. Using map_err keeps it idiomatic and clearer.

        let code =
            hvdef::HypercallCode(code.try_into().ok().ok_or(TmkError::InvalidHypercallCode)?);

opentmk/src/platform/hyperv/ctx.rs:174

• The input page is only partially overwritten (0..inp_len). If the current hypercall uses a smaller input than the previous one, the remaining bytes in the page retain stale data, which can affect hypercall behavior and contradict the doc comment about truncation. Clearing the pages before copying avoids leaking/using leftover bytes.

        let inp_len = input.len().min(self.hvcall.input_page.buffer.len());
        let out_len = output.len().min(self.hvcall.output_page.buffer.len());

        // Write to input page
        self.hvcall.input_page.buffer[0..inp_len].copy_from_slice(&input[0..inp_len]);

[Copilot]

Pull request overview

Copilot reviewed 23 out of 26 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

opentmk/src/arch/x86_64/serial.rs:142

• write_byte/fmt::Write serialize port access with self.mutex, but the newly-public read_byte and drain perform port I/O without taking the same lock. That allows concurrent readers/writers to interleave register accesses, which can corrupt UART state or lead to unexpected blocking.

    pub fn read_byte(&self) -> u8 {
        // SAFETY: Reading and writing text to the serial device is safe.
        unsafe {
            while self.io.inb(self.serial_port.value() + 5) & 1 == 0 {
                core::hint::spin_loop();

opentmk/src/platform/hyperv/ctx.rs:191

• hypercall() copies only the provided input prefix into the shared input page and (in the slow path) copies only the output prefix back. Any remaining bytes in the input/output pages (or in output when it’s larger than the page) keep stale data from prior calls, which can make hypercall behavior nondeterministic and can leak old data to callers.

        let inp_len = input.len().min(self.hvcall.input_page.buffer.len());
        let out_len = output.len().min(self.hvcall.output_page.buffer.len());

        // Write to input page
        self.hvcall.input_page.buffer[0..inp_len].copy_from_slice(&input[0..inp_len]);

[github-actions]

2 Petri tests failed (2 unstable)