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.
>The latter can be
>optionally built to feed into qemu log, as well as the other dynamic trace
>backends.
The use of qemu_log() in qemu_log_mask() seems like an implementation detail to
me. Theoretically, qemu_log_mask() could use a different backend if this got
implemented, and wouldn't require code changes throughout QEMU.
Best regards,
Bernhard
>
>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
