On Fri, 25 Apr 2025 at 22:03, Daniel Henrique Barboza
<dbarb...@ventanamicro.com> wrote:
>
>
>
> On 4/24/25 6:41 AM, Joel Stanley wrote:
> > On Wed, 23 Apr 2025 at 20:39, Daniel Henrique Barboza
> > <dbarb...@ventanamicro.com> wrote:
> >>
> >> We can avoid the 'long' casts by using PRIx64 and HWADDR_PRIx on the fmt
> >> strings for uint64_t and hwaddr types.
> >>
> >> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
> >> ---
> >> hw/riscv/virt.c | 29 +++++++++++++++--------------
> >> 1 file changed, 15 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> >> index 036a0a9bfb..075c035f25 100644
> >> --- a/hw/riscv/virt.c
> >> +++ b/hw/riscv/virt.c
> >> @@ -309,7 +309,7 @@ static void create_fdt_socket_memory(RISCVVirtState
> >> *s, int socket)
> >>
> >> addr = s->memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms,
> >> socket);
> >> size = riscv_socket_mem_size(ms, socket);
> >> - mem_name = g_strdup_printf("/memory@%lx", (long)addr);
> >> + mem_name = g_strdup_printf("/memory@%"PRIx64, addr);
> >
> > I wondered why this wasn't a HWADDR_PRIx.
> >
> > addr (and NodeInfo::node_mem?) could be a hwaddr? That would make
> > everything more consistent.
>
> I didn't put too much thought about it in this patch. I saw that 'addr' was an
> uint64_t and just matched the format string.
>
> As for whether this 'addr' var and NodeInfo::node_mem could be a hwaddr, at
> first
> glance I don't see why not. There are lots of 'hwaddr' references in the
> memory API
> (memory_region_* functions, address_space* functions, etc) so using hwaddr in
> the
> memory context is valid.
>
> If we want to go further, the patch that introduced hwaddr in QEMU (commit
> a8170e5e)
> states:
>
> ---------
> Rename target_phys_addr_t to hwaddr
>
> target_phys_addr_t is unwieldly, violates the C standard (_t suffixes are
> reserved) and its purpose doesn't match the name (most target_phys_addr_t
> addresses are not target specific). Replace it with a finger-friendly,
> standards conformant hwaddr.
> ---------
>
> So it seems to me that the idea is to have an abstraction of target
> independent
> physical addresses, and memory is a good example of that. I believe we're not
> making full use of hwaddr and overusing uint64_t. Thanks,
Cool. I've got a little series that I'll post. It cleans this one up,
and cleans up some other device tree things.
Cheers,
Joel
