Hi Bharata, Am 22.02.2016 um 06:01 schrieb Bharata B Rao: > This is an attempt to implement David Gibson's RFC that was posted at > https://lists.gnu.org/archive/html/qemu-ppc/2016-02/msg00000.html > I am not sure if I have followed all the aspects of the RFC fully, but we > can make changes going forward.
I am not familiar with David's RFC beyond what was portrayed on the KVM call - this is not what we discussed on the call and I don't like it. Further, your commits are pretty cryptic to me. Please improve your commit messages. For example, you add a cpu_type field and you assign it the value TYPE_POWERPC_CPU. That's not the user-chosen CPU type then, it's a base CPU type that cannot be instantiated. Either name it cpu_base_type or fill it in with proper values in one patch - that patch on its own does not create value and does not explain your claim: "Storing CPU typename in MachineState lets us to create CPU threads for all architectures in uniform manner from arch-neutral code." I'm pretty sure that CPU threads cannot be created from that type, as it would run into an assertion. Next, you make a functionally correct refactoring of cpu_generic_init(), but I don't understand why you duplicate that code. cpu_foo_init() still expects things to be realized, so instead of realizing once in a central place you do it in nine different places. Had you touched all helper functions we might be able to move that to three places, once for softmmu, once or twice for linux-user and once for bsd-user. But I rather get the feeling that you misunderstand those legacy helper functions, they're for -cpu handling and not to my knowledge used for cpu-add at all. You should not be using them and then won't need to touch them in this way. By using them in your supposedly QOM code you are hiding an object_new() call inside deep layers of helper functions instead of using QOM native functions such as object_initialize(), object_new() and object_property_set*(). Is "CPU package" some IBM sPAPR term? It is new to me and does not match -smp precedence, so I really don't think we should be forcing that term on all architectures for no good reason. Regards, Andreas -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)