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

2025-06-11 Thread Manos Pitsidianakis 
On Tue, Jun 10, 2025 at 11:21 PM Bernhard Beschow  wrote:
>
> Now that there is logging support in Rust for QEMU, use it in the pl011
> device.
>
> Signed-off-by: Bernhard Beschow 
> ---
>  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

2025-06-11 Thread Bernhard Beschow 



Am 11. Juni 2025 07:55:20 UTC schrieb Paolo Bonzini :
>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

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

2025-06-11 Thread Paolo Bonzini 

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;

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

2025-06-10 Thread Bernhard Beschow 
Now that there is logging support in Rust for QEMU, use it in the pl011
device.

Signed-off-by: Bernhard Beschow 
---
 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