Re: [Qemu-devel] [PATCH 11/20] target-i386: do not set vendor_override in x86_cpuid_set_vendor()

[Andreas Färber]

Am 19.12.2012 18:38, schrieb Eduardo Habkost:
> On Mon, Dec 17, 2012 at 05:01:23PM +0100, Igor Mammedov wrote:
>> commit d480e1af which introduced vendor property was setting
>> env->cpuid_vendor_override = 1, which prevents using vendor property
>> on its own without triggering vendor override.
>> Fix it by removing setting cpuid_vendor_override in x86_cpuid_set_vendor()
>> to allow to use vendor property in other places that doesn't require
>> cpuid_vendor_override to be set to 1.
> 
> By making "vendor" not force override, you are making "-cpu vendor=xxx"
> behave differently from setting "vendor" using all other interfaces
> (e.g. -device, -global, QMP commands).
> 
> What about taking the opposite approach? Setting "vendor" could always
> force vendor override, but the code that initialize the defaults would
> take care of not overriding the vendor ID if unsafe. e.g.: it could just
> do this:
> 
>  if (!kvm_enabled() || def->vendor_override) {
>    object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
>  } /* else, leave the "vendor" property untouched" */
> 
> (something equivalent could be done inside class_init() when we
> introduce subclasses)
> 
> On all I cases I can think of somebody setting the "vendor" property
> (e.g. using -cpu, QMP, -device, or -global), it means they want vendor
> override (otherwise, what's the point of setting the property?). Setting
> vendor in no-override mode is the special case, not the other way
> around.

+1

I was thinking it might be possible to just reset vendor_override to
false when set internally via property - I didn't get to trying out
whether there is a second place affected though.

Andreas

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