From patchwork Mon Dec 2 11:06:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 13890269 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 508FAD7830D for ; Mon, 2 Dec 2024 11:07:40 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tI4GC-00053y-I8; Mon, 02 Dec 2024 06:06:36 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tI4Fy-0004ye-Pv for qemu-devel@nongnu.org; Mon, 02 Dec 2024 06:06:24 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tI4Fv-0005Be-I9 for qemu-devel@nongnu.org; Mon, 02 Dec 2024 06:06:21 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1733137578; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vFZVeuOocEdpSoBqzsPyrTYWx9pROq8j1tqsBgVVe5Y=; b=dSaRb0ujGtQ32/7UrDOx98ROlklXa4UkNQUFAEhbvBrg9TNkcXQORYAjaaxwJRKZnpwSBC GLUimUlFpbwnObLSp8CvXbhT1iY4KYDnbT2wWNkkwODE8EYxTAA8Kr5uMZphIjC/2/5aSf PqQQYAZX1sbBGu/PJBgxfRZE9rzlRH8= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-574-bstx8CvaP96unPcLyAFuQg-1; Mon, 02 Dec 2024 06:06:17 -0500 X-MC-Unique: bstx8CvaP96unPcLyAFuQg-1 X-Mimecast-MFC-AGG-ID: bstx8CvaP96unPcLyAFuQg Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-466cab92c3aso49957051cf.2 for ; Mon, 02 Dec 2024 03:06:17 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733137576; x=1733742376; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vFZVeuOocEdpSoBqzsPyrTYWx9pROq8j1tqsBgVVe5Y=; b=rMICF2e9oncUgLQene27Ys/dtgRXxUfyzxKG0N+DPho7hvjHShUn/TkGGkD0FMnAAf mic9xr4xI1mWi8QxwRM7Rom3mSPjMvNSg6+Zs7XVdpGmYOPD43VPMuIV3ZHbSdO0K7R1 jqmBwvUVzjavlHUfAZVNZ2lrIZNnQVRbRp4LVtPspsmcv4TbBqGcKswYuP3B3YCEXHi7 /aa8NNPh5LX6Q5/6S2nu10ErymnAfrj2SZ/rQuOMAMDnaduX8GGze+idpkpMoCXUwS+q tw+FqiOLj+dn/9znBEdqw0NnjBQgFo37gASriYrtTUXHclsgK1ghce66xWCazsnD1WrX jQXg== X-Gm-Message-State: AOJu0YwEGtADTQ4msL78Wh0cN4/D+GiuU42ESgBz9DjZLHnMbOTjO1hf 1aiOUit56fLg9RNEYv1QLbxdLWq3O2kQ+aElxEEda2lwLLLJaf1nVA71f1mFpkPV9rIfQMvXMIj Y7YQj33PhwfBWXok0lDtjPsycKeiGeVVF8HmUU5qMxaxSa7JGEHhkDkoZ/9+L5AMM9SHLx95HmI ayw5LcsubDQUfaXAQmcFBqYVVwkg40cBwjKric X-Gm-Gg: ASbGnctH13O9UHhyNpgRKxJqwzKPs4R/VsZl38o1LmREPWd4rqwCeikikdlnOp67ubS aguQLWgKJ1PEhG1pD2LY2VqRJ6VMm+Pe/+QNaa7TYwargXVsxiyUaXaiO3NfYPu71WlQhk3AhNc c4NIMguxqzoA9XUFAuo86T9f6W9KqxbLMILjZR0kvjZ9BQKhnuGIlCY/iKOWMjVwY6ZKkchaLKS 3pV1/zt4SotJdU+HRlxYTMSTwOOelrFjTfBp1GaOX5ZW5s1TDWWT7yEN+FZkDnlYC/4QHGX5myQ 27g= X-Received: by 2002:a05:622a:54b:b0:460:ac8f:3bcd with SMTP id d75a77b69052e-466b36907c2mr339741081cf.45.1733137575753; Mon, 02 Dec 2024 03:06:15 -0800 (PST) X-Google-Smtp-Source: AGHT+IFfsZJO5oK63TazZCpQWjKPNuzi/ehU/+0hdR27r9yQHQ9y77lj7wwDVglADmdjasAgOse/zA== X-Received: by 2002:a05:622a:54b:b0:460:ac8f:3bcd with SMTP id d75a77b69052e-466b36907c2mr339740561cf.45.1733137575109; Mon, 02 Dec 2024 03:06:15 -0800 (PST) Received: from [192.168.122.1] (nat-pool-mxp-t.redhat.com. [149.6.153.186]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7b68492121esm392311385a.3.2024.12.02.03.06.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 02 Dec 2024 03:06:14 -0800 (PST) From: Paolo Bonzini To: qemu-devel@nongnu.org Cc: qemu-rust@nongnu.org Subject: [PATCH v2 1/2] rust: add BQL-enforcing Cell variant Date: Mon, 2 Dec 2024 12:06:08 +0100 Message-ID: <20241202110609.36775-2-pbonzini@redhat.com> X-Mailer: git-send-email 2.47.0 In-Reply-To: <20241202110609.36775-1-pbonzini@redhat.com> References: <20241202110609.36775-1-pbonzini@redhat.com> MIME-Version: 1.0 Received-SPF: pass client-ip=170.10.133.124; envelope-from=pbonzini@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -50 X-Spam_score: -5.1 X-Spam_bar: ----- X-Spam_report: (-5.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-2.996, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org QEMU objects usually have their pointer shared with the "outside world" very early in their lifetime, for example when they create their MemoryRegions. Because at this point it is not valid anymore to create a &mut reference to the device, individual parts of the device struct must be made mutable in a controlled manner. QEMU's Big Lock (BQL) effectively turns multi-threaded code into single-threaded code while device code runs, as long as the BQL is not released while the device is borrowed (because C code could sneak in and mutate the device). We can then introduce custom interior mutability primitives that are semantically similar to the standard library's (single-threaded) Cell and RefCell, but account for QEMU's threading model. Accessing the "BqlCell" or borrowing the "BqlRefCell" requires proving that the BQL is held, and attempting to access without the BQL is a runtime panic, similar to RefCell's already-borrowed panic. With respect to naming I also considered omitting the "Bql" prefix or moving it to the module, e.g. qemu_api::bql::{Cell, RefCell}. However, this could easily lead to mistakes and confusion; for example rustc could suggest the wrong import, leading to subtle bugs. As a start introduce the an equivalent of Cell. Almost all of the code was taken from Rust's standard library, while removing unstable features and probably-unnecessary functionality that constitute a large of the original code. A lot of what's left is documentation, as well as unit tests in the form of doctests. These are not yet integrated in "make check" but can be run with "cargo test --doc". Signed-off-by: Paolo Bonzini Reviewed-by: Junjie Mao --- rust/qemu-api/meson.build | 1 + rust/qemu-api/src/cell.rs | 298 ++++++++++++++++++++++++++++++++++++++ rust/qemu-api/src/lib.rs | 1 + 3 files changed, 300 insertions(+) create mode 100644 rust/qemu-api/src/cell.rs diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build index d719c13f46d..edc21e1a3f8 100644 --- a/rust/qemu-api/meson.build +++ b/rust/qemu-api/meson.build @@ -13,6 +13,7 @@ _qemu_api_rs = static_library( [ 'src/lib.rs', 'src/bindings.rs', + 'src/cell.rs', 'src/c_str.rs', 'src/definitions.rs', 'src/device_class.rs', diff --git a/rust/qemu-api/src/cell.rs b/rust/qemu-api/src/cell.rs new file mode 100644 index 00000000000..373be23ba8b --- /dev/null +++ b/rust/qemu-api/src/cell.rs @@ -0,0 +1,298 @@ +// SPDX-License-Identifier: MIT +// +// This file is based on library/core/src/cell.rs from +// Rust 1.82.0. +// +// Permission is hereby granted, free of charge, to any +// person obtaining a copy of this software and associated +// documentation files (the "Software"), to deal in the +// Software without restriction, including without +// limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of +// the Software, and to permit persons to whom the Software +// is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice +// shall be included in all copies or substantial portions +// of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF +// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED +// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A +// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR +// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +//! BQL-protected mutable containers. +//! +//! Rust memory safety is based on this rule: Given an object `T`, it is only +//! possible to have one of the following: +//! +//! - Having several immutable references (`&T`) to the object (also known as +//! **aliasing**). +//! - Having one mutable reference (`&mut T`) to the object (also known as +//! **mutability**). +//! +//! This is enforced by the Rust compiler. However, there are situations where +//! this rule is not flexible enough. Sometimes it is required to have multiple +//! references to an object and yet mutate it. In particular, QEMU objects +//! usually have their pointer shared with the "outside world very early in +//! their lifetime", for example when they create their +//! [`MemoryRegion`s](crate::bindings::MemoryRegion). Therefore, individual +//! parts of a device must be made mutable in a controlled manner through the +//! use of cell types. +//! +//! This module provides a way to do so via the Big QEMU Lock. While +//! [`BqlCell`] is essentially the same single-threaded primitive that is +//! available in `std::cell`, the BQL allows it to be used from a multi-threaded +//! context and to share references across threads, while maintaining Rust's +//! safety guarantees. For this reason, unlike its `std::cell` counterpart, +//! `BqlCell` implements the `Sync` trait. +//! +//! BQL checks are performed in debug builds but can be optimized away in +//! release builds, providing runtime safety during development with no overhead +//! in production. +//! +//! Warning: While `BqlCell` is similar to its `std::cell` counterpart, the two +//! are not interchangeable. Using `std::cell` types in QEMU device +//! implementations is usually incorrect and can lead to thread-safety issues. +//! +//! ## `BqlCell` +//! +//! [`BqlCell`] implements interior mutability by moving values in and out of +//! the cell. That is, an `&mut T` to the inner value can never be obtained as +//! long as the cell is shared. The value itself cannot be directly obtained +//! without copying it, cloning it, or replacing it with something else. This +//! type provides the following methods, all of which can be called only while +//! the BQL is held: +//! +//! - For types that implement [`Copy`], the [`get`](BqlCell::get) method +//! retrieves the current interior value by duplicating it. +//! - For types that implement [`Default`], the [`take`](BqlCell::take) method +//! replaces the current interior value with [`Default::default()`] and +//! returns the replaced value. +//! - All types have: +//! - [`replace`](BqlCell::replace): replaces the current interior value and +//! returns the replaced value. +//! - [`set`](BqlCell::set): this method replaces the interior value, +//! dropping the replaced value. + +use std::{cell::UnsafeCell, cmp::Ordering, fmt, mem}; + +use crate::bindings; + +// TODO: When building doctests do not include the actual BQL, because cargo +// does not know how to link them to libqemuutil. This can be fixed by +// running rustdoc from "meson test" instead of relying on cargo. +fn bql_locked() -> bool { + // SAFETY: the function does nothing but return a thread-local bool + !cfg!(MESON) || unsafe { bindings::bql_locked() } +} + +/// A mutable memory location that is protected by the Big QEMU Lock. +/// +/// # Memory layout +/// +/// `BqlCell` has the same in-memory representation as its inner type `T`. +#[repr(transparent)] +pub struct BqlCell { + value: UnsafeCell, +} + +// SAFETY: Same as for std::sync::Mutex. In the end this *is* a Mutex, +// except it is stored out-of-line +unsafe impl Send for BqlCell {} +unsafe impl Sync for BqlCell {} + +impl Clone for BqlCell { + #[inline] + fn clone(&self) -> BqlCell { + BqlCell::new(self.get()) + } +} + +impl Default for BqlCell { + /// Creates a `BqlCell`, with the `Default` value for T. + #[inline] + fn default() -> BqlCell { + BqlCell::new(Default::default()) + } +} + +impl PartialEq for BqlCell { + #[inline] + fn eq(&self, other: &BqlCell) -> bool { + self.get() == other.get() + } +} + +impl Eq for BqlCell {} + +impl PartialOrd for BqlCell { + #[inline] + fn partial_cmp(&self, other: &BqlCell) -> Option { + self.get().partial_cmp(&other.get()) + } +} + +impl Ord for BqlCell { + #[inline] + fn cmp(&self, other: &BqlCell) -> Ordering { + self.get().cmp(&other.get()) + } +} + +impl From for BqlCell { + /// Creates a new `BqlCell` containing the given value. + fn from(t: T) -> BqlCell { + BqlCell::new(t) + } +} + +impl fmt::Debug for BqlCell { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.get().fmt(f) + } +} + +impl fmt::Display for BqlCell { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.get().fmt(f) + } +} + +impl BqlCell { + /// Creates a new `BqlCell` containing the given value. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// ``` + #[inline] + pub const fn new(value: T) -> BqlCell { + BqlCell { + value: UnsafeCell::new(value), + } + } + + /// Sets the contained value. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// + /// c.set(10); + /// ``` + #[inline] + pub fn set(&self, val: T) { + self.replace(val); + } + + /// Replaces the contained value with `val`, and returns the old contained + /// value. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let cell = BqlCell::new(5); + /// assert_eq!(cell.get(), 5); + /// assert_eq!(cell.replace(10), 5); + /// assert_eq!(cell.get(), 10); + /// ``` + #[inline] + pub fn replace(&self, val: T) -> T { + assert!(bql_locked()); + // SAFETY: This can cause data races if called from multiple threads, + // but it won't happen as long as C code accesses the value + // under BQL protection only. + mem::replace(unsafe { &mut *self.value.get() }, val) + } + + /// Unwraps the value, consuming the cell. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// let five = c.into_inner(); + /// + /// assert_eq!(five, 5); + /// ``` + pub fn into_inner(self) -> T { + assert!(bql_locked()); + self.value.into_inner() + } +} + +impl BqlCell { + /// Returns a copy of the contained value. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// + /// let five = c.get(); + /// ``` + #[inline] + pub fn get(&self) -> T { + assert!(bql_locked()); + // SAFETY: This can cause data races if called from multiple threads, + // but it won't happen as long as C code accesses the value + // under BQL protection only. + unsafe { *self.value.get() } + } +} + +impl BqlCell { + /// Returns a raw pointer to the underlying data in this cell. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// + /// let ptr = c.as_ptr(); + /// ``` + #[inline] + pub const fn as_ptr(&self) -> *mut T { + self.value.get() + } +} + +impl BqlCell { + /// Takes the value of the cell, leaving `Default::default()` in its place. + /// + /// # Examples + /// + /// ``` + /// use qemu_api::cell::BqlCell; + /// + /// let c = BqlCell::new(5); + /// let five = c.take(); + /// + /// assert_eq!(five, 5); + /// assert_eq!(c.into_inner(), 0); + /// ``` + pub fn take(&self) -> T { + self.replace(Default::default()) + } +} diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index 440aff3817d..b04d110b3f5 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -8,6 +8,7 @@ pub mod bindings; pub mod c_str; +pub mod cell; pub mod definitions; pub mod device_class; pub mod offset_of; From patchwork Mon Dec 2 11:06:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 13890268 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 95DD9D7830C for ; Mon, 2 Dec 2024 11:07:32 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tI4GC-00053N-6H; Mon, 02 Dec 2024 06:06:36 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tI4G4-00050p-SP for qemu-devel@nongnu.org; Mon, 02 Dec 2024 06:06:30 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tI4G2-0005Ge-FF for qemu-devel@nongnu.org; Mon, 02 Dec 2024 06:06:28 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1733137584; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=CPvWGaC61x5SIWSt7f1hm5iB12nylqAohLxeFtFVppQ=; b=is+n1apB6zSF3kwAtIzavR+kibkvTrL2KqWaHTCXwGyvuHa0vw19qfP4jY/r+2kh13PFi9 /xvZDSMHVWMDourTZsOGm50OVXT1JZqDUuxteGuApD5zBj4VktklFjhDhvU7l6rOnBXQ37 DreTcr3t8Wn0SEYhF1ntFdKVrLh9028= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-563-JdJUyA7yMAuVzCqvymVO0g-1; Mon, 02 Dec 2024 06:06:20 -0500 X-MC-Unique: JdJUyA7yMAuVzCqvymVO0g-1 X-Mimecast-MFC-AGG-ID: JdJUyA7yMAuVzCqvymVO0g Received: by mail-qv1-f70.google.com with SMTP id 6a1803df08f44-6d88d56beb7so26875796d6.3 for ; Mon, 02 Dec 2024 03:06:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733137579; x=1733742379; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=CPvWGaC61x5SIWSt7f1hm5iB12nylqAohLxeFtFVppQ=; b=SnZxxEY7J4Knbtc9N1mBAjKJlevuGBkc9LSP6Bu3CNBfVMvUUULNFEgAkSfJ3J/efd dLq7QXUpRSkNIicsLhfTc807FlftHgD5H5JTQazJ6rLpMreGEBBi3sh/nHgajFGRTRNU NZuFrvrzs/dN9c5kodiDpekbshXpLp4E4VCEKNygPEE/qcEElt7nzF2mIhF2dP5yOj5t 1EgJrR/OSfhAaAg0yGs540kcnSSx5xejGaC8b79Q/3KpRiOen3c7MKkqVzjwWHsn0Tl8 FVtWL72uxP+LymC4L21tgP4DbxboPk63CcPlwEGzt7tymOpaBOItzYkyO5Au6zFwQuOd kyjQ== X-Gm-Message-State: AOJu0Yz/fKv5brJk43hBjoAiAWRtm0U/YxD30LF8tEcmUPv6sFlGtWKZ 7RGpznrIDeh0BJ8xguyw6HE3vEAbSMd23Ww03C7ioEJD9YhGFHdzV6PBiXnriuhAvHSizDH6OaG T6vhHbuZMajmokN+dZgd6KtYE09CZtoBHA5+2cRBuhkTJBb4nPA+9ce52/J1webit2ktJmejixk EL0uvJX4wMtFQXFcVBRj4ac2UvTz0w54aXOPkr X-Gm-Gg: ASbGncvLpzOmOnO1MXjqp1P4XU7EeUTmwoLBG+R/Ru1NxbEx9R5o1nTk8k9BBjnH8oS viwh2rQIkyRi2LX9Ky+0KM2u73WczGpMVJQHE+BR7LbwsLFaiUdRYOL+/2UJUq5272lByLhvJPr ufDy2Amiw9noc4GUS99Zag16dRGD2zBqzsm75C90McYJVDxJTcPIL/+FqiMhI8tGF+/k89O4cKw tbwVYdDGPKEq/XbdOa+oF6fGvaALe0HXL61neFTZxJNmsLdS8L/VO1bAe4Tyx1DDPlzEQb5Tjju r1k= X-Received: by 2002:ad4:5bef:0:b0:6d8:8f81:e2ea with SMTP id 6a1803df08f44-6d88f81e6cdmr154491396d6.31.1733137578672; Mon, 02 Dec 2024 03:06:18 -0800 (PST) X-Google-Smtp-Source: AGHT+IGjzuewtsoavkEm3OOJit9AmU7UeGAiGac4Ty3pvp6DGtcjaNlWqbjRqJK9teeS8VTfhgGFBQ== X-Received: by 2002:ad4:5bef:0:b0:6d8:8f81:e2ea with SMTP id 6a1803df08f44-6d88f81e6cdmr154491026d6.31.1733137578164; Mon, 02 Dec 2024 03:06:18 -0800 (PST) Received: from [192.168.122.1] (nat-pool-mxp-t.redhat.com. [149.6.153.186]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7b6849aabcasm390853385a.86.2024.12.02.03.06.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 02 Dec 2024 03:06:17 -0800 (PST) From: Paolo Bonzini To: qemu-devel@nongnu.org Cc: qemu-rust@nongnu.org Subject: [PATCH v2 2/2] rust: add bindings for interrupt sources Date: Mon, 2 Dec 2024 12:06:09 +0100 Message-ID: <20241202110609.36775-3-pbonzini@redhat.com> X-Mailer: git-send-email 2.47.0 In-Reply-To: <20241202110609.36775-1-pbonzini@redhat.com> References: <20241202110609.36775-1-pbonzini@redhat.com> MIME-Version: 1.0 Received-SPF: pass client-ip=170.10.129.124; envelope-from=pbonzini@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -50 X-Spam_score: -5.1 X-Spam_bar: ----- X-Spam_report: (-5.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-2.996, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org The InterruptSource bindings let us call qemu_set_irq() and sysbus_init_irq() as safe code. Interrupt sources, qemu_irq in C code, are pointers to IRQState objects. They are QOM link properties and can be written to outside the control of the device (i.e. from a shared reference); therefore they must be interior-mutable in Rust. Since thread-safety is provided by the BQL, what we want here is the newly-introduced BqlCell. A pointer to the contents of the BqlCell (an IRQState**, or equivalently qemu_irq*) is then passed to the C sysbus_init_irq function. Signed-off-by: Paolo Bonzini Reviewed-by: Zhao Liu --- rust/hw/char/pl011/src/device.rs | 22 ++++---- rust/qemu-api/meson.build | 2 + rust/qemu-api/src/irq.rs | 91 ++++++++++++++++++++++++++++++++ rust/qemu-api/src/lib.rs | 2 + rust/qemu-api/src/sysbus.rs | 26 +++++++++ 5 files changed, 133 insertions(+), 10 deletions(-) create mode 100644 rust/qemu-api/src/irq.rs create mode 100644 rust/qemu-api/src/sysbus.rs diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs index 317a9b3c5ad..c5c8c463d37 100644 --- a/rust/hw/char/pl011/src/device.rs +++ b/rust/hw/char/pl011/src/device.rs @@ -13,6 +13,7 @@ c_str, definitions::ObjectImpl, device_class::TYPE_SYS_BUS_DEVICE, + irq::InterruptSource, }; use crate::{ @@ -94,7 +95,7 @@ pub struct PL011State { /// * sysbus IRQ 5: `UARTEINTR` (error interrupt line) /// ``` #[doc(alias = "irq")] - pub interrupts: [qemu_irq; 6usize], + pub interrupts: [InterruptSource; IRQMASK.len()], #[doc(alias = "clk")] pub clock: NonNull, #[doc(alias = "migrate_clk")] @@ -139,7 +140,8 @@ impl PL011State { unsafe fn init(&mut self) { const CLK_NAME: &CStr = c_str!("clk"); - let dev = addr_of_mut!(*self).cast::(); + let sbd = unsafe { &mut *(addr_of_mut!(*self).cast::()) }; + // SAFETY: // // self and self.iomem are guaranteed to be valid at this point since callers @@ -153,12 +155,15 @@ unsafe fn init(&mut self) { Self::TYPE_INFO.name, 0x1000, ); - let sbd = addr_of_mut!(*self).cast::(); sysbus_init_mmio(sbd, addr_of_mut!(self.iomem)); - for irq in self.interrupts.iter_mut() { - sysbus_init_irq(sbd, irq); - } } + + for irq in self.interrupts.iter() { + sbd.init_irq(irq); + } + + let dev = addr_of_mut!(*self).cast::(); + // SAFETY: // // self.clock is not initialized at this point; but since `NonNull<_>` is Copy, @@ -498,10 +503,7 @@ pub fn put_fifo(&mut self, value: c_uint) { pub fn update(&self) { let flags = self.int_level & self.int_enabled; for (irq, i) in self.interrupts.iter().zip(IRQMASK) { - // SAFETY: self.interrupts have been initialized in init(). - unsafe { - qemu_set_irq(*irq, i32::from(flags & i != 0)); - } + irq.set(flags & i != 0); } } diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build index edc21e1a3f8..973cfbcfb4a 100644 --- a/rust/qemu-api/meson.build +++ b/rust/qemu-api/meson.build @@ -17,7 +17,9 @@ _qemu_api_rs = static_library( 'src/c_str.rs', 'src/definitions.rs', 'src/device_class.rs', + 'src/irq.rs', 'src/offset_of.rs', + 'src/sysbus.rs', 'src/vmstate.rs', 'src/zeroable.rs', ], diff --git a/rust/qemu-api/src/irq.rs b/rust/qemu-api/src/irq.rs new file mode 100644 index 00000000000..011dbcf3dba --- /dev/null +++ b/rust/qemu-api/src/irq.rs @@ -0,0 +1,91 @@ +// Copyright 2024 Red Hat, Inc. +// Author(s): Paolo Bonzini +// SPDX-License-Identifier: GPL-2.0-or-later + +//! Bindings for interrupt sources + +use core::ptr; +use std::{marker::PhantomData, os::raw::c_int}; + +use crate::{ + bindings::{qemu_set_irq, IRQState}, + cell::BqlCell, +}; + +/// Interrupt sources are used by devices to pass changes to a value (typically +/// a boolean). The interrupt sink is usually an interrupt controller or +/// GPIO controller. +/// +/// As far as devices are concerned, interrupt sources are always active-high: +/// for example, `InterruptSource`'s [`raise`](InterruptSource::raise) +/// method sends a `true` value to the sink. If the guest has to see a +/// different polarity, that change is performed by the board between the +/// device and the interrupt controller. +/// +/// Interrupts are implemented as a pointer to the interrupt "sink", which has +/// type [`IRQState`]. A device exposes its source as a QOM link property using +/// a function such as +/// [`SysBusDevice::init_irq`](crate::sysbus::SysBusDevice::init_irq), and +/// initially leaves the pointer to a NULL value, representing an unconnected +/// interrupt. To connect it, whoever creates the device fills the pointer with +/// the sink's `IRQState *`, for example using `sysbus_connect_irq`. Because +/// devices are generally shared objects, interrupt sources are an example of +/// the interior mutability pattern. +/// +/// Interrupt sources can only be triggered under the Big QEMU Lock; `BqlCell` +/// allows access from whatever thread has it. +#[derive(Debug)] +#[repr(transparent)] +pub struct InterruptSource +where + c_int: From, +{ + cell: BqlCell<*mut IRQState>, + _marker: PhantomData, +} + +impl InterruptSource { + /// Send a low (`false`) value to the interrupt sink. + pub fn lower(&self) { + self.set(false); + } + + /// Send a high-low pulse to the interrupt sink. + pub fn pulse(&self) { + self.set(true); + self.set(false); + } + + /// Send a high (`true`) value to the interrupt sink. + pub fn raise(&self) { + self.set(true); + } +} + +impl InterruptSource +where + c_int: From, +{ + /// Send `level` to the interrupt sink. + pub fn set(&self, level: T) { + let ptr = self.cell.get(); + // SAFETY: the pointer is retrieved under the BQL and remains valid + // until the BQL is released, which is after qemu_set_irq() is entered. + unsafe { + qemu_set_irq(ptr, level.into()); + } + } + + pub(crate) const fn as_ptr(&self) -> *mut *mut IRQState { + self.cell.as_ptr() + } +} + +impl Default for InterruptSource { + fn default() -> Self { + InterruptSource { + cell: BqlCell::new(ptr::null_mut()), + _marker: PhantomData, + } + } +} diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index b04d110b3f5..aa692939688 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -11,7 +11,9 @@ pub mod cell; pub mod definitions; pub mod device_class; +pub mod irq; pub mod offset_of; +pub mod sysbus; pub mod vmstate; pub mod zeroable; diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs new file mode 100644 index 00000000000..1a9b8a1f971 --- /dev/null +++ b/rust/qemu-api/src/sysbus.rs @@ -0,0 +1,26 @@ +// Copyright 2024 Red Hat, Inc. +// Author(s): Paolo Bonzini +// SPDX-License-Identifier: GPL-2.0-or-later + +use std::ptr::addr_of; + +pub use bindings::{SysBusDevice, SysBusDeviceClass}; + +use crate::{bindings, irq::InterruptSource}; + +impl SysBusDevice { + /// Return `self` cast to a mutable pointer, for use in calls to C code. + const fn as_mut_ptr(&self) -> *mut SysBusDevice { + addr_of!(*self) as *mut _ + } + + /// Expose an interrupt source outside the device as a qdev GPIO output. + /// Note that the ordering of calls to `init_irq` is important, since + /// whoever creates the sysbus device will refer to the interrupts with + /// a number that corresponds to the order of calls to `init_irq`. + pub fn init_irq(&self, irq: &InterruptSource) { + unsafe { + bindings::sysbus_init_irq(self.as_mut_ptr(), irq.as_ptr()); + } + } +}