Glauber Costa wrote: > On Fri, Apr 17, 2009 at 10:22 AM, Glauber Costa <glommer@xxxxxxxxx> wrote: >>> 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. >> > After reading the manual, my understanding is that only the flag must > be set. I tried, > and in fact: > > return s ? s->apicbase : MSR_IA32_APICBASE_ENABLE; > > still fixes it. > If it works for you, I believe this is a good solution, and will send > a descriptive patch. > If we ever read the apic as disabled, we will have problems enabling > it again. So for > my test case, kvm should never see a disabled apic. > > For yours, you'll still see the apic base address as zero. > > what do you think? > My use case (you can try it yourself: -M isapc) will likely already be fine if there is no segv on accidentally reading that msr. I was more concerned about the correct value according to the spec - if that case isn't simply "undefined". On the other hand, I didn't get the precise race yet, and my feeling about this patch as a fix for something else than an inexistent apic is not that good. But it's just a feeling. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux -- 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
