Re: [Qemu-devel] [PATCH 4/7] savevm: Add VMSTATE_ helpers for target_phys_addr_t

[David Gibson]

On Thu, Oct 11, 2012 at 04:07:24PM +0100, Peter Maydell wrote:
> On 11 October 2012 02:57, David Gibson <address@hidden> wrote:
> > Actually, turns out I had another use of these helpers.  That was to
> > store the real page address from the ppcmeb_tlb_t structure.  That
> > structure is used to represent TLB entries on a number of different
> > embedded chips, which don't all have the same physical bus width.  So
> > target_phys_addr_t does seem like the correct type there.
> 
> > Obviously I could change the type to a fixed uint64_t, but I'm not
> > sure if that's a better idea than bringing in the VMSTATE_TPA
> > helpers.  Advice?
> 
> PPC has had 64 bit target_phys_addr_t even before the recent change.
> Either these various CPUs all actually have physical hardware
> TLB configs which hold the full 64 bits (use uint64_t) or they
> have physical configs which vary between them (in which case
> you're presumably mismodelling some of them) and you need to
> use different types or alternatively always use uint64_t and
> do masking on writes to the fields or something.

Masking will be done in the handling of the instructions that access
the TLB.  They do have different physical configs, but they also have
different access methods, the data type as it is is sufficient for
both of them.

> If you're happy to continue to bake the assumption into your
> code that target_phys_addr_t is 64 bits, you can just use
> VMSTATE_UINT64 which (I think) should work OK when t_p_a_t
> really is 64 bits, and will give a helpful compile failure
> if the assumption is ever untrue...
> 
> Basically I think that to the extent that we contemplate
> t_p_a_t as distinct from "64 bit uint" it's a bad idea to put
> it into vmstate because it means potential migration
> compatibility breaks if it changes.

Hm, yeah, that I'll grant you.  Ok, I'll remove it.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson