qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v6 03/16] target-i386: Add cpu object access rou


From: Don Slutz
Subject: Re: [Qemu-devel] [PATCH v6 03/16] target-i386: Add cpu object access routines for Hypervisor level.
Date: Thu, 11 Oct 2012 18:20:01 -0400
User-agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:15.0) Gecko/20120907 Thunderbird/15.0.1

On 10/10/12 11:40, Andreas Färber wrote:
Am 10.10.2012 17:22, schrieb Don Slutz:
On 10/09/12 15:13, Don Slutz wrote:
On 10/09/12 12:25, Marcelo Tosatti wrote:
On Mon, Sep 24, 2012 at 10:32:05AM -0400, Don Slutz wrote:
+static void x86_cpuid_set_hv_level(Object *obj, Visitor *v, void
*opaque,
+                                const char *name, Error **errp)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    uint32_t value;
+
+    visit_type_uint32(v, &value, name, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (value != 0 && value < 0x40000000) {
+        value += 0x40000000;
+    }
Whats the purpose of this? Adds ambiguity.
[...]
This is direct copy with adjustment from x86_cpuid_set_xlevel():

     if (value < 0x80000000) {
         value += 0x80000000;
     }

(Pending patch:
http://comments.gmane.org/gmane.comp.emulators.qemu/172703 adds this)
(Any pending patch can be changed ;))

The adjustment is that 0 is a legal value. See
http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html

This does mean that just like xlevel=1 and xlevel=0x80000001 are the
same; hypervisor-level=1 and hypervisor-level=0x4000001 are the same.
If this is not wanted, I have no issue with removing it.
I have no strong opinion either way, but if there's only one call site,
I'd prefer to apply these fixups to user input before setting the
property and to have the property setter error out on invalid values. I
consider that cleaner than silently fixing up values inside the setter.

Regards,
Andreas

I find more then one call site. And one of them is converting the predefined x86 cpus (like 486). So I am not planning on a change.

I have finished up the v7 changes except for this. I will wait until some time tomorrow to send it in case there is more on this topic.
    -Don Slutz





reply via email to

[Prev in Thread] Current Thread [Next in Thread]