[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Use of TARGET_FMT_plx in hw/tpm/trace-events
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] Use of TARGET_FMT_plx in hw/tpm/trace-events |
Date: |
Mon, 22 Jul 2019 16:40:04 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) |
Thomas Huth <address@hidden> writes:
> On 20/07/2019 11.42, Philippe Mathieu-Daudé wrote:
>> On 7/20/19 8:39 AM, Markus Armbruster wrote:
>>> Consider hw/tpm/trace-events
>>>
>>> # tpm_crb.c
>>> tpm_crb_mmio_read(uint64_t addr, unsigned size, uint32_t val) "CRB read
>>> 0x" TARGET_FMT_plx " len:%u val: 0x%" PRIx32
>>> tpm_crb_mmio_write(uint64_t addr, unsigned size, uint32_t val) "CRB
>>> write 0x" TARGET_FMT_plx " len:%u val: 0x%" PRIx32
>>>
>>> Format is TARGET_FMT_plx formats a hwaddr, but the parameter type is
>>> uint64_t. They happen to be the same. Is this kosher?
>>>
>>
>> Missed when converting from DPRINTF() to trace-events:
>> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=ec427498;hp=8cb340c613
>>
>> PRIx64 certainly makes sense here.
>>
>> Since it is the single use, once updated we can remote this hunk from
>> scripts/tracetool/format/log_stap.py:
>>
>> if macro == "TARGET_FMT_plx":
>> return "%016x"
>>
>> I guess remember a thread with Thomas talking about TARGET_FMT_plx but I
>> can't find it, maybe I dreamed about it...
>
> That was:
>
> https://patchwork.kernel.org/patch/10930327/
>
> ... I think we should rename it to HWADDR_PRI0x or so.
I'd expect "PRI" macros to expand into just a format specifier.
"%016" PRIx64 is a full conversion specification. Something like
HWADDR_FMT0x would work for me.
"Use HWADDR_PRIx and slowly kill TARGET_FMT_plx" (suggested in the
thread you linked) would also work for me.