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

Bernhard Beschow posted 2 patches 5 months, 1 week ago
Maintainers: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
There is a newer version of this series
[PATCH v2 2/2] rust/hw/char/pl011/src/device: Implement logging
Posted by Bernhard Beschow 5 months, 1 week ago
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..17a4e9269c 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,
     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!(Log::Unimp, "pl011: DMA not implemented\n");
                 }
             }
         }
@@ -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!(Log::GuestError, "pl011_read: Bad offset {offset}\n");
                 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!(
+                Log::GuestError,
+                "pl011_write: Bad offset {offset} value {value}\n"
+            );
         }
         if update_irq {
             self.update();
-- 
2.49.0
Re: [PATCH v2 2/2] rust/hw/char/pl011/src/device: Implement logging
Posted by Manos Pitsidianakis 5 months, 1 week ago
On Tue, Jun 10, 2025 at 11:21 PM Bernhard Beschow <shentey@gmail.com> 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 be8387f6f2..17a4e9269c 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,
>      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!(Log::Unimp, "pl011: DMA not implemented\n");
>                  }
>              }
>          }
> @@ -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!(Log::GuestError, "pl011_read: Bad offset {offset}\n");

Nit:

log_mask!(Log::GuestError, "pl011_read: Bad offset 0x{offset:x}\n");

Also, pl011_read is the C device function. You can put
`PL011State::read: ` instead.

>                  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!(
> +                Log::GuestError,
> +                "pl011_write: Bad offset {offset} value {value}\n"
> +            );

Ditto



--
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd
Re: [PATCH v2 2/2] rust/hw/char/pl011/src/device: Implement logging
Posted by Paolo Bonzini 5 months, 1 week ago
On 6/10/25 22:21, Bernhard Beschow wrote:
> Now that there is logging support in Rust for QEMU, use it in the pl011
> device.
Adding also this to match the C code:

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 42dfa9509dc..e505abfae86 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -305,6 +305,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!(Log::GuestError, "PL011 data written to disabled UART\n");
+        }
+        if !self.control.enable_transmit() {
+            log_mask!(Log::GuestError, "PL011 data written to disabled TX UART\n");
+        }
          // interrupts always checked
          let _ = self.loopback_tx(value.into());
          self.int_level |= Interrupt::TX;
Re: [PATCH v2 2/2] rust/hw/char/pl011/src/device: Implement logging
Posted by Bernhard Beschow 5 months, 1 week ago

Am 11. Juni 2025 07:55:20 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>On 6/10/25 22:21, Bernhard Beschow wrote:
>> Now that there is logging support in Rust for QEMU, use it in the pl011
>> device.
>Adding also this to match the C code:
>
>diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
>index 42dfa9509dc..e505abfae86 100644
>--- a/rust/hw/char/pl011/src/device.rs
>+++ b/rust/hw/char/pl011/src/device.rs
>@@ -305,6 +305,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!(Log::GuestError, "PL011 data written to disabled UART\n");
>+        }
>+        if !self.control.enable_transmit() {
>+            log_mask!(Log::GuestError, "PL011 data written to disabled TX UART\n");
>+        }
>         // interrupts always checked
>         let _ = self.loopback_tx(value.into());
>         self.int_level |= Interrupt::TX;
>

Will add as separate patch in v3.

Best regards,
Bernhard