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?
In those cases, I don't see what's the benefit of doing something
different from what the user explicitly asked for (considering
that it would require adding more compat code to keep the pc-2.7
behavior).
>
> > + /* 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
> compatibility.
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?).
> You're not considering adding a new one when cpuid[8].ebx is
> invented are you?
I'm not. Those were added only to allow us to implement pc-2.7
compatibility.
--
Eduardo
