[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] return default values for apic probe functions.
From: |
Glauber Costa |
Subject: |
[Qemu-devel] Re: [PATCH] return default values for apic probe functions. |
Date: |
Fri, 17 Apr 2009 10:22:14 -0300 |
> 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."