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

["Daniel P. Berrangé" <berrange@redhat.com>]

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|