Re: [PATCH] return default values for apic probe functions.

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

 



> Even on sunny days, this collides with QEMU commit #7048. :)
>
> Does Intel specify what non-existent MSRs should return, ie. is your
> version still correct if !s->apicbase means that there is actually no
> APIC? And does kvm depend on the default base? If so, I would say:
> provide a patch against upstream.

hummm, I missed this one going in.
But sadly, your patch also breaks cpu hotplug. Not a segfault anymore, but the
VM will freeze instead of shutting down, if we ask too. It does not even respond
to ^C anymore.

By leaving your patch as is, and changing the apic base return to

   return s ? s->apicbase : (0xfee00000 | MSR_IA32_APICBASE_ENABLE);

fixes the issue.

I'm not sure about what the manual says (will check now), but I
believe if we ever try to
read from apic, we should return a meaningful value. Can you verify if
this also works for your
test case?

>
>>
>>  void cpu_set_apic_tpr(CPUX86State *env, uint8_t val)
>> @@ -314,7 +319,10 @@ void cpu_set_apic_tpr(CPUX86State *env, uint8_t val)
>>  uint8_t cpu_get_apic_tpr(CPUX86State *env)
>>  {
>>      APICState *s = env->apic_state;
>> -    return s->tpr >> 4;
>> +    if (s)
>> +        return s->tpr >> 4;
>> +    else
>> +        return 0;
>>  }
>>
>>  /* return -1 if no bit is set */
>
> This is already upstream.

Yeah, and the rest of your patch is totally ok for me.


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."
--
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

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux