diff mbox series

[2/2] rust/hw/char/pl011/src/device: Implement logging

Message ID 20250330205857.1615-3-shentey@gmail.com (mailing list archive)
State New
Headers show
Series Initial logging support for Rust | expand

Commit Message

Bernhard Beschow March 30, 2025, 8:58 p.m. UTC 
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(-)

Comments

Daniel P. Berrangé March 31, 2025, 9:18 a.m. UTC | #1 
On Sun, Mar 30, 2025 at 10:58:57PM +0200, Bernhard Beschow wrote:
> 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 bf88e0b00a..d5470fae11 100644
> --- a/rust/hw/char/pl011/src/device.rs
> +++ b/rust/hw/char/pl011/src/device.rs
> @@ -8,9 +8,11 @@
>      chardev::{CharBackend, Chardev, Event},
>      impl_vmstate_forward,
>      irq::{IRQState, InterruptSource},
> +    log::{LOG_GUEST_ERROR, LOG_UNIMP},
>      memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
>      prelude::*,
>      qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl},
> +    qemu_log_mask,
>      qom::{ObjectImpl, Owned, ParentField},
>      static_assert,
>      sysbus::{SysBusDevice, SysBusDeviceImpl},
> @@ -298,8 +300,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");
> +                    qemu_log_mask!(LOG_UNIMP, "pl011: DMA not implemented\n");
>                  }
>              }
>          }
> @@ -535,7 +536,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);
> +                qemu_log_mask!(LOG_GUEST_ERROR, "pl011_read: Bad offset {offset}\n");
>                  0
>              }
>              Ok(field) => {
> @@ -567,7 +568,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}");
> +            qemu_log_mask!(
> +                LOG_GUEST_ERROR,
> +                "pl011_write: Bad offset {offset} value {value}\n"
> +            );
>          }

General conceptual question .....  I've never understood what the dividing
line is between use of 'qemu_log_mask' and trace points. The latter can be
optionally built to feed into qemu log, as well as the other dynamic trace
backends.

Is there a compelling reason to use 'qemu_log', that isn't acceptable for
trace probe points ?

This is an indirect way of asking whether qemu_log_mask should be exposed
to rust, or would exposing tracing be sufficient ?

With regards,
Daniel
diff mbox series

Patch

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index bf88e0b00a..d5470fae11 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -8,9 +8,11 @@ 
     chardev::{CharBackend, Chardev, Event},
     impl_vmstate_forward,
     irq::{IRQState, InterruptSource},
+    log::{LOG_GUEST_ERROR, LOG_UNIMP},
     memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
     prelude::*,
     qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl},
+    qemu_log_mask,
     qom::{ObjectImpl, Owned, ParentField},
     static_assert,
     sysbus::{SysBusDevice, SysBusDeviceImpl},
@@ -298,8 +300,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");
+                    qemu_log_mask!(LOG_UNIMP, "pl011: DMA not implemented\n");
                 }
             }
         }
@@ -535,7 +536,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);
+                qemu_log_mask!(LOG_GUEST_ERROR, "pl011_read: Bad offset {offset}\n");
                 0
             }
             Ok(field) => {
@@ -567,7 +568,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}");
+            qemu_log_mask!(
+                LOG_GUEST_ERROR,
+                "pl011_write: Bad offset {offset} value {value}\n"
+            );
         }
         if update_irq {
             self.update();