Am 11.01.2013 03:10, schrieb Igor Mammedov: > Igor Mammedov (17): > target-i386: move setting defaults out of cpu_x86_parse_featurestr() > target-i386: cpu_x86_register() consolidate freeing resources > target-i386: move kvm_check_features_against_host() check to realize > time
Thanks, I've applied 1-3 to qom-cpu: https://github.com/afaerber/qemu-cpu/commits/qom-cpu I really like patch 3, which moves more logic into our realizefn! When the QOM realize series gets applied, I have a queue prepared already to change today's signatures and to hook up to the infrastructure and to make this uniform across targets. > target-i386: add x86_cpu_vendor_words2str() Patch 4 v3 looks okay too, but it mentions "next patch" and doesn't provide any value on its own: > target-i386: replace uint32_t vendor fields by vendor string in > x86_def_t I raised some (non-)issues on patch 5 that I would like to give others a chance to review rather than taking the lonely decision of putting this in a PULL tonight; a new idea there would be a union { uint32_t words[3]; char str[12]; } to keep both options open while avoiding the string -> 3x uint32_t -> string mess that patch 5 rightfully attempts to clean up. Once consensus is reached and it is still needed, I would prefer to squash patch 4 as justification, being a rather trivial code movement. > target-i386: remove vendor_override field from CPUX86State > target-i386: prepare cpu_x86_parse_featurestr() to return a set of > key,value property pairs Didn't get around to fully reviewing the remainder of the series yet. However I am still unclear and skeptic towards this patch using a QDict to set properties on the CPU. I was told this was for -feat and +feat backwards compatibility but the properties it is being used for are not features but regular properties that I would like to always set to get errors from each property rather than papering over, e.g., "...,tsc_freq=invalid,tsc_freq=1G,..." (IIUC). By operating on the X86CPU, the intermediate x86_def_t can be spared too, as shown in my subclasses patch. Maybe I'm overlooking something, needs calm review from my side and some trial-and-error. As a reminder, I have been pushing for converting to subclasses early because that is a prerequisite for doing some CPUClass-level changes across targets to clean up the way CPUs get created wrt cpu_init() and cpu_copy(). Feature properties are, as far as I have seen and heard, an x86-only project that could thus well be done on top of that common infrastructure IMO. Regards, Andreas > target-i386: set custom 'vendor' without intermediate x86_def_t > target-i386: print deprecated warning if xlevel < 0x80000000 > target-i386: set custom 'xlevel' without intermediate x86_def_t > target-i386: set custom 'level' without intermediate x86_def_t > target-i386: set custom 'model-id' without intermediate x86_def_t > target-i386: set custom 'stepping' without intermediate x86_def_t > target-i386: set custom 'model' without intermediate x86_def_t > target-i386: set custom 'family' without intermediate x86_def_t > target-i386: set custom 'tsc-frequency' without intermediate > x86_def_t > target-i386: remove setting tsc-frequency from x86_def_t > > target-i386/cpu.c | 314 > ++++++++++++++++++++++------------------------------- > target-i386/cpu.h | 7 +- > 2 files changed, 131 insertions(+), 190 deletions(-) -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg