On 21.11.2012, at 02:14, David Gibson wrote: > 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 <ag...@suse.de> 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?
It means I'll wait for one that converts more than just this one field :). According to the above rationale, there shouldn't be any hwaddr fields in CPUState, right? Alex
