Add go bindings for libpqos - #297
Conversation
0ef849c to
95683ad
Compare
|
Hi Michael, |
|
Hi Michael, |
There was a problem hiding this comment.
Pull request overview
This PR introduces comprehensive Go bindings for the Intel RDT PQoS library, enabling Go applications to utilize Resource Director Technology features. The bindings provide idiomatic Go interfaces for cache allocation, memory bandwidth management, and resource monitoring capabilities.
Key Changes:
- Added complete CGO bindings for core PQoS library functionality (initialization, capability detection, resource allocation)
- Implemented resource monitoring APIs for cores and processes with support for LLC occupancy and memory bandwidth metrics
- Created example programs demonstrating capability detection and basic library usage
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/go/pqos/pqos.go | Core library initialization, configuration, and singleton pattern implementation |
| lib/go/pqos/capability.go | Capability detection for L3/L2 CAT, MBA, and monitoring features |
| lib/go/pqos/cpuinfo.go | CPU topology information and cache hierarchy utilities |
| lib/go/pqos/allocation.go | Cache and memory bandwidth allocation APIs with COS management |
| lib/go/pqos/monitoring.go | Resource monitoring for cores/processes with event data structures |
| lib/go/pqos/pqos_test.go | Unit tests for core functionality and CPU topology methods |
| lib/go/examples/simple/main.go | Basic example demonstrating initialization and capability detection |
| lib/go/examples/capability/main.go | Comprehensive example showing detailed capability inspection |
| lib/go/Makefile | Build configuration with dependency checking and example compilation |
| lib/go/go.mod | Go module definition requiring Go 1.21 |
| lib/go/API.md | Complete API documentation with usage examples |
| lib/go/.gitignore | Git ignore patterns for build artifacts |
Comments suppressed due to low confidence (1)
lib/go/pqos/pqos.go:1
- The helper functions
safeCStringandfreeCStringare defined but never used in the codebase. Remove these unused functions or add documentation explaining their intended future use.
// Copyright(c) 2025 Zededa, Inc.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| err := p.Init(nil) | ||
| if err != nil { |
There was a problem hiding this comment.
The test TestInitWithNilConfig lacks verification of the initialized state's configuration. Add assertions to verify that when nil config is passed, the library initializes with expected defaults (InterAuto interface and LogVerbosityDefault).
| } | ||
|
|
||
| ret := C.pqos_mon_poll(&cGroups[0], C.uint(len(groups))) | ||
| if ret != C.PQOS_RETVAL_OK && ret != C.PQOS_RETVAL_OVERFLOW { |
There was a problem hiding this comment.
The overflow error message on line 319 loses information about which group(s) experienced the overflow. Consider enhancing the error message to include the group indices that had overflow to aid debugging.
|
|
||
| check-deps: | ||
| @echo "Checking dependencies..." | ||
| @if [ ! -f "$(LIBPQOS_PATH)/libpqos.so.6" ]; then \ |
There was a problem hiding this comment.
The hardcoded version number '.6' in the library name makes the build fragile to version changes. Consider using a wildcard pattern like libpqos.so* or extracting the version to a variable for easier maintenance.
| @if [ ! -f "$(LIBPQOS_PATH)/libpqos.so.6" ]; then \ | |
| @if [ -z "$(wildcard $(LIBPQOS_PATH)/libpqos.so*)" ]; then \ |
| @@ -0,0 +1,7 @@ | |||
| module github.com/intel/intel-cmt-cat/lib/go | |||
|
|
|||
| go 1.21 | |||
There was a problem hiding this comment.
The minimum Go version requirement of 1.21 should be documented in the PR description or README, explaining why this specific version is required (e.g., specific language features, standard library dependencies).
| func TestCPUInfoL2Methods(t *testing.T) { | ||
| // Create a mock CPUInfo with some test data | ||
| cpuInfo := &CPUInfo{ |
There was a problem hiding this comment.
The L2 topology tests use hardcoded mock data but don't test against actual hardware capabilities. Add integration tests that verify L2 methods against real CPU topology when available, potentially skipping if hardware isn't present.
95683ad to
7fac01e
Compare
d749698 to
0c9b9d4
Compare
Topology consumers need to identify SMT siblings as logical CPUs that share the same physical core. L2 cache id is not sufficient for this: on Intel E-core modules a single L2 is shared by 4 distinct physical cores. Add an unsigned core_id field to struct pqos_coreinfo and populate it on Linux from /sys/devices/system/cpu/cpu%u/topology/core_id. The OS interface reads it in os_cpuinfo_topology() via a new os_cpuinfo_cpu_core() (mirroring os_cpuinfo_cpu_socket()); the MSR/MMIO CPUID topology builder (cpuinfo_build_topo()) reads the same sysfs value. SMT siblings are then the logical CPUs sharing (socket, core_id). Signed-off-by: Mikhail Malyshev <mike.malyshev@gmail.com>
CGO bindings for libpqos (init/fini, capability detection, cpuinfo, allocation and monitoring). CoreInfo exposes the physical core id as CoreID so callers can group logical CPUs into SMT siblings by (Socket, CoreID). Signed-off-by: Mikhail Malyshev <mike.malyshev@gmail.com>
On CONFIG_STRICT_DEVMEM=y kernels, acpi_get_sig() falling back to acpi_get_mmap() maps and reads physical memory via /dev/mem, which the kernel restricts. The mmap of the RSDP/BIOS region either fails or, worse, succeeds and then faults on access, taking down a CGO host process such as the Go bindings. When the kernel exposes ACPI tables under /sys/firmware/acpi/tables but the requested table is not present there, return NULL instead of scanning /dev/mem. The memory fallback is still used on legacy kernels that do not expose the sysfs ACPI tables directory at all. Fixes: intel#307 Signed-off-by: Mikhail Malyshev <mike.malyshev@gmail.com>
This PR introduces CGO bindings for libpqos library. Almost all functionality is covered except for some specialized features
NOTE This is just a draft PR to start a discussion. It may be updated as I proceed with testing
Description
The Go bindings provide complete coverage of the core PQoS API (initialization, capability detection, cache/memory allocation, monitoring) but are missing some advanced/specialized features from the C library.
Coverage: ~85% of commonly-used APIs, ~65% of total C APIs
Fully Implemented
Core Library (100%)
pqos_init()- Initialize librarypqos_fini()- Finalize librarypqos_cap_get()- Get capabilitiespqos_inter_get()- Get interface typeCapability Detection (100%)
pqos_cap_get_type()- Get capability by type (via HasL3CA(), HasL2CA(), etc.)L3 Cache Allocation (100%)
pqos_l3ca_set()- Set L3 CAT configurationpqos_l3ca_get()- Get L3 CAT configurationpqos_l3ca_get_min_cbm_bits()- Get minimum CBM bitsL2 Cache Allocation (100%)
pqos_l2ca_set()- Set L2 CAT configurationpqos_l2ca_get()- Get L2 CAT configurationpqos_l2ca_get_min_cbm_bits()- Get minimum CBM bitsMemory Bandwidth Allocation (100%)
pqos_mba_set()- Set MBA configurationpqos_mba_get()- Get MBA configurationCore/Process Association (100%)
pqos_alloc_assoc_set()- Associate core with COSpqos_alloc_assoc_get()- Get core's COSpqos_alloc_assoc_set_pid()- Associate PID with COSpqos_alloc_assoc_get_pid()- Get PID's COSpqos_alloc_assign()- Auto-assign COS to corespqos_alloc_release()- Release cores to defaultpqos_alloc_assign_pid()- Auto-assign COS to PIDspqos_alloc_release_pid()- Release PIDs to defaultpqos_alloc_reset()- Reset allocationResource Monitoring - Cores (100%)
pqos_mon_start_cores()- Start core monitoringpqos_mon_stop()- Stop monitoringpqos_mon_poll()- Poll monitoring datapqos_mon_reset()- Reset monitoringpqos_mon_assoc_get()- Get core RMIDResource Monitoring - Processes (100%)
pqos_mon_start_pids2()- Start PID monitoringpqos_mon_add_pids()- Add PIDs to grouppqos_mon_remove_pids()- Remove PIDs from groupEvent Values (100%)
Partially Implemented
Capability Queries (50%)
Implemented:
Missing:
pqos_l3ca_get_cos_num()- Get number of COSpqos_l2ca_get_cos_num()- Get number of COSpqos_mba_get_cos_num()- Get number of COSpqos_smba_get_cos_num()- Get number of COSpqos_l3ca_cdp_enabled()- Check CDP statuspqos_l2ca_cdp_enabled()- Check CDP statuspqos_mba_ctrl_enabled()- Check MBA controller statuspqos_l3ca_iordt_enabled()- Check I/O RDT statuspqos_cap_get_event()- Get monitoring event detailsWorkaround: These can be accessed via capability structures, e.g.,
l3ca.NumClasses,l3ca.CDPOnNot Implemented
I/O RDT - Device/Channel Monitoring (0%)
pqos_mon_start_channels()- Monitor I/O channelspqos_mon_start_dev()- Monitor specific devicepqos_mon_assoc_get_channel()- Get channel RMIDpqos_mon_assoc_get_dev()- Get device RMIDImpact: Cannot monitor I/O RDT devices (PCIe, etc.)
I/O RDT - Device Allocation (0%)
pqos_alloc_assoc_set_channel()- Associate channel with COSpqos_alloc_assoc_get_channel()- Get channel's COSpqos_alloc_assoc_set_dev()- Associate device with COSpqos_alloc_assoc_get_dev()- Get device's COSImpact: Cannot allocate resources to I/O devices
Uncore Monitoring (0%)
pqos_mon_start_uncore()- Monitor uncore/socket levelImpact: Cannot do socket-level monitoring, only core/process
Advanced Configuration (0%)
pqos_sysconfig_get()- Get system configurationpqos_mon_reset_config()- Reset with configpqos_alloc_reset_config()- Reset allocation with configImpact: Cannot use advanced reset/configuration options
CPU Topology Helpers (0%)
pqos_cpu_get_one_by_mba_id()- Get core by MBA IDpqos_cpu_get_one_by_smba_id()- Get core by SMBA IDpqos_cpu_get_one_core()- Get one core from socketpqos_cpu_get_one_by_numaid()- Get core by NUMA IDpqos_cpu_get_one_by_l3cat_id()- Get core by L3 CAT IDpqos_cpu_get_one_by_l2id()- Get core by L2 IDpqos_cpu_check_core()- Check if core is validpqos_cpu_get_socketid()- Get socket ID for corepqos_cpu_get_numaid()- Get NUMA ID for corepqos_cpu_get_clusterid()- Get cluster ID for coreImpact: Need to manually search core arrays
Workaround: Use CPUInfo methods like
FindCore(),GetCoresBySocket()Monitoring Value Helpers (0%)
pqos_mon_get_value()- Get specific event valuepqos_mon_get_ipc()- Get IPC valueImpact: Minor - values accessible via MonData.Values struct
Workaround: Use
MonData.GetIPC(),MonData.Values.LLC, etc.Deprecated Functions (Intentionally Skipped)
pqos_mon_start()- Deprecated, usepqos_mon_start_cores()pqos_mon_start_pid()- Deprecated, usepqos_mon_start_pids2()pqos_mon_start_pids()- Deprecated, usepqos_mon_start_pids2()Missing Structs/Types
Not Implemented
pqos_sysconfig- System configurationpqos_devinfo- Device informationpqos_dev- Device structurepqos_channel- Channel structurepqos_mon_config- Monitoring configurationpqos_alloc_config- Allocation configurationpqos_channel_t- Channel ID typeAffected parts
Motivation and Context
These bindings are required to add a support for Intel RTD technologies into EVE OS https://github.com/lf-edge/eve
How Has This Been Tested?
The test is ongoing. This is a draft PR to start a discussion
Types of changes
Checklist: