qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Initial logging support for Rust
@ 2025-06-15 11:20 Bernhard Beschow
  2025-06-15 11:20 ` [PATCH v3 1/4] rust/qemu-api: Add initial logging support based on C API Bernhard Beschow
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Bernhard Beschow @ 2025-06-15 11:20 UTC (permalink / raw)
  To: qemu-devel
  Cc: Manos Pitsidianakis, Paolo Bonzini, qemu-rust, Bernhard Beschow

This series introduces a log_mask_ln! macro which is inspired by the C version
and is just a thin wrapper around qemu_log(). It caters to Rust expectations by
accepting an enum for logging categories and working like the format! macro. The
macro then gets used in the pl011 device which either had its logging commented
out or relied on eprintln!() which can't be silenced by users.

Testing done:
* Run `make clippy`
* Purposefully trigger warnings with varying number of arguments

v3:
* Add macro to prelude (Manos)
* Have log_mask_ln! rather than log_mask! macro (Manos, Paolo)
* rustdoc improvements (Paolo)
* Fix logged function names (Manos)
* Add missing logging to pl011 to match C version (Paolo)
* Add patch to make clippy happier (which still complains about "PrimeCell" in
  rustdoc comment)

v2:
* Drop the qemu_ prefix from the macro name (Paolo)
* Use an enum for the logging categories in PascalCase as suggested by Paolo

Bernhard Beschow (4):
  rust/qemu-api: Add initial logging support based on C API
  rust/hw/char/pl011/src/device: Implement logging
  rust/hw/char/pl011/src/device: Add missing logging to match C version
  rust/qemu-api: Fix clippy lint `missing_const_for_fn`

 docs/devel/rust.rst              |  1 +
 rust/wrapper.h                   |  2 +
 rust/hw/char/pl011/src/device.rs | 18 ++++++--
 rust/qemu-api/meson.build        |  1 +
 rust/qemu-api/src/cell.rs        |  2 +-
 rust/qemu-api/src/lib.rs         |  1 +
 rust/qemu-api/src/log.rs         | 78 ++++++++++++++++++++++++++++++++
 rust/qemu-api/src/prelude.rs     |  2 +
 rust/qemu-api/src/qom.rs         |  2 +-
 9 files changed, 101 insertions(+), 6 deletions(-)
 create mode 100644 rust/qemu-api/src/log.rs

-- 
2.49.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/4] rust/qemu-api: Add initial logging support based on C API
  2025-06-15 11:20 [PATCH v3 0/4] Initial logging support for Rust Bernhard Beschow
@ 2025-06-15 11:20 ` Bernhard Beschow
  2025-06-15 11:20 ` [PATCH v3 2/4] rust/hw/char/pl011/src/device: Implement logging Bernhard Beschow
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Bernhard Beschow @ 2025-06-15 11:20 UTC (permalink / raw)
  To: qemu-devel
  Cc: Manos Pitsidianakis, Paolo Bonzini, qemu-rust, Bernhard Beschow

A log_mask_ln!() macro is provided which expects similar arguments as the
C version. However, the formatting works as one would expect from Rust.

To maximize code reuse the macro is just a thin wrapper around
qemu_log(). Also, just the bare minimum of logging masks is provided
which should suffice for the current use case of Rust in QEMU.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 docs/devel/rust.rst          |  1 +
 rust/wrapper.h               |  2 +
 rust/qemu-api/meson.build    |  1 +
 rust/qemu-api/src/lib.rs     |  1 +
 rust/qemu-api/src/log.rs     | 78 ++++++++++++++++++++++++++++++++++++
 rust/qemu-api/src/prelude.rs |  2 +
 6 files changed, 85 insertions(+)
 create mode 100644 rust/qemu-api/src/log.rs

diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index 47e9677fcb..dc8c44109e 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -162,6 +162,7 @@ module           status
 ``errno``        complete
 ``error``        stable
 ``irq``          complete
+``log``          proof of concept
 ``memory``       stable
 ``module``       complete
 ``qdev``         stable
diff --git a/rust/wrapper.h b/rust/wrapper.h
index 6060d3ba1a..15a1b19847 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -48,6 +48,8 @@ typedef enum memory_order {
 #endif /* __CLANG_STDATOMIC_H */
 
 #include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/log-for-trace.h"
 #include "qemu/module.h"
 #include "qemu-io.h"
 #include "system/system.h"
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index cac8595a14..33caee3c4f 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -21,6 +21,7 @@ _qemu_api_rs = static_library(
       'src/errno.rs',
       'src/error.rs',
       'src/irq.rs',
+      'src/log.rs',
       'src/memory.rs',
       'src/module.rs',
       'src/prelude.rs',
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 93902fc94b..e20be35460 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -21,6 +21,7 @@
 pub mod errno;
 pub mod error;
 pub mod irq;
+pub mod log;
 pub mod memory;
 pub mod module;
 pub mod qdev;
diff --git a/rust/qemu-api/src/log.rs b/rust/qemu-api/src/log.rs
new file mode 100644
index 0000000000..0d7f06e63c
--- /dev/null
+++ b/rust/qemu-api/src/log.rs
@@ -0,0 +1,78 @@
+// Copyright 2025 Bernhard Beschow <shentey@gmail.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! Bindings for QEMU's logging infrastructure
+
+#[repr(u32)]
+/// Represents specific error categories within QEMU's logging system.
+///
+/// The `Log` enum provides a Rust abstraction for logging errors, corresponding
+/// to a subset of the error categories defined in the C implementation.
+pub enum Log {
+    /// Log invalid access caused by the guest.
+    /// Corresponds to `LOG_GUEST_ERROR` in the C implementation.
+    GuestError = crate::bindings::LOG_GUEST_ERROR,
+
+    /// Log guest access of unimplemented functionality.
+    /// Corresponds to `LOG_UNIMP` in the C implementation.
+    Unimp = crate::bindings::LOG_UNIMP,
+}
+
+/// A macro to log messages conditionally based on a provided mask.
+///
+/// The `log_mask_ln` macro checks whether the given mask matches the current
+/// log level and, if so, formats and logs the message. It is the Rust
+/// counterpart of the `qemu_log_mask()` macro in the C implementation.
+///
+/// # Parameters
+///
+/// - `$mask`: A log level mask. This should be a variant of the `Log` enum.
+/// - `$fmt`: A format string following the syntax and rules of the `format!`
+///   macro. It specifies the structure of the log message.
+/// - `$args`: Optional arguments to be interpolated into the format string.
+///
+/// # Example
+///
+/// ```
+/// use qemu_api::log::Log;
+/// use qemu_api::log_mask_ln;
+///
+/// let error_address = 0xbad;
+/// log_mask_ln!(
+///     Log::GuestError,
+///     "Address 0x{error_address:x} out of range"
+/// );
+/// ```
+///
+/// It is also possible to use printf-style formatting, as well as having a
+/// trailing `,`:
+///
+/// ```
+/// use qemu_api::log::Log;
+/// use qemu_api::log_mask_ln;
+///
+/// let error_address = 0xbad;
+/// log_mask_ln!(
+///     Log::GuestError,
+///     "Address 0x{:x} out of range",
+///     error_address,
+/// );
+/// ```
+#[macro_export]
+macro_rules! log_mask_ln {
+    ($mask:expr, $fmt:tt $($args:tt)*) => {{
+        // Type assertion to enforce type `Log` for $mask
+        let _: Log = $mask;
+
+        if unsafe {
+            (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0
+        } {
+            let formatted_string = format!("{}\n", format_args!($fmt $($args)*));
+            let c_string = std::ffi::CString::new(formatted_string).unwrap();
+
+            unsafe {
+                ::qemu_api::bindings::qemu_log(c_string.as_ptr());
+            }
+        }
+    }};
+}
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index 43bfcd5fca..8f9e23ee2c 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -11,6 +11,8 @@
 
 pub use crate::errno;
 
+pub use crate::log_mask_ln;
+
 pub use crate::qdev::DeviceMethods;
 
 pub use crate::qom::InterfaceType;
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/4] rust/hw/char/pl011/src/device: Implement logging
  2025-06-15 11:20 [PATCH v3 0/4] Initial logging support for Rust Bernhard Beschow
  2025-06-15 11:20 ` [PATCH v3 1/4] rust/qemu-api: Add initial logging support based on C API Bernhard Beschow
@ 2025-06-15 11:20 ` Bernhard Beschow
  2025-06-15 11:20 ` [PATCH v3 3/4] rust/hw/char/pl011/src/device: Add missing logging to match C version Bernhard Beschow
  2025-06-15 11:20 ` [PATCH v3 4/4] rust/qemu-api: Fix clippy lint `missing_const_for_fn` Bernhard Beschow
  3 siblings, 0 replies; 5+ messages in thread
From: Bernhard Beschow @ 2025-06-15 11:20 UTC (permalink / raw)
  To: qemu-devel
  Cc: Manos Pitsidianakis, Paolo Bonzini, qemu-rust, Bernhard Beschow

Now that there is logging support in Rust for QEMU, use it in the pl011
device.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 rust/hw/char/pl011/src/device.rs | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index be8387f6f2..fa591c3ef6 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -8,6 +8,8 @@
     chardev::{CharBackend, Chardev, Event},
     impl_vmstate_forward,
     irq::{IRQState, InterruptSource},
+    log::Log,
+    log_mask_ln,
     memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
     prelude::*,
     qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl},
@@ -275,8 +277,7 @@ pub(self) fn write(
             DMACR => {
                 self.dmacr = value;
                 if value & 3 > 0 {
-                    // qemu_log_mask(LOG_UNIMP, "pl011: DMA not implemented\n");
-                    eprintln!("pl011: DMA not implemented");
+                    log_mask_ln!(Log::Unimp, "pl011: DMA not implemented");
                 }
             }
         }
@@ -538,7 +539,7 @@ fn read(&self, offset: hwaddr, _size: u32) -> u64 {
                 u64::from(device_id[(offset - 0xfe0) >> 2])
             }
             Err(_) => {
-                // qemu_log_mask(LOG_GUEST_ERROR, "pl011_read: Bad offset 0x%x\n", (int)offset);
+                log_mask_ln!(Log::GuestError, "PL011State::read: Bad offset {offset}");
                 0
             }
             Ok(field) => {
@@ -570,7 +571,10 @@ fn write(&self, offset: hwaddr, value: u64, _size: u32) {
                 .borrow_mut()
                 .write(field, value as u32, &self.char_backend);
         } else {
-            eprintln!("write bad offset {offset} value {value}");
+            log_mask_ln!(
+                Log::GuestError,
+                "PL011State::write: Bad offset {offset} value {value}"
+            );
         }
         if update_irq {
             self.update();
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 3/4] rust/hw/char/pl011/src/device: Add missing logging to match C version
  2025-06-15 11:20 [PATCH v3 0/4] Initial logging support for Rust Bernhard Beschow
  2025-06-15 11:20 ` [PATCH v3 1/4] rust/qemu-api: Add initial logging support based on C API Bernhard Beschow
  2025-06-15 11:20 ` [PATCH v3 2/4] rust/hw/char/pl011/src/device: Implement logging Bernhard Beschow
@ 2025-06-15 11:20 ` Bernhard Beschow
  2025-06-15 11:20 ` [PATCH v3 4/4] rust/qemu-api: Fix clippy lint `missing_const_for_fn` Bernhard Beschow
  3 siblings, 0 replies; 5+ messages in thread
From: Bernhard Beschow @ 2025-06-15 11:20 UTC (permalink / raw)
  To: qemu-devel
  Cc: Manos Pitsidianakis, Paolo Bonzini, qemu-rust, Bernhard Beschow

Co-developed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 rust/hw/char/pl011/src/device.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index fa591c3ef6..48b4bca6b8 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -304,6 +304,12 @@ fn read_data_register(&mut self, update: &mut bool) -> u32 {
     }
 
     fn write_data_register(&mut self, value: u32) -> bool {
+        if !self.control.enable_uart() {
+            log_mask_ln!(Log::GuestError, "PL011 data written to disabled UART");
+        }
+        if !self.control.enable_transmit() {
+            log_mask_ln!(Log::GuestError, "PL011 data written to disabled TX UART");
+        }
         // interrupts always checked
         let _ = self.loopback_tx(value.into());
         self.int_level |= Interrupt::TX;
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 4/4] rust/qemu-api: Fix clippy lint `missing_const_for_fn`
  2025-06-15 11:20 [PATCH v3 0/4] Initial logging support for Rust Bernhard Beschow
                   ` (2 preceding siblings ...)
  2025-06-15 11:20 ` [PATCH v3 3/4] rust/hw/char/pl011/src/device: Add missing logging to match C version Bernhard Beschow
@ 2025-06-15 11:20 ` Bernhard Beschow
  3 siblings, 0 replies; 5+ messages in thread
From: Bernhard Beschow @ 2025-06-15 11:20 UTC (permalink / raw)
  To: qemu-devel
  Cc: Manos Pitsidianakis, Paolo Bonzini, qemu-rust, Bernhard Beschow

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 rust/qemu-api/src/cell.rs | 2 +-
 rust/qemu-api/src/qom.rs  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-api/src/cell.rs b/rust/qemu-api/src/cell.rs
index 27063b049d..851573f8ef 100644
--- a/rust/qemu-api/src/cell.rs
+++ b/rust/qemu-api/src/cell.rs
@@ -965,7 +965,7 @@ impl<T> Opaque<T> {
     /// # Safety
     ///
     /// The pointer must be valid, though it need not point to a valid value.
-    pub unsafe fn from_raw<'a>(ptr: *mut T) -> &'a Self {
+    pub const unsafe fn from_raw<'a>(ptr: *mut T) -> &'a Self {
         let ptr = NonNull::new(ptr).unwrap().cast::<Self>();
         // SAFETY: Self is a transparent wrapper over T
         unsafe { ptr.as_ref() }
diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
index 14f98fee60..29d1b02720 100644
--- a/rust/qemu-api/src/qom.rs
+++ b/rust/qemu-api/src/qom.rs
@@ -634,7 +634,7 @@ impl<T: ObjectType> Owned<T> {
     /// back to `from_raw()` (assuming the original `Owned` was valid!),
     /// since the owned reference remains there between the calls to
     /// `into_raw()` and `from_raw()`.
-    pub unsafe fn from_raw(ptr: *const T) -> Self {
+    pub const unsafe fn from_raw(ptr: *const T) -> Self {
         // SAFETY NOTE: while NonNull requires a mutable pointer, only
         // Deref is implemented so the pointer passed to from_raw
         // remains const
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-06-15 11:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-15 11:20 [PATCH v3 0/4] Initial logging support for Rust Bernhard Beschow
2025-06-15 11:20 ` [PATCH v3 1/4] rust/qemu-api: Add initial logging support based on C API Bernhard Beschow
2025-06-15 11:20 ` [PATCH v3 2/4] rust/hw/char/pl011/src/device: Implement logging Bernhard Beschow
2025-06-15 11:20 ` [PATCH v3 3/4] rust/hw/char/pl011/src/device: Add missing logging to match C version Bernhard Beschow
2025-06-15 11:20 ` [PATCH v3 4/4] rust/qemu-api: Fix clippy lint `missing_const_for_fn` Bernhard Beschow

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).