Skip to content

dev-qemu: add sp-serial feature for the SP link#35

Open
dymk wants to merge 1 commit into
OpenDevicePartnership:mainfrom
dymk:dymk/f1.1-on-modernized-ec
Open

dev-qemu: add sp-serial feature for the SP link#35
dymk wants to merge 1 commit into
OpenDevicePartnership:mainfrom
dymk:dymk/f1.1-on-modernized-ec

Conversation

@dymk

@dymk dymk commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

What

Add an off-by-default sp-serial feature to dev-qemu that selects uart-service's MctpSerialMedium (DSP0253 framing) instead of the default SmbusEspiMedium, for the two-QEMU SP ↔ EC serial link. Pure compile-time selector — both medium constructors already live in uart-service; dev-qemu holds no wire addressing.

Why

The default build keeps SmbusEspi (the framing ec-test-cli speaks), so the single-QEMU integration-test stays green. The sp-serial feature selects the serial medium the Secure Partition speaks over the two-QEMU link; the qemu-sbsa harness builds --features sp-serial.

Context

Builds on #33 (EC modernized to embedded-services main): default_mctp_serial now resolves directly from upstream embedded-services main — no fork [patch].

Verification

  • Both configs build clean (release/LTO): default (SmbusEspi) and --features sp-serial (DSP0253).
  • fmt --check + clippy clean.
  • Two-QEMU SP ↔ EC e2e (odp-platform-qemu-sbsa test-sp-ec-link): the full UEFI ↔ FFA ↔ SP ↔ DSP0253-serial ↔ EC thermal GetTmp round-trip passes (ffa_version / ffa_id_get / partition_discovery / thermal_get_temperature all green). The sp-serial code path is unchanged by the rebase onto merged main.

Assisted-by: GitHub Copilot:claude-opus-4.8

Add an off-by-default `sp-serial` feature to dev-qemu that selects
uart-service's MctpSerialMedium (DSP0253) instead of the default
SmbusEspiMedium, for the two-QEMU SP <-> EC serial link. The default
build keeps SmbusEspi so the ec-test-cli integration-test stays green;
the harness builds --features sp-serial.

This is F1.1 re-pinned onto the modernized EC: default_mctp_serial now
resolves directly from upstream embedded-services main (6d7dbbf, the
merged #906 ctor) with no fork [patch] override.

Assisted-by: GitHub Copilot:claude-opus-4.8
Copilot AI review requested due to automatic review settings June 29, 2026 19:49
@dymk dymk requested a review from a team as a code owner June 29, 2026 19:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds an off-by-default sp-serial feature to dev-qemu to select the UART service’s MCTP serial (DSP0253) medium at compile time, while keeping the default SMBus/eSPI framing for existing ec-test-cli integration flows.

Changes:

  • Introduces a sp-serial Cargo feature for dev-qemu (default remains unchanged).
  • Refactors dev-qemu’s UART service initialization to conditionally select DefaultService vs MctpSerialService via cfg(feature = "sp-serial").

Reviewed changes

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

File Description
platform/dev-qemu/src/main.rs Adds compile-time selection for the uart-service type + constructor based on sp-serial.
platform/dev-qemu/Cargo.toml Defines the off-by-default sp-serial feature and documents its intent.

Comment on lines +27 to +29
fn new_uart_service(relay: MockOdpRelayHandler) -> EcUartService {
uart_service::DefaultService::default_smbusespi(relay).unwrap()
}
Comment on lines +30 to +33
#[cfg(feature = "sp-serial")]
fn new_uart_service(relay: MockOdpRelayHandler) -> EcUartService {
uart_service::MctpSerialService::default_mctp_serial(relay).unwrap()
}

@kurtjd kurtjd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not necessarily blocking, but I think I'd rather just see everything updated to using your new framing protocol. I don't see any real need to support the smbus framing if we just update the ec-test-cli/tui to use your new one, then we can just update all the platforms here to use default_mctp_serial.

Let me know if you are opposed to that and we can talk, but I think supporting the two framing protocols is unnecessary, better to just pick one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants