On Fri, Mar 4, 2016 at 1:00 AM, Thomas Huth <thuth@xxxxxxxxxx> wrote:
> On 04.03.2016 09:49, Paolo Bonzini wrote:
>>
>>
>> On 04/03/2016 09:47, Thomas Huth wrote:
>>>>> asm volatile("mov %%dr7,%0" : "=r" (dr7));
>>>>> debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
>>>>> /* Commented out: KVM does not support DEBUGCTL so far */
>>>>> + assert(debugctl == debugctl);
>>>>> report("Guest=host debug controls", dr7 == 0x402 /* && debugctl == 0x1 */);
>>> Now these assert()s are really ugly. Wouldn't it be better do comment
>>> out the "debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);" line instead?
>>
>> This also looks better than the asserts:
>>
>> (void)debugctl;
>>
>> Thomas, if you're okay with it I can do the change locally.
>
> I personally would rather prefer to put comments around "debugctl =",
> but your suggestion looks at least better to me than the solution with
> assert() ... (since assert() can also be #undef in normal C projects, so
> IMHO it should not be used for something that has influence on the
> behavior of the normal C code).
I wasn't too happy with the asserts either. I didn't just comment it
all out because I wanted to leave in the rdmsr in case there were side
effects (which there shouldn't be, but that's the point of testing!).
I'm fine with the (void) as well. Paolo, I'll assume you're going to
massage the patch unless you say otherwise.
Thanks,
Peter
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
