On 15.08.2013, at 16:47, Andreas Färber wrote: > Am 15.08.2013 15:55, schrieb Alexey Kardashevskiy: >> On 08/15/2013 09:48 PM, 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. :/ >>> >>> We do have the following: >>> >>> "object" >>> +- "device" >>> +- "cpu" >>> +- "powerpc64-cpu" >>> +- "POWER7-family-powerpc64-cpu" -> POWERPC_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. >>> >>> 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. >>> >>> 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. >>> >>>> 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. 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... :( >> >> >> I am not trying to add any additional POWER7. We do not have POWER7 in QEMU >> at all, just some approaching/approximation (whatever word suits, sorry for >> my weak, terrible english). POWER7 (forget about POWER7+ and others) with >> PVR=0x003FAABB would still be absolutely valid POWER7 everywhere but QEMU >> (until we support the exact PVR with the specific patch which would add >> _anything_ new but just definition). Sorry for my deep ignorance if I miss >> the point. Thank you. > > There is nothing wrong with finding a mask or wildcard solution to that > problem, I already indicated so on the original POWER+ patch. The point > of the whole discussion is how to get there in the least invasive way. > Not whether, just how. > > I think - unlike Alex apparently - that the least invasive way is to > leave models as they are and to add masking support to families and KVM > code only.
Not sure I understand. What is KVM specific about this? > I'm already trying to get away from extending the > POWERPC_DEF* macros for Prerna's fw_name, which are starting to get a > big conflict point these days and a future pain if everyone extends them > for the feature of the day. Note that I started with reading v3, not > everything from the start, and am therefore not pointing fingers at > anyone. It may be that you were given some unfortunate suggestions and > too quick in implementing them. > > When we instantiate a -cpu POWER9 then having one POWER9_vX.Y around to > back it doesn't really hurt. Unlike ARM's MIDR there doesn't seem to be > an encoding of IBM vendor or POWER family in the PVR. The macros and > their new implementation are not the way they are because I consider > them the nicest thing in the world but because the name+pvr+svr+family > combination made them work for the whole zoo of models we carry around > and started to give us some inheritance through QOM. Making the POWER7 > family non-abstract would require the same kind of macro "overloading" > for POWERPC_FAMILY that I'm trying to contain for POWERPC_DEF ATM. So > what I am still thinking about is how to handle there being multiple > matches for a PVR - I am considering putting them into a list and > comparing values for closest match. So that if you have a v2.4 and QEMU > knows v2.1 and v2.3 we take v2.3 and fill in the v2.4 PVR. I think this goes into the wrong direction. We should have one single unified scheme to model core versions and -cpu host should be able to override them for a family, no? I don't see how instantiating a POWER7_v20 object on a POWER7_v23 system is any improvement over instantiating a POWER7 object. Alex
