Re: [Qemu-devel] [PATCH] build: fix target_phys_addr_t to 64-bit

[Anthony Liguori]

On 01/12/2012 04:46 PM, Peter Maydell wrote:
> On 12 January 2012 22:42, Peter Maydell<address@hidden>  wrote:
> > You're the one changing what was previously a known-to-be-32-bit
> > type to one that's much bigger, you get to fix the printing
> > issues.

This code is broken in its current form.  target_phys_addr_t has an unspecified 
width which is why we provide a FMT for it.

Assuming it's 32-bit is just as bad as assuming that all hosts are 64-bit or all 
guests are little endian.  It may have worked up until now for a particular 
device, but it doesn't change the fact that it was wrong.

> ...which isn't to say that I don't think this is a good
> plan (indeed I suspect I'm going to need wider physaddrs
> for Cortex-A15 at some point :-)), just that I think we
> ought to have at least a sketch of how the average device
> for which the offset is going to be<4096,

I think a reasonable thing to do is:

#define PRIp64 "0x%08" PRIx64

s:TARGET_FMT_plx:PRIp64:g

Then in places where desired, you can just use PRIx64 directly to specify a 
custom width.

But that's a touch-everything change that I think should be done in a separate 
series.

> let alone<32bits
> can print messages involving the offsets without it looking
> really ugly in either the code or the output (and that
> where in this patch you're actually touching output formats
> you should use whatever the reasonable-looking thing is
> rather than just something that compiles.)

To print a target_phys_addr_t, you need to use TARGET_FMT_plx.  If code wasn't 
using TARGET_FMT_plx, then it was broken.

If you want something more flexible than TARGET_FMT_plx, I'm supportive of that, 
but that should have been done to begin with instead of making bad assumptions 
about sizeof(target_phys_addr_t).

Regards,

Anthony Liguori

> -- PMM