On 09/21/2016 01:14 PM, Eduardo Habkost wrote:
On Wed, Sep 21, 2016 at 12:53:08PM -0700, Richard Henderson wrote:
On 09/21/2016 11:26 AM, Eduardo Habkost wrote:
+    /* Set cpuid_*level* based on cpuid_min_*level, if not explicitly set */
+    if (!env->cpuid_level) {
+        env->cpuid_level = env->cpuid_min_level;
+    }
+    if (!env->cpuid_xlevel) {
+        env->cpuid_xlevel = env->cpuid_min_xlevel;
+    }
+    if (!env->cpuid_xlevel2) {
+        env->cpuid_xlevel2 = env->cpuid_min_xlevel2;

Why are we not bounding them by MIN, if it's really a minimum?

Not sure I understand what you mean. Do you mean silently
changing the value even if it was explicitly set by the user?

You're changing it if the user explicitly set the level to 0, aren't you?

Or is that merely an oversight and you really need the levels defaulted to some magic value like -1?

+    /* Enable auto level-increase for CPUID[7].ECX features */
+    bool cpuid_auto_level_7_0_ecx;
+    /* Enable auto level-increase for CPUID[6] features */
+    bool cpuid_auto_level_6;

Why two variables?  Seems like only one is needed for backward

It's true that we don't really need two variables to implement
pc-2.7 compatibility. I just implemented it this way because
having two separate variables looks clearer to me. I wouldn't
know how I would name the variable if it controlled both
CPUID[7].ECX and CPUID[6] at the same time (any suggestions?).



Reply via email to