From a2975121cab7cd2255776791cc918879b568d2d6 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Tue, 5 Mar 2024 17:17:33 +0000 Subject: [PATCH 1/7] Add `serde` support for various bindings Add implementations of `serde::{Serialize, Deserialize}` for kvm-bindings structures commonly used for live migration/snapshotting. The selections of structures to add the derives to was determined by finding the minimal set that allows both Firecracker and CloudHypervisor to compile against this branch. Serialization happens as opaque binary blobs via the `zerocopy` crate. This is to support live migration across different versions of kvm-bindings (which can potentially be using bindings generated from different versions of the kernel headers). Since the kernel's ABI is backward-compatible, the only changes that can happen are: - "reserved" areas of structures acquire meaning. This is a change purely at the representational level, and has no impact on the underlying binary representation. Thus, our serialization is roboust to these changes (whereas simply deriving the serde traits would cause incompatibilities here). - Adding new fields to the end of a structure. When deserializating, we compare the length of the deserialized data with the size of the struct, and pad with zeros/truncate as appropriate. This allows seamless live update. We pick the `zerocopy` crate for binary serialization as it is already in use in the rust-vmm ecosystem (in `acpi_tables`), and it has a very strong security stance (for example, using formal verification in their CI). The alternative would have been using `ByteValued` from `vm-memory`, but I do not want `kvm-bindings` to gain a dependency on `vm-memory` (especially since `vm-memory` brings in a lot more functionality that `kvm-bindings` does not need). Signed-off-by: Patrick Roy --- Cargo.toml | 6 ++ README.md | 12 +++- src/arm64/bindings.rs | 24 ++++++++ src/arm64/mod.rs | 3 + src/arm64/serialize.rs | 53 +++++++++++++++++ src/lib.rs | 10 ++++ src/serialize.rs | 43 ++++++++++++++ src/x86_64/bindings.rs | 126 +++++++++++++++++++++++++++++++++++++++- src/x86_64/mod.rs | 3 + src/x86_64/serialize.rs | 119 +++++++++++++++++++++++++++++++++++++ 10 files changed, 394 insertions(+), 5 deletions(-) create mode 100644 src/arm64/serialize.rs create mode 100644 src/serialize.rs create mode 100644 src/x86_64/serialize.rs diff --git a/Cargo.toml b/Cargo.toml index aadf388..c4ff6db 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,12 @@ rustdoc-args = ["--cfg", "docsrs"] [features] fam-wrappers = ["vmm-sys-util"] +# It is not needed to enable the `serde` feature of `vmm-sys-util` here, because due to how cargo merges features, +# if a downstream crate enables vmm-sys-util in its Cargo.toml, it will get enabled globally. +serde = ["dep:serde", "serde/derive", "dep:zerocopy"] + [dependencies] vmm-sys-util = { version = "0.12.1", optional = true } +serde = { version = "1.0.0", optional = true, features = ["derive"] } +zerocopy = { version = "0.7.32", optional = true, features = ["derive"] } diff --git a/README.md b/README.md index a7d467c..02d5a26 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,6 @@ Rust FFI bindings to KVM, generated using [bindgen](https://crates.io/crates/bindgen). It currently has support for the following target architectures: - x86_64 -- arm - arm64 The bindings exported by this crate are statically generated using header files @@ -17,7 +16,7 @@ kernel version they are using. For example, the `immediate_exit` field from the capability is available. Using invalid fields or features may lead to undefined behaviour. -# Usage +## Usage First, add the following to your `Cargo.toml`: ```toml kvm-bindings = "0.3" @@ -35,7 +34,14 @@ this crate. Example: kvm-bindings = { version = "0.3", features = ["fam-wrappers"]} ``` -# Dependencies +## Dependencies The crate has an `optional` dependency to [vmm-sys-util](https://crates.io/crates/vmm-sys-util) when enabling the `fam-wrappers` feature. + +It also has an optional dependency on [`serde`](serde.rs) when enabling the +`serde` feature, to allow serialization of bindings. Serialization of +bindings happens as opaque binary blobs via [`zerocopy`](https://google.github.io/comprehensive-rust/bare-metal/useful-crates/zerocopy.html). +Due to the kernel's ABI compatibility, this means that bindings serialized +in version `x` of `kvm-bindings` can be deserialized in version `y` of the +crate, even if the bindings have had been regenerated in the meantime. diff --git a/src/arm64/bindings.rs b/src/arm64/bindings.rs index 1e7e6e2..f6f60ea 100644 --- a/src/arm64/bindings.rs +++ b/src/arm64/bindings.rs @@ -955,6 +955,10 @@ pub type __wsum = __u32; pub type __poll_t = ::std::os::raw::c_uint; #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct user_pt_regs { pub regs: [__u64; 31usize], pub sp: __u64, @@ -1019,6 +1023,10 @@ fn bindgen_test_layout_user_pt_regs() { #[repr(C)] #[repr(align(16))] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct user_fpsimd_state { pub vregs: [__uint128_t; 32usize], pub fpsr: __u32, @@ -1499,6 +1507,10 @@ fn bindgen_test_layout_user_za_header() { #[repr(C)] #[repr(align(16))] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_regs { pub regs: user_pt_regs, pub sp_el1: __u64, @@ -1574,6 +1586,10 @@ fn bindgen_test_layout_kvm_regs() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_vcpu_init { pub target: __u32, pub features: [__u32; 7usize], @@ -6511,6 +6527,10 @@ fn bindgen_test_layout_kvm_vapic_addr() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_mp_state { pub mp_state: __u32, } @@ -8786,6 +8806,10 @@ fn bindgen_test_layout_kvm_reg_list() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_one_reg { pub id: __u64, pub addr: __u64, diff --git a/src/arm64/mod.rs b/src/arm64/mod.rs index a331b07..50f4703 100644 --- a/src/arm64/mod.rs +++ b/src/arm64/mod.rs @@ -7,6 +7,9 @@ pub mod bindings; #[cfg(feature = "fam-wrappers")] pub mod fam_wrappers; +#[cfg(feature = "serde")] +mod serialize; + pub use self::bindings::*; #[cfg(feature = "fam-wrappers")] pub use self::fam_wrappers::*; diff --git a/src/arm64/serialize.rs b/src/arm64/serialize.rs new file mode 100644 index 0000000..bfa78a8 --- /dev/null +++ b/src/arm64/serialize.rs @@ -0,0 +1,53 @@ +use bindings::{ + kvm_mp_state, kvm_one_reg, kvm_regs, kvm_vcpu_init, user_fpsimd_state, user_pt_regs, +}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use zerocopy::{transmute, AsBytes}; + +serde_impls! { + user_pt_regs, + user_fpsimd_state, + kvm_regs, + kvm_vcpu_init, + kvm_mp_state, + kvm_one_reg +} + +#[cfg(test)] +mod tests { + use bindings::*; + use serde::{Deserialize, Serialize}; + + fn is_serde Deserialize<'de>>() {} + + #[test] + fn static_assert_serde_implementations() { + // This test statically (= at compile-time) asserts that various bindgen generated + // structures implement serde's `Serialize` and `Deserialize` traits. + // This is to make sure that we do not accidentally remove those implementations + // when regenerating bindings. If this test fails to compile, please add + // + // #[cfg_attr( + // feature = "serde", + // derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) + // )] + // + // to all structures causing compilation errors (we need the zerocopy traits, as the + // `Serialize` and `Deserialize` implementations are provided by the `serde_impls!` macro + // above, which implements serialization based on zerocopy's `FromBytes` and `AsBytes` + // traits that it expects to be derived). + // + // NOTE: This only include "top-level" items, and does not list out bindgen-anonymous types + // (e.g. types like `kvm_vcpu_events__bindgen_ty_5`). These types can change name across + // bindgen versions. If after re-adding the derives to all the below items you can compile + // errors about anonymous types not implementing `Serialize`/`Deserialize`, please also add + // the derives to all anonymous types references in the definitions of the below items. + + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + } +} diff --git a/src/lib.rs b/src/lib.rs index fc0ed6f..3b82731 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,6 +12,16 @@ #[cfg(feature = "fam-wrappers")] extern crate vmm_sys_util; +#[cfg(feature = "serde")] +extern crate serde; + +#[cfg(feature = "serde")] +extern crate zerocopy; + +#[cfg(feature = "serde")] +#[macro_use] +mod serialize; + #[cfg(target_arch = "x86_64")] mod x86_64; #[cfg(target_arch = "x86_64")] diff --git a/src/serialize.rs b/src/serialize.rs new file mode 100644 index 0000000..eaee42c --- /dev/null +++ b/src/serialize.rs @@ -0,0 +1,43 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +//! Module containing serialization utilities + +/// Macro that generates serde::Serialize and serde::Deserialize implementations for the given types. +/// This macro assumes that the types implement zerocopy::FromBytes and zerocopy::AsBytes, and uses +/// these implementations to serialize as opaque byte arrays. During deserialization, it will +/// try to deserialize as a `Vec`. If this deserialized `Vec` has a length that equals `size_of::`, +/// it will transmute to `T` (using zerocopy), otherwise the `Vec` will either be zero-padded, or truncated. +/// This will hopefully allow live update of bindings across kernel versions even if the kernel adds +/// new fields to the end of some struct (we heavily rely on the kernel not making ABI breaking changes here). +macro_rules! serde_impls { + ($($typ: ty),*) => { + $( + impl Serialize for $typ { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer + { + let bytes = self.as_bytes(); + serializer.serialize_bytes(bytes) + } + } + + impl<'de> Deserialize<'de> for $typ { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de> + { + let bytes = Vec::::deserialize(deserializer)?; + + let mut backing = [0u8; std::mem::size_of::<$typ>()]; + + let limit = bytes.len().min(backing.len()); + + backing[..limit].copy_from_slice(&bytes[..limit]); + + Ok(transmute!(backing)) + } + } + )* + } +} diff --git a/src/x86_64/bindings.rs b/src/x86_64/bindings.rs index 8b56d01..414ca49 100644 --- a/src/x86_64/bindings.rs +++ b/src/x86_64/bindings.rs @@ -1,7 +1,11 @@ /* automatically generated by rust-bindgen 0.64.0 */ -#[repr(C)] +#[repr(transparent)] #[derive(Copy, Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct __BindgenBitfieldUnit { storage: Storage, } @@ -80,8 +84,12 @@ where } } } -#[repr(C)] +#[repr(transparent)] #[derive(Default)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct __IncompleteArrayField(::std::marker::PhantomData, [T; 0]); impl __IncompleteArrayField { #[inline] @@ -879,6 +887,10 @@ pub type __wsum = __u32; pub type __poll_t = ::std::os::raw::c_uint; #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_pic_state { pub last_irr: __u8, pub irr: __u8, @@ -1074,6 +1086,10 @@ fn bindgen_test_layout_kvm_pic_state() { } #[repr(C)] #[derive(Copy, Clone)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_ioapic_state { pub base_address: __u64, pub ioregsel: __u32, @@ -1084,12 +1100,20 @@ pub struct kvm_ioapic_state { } #[repr(C)] #[derive(Copy, Clone)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub union kvm_ioapic_state__bindgen_ty_1 { pub bits: __u64, pub fields: kvm_ioapic_state__bindgen_ty_1__bindgen_ty_1, } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_ioapic_state__bindgen_ty_1__bindgen_ty_1 { pub vector: __u8, pub _bitfield_align_1: [u8; 0], @@ -1426,6 +1450,10 @@ impl ::std::fmt::Debug for kvm_ioapic_state { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_regs { pub rax: __u64, pub rbx: __u64, @@ -1643,6 +1671,10 @@ fn bindgen_test_layout_kvm_regs() { } #[repr(C)] #[derive(Debug, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_lapic_state { pub regs: [::std::os::raw::c_char; 1024usize], } @@ -1682,6 +1714,10 @@ impl Default for kvm_lapic_state { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_segment { pub base: __u64, pub limit: __u32, @@ -1844,6 +1880,10 @@ fn bindgen_test_layout_kvm_segment() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_dtable { pub base: __u64, pub limit: __u16, @@ -1896,6 +1936,10 @@ fn bindgen_test_layout_kvm_dtable() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_sregs { pub cs: kvm_segment, pub ds: kvm_segment, @@ -2481,6 +2525,10 @@ fn bindgen_test_layout_kvm_fpu() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_msr_entry { pub index: __u32, pub reserved: __u32, @@ -2533,6 +2581,10 @@ fn bindgen_test_layout_kvm_msr_entry() { } #[repr(C)] #[derive(Debug, Default)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_msrs { pub nmsrs: __u32, pub pad: __u32, @@ -2885,6 +2937,10 @@ fn bindgen_test_layout_kvm_cpuid() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_cpuid_entry2 { pub function: __u32, pub index: __u32, @@ -2992,6 +3048,10 @@ fn bindgen_test_layout_kvm_cpuid_entry2() { } #[repr(C)] #[derive(Debug, Default)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_cpuid2 { pub nent: __u32, pub padding: __u32, @@ -3044,6 +3104,10 @@ fn bindgen_test_layout_kvm_cpuid2() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_pit_channel_state { pub count: __u32, pub latched_count: __u16, @@ -3341,6 +3405,10 @@ fn bindgen_test_layout_kvm_pit_state() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_pit_state2 { pub channels: [kvm_pit_channel_state; 3usize], pub flags: __u32, @@ -3434,6 +3502,10 @@ fn bindgen_test_layout_kvm_reinject_control() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_vcpu_events { pub exception: kvm_vcpu_events__bindgen_ty_1, pub interrupt: kvm_vcpu_events__bindgen_ty_2, @@ -3448,6 +3520,10 @@ pub struct kvm_vcpu_events { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_vcpu_events__bindgen_ty_1 { pub injected: __u8, pub nr: __u8, @@ -3523,6 +3599,10 @@ fn bindgen_test_layout_kvm_vcpu_events__bindgen_ty_1() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_vcpu_events__bindgen_ty_2 { pub injected: __u8, pub nr: __u8, @@ -3587,6 +3667,10 @@ fn bindgen_test_layout_kvm_vcpu_events__bindgen_ty_2() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_vcpu_events__bindgen_ty_3 { pub injected: __u8, pub pending: __u8, @@ -3651,6 +3735,10 @@ fn bindgen_test_layout_kvm_vcpu_events__bindgen_ty_3() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_vcpu_events__bindgen_ty_4 { pub smm: __u8, pub pending: __u8, @@ -3715,6 +3803,10 @@ fn bindgen_test_layout_kvm_vcpu_events__bindgen_ty_4() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_vcpu_events__bindgen_ty_5 { pub pending: __u8, } @@ -3861,6 +3953,10 @@ fn bindgen_test_layout_kvm_vcpu_events() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_debugregs { pub db: [__u64; 4usize], pub dr6: __u64, @@ -3935,10 +4031,15 @@ fn bindgen_test_layout_kvm_debugregs() { } #[repr(C)] #[derive(Debug)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_xsave { pub region: [__u32; 1024usize], pub extra: __IncompleteArrayField<__u32>, } + #[test] fn bindgen_test_layout_kvm_xsave() { const UNINIT: ::std::mem::MaybeUninit = ::std::mem::MaybeUninit::uninit(); @@ -3985,6 +4086,10 @@ impl Default for kvm_xsave { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_xcr { pub xcr: __u32, pub reserved: __u32, @@ -4037,6 +4142,10 @@ fn bindgen_test_layout_kvm_xcr() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_xcrs { pub nr_xcrs: __u32, pub flags: __u32, @@ -5032,6 +5141,10 @@ impl ::std::fmt::Debug for kvm_irq_level { } #[repr(C)] #[derive(Copy, Clone)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_irqchip { pub chip_id: __u32, pub pad: __u32, @@ -5044,6 +5157,7 @@ pub union kvm_irqchip__bindgen_ty_1 { pub pic: kvm_pic_state, pub ioapic: kvm_ioapic_state, } + #[test] fn bindgen_test_layout_kvm_irqchip__bindgen_ty_1() { const UNINIT: ::std::mem::MaybeUninit = @@ -9167,6 +9281,10 @@ fn bindgen_test_layout_kvm_vapic_addr() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_mp_state { pub mp_state: __u32, } @@ -11404,6 +11522,10 @@ fn bindgen_test_layout_kvm_irqfd() { } #[repr(C)] #[derive(Debug, Default, Copy, Clone, PartialEq)] +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] pub struct kvm_clock_data { pub clock: __u64, pub flags: __u32, diff --git a/src/x86_64/mod.rs b/src/x86_64/mod.rs index 341ec3a..08e1bab 100644 --- a/src/x86_64/mod.rs +++ b/src/x86_64/mod.rs @@ -6,6 +6,9 @@ pub mod bindings; #[cfg(feature = "fam-wrappers")] pub mod fam_wrappers; +#[cfg(feature = "serde")] +mod serialize; + pub use self::bindings::*; #[cfg(feature = "fam-wrappers")] pub use self::fam_wrappers::*; diff --git a/src/x86_64/serialize.rs b/src/x86_64/serialize.rs new file mode 100644 index 0000000..da3e546 --- /dev/null +++ b/src/x86_64/serialize.rs @@ -0,0 +1,119 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +use bindings::{ + kvm_clock_data, kvm_cpuid2, kvm_cpuid_entry2, kvm_debugregs, kvm_dtable, kvm_irqchip, + kvm_irqchip__bindgen_ty_1, kvm_lapic_state, kvm_mp_state, kvm_msr_entry, kvm_msrs, + kvm_pit_channel_state, kvm_pit_state2, kvm_regs, kvm_segment, kvm_sregs, kvm_vcpu_events, + kvm_xcr, kvm_xcrs, kvm_xsave, +}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use zerocopy::{transmute, AsBytes, FromBytes, FromZeroes}; + +serde_impls!( + kvm_regs, + kvm_segment, + kvm_dtable, + kvm_sregs, + kvm_msr_entry, + kvm_cpuid_entry2, + kvm_pit_channel_state, + kvm_pit_state2, + kvm_vcpu_events, + kvm_debugregs, + kvm_xcr, + kvm_xcrs, + kvm_mp_state, + kvm_clock_data, + kvm_lapic_state, + kvm_msrs, + kvm_cpuid2, + kvm_xsave, + kvm_irqchip +); + +// SAFETY: zerocopy's derives explicitly disallow deriving for unions where +// the fields have different sizes, due to the smaller fields having padding. +// Miri however does not complain about these implementations (e.g. about +// reading the "padding" for one union field as valid data for a bigger one) +unsafe impl FromZeroes for kvm_irqchip__bindgen_ty_1 { + fn only_derive_is_allowed_to_implement_this_trait() + where + Self: Sized, + { + } +} + +// SAFETY: zerocopy's derives explicitly disallow deriving for unions where +// the fields have different sizes, due to the smaller fields having padding. +// Miri however does not complain about these implementations (e.g. about +// reading the "padding" for one union field as valid data for a bigger one) +unsafe impl FromBytes for kvm_irqchip__bindgen_ty_1 { + fn only_derive_is_allowed_to_implement_this_trait() + where + Self: Sized, + { + } +} + +// SAFETY: zerocopy's derives explicitly disallow deriving for unions where +// the fields have different sizes, due to the smaller fields having padding. +// Miri however does not complain about these implementations (e.g. about +// reading the "padding" for one union field as valid data for a bigger one) +unsafe impl AsBytes for kvm_irqchip__bindgen_ty_1 { + fn only_derive_is_allowed_to_implement_this_trait() + where + Self: Sized, + { + } +} + +#[cfg(test)] +mod tests { + use super::*; + use bindings::*; + + fn is_serde Deserialize<'de>>() {} + + #[test] + fn static_assert_serde_implementations() { + // This test statically (= at compile-time) asserts that various bindgen generated + // structures implement serde's `Serialize` and `Deserialize` traits. + // This is to make sure that we do not accidentally remove those implementations + // when regenerating bindings. If this test fails to compile, please add + // + // #[cfg_attr( + // feature = "serde", + // derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) + // )] + // + // to all structures causing compilation errors (we need the zerocopy traits, as the + // `Serialize` and `Deserialize` implementations are provided by the `serde_impls!` macro + // above, which implements serialization based on zerocopy's `FromBytes` and `AsBytes` + // traits that it expects to be derived). + // + // NOTE: This only include "top-level" items, and does not list out bindgen-anonymous types + // (e.g. types like `kvm_vcpu_events__bindgen_ty_5`). These types can change name across + // bindgen versions. If after re-adding the derives to all the below items you can compile + // errors about anonymous types not implementing `Serialize`/`Deserialize`, please also add + // the derives to all anonymous types references in the definitions of the below items. + + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + is_serde::(); + } +} From e3c22d786836ba1589be6f37124f0ec5d35cb155 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Thu, 7 Mar 2024 10:43:30 +0000 Subject: [PATCH 2/7] test: test with-serde feature in CI Do a build-test of the `with-serde` feature. Also do some simple round-trip testing to ensure consistent serialization (particularly, for the items that have manual (De)Serialize implementations). Signed-off-by: Patrick Roy --- .buildkite/custom-tests.json | 20 ++++++++++++++++++-- Cargo.toml | 3 +++ coverage_config_aarch64.json | 2 +- coverage_config_x86_64.json | 4 ++-- src/arm64/serialize.rs | 9 ++++++++- src/x86_64/serialize.rs | 9 ++++++++- 6 files changed, 40 insertions(+), 7 deletions(-) diff --git a/.buildkite/custom-tests.json b/.buildkite/custom-tests.json index 009f457..1ffa2fe 100644 --- a/.buildkite/custom-tests.json +++ b/.buildkite/custom-tests.json @@ -17,8 +17,24 @@ ] }, { - "test_name": "check-warnings-fam", - "command": "RUSTFLAGS=\"-D warnings\" cargo check --features=fam-wrappers", + "test_name": "build-serde-gnu", + "command": "cargo build --release --features=serde", + "platform": [ + "x86_64", + "aarch64" + ] + }, + { + "test_name": "build-serde-musl", + "command": "cargo build --release --features=serde --target {target_platform}-unknown-linux-musl", + "platform": [ + "x86_64", + "aarch64" + ] + }, + { + "test_name": "check-warnings-all-features", + "command": "RUSTFLAGS=\"-D warnings\" cargo check --all-features", "platform": [ "x86_64", "aarch64" diff --git a/Cargo.toml b/Cargo.toml index c4ff6db..1cdd6f2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,3 +23,6 @@ serde = ["dep:serde", "serde/derive", "dep:zerocopy"] vmm-sys-util = { version = "0.12.1", optional = true } serde = { version = "1.0.0", optional = true, features = ["derive"] } zerocopy = { version = "0.7.32", optional = true, features = ["derive"] } + +[dev-dependencies] +bincode = "1.3.3" diff --git a/coverage_config_aarch64.json b/coverage_config_aarch64.json index 54fd8ea..df5b99f 100644 --- a/coverage_config_aarch64.json +++ b/coverage_config_aarch64.json @@ -1,5 +1,5 @@ { "coverage_score": 60.9, "exclude_path": "", - "crate_features": "fam-wrappers" + "crate_features": "fam-wrappers,serde" } diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index 553e7aa..0026598 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1,5 +1,5 @@ { - "coverage_score": 100, + "coverage_score": 64, "exclude_path": ".*bindings\\.rs", - "crate_features": "fam-wrappers" + "crate_features": "fam-wrappers,serde" } diff --git a/src/arm64/serialize.rs b/src/arm64/serialize.rs index bfa78a8..e69f27b 100644 --- a/src/arm64/serialize.rs +++ b/src/arm64/serialize.rs @@ -18,7 +18,14 @@ mod tests { use bindings::*; use serde::{Deserialize, Serialize}; - fn is_serde Deserialize<'de>>() {} + fn is_serde Deserialize<'de> + Default>() { + let serialized = bincode::serialize(&T::default()).unwrap(); + let deserialized = bincode::deserialize::(serialized.as_ref()).unwrap(); + let serialized_again = bincode::serialize(&deserialized).unwrap(); + // Compare the serialized state after a roundtrip, to work around issues with + // bindings not implementing `PartialEq`. + assert_eq!(serialized, serialized_again); + } #[test] fn static_assert_serde_implementations() { diff --git a/src/x86_64/serialize.rs b/src/x86_64/serialize.rs index da3e546..e39a0b9 100644 --- a/src/x86_64/serialize.rs +++ b/src/x86_64/serialize.rs @@ -73,7 +73,14 @@ mod tests { use super::*; use bindings::*; - fn is_serde Deserialize<'de>>() {} + fn is_serde Deserialize<'de> + Default>() { + let serialized = bincode::serialize(&T::default()).unwrap(); + let deserialized = bincode::deserialize::(serialized.as_ref()).unwrap(); + let serialized_again = bincode::serialize(&deserialized).unwrap(); + // Compare the serialized state after a roundtrip, to work around issues with + // bindings not implementing `PartialEq`. + assert_eq!(serialized, serialized_again); + } #[test] fn static_assert_serde_implementations() { From df9fc11574e2b535617e1ec6da491d4b4999d676 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Wed, 10 Apr 2024 10:18:06 +0100 Subject: [PATCH 3/7] fix: Remove "check-warnings" step from custom ci steps We already have a `cargo check --all-targets --all-features` with `-D warnings` in the default CI inherited from rust-vmm-ci, so remove this duplicate. Signed-off-by: Patrick Roy --- .buildkite/custom-tests.json | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.buildkite/custom-tests.json b/.buildkite/custom-tests.json index 1ffa2fe..0977607 100644 --- a/.buildkite/custom-tests.json +++ b/.buildkite/custom-tests.json @@ -31,14 +31,6 @@ "x86_64", "aarch64" ] - }, - { - "test_name": "check-warnings-all-features", - "command": "RUSTFLAGS=\"-D warnings\" cargo check --all-features", - "platform": [ - "x86_64", - "aarch64" - ] } ] } From 50fcbc15b8bf3caae211baaad5facc68a52223c6 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Thu, 7 Mar 2024 10:55:55 +0000 Subject: [PATCH 4/7] Support treating kvm_xsave as a FamStruct In linux 5.17, kvm_xsave got turned into a FamStruct by adding the flexible "extra" member to its definition. However, unlike all other such structs, it does not contain a "length" field. Instead, the length of the flexible array member has to be determined by querying the `KVM_CAP_XSAVE2` capability. This requires access to a VM file descriptor, and thus cannot happen in the `FamStruct::len` trait method. To work around this, define a wrapper struct that caches the length of a previous `KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2)` call, and implement `FamStruct` for this wrapper. Then in kvm-ioctls, we can expose a function that first query `KVM_CAP_XSAVE2`, then invokes `KVM_GET_XSAVE2` to retrives the `kvm_xsave` structure, and then combine them into the below `kvm_xsave2` structure to be managed as a `FamStruct`. Signed-off-by: Patrick Roy --- src/x86_64/fam_wrappers.rs | 58 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/x86_64/fam_wrappers.rs b/src/x86_64/fam_wrappers.rs index 032879d..451bc34 100644 --- a/src/x86_64/fam_wrappers.rs +++ b/src/x86_64/fam_wrappers.rs @@ -94,6 +94,64 @@ impl PartialEq for kvm_msr_list { /// [FamStructWrapper](../vmm_sys_util/fam/struct.FamStructWrapper.html). pub type MsrList = FamStructWrapper; +/// Helper structure to treat post-5.17 [`kvm_xsave`] as a FamStruct. +/// +/// See also: [`Xsave`]. +#[repr(C)] +pub struct kvm_xsave2 { + pub len: usize, + pub xsave: kvm_xsave, +} + +// SAFETY: +// - `kvm_xsave2` is a POD +// - `kvm_xsave2` contains a flexible array member as its final field, due to `kvm_xsave` containing +// one, and being `repr(C)` +// - `Entry` is a POD +unsafe impl FamStruct for kvm_xsave2 { + type Entry = __u32; + + fn len(&self) -> usize { + self.len + } + + unsafe fn set_len(&mut self, len: usize) { + self.len = len; + } + + fn max_len() -> usize { + __u32::MAX as usize + } + + fn as_slice(&self) -> &[::Entry] { + let len = self.len(); + // SAFETY: By the invariants that the caller of `set_len` has to uphold, `len` matches + // the actual in-memory length of the FAM + unsafe { self.xsave.extra.as_slice(len) } + } + + fn as_mut_slice(&mut self) -> &mut [::Entry] { + let len = self.len(); + // SAFETY: By the invariants that the caller of `set_len` has to uphold, `len` matches + // the actual in-memory length of the FAM + unsafe { self.xsave.extra.as_mut_slice(len) } + } +} + +/// Wrapper over the post-5.17 [`kvm_xsave`] structure. +/// +/// In linux 5.17, kvm_xsave got turned into a FamStruct by adding the flexible "extra" member +/// to its definition. However, unlike all other such structs, it does not contain a "length" +/// field. Instead, the length of the flexible array member has to be determined by querying +/// the [`KVM_CAP_XSAVE2`] capability. This requires access to a VM file descriptor, and thus +/// cannot happen in the [`FamStruct::len`] trait method. To work around this, we define a wrapper +/// struct that caches the length of a previous `KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2)` call, +/// and implement [`FamStruct`] for this wrapper. Then in kvm-ioctls, we can expose a function +/// that first queries `KVM_CAP_XSAVE2`, then invokes [`KVM_GET_XSAVE2`] to retrives the `kvm_xsave` +/// structure, and finally combine them into the [`kvm_xsave2`] helper structure to be managed as a +/// `FamStruct`. +pub type Xsave = FamStructWrapper; + #[cfg(test)] mod tests { use super::{CpuId, MsrList, Msrs}; From b2d97d6700f6cb2e5b278204cb0adb5b08178649 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Fri, 15 Mar 2024 11:23:17 +0000 Subject: [PATCH 5/7] doc: Add instructions for generating bindings Describe what needs to be done to not break serialization support. Signed-off-by: Patrick Roy --- CONTRIBUTING.md | 26 ++++++++++++++++++++++++++ README.md | 1 + 2 files changed, 27 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index eeaa7c4..8f6d284 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,6 +68,32 @@ cp arm/mod.rs arm64/ Also, you will need to add the new architecture to `kvm-bindings/lib.rs`. +When regenerating bindings, care must be taken to re-add various `zerocopy` +derives under the `serde` feature. All items that require derives are +listed in the `x86_64/serialize.rs` and `arm64/serialize.rs` inside the +`serde_impls!` macro invocation, and missing derives will cause these +modules to fail compilation. For all items listed here, the following +derive should be present: + +```rs +#[cfg_attr( + feature = "serde", + derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes) +)] +``` + +Any types whose name contains a suffix akin to `__bindgen_ty_` and +which is contained in any struct listed in `serialize.rs` will also need +to have this derive added (otherwise compilation will fail). Note that +these types are not explicitly listed in `serialize.rs`, as their names +can change across `bindgen.rs` versions. + +Lastly, in `x86_64/bindings.rs`, the derives also need to be added to +`struct __BindgenBitfieldUnit` and `struct __IncompleteArrayField`. +Additionally, these structs need to have their layout changed from `#[repr(C)]` +to `#[repr(transparent)]`. This is needed because `zerocopy` traits can only be +derived on generic structures that are `repr(transparent)` or `repr(packed)`. + ### Future Improvements All the above steps are scriptable, so in the next iteration I will add a script to generate the bindings. diff --git a/README.md b/README.md index 02d5a26..417af37 100644 --- a/README.md +++ b/README.md @@ -45,3 +45,4 @@ bindings happens as opaque binary blobs via [`zerocopy`](https://google.github.i Due to the kernel's ABI compatibility, this means that bindings serialized in version `x` of `kvm-bindings` can be deserialized in version `y` of the crate, even if the bindings have had been regenerated in the meantime. + From 102cc0ace6f6a404602a35e5d1ce6c9043b4ea91 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Wed, 10 Apr 2024 15:42:41 +0100 Subject: [PATCH 6/7] doc: Add reference to CONTRIBUTING.md to README The contributing file contains instructions for how to (re)generate bindings, but this is a somewhat unintuitive location for these instructions. Thus explicitly refer to it from the README. Signed-off-by: Patrick Roy --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 417af37..4849169 100644 --- a/README.md +++ b/README.md @@ -46,3 +46,7 @@ Due to the kernel's ABI compatibility, this means that bindings serialized in version `x` of `kvm-bindings` can be deserialized in version `y` of the crate, even if the bindings have had been regenerated in the meantime. +## Regenerating Bindings + +Please see [`CONTRIBUTING.md`](CONTRIBUTING.md) for details on how to generate the bindings +or add support for new architectures. \ No newline at end of file From 3329420b7e46a8984a6cb742e3b3cb59b0021669 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Wed, 10 Apr 2024 15:51:15 +0100 Subject: [PATCH 7/7] doc: Add CHANGELOG entry Signed-off-by: Patrick Roy --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1aa0ecf..c76401f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ # Changelog ## [Unreleased] +### Added + +- An opt-in feature `serde` that enables [`serde`](https://serde.rs)-based + (de)serialization of various bindings. + ## [0.7.0] ### Changed