On 15.08.2013, at 13:48, Andreas Färber wrote: > Am 15.08.2013 13:03, schrieb Alexander Graf: >> >> On 15.08.2013, at 12:52, Andreas Färber wrote: >> >>> Am 15.08.2013 10:45, schrieb Alexander Graf: >>>> >>>> Yes, I think it makes sense to keep the full PVR around when we want to be >>>> specific. What I'm referring to is class specific logic that can assemble >>>> major/minor numbers from the command line. So >>>> >>>> -cpu POWER7,major=2,minor=0 >>>> >>>> would result in a PVR value that is identical to POWER7_v2.0. The assembly >>>> of this PVR value is class specific, because different classes of CPUs >>>> have different semantics for their major and minor numbers. >>>> >>>> That way in the future we won't have to add any new version specific CPU >>>> types but instead the user can assemble those himself, making everyone's >>>> life a lot easier. >>>> >>>> My point was that if we have that logic, we could at the same place just >>>> say "if my major/minor is 0, default to something reasonable". >>>> >>>> But let's ask Andreas whether he has a better idea here :). >>> >>> If you read the previous discussion on the initial POWER7+ patch, I >>> believe I had proposed major-version / minor-version or so properties at >>> family level, to be able to use different implementations or none at all >>> where we don't see a scheme. >> >> Sounds like a good idea. >> >>> However if we want to use that from -cpu as >>> in your example above, we would have to implement custom parsing code >>> for cpu_model, which I would rather avoid, given we want to replace it >>> with -device in the future. >> >> Can't we make this generic QOM property parsing code? >> >> -cpu POWER7,major-version=2,minor-version=0 >> >> would do >> >> cpu = new POWER7(major-version = 2, minor_version = 0); >> >> and then the POWER7 class can decide what to do with this additional >> information? > > That is "custom parsing code for cpu_model" in target-ppc then. x86 has > its own implementation and so does sparc, both not fully QOM'ified yet, > so there is no one-size-fits-all. > >>> But maybe I didn't fully catch the exact question. :) >>> >>> The custom parenting strikes me as a wrong consequence of us not having >>> fully QOM'ified / cleaned up the family classes yet. We had discussed >>> two ways: Either have, e.g., POWER7+ inherit from POWER7 (which looks >>> like the only reason this is being done here) and/or have, e.g., POWER5+ >>> copy and modify 970fx values via #defines. >> >> IIUC the family parenting is orthogonal to this. Here we're looking at >> having families as classes at all. Currently we don't - we only have >> explicit versioned implementations as classes. > > That's simply not true!!! All is hidden by macros as requested by you - > sounds as if that was a bad idea after all. :/
Maybe. I don't know - it's always a tradeoff of readability on different cases. Maybe we simply have too many CPU models too ;). > > We do have the following: > > "object" > +- "device" > +- "cpu" > +- "powerpc64-cpu" > +- "POWER7-family-powerpc64-cpu" -> POWERPC_FAMILY() Ah, there is the family :). > +- "POWER7_v2.0-powerpc64-cpu" -> POWERPC_DEF_SVR() > +- "host-powerpc64-cpu" (depending on host PVR) > > That's why I was saying: If we need POWER7+-specific family code, we > need to have a POWER7P family and not reuse POWER7 as conveniently done > today. All is there to implement properties or whatever at that level. Ah, so your point is that POWER7+ is-a POWER7 today? That's most likely wrong, yes. Who pulled in that code? > And that's also why trying to do the parent tweaking in > POWERPC_DEF_FAMILY_MEMBER() is bogus. The existing infrastructure just > needs to be used the right way, sigh. That's not what this patch is about. It's about making the family classes instantiable. Could we just remove the abstract = true piece from the POWER7 class and create that one as our generic POWER7 CPU? > And to clean up the aliases business, we should simply move them into > the POWER7_v2.0-powerpc64-cpu level class as an array, I think. That > would greatly simplify -cpu ?, and the alias-to-type lookup would get > faster at the same time since we wouldn't be looking at unavailable > models anymore. Not sure I follow you on this one :). > >> Whether we have >> >> PowerPC >> `- POWER7 >> `- POWER7+ >> `- POWER7+ v1.0 >> >> or >> >> PowerPC >> `- POWER7+ >> `- POWER7+ v1.0 >> >> is a different question I think. > > My question is: Why are you guys trying to create yet another type for > "POWER7" when we already have one. Because we're being dumb ;). > The only plausible-to-me explanation > was that avoidance of separate POWER7P family was the core cause, but > apparently the core problem is that no one except me is actually > grasping the macro'fied code or at least you lost the overview during > your vacation... :( That's the reason, yes :). There's just way too much complexity in this code. The macros were supposed to reduce that, but either they failed or the code is just still too complex despite them. Alex
