Re: [Qemu-devel] [Qemu-ppc] [PATCH 08/12] target-ppc: Convert ppcemb_tlb_t to use fixed 64-bit RPN

[David Gibson]

On Tue, Nov 20, 2012 at 10:55:50AM +0100, Alexander Graf wrote:
> 
> On 20.11.2012, at 10:53, Peter Maydell wrote:
> 
> > On 20 November 2012 09:29, Alexander Graf <address@hidden> wrote:
> >> On 19.11.2012, at 23:48, David Gibson wrote:
> >>> On Mon, Nov 19, 2012 at 05:26:45PM +0100, Alexander Graf wrote:
> >>>> On 13.11.2012, at 03:46, David Gibson wrote:
> >>>>> This patch therefore changes ppcemb_tlb_t to use a fixed 64-bit integer
> >>>>> which we know is sufficient for all the machines which use this 
> >>>>> structure.
> >>>> 
> >>>> hwaddr is always defined to 64bit by now.
> >>> 
> >>> I know, but there aren't state save helpers for hwaddr, and there are
> >>> objections to creating them.
> > 
> > (previous discussion on this point:
> > https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg01456.html )
> > 
> >> Sure, but you can just use the 64bit save helpers now that hwaddr == 
> >> uint64_t, no?
> > 
> > That would be one approach. I'm a bit sceptical about putting hwaddr
> > fields in CPU state, though -- it's suggestive that something's
> > not modelled right. hwaddr is conceptually "big enough for the
> > biggest bus in the system", and no single component should have
> > internal state whose size depends on that.

Right, that's the reason I was given for not adding VMSTATE helpers
for hwaddr too.

But more directly, as long as hwaddr is a different type from
uint64_t, to me that at least admits the possibility that it could be
changed again some day.  And if we're using a uint64_t based VMSTATE
helper on a type that could change, that could go badly wrong.
Basically it's a subtle and ungreppable dependency on the fact that a
hwaddr is actually a uint64_t, which seems like a bad idea.

> *shrug* I'm more than happy to get a patch that just converts all
> *the hwaddr fields in CPUState to uint64_t.

So.. does that mean you'll apply this one or not?

-- 
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