Re: [PATCH] Add cpu model configuration support.. (resend)

john cooper
2010-02-02T19:47:29Z

Andre Przywara wrote:

>> +[cpudef]
>> + name = "Conroe"
>> + level = "2"
>> + vendor = "GenuineIntel"
>> + family = "6"
>> + model = "2"
>> + stepping = "3"
>> + feature_edx = "sse2 sse fxsr mmx pat cmov pge sep apic cx8 mce pae
>> msr tsc pse de fpu mtrr clflush mca pse36"
>> + feature_ecx = "sse3 ssse3"
>> + extfeature_edx = "fxsr mmx pat cmov pge apic cx8 mce pae msr tsc
>> pse de fpu lm syscall nx"
>> + extfeature_ecx = "lahf_lm"
> Wouldn't it be much more user friendly to merge them all into one
> string? Just from the feature names it is quite obscure to guess which
> flag belongs into which string (especially since we lack the EXTn_
> prefix we had in helper.c). I haven't tried it, but the parsing code
> looks like this shouldn't be too hard.
> To avoid overlong lines one could think about a += operator.

That's true. Although I expect setup of a cpu model to
be a rather infrequent occurrence by the expert (+/-)
user so the above didn't strike me as a significant issue.
Also "-cpu ?cpuid" dumps out the entire motley crew of
flags relative to each grouping for reference.

That said the current config file syntax seems rather
rigid and I think your suggestion makes sense. I avoided
modifying the parser at this point just in the interest of
minimizing the sprawl of this patch.

> I would just drop all definitions here except qemu{32,64} and
> kvm{32,64}. The other models should be described in the config file.

That's the goal but I wanted to leave an interim firewall
of sorts. If the target-x86_64.conf isn't installed for
whatever reason, qemu still can fall back to the internal
definitions. Even here it isn't strictly necessary to
remove an internal def as it can be redefined in the
config file which will override the internal version.
In general -cpu "?model" will indicate internal vs.
externally defined models by enclosing internal model names
in brackets:

:
x86 Opteron_G3 AMD Opteron 23xx (Gen 3 Class Opteron)
:
x86 [athlon] QEMU Virtual CPU version 0.12.50
:

It also seems worth dropping a hint to the user in the case qemu
fails to find a target config file rather than leaving them to
puzzle out why an external model has gone missing.

Thanks for the feedback.

-john