On Wed, Apr 02, 2025 at 09:33:16AM +0000, Bernhard Beschow wrote:
>
>
> Am 31. März 2025 09:18:05 UTC schrieb "Daniel P. Berrangé"
> <berra...@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 <shen...@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.
>
> I *think* it's the perspective: If you want to see any issues, regardless
> of which device, use the -l option, i.e. qemu_log_mask(). If, however,
> you want to see what a particular device does, use tracepoints.
I guess I'd say that the latter ought to be capable of satisfying the
former use case too, given a suitable trace point selection. If it
can't, then perhaps that's telling us the way we select trace points
is insufficiently expressive ?
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 :|
