Re: [Qemu-devel] [PATCH 3/3] target-i386: replace cpuid_*features fields with a feature word array

[Andreas Färber]

Am 14.12.2012 13:27, schrieb Eduardo Habkost:
> On Fri, Dec 14, 2012 at 10:38:50AM +0100, Igor Mammedov wrote:
>> On Wed, 12 Dec 2012 20:22:26 -0200
>> Eduardo Habkost <address@hidden> wrote:
>>
>>> This replaces the feature-bit fields on both X86CPU and x86_def_t
>>> structs with an array.
>>>
>>> With this, we will be able to simplify code that simply does the same
>>> operation on all feature words (e.g. kvm_check_features_against_host(),
>>> filter_features_for_kvm(), add_flagname_to_bitmaps(), and CPU
>>> feature-bit property lookup/registration).
>>>
>>
>> do you have a patch that simplifies kvm_check_features_against_host() using
>> this?
> 
> I have a very old one, based on an older (and more complex) version of
> this series:
> https://github.com/ehabkost/qemu-hacks/commit/eb01d374baecf6df26fd6f0d0bb23f2e1547f499
> 
> It's in the work/cpuid-refactor-v0.22-2012-08-31 branch in my git
> repository.
> 
> That branch also has some patches to merge kvm_check_features_against_host()
> and filter_features_for_kvm() (because the purpose of
> kvm_check_features_against_host() is simply to check if anything is
> going to be filtered out by filter_features_for_kvm()).
> 
> If people are happy with the approach in this series, I plan to write
> and submit cleanups for kvm_cpu_fill_host(),
> kvm_check_features_against_host(), filter_features_for_kvm(),
> add_flagname_to_bitmaps(), and the cpudef -> CPU feature copying code.
> 
> There's so much code that could be cleaned up using the array, that I am
> afraid that it would cause too much conflicts in the CPU properties
> work. So I can wait until the CPU properties series are submitted before
> making the cleanups, if necessary.

As stated elsewhere, for me a proper way to define new CPU models has
higher priority than feature properties, especially now that we're
headed for a class_init based approach where properties cannot be used
for original initialization.

As suggested with my RFC I would like to take a quick, simplistic route
to subclasses where no major refactoring of data fields happens. If we
prepend a couple coding style and function movement patches, that's fine
with me. But if we do large functional refactorings > 10 patches that
change history will be clobbered by the touch-all conversion and we need
to do intensive functional testing, for which I don't see sufficient
time before the Soft Freeze, given the holidays.

When the feature bits are stored in the classes, setting them via
properties in instance_init seems like overkill (visitor overhead); we
no longer dump them in -cpu ?foo; only +feature/-feature would benefit.
Thus I assume them to mainly conflict where function signatures change
and trivially where def changes to env, making them rather orthogonal to
each other.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg