Skip to content

Add virtio-scmi backend and comprehensive documentation#84

Open
Inquisitor-201 wants to merge 16 commits into
mainfrom
feat-virtio-scmi
Open

Add virtio-scmi backend and comprehensive documentation#84
Inquisitor-201 wants to merge 16 commits into
mainfrom
feat-virtio-scmi

Conversation

@Inquisitor-201

Copy link
Copy Markdown
Contributor

PR: Implement VirtIO-SCMI Virtualization Feature

Feature Overview

This PR implements the VirtIO-SCMI virtualization framework, allowing virtual machines (ZoneU) to securely access hardware resources on the host (Zone0) through the VirtIO transport layer, including:

  • Clock Resource Management: Query clock attributes, get/set clock frequencies, enable/disable clocks
  • Reset Domain Management: Control reset domain operations
  • Resource Isolation and Mapping: Implement resource access control and ID mapping through configuration

Implementation Details

Core Changes

  1. Build System:

    • Added VIRTIO_SCMI configuration option in Makefile
    • Passed VIRTIO_SCMI parameter to driver build
    • Updated build variable strings to include virtio_scmi configuration
  2. Kernel Driver:

    • Renamed hvisor.c to hvisor_main.c
    • Added scmi_server.c: Implements SCMI server functionality
    • Added reset-domains.c: Implements reset domain management
    • Added scmi_server.h: Defines SCMI server interface
    • Added SCMI-related ioctl command handling
  3. User Space Tools:

    • Implemented VirtIO-SCMI device emulation
    • Implemented SCMI protocol handling (Base, Clock, Reset)
    • Support for resource mapping and access control
  4. Configuration System:

    • Device tree configuration support (phandle references)
    • JSON configuration file support (resource mapping and access control)

Technical Architecture

The VirtIO-SCMI system is divided into three layers:

  • User Space Tools Layer: VirtIO device emulation and SCMI protocol processing
  • Kernel Driver Layer: Actual hardware operations and resource management
  • Configuration Layer: Device tree and JSON configuration

Key Features

  • Security Isolation: All hardware operations are verified and executed through Zone0
  • Resource Mapping: Support for mapping virtual IDs to physical IDs
  • Access Control: Resource access restricted through allowlists
  • Flexible Configuration: Support for resource configuration on different boards

Configuration Guide

Device Tree Configuration

ZoneU device tree needs to define:

  1. SCMI protocol nodes (Base, Clock, Reset)
  2. VirtIO transport channel (virtio-mmio)
  3. Device references to SCMI resources

JSON Configuration

Configure in virtio_cfg.json:

  • SCMI device basic information (address, interrupt, etc.)
  • Clock and reset provider phandles
  • Resource mapping table (virtual ID → physical ID)
  • Access control list

Testing Instructions

Build Testing

# Enable VirtIO-SCMI build
make VIRTIO_SCMI=y KDIR=/path/to/kernel

# Build user space tools and kernel driver
make tools driver

Runtime Testing

  1. Configure device tree and JSON files
  2. Start hvisor tool
  3. Load SCMI driver in ZoneU
  4. Test clock and reset operations

Examples

Complete RK3588 example configuration is provided:

  • examples/rk3588-aarch64/with_virtio_scmi/README.md: Detailed configuration and usage instructions
  • Includes device tree examples and JSON configuration examples

Dependencies

  • Linux kernel clock subsystem
  • Linux kernel reset subsystem
  • VirtIO framework
  • cJSON library (for configuration parsing)

This implementation adds complete VirtIO-SCMI support to the hvisor project, enabling virtual machines to securely access and manage hardware resources.

@Inquisitor-201 Inquisitor-201 force-pushed the feat-virtio-scmi branch 2 times, most recently from a1cde9a to 2818939 Compare April 6, 2026 08:28
@Inquisitor-201 Inquisitor-201 requested a review from dallasxy April 6, 2026 08:29
Comment thread driver/hvisor_main.c Outdated
Comment thread driver/reset-domains.c
Comment on lines +21 to +26
/* Forward declarations from core.c */
extern struct mutex reset_list_mutex;
extern struct list_head reset_controller_list;
extern struct reset_control *
__reset_control_get_internal(struct reset_controller_dev *rcdev,
unsigned int index, bool shared, bool acquired);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With the header issue worked around (by providing the missing reset-domains.h locally), the compilation now passes, but the module fails at modpost with three undefined symbols:

ERROR: modpost: "reset_controller_list" [hvisor.ko] undefined!
ERROR: modpost: "__reset_control_get_internal" [hvisor.ko] undefined!
ERROR: modpost: "reset_list_mutex" [hvisor.ko] undefined!

I noticed that reset-domains.c declares them as extern:

extern struct mutex reset_list_mutex;
extern struct list_head reset_controller_list;
extern struct reset_control *
__reset_control_get_internal(struct reset_controller_dev *rcdev,
                              unsigned int index, bool shared, bool acquired);

but they don't appear to be defined anywhere in the module. These also aren't exported by either the 6.1 SDK kernel or upstream 7.1-rc2:

# SDK kernel 6.1
$ grep -E "EXPORT_SYMBOL.*(reset_controller_list|__reset_control_get_internal|reset_list_mutex)" drivers/reset/core.c
(empty)

# Upstream 7.1-rc2
$ grep -E "EXPORT_SYMBOL.*(reset_controller_list|__reset_control_get_internal|reset_list_mutex)" drivers/reset/core.c
(empty)

Just wanted to check — were these symbols meant to be provided by a local implementation (perhaps in reset-domains.c itself or another file) that didn't make it into this PR because of the
missing header? Or were you thinking of exporting them from the SDK kernel?

Happy to help sort it out either way — let me know what you think!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for looking into this! I've investigated the issue across multiple kernel versions (5.10, 6.1, 6.6), and you're right —
reset_list_mutex, reset_controller_list, and __reset_control_get_internal are all declared as static in drivers/reset/core.c
and are never exported via EXPORT_SYMBOL in any upstream kernel.

Since we need to access these internal reset controller symbols from an out-of-tree module, the fix I'm using is to manually
add EXPORT_SYMBOL_GPL for these three symbols in the kernel's drivers/reset/core.c, as part of the SDK kernel patch set.
Specifically:

// In drivers/reset/core.c, after each definition:

EXPORT_SYMBOL_GPL(reset_list_mutex);
EXPORT_SYMBOL_GPL(reset_controller_list);
EXPORT_SYMBOL_GPL(__reset_control_get_internal);

@Inquisitor-201

Copy link
Copy Markdown
Contributor Author

@agicy Fixed. It should be OK now

@agicy agicy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

Comment thread tools/virtio/devices/scmi/virtio_scmi.c Outdated
}

// Update used ring with total length (request + response)
update_used_ring(vq, desc_idx, iov[0].iov_len + iov[1].iov_len);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is a logical error in the call to update_used_ring of virtq_tx_handle_one_request.

Implementation

In virtq_tx_handle_one_request, &iov[0] is passed as req_iov (request, device-readable) and &iov[1] is passed as resp_iov (response, device-writable).

// tools/virtio/devices/scmi/virtio_scmi.c:70-72
    // Dispatch request to protocol handler
    int status =
        scmi_handle_message(dev, protocol_id, msg_id, token, &iov[0], &iov[1]);
int scmi_handle_message(SCMIDev *dev, uint8_t protocol_id, uint8_t msg_id,
                        uint16_t token, const struct iovec *req_iov,
                        struct iovec *resp_iov);

Specifications

This confirms that iov[0] represents the Request sent by the guest, and iov[1] is the Response written by the device.

Device reports the number of bytes it has written to memory for each buffer it uses. This is referred to as "used length".

References: Virtual I/O Device (VIRTIO) Version 1.2 - 2.6 Virtqueues

The virtio_scmi_response fields are interpreted as follows:

  • hdr: (device-writable) contains the SCMI message header
  • ret_values: (device-writable) comprises the SCMI message return values

References: Virtual I/O Device (VIRTIO) Version 1.2 - 5.17.6.1 cmdq Operation

Conclusion

The device only writes data into iov[1] (the device-writable response buffer). Including iov[0].iov_len in used_len reports uninitialized bytes to the guest, causing the guest SCMI driver to see a response larger than the actual payload. This triggers the "Malformed reply" warning on every message exchange.

The fix is straightforward: pass only iov[1].iov_len to update_used_ring.

Results

Before the fix, the guest SCMI driver saw a 124-byte response for a message whose payload is only 4 bytes:

[  134.973615] arm-scmi firmware:scmi: Malformed reply - real_sz:124  calc_sz:4  (loop_num_ret:3)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this! The fix has been applied in commit 76e3113 — now passing only iov[1].iov_len to update_used_ring.

@agicy agicy self-requested a review May 16, 2026 09:40
Inquisitor-201 and others added 15 commits May 19, 2026 23:34
Introduces a comprehensive guide for deploying virtio-scmi on hvisor.
Key sections include:
- ZoneU Device Tree configuration (SCMI protocols, VirtIO nodes).
- SoC device re-mapping for clock and reset providers.
- Hvisor configuration requirements (config.json & virtio_cfg.json).
- Linux kernel backporting instructions for versions < 5.15.
- Current status: rebased to stable hvisor-tool (pre-refactor version).

Note: Added warnings regarding incompatibility with the latest
hvisor-tool device-based architecture.
…O-SCMI framework, including architecture, configuration, and usage examples
This header was from an out-of-tree kernel patch and is no longer needed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The device only writes data into iov[1] (response buffer). Including
iov[0].iov_len (request header) in used_len made the guest see a response
larger than the actual payload, triggering a "Malformed reply" warning on
every message exchange.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@agicy

agicy commented May 27, 2026

Copy link
Copy Markdown
Contributor

Problem

Response iov_len never updated

scmi_make_response() fills in the header but does not set resp_iov->iov_len:

int scmi_make_response(SCMIDev *dev, uint16_t token, struct iovec *resp_iov,
                        int32_t status) {
    // ...
    struct scmi_response *resp = resp_iov->iov_base;
    resp->header = SCMI_RESP_HDR(token);
    resp->status = status;
    return 0;                         // resp_iov->iov_len unchanged
}

Every handler writes payload after scmi_make_response() but never adds the payload size. Examples:

  • base.c:87-88: *version = SCMI_BASE_VERSION (4 bytes), iov_len unchanged.
  • base.c:65-66: attr->num_protocols, attr->num_agents written, iov_len unchanged.
  • clock.c:116-117: *version = SCMI_CLOCK_VERSION, iov_len unchanged.

So when we call update_used_ring(vq, desc_idx, iov[1].iov_len), iov[1].iov_len is still the guest's original buffer allocation, not the actual response size. The guest sees correct data followed by garbage in the reported trailing bytes.

handle_base_protocol_list reuses iov_len after it is shrunk

handle_base_protocol_list() computes remaining buffer capacity after scmi_make_response():

Once the previous fix is applied (scmi_make_response sets iov_len = 8), this becomes 8 - 8 = 0. get_protocol_list() receives max = 0, returns zero protocols, and the guest concludes clock and reset are "not implemented".

Suggestion

Root cause

The two remaining issues share the same root cause:

  • raw struct iovec manipulation with no abstraction.
  • The iov_len field serves double duty:
    • The allocated buffer capacity (set by the guest).
    • Expected to be the actual bytes written (set by the device).

With no wrapper enforcing this distinction, every handler must remember to update the field manually. This cannot scale across authors and time.

Short-term fix

Introduce a lightweight scmi_resp_ctx builder that owns the struct iovec and tracks written length internally:

struct scmi_resp_ctx {
    struct iovec *iov;
    size_t written;       // auto-incremented by scmi_make_response and helpers
    size_t capacity;      // captured once, never modified
};

int scmi_make_response(SCMIDev *dev, uint16_t token,
                        struct scmi_resp_ctx *ctx, int32_t status);
void *scmi_resp_payload(struct scmi_resp_ctx *ctx, size_t size);
  • scmi_make_response initializes written = sizeof(struct scmi_response).
  • scmi_resp_payload returns a write pointer and advances written by size.
  • The caller uses ctx.capacity for bounds checks and ctx.written for update_used_ring.

scmi_resp_payload is just a placeholder, we may think of a better name.

Long-term direction

A stronger type system would eliminate this class of bug. A gradual rewrite of the hvisor-tool virtio daemon in Rust, or selectively adopting C++ with RAII for the virtio layer, would be the principled long-term direction.

- Replace CCF enumeration in scmi_clock_get_count with config value clock_max_num
- Rename "invalid_clock_" prefix to "inv_" to shorten clock name
- Remove unused cached_clock_count and related cache logic
@Inquisitor-201

Copy link
Copy Markdown
Contributor Author

修复:时钟数量获取方式与命名

1. 时钟数量不再枚举 CCF

scmi_clock_get_count() 此前通过 ioctl 绕道内核驱动去遍历 clock provider 以获取时钟总数(枚举 CCF),现改为直接使用 config 中配置的 clock_max_num 字段,去掉了冗余的内核态往返和缓存逻辑。

2. 缩短无效时钟名前缀

"invalid_clock_%u""inv_%u",避免名称过长。

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