Re: [Qemu-devel] [PATCH] qemu: x86: report lapic version as 0x14 instead of 0x11
On Sat, Mar 01, 2014 at 07:17:07PM -0500, Gabriel L. Somlo wrote: On Sat, Mar 01, 2014 at 03:46:22PM +0100, Paolo Bonzini wrote: Il 28/02/2014 20:14, Gabriel L. Somlo ha scritto: Some guests (e.g. 0S X) insist on a minimum lapic version of 0x14. This patch bumps the emulated lapic version to 0x14 to accomodate that. Signed-off-by: Gabriel L. Somlo so...@cmu.edu Looks good, but you have to make this a qdev property so that older versions keep using version 0x11. After digging around a bit, I think adding a uint8_t lapic_version member to to struct x86_def_t might be even better, as the version is pretty much a property of the on-chip local APIC, not necessarily something best left up to the user... It could default to 0x11 for anything purely virtual (e.g. qemu64, kvm64, etc.) and to the appropriate value for chips with a real-world physical correspondent (core2duo, haswell, westmere, etc) Although, on KVM, it's simply hardcoded to 0x14 rather than exposing to the guest whatever the host CPU's apic version happens to be, or trying to match it to the CPU model: [somlo@foober kvm]$ grep -i version arch/x86/kvm/lapic.c ... /* 14 is the version for Xeon and Pentium 8.4.8*/ #define APIC_VERSION(0x14UL | ((APIC_LVT_NUM - 1) 16)) ... I'd honestly prefer to stick to 0x14 (because it's simple :) ) but if you're sure that's a bad idea, how about the struct x86_def_t rather than qdev ? So far, only OS X seems to even care at all about the version... Thanks, --Gabriel No, your patch is fine generally, it's not about users tweaking the version. It's about users getting compatible behaviour when they specify e.g. -M pc-1.6 Using property here is an implementation detail not exposed to users. Look for all the PC_COMPAT macros as an example of that. -- MST
Re: [Qemu-devel] [PATCH] qemu: x86: report lapic version as 0x14 instead of 0x11
Il 02/03/2014 01:17, Gabriel L. Somlo ha scritto: Although, on KVM, it's simply hardcoded to 0x14 rather than exposing to the guest whatever the host CPU's apic version happens to be, or trying to match it to the CPU model: [somlo@foober kvm]$ grep -i version arch/x86/kvm/lapic.c ... /* 14 is the version for Xeon and Pentium 8.4.8*/ #define APIC_VERSION(0x14UL | ((APIC_LVT_NUM - 1) 16)) ... I'd honestly prefer to stick to 0x14 (because it's simple :) ) I'd also prefer that, because I like having the same for KVM and TCG, but I'm not sure it'd fly with others. :) but if you're sure that's a bad idea, how about the struct x86_def_t rather than qdev ? So far, only OS X seems to even care at all about the version... Andreas, Michael, what do you think? Paolo
Re: [Qemu-devel] [PATCH] qemu: x86: report lapic version as 0x14 instead of 0x11
On Sun, Mar 02, 2014 at 09:56:50AM +0100, Paolo Bonzini wrote:
Il 02/03/2014 01:17, Gabriel L. Somlo ha scritto:
Although, on KVM, it's simply hardcoded to 0x14 rather than exposing to
the guest whatever the host CPU's apic version happens to be, or
trying to match it to the CPU model:
[somlo@foober kvm]$ grep -i version arch/x86/kvm/lapic.c
...
/* 14 is the version for Xeon and Pentium 8.4.8*/
#define APIC_VERSION(0x14UL | ((APIC_LVT_NUM - 1) 16))
...
I'd honestly prefer to stick to 0x14 (because it's simple :) )
I'd also prefer that, because I like having the same for KVM and
TCG, but I'm not sure it'd fly with others. :)
but if you're sure that's a bad idea, how about the struct x86_def_t
rather than qdev ?
So far, only OS X seems to even care at all about the version...
Andreas, Michael, what do you think?
Paolo
I note that OSX is not the only one that cares,
Linux does this:
static int modern_apic(void)
{
/* AMD systems use old APIC versions, so check the CPU */
if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD
boot_cpu_data.x86 = 0xf)
return 1;
return lapic_get_version() = 0x14;
}
So I think this commit should include some documentation
analysing the reasons that this is a safe change.
In any case, we really should also use old lapic version for
compat machine types.
--
MST
Re: [Qemu-devel] [PATCH] qemu: x86: report lapic version as 0x14 instead of 0x11
On Sun, Mar 02, 2014 at 04:31:12PM +0200, Michael S. Tsirkin wrote:
On Sun, Mar 02, 2014 at 09:56:50AM +0100, Paolo Bonzini wrote:
Il 02/03/2014 01:17, Gabriel L. Somlo ha scritto:
Although, on KVM, it's simply hardcoded to 0x14 rather than exposing to
the guest whatever the host CPU's apic version happens to be, or
trying to match it to the CPU model:
[somlo@foober kvm]$ grep -i version arch/x86/kvm/lapic.c
...
/* 14 is the version for Xeon and Pentium 8.4.8*/
#define APIC_VERSION(0x14UL | ((APIC_LVT_NUM - 1)
16))
...
I'd honestly prefer to stick to 0x14 (because it's simple :) )
I'd also prefer that, because I like having the same for KVM and
TCG, but I'm not sure it'd fly with others. :)
but if you're sure that's a bad idea, how about the struct x86_def_t
rather than qdev ?
So far, only OS X seems to even care at all about the version...
Andreas, Michael, what do you think?
Paolo
I note that OSX is not the only one that cares,
Linux does this:
static int modern_apic(void)
{
/* AMD systems use old APIC versions, so check the CPU */
if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD
boot_cpu_data.x86 = 0xf)
return 1;
return lapic_get_version() = 0x14;
}
So I think this commit should include some documentation
analysing the reasons that this is a safe change.
Also
arch/x86/include/asm/apicdef.h:#define APIC_XAPIC(x) ((x) = 0x14)
In any case, we really should also use old lapic version for
compat machine types.
--
MST
Re: [Qemu-devel] [PATCH] qemu: x86: report lapic version as 0x14 instead of 0x11
Il 28/02/2014 20:14, Gabriel L. Somlo ha scritto: Some guests (e.g. 0S X) insist on a minimum lapic version of 0x14. This patch bumps the emulated lapic version to 0x14 to accomodate that. Signed-off-by: Gabriel L. Somlo so...@cmu.edu --- Along with the TCG ioapic polarity fix, this allows me to boot OS X without KVM acceleration. I dug around the Intel docs and searched the Web, and there was nothing there to indicate any difference in functionality between lapic versions 0x11 and 0x14. It appears to me that lapic version is loosely correlated with the generation of the CPU it's attached to, and Apple simply decided to include an extra check that basically says we don't support CPUs older than foo, where the oldest foo they support ships with lapics that were versioned to 0x14 :) For example: http://www.opensource.apple.com/source/xnu/xnu-1456.1.26/osfmk/i386/lapic.c Let me know what you think. Thanks, Gabriel hw/intc/apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/intc/apic.c b/hw/intc/apic.c index 361ae90..67365b7 100644 --- a/hw/intc/apic.c +++ b/hw/intc/apic.c @@ -675,7 +675,7 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr) val = s-id 24; break; case 0x03: /* version */ -val = 0x11 | ((APIC_LVT_NB - 1) 16); /* version 0x11 */ +val = 0x14 | ((APIC_LVT_NB - 1) 16); /* version 0x14 */ break; case 0x08: apic_sync_vapic(s, SYNC_FROM_VAPIC); Looks good, but you have to make this a qdev property so that older versions keep using version 0x11. Paolo
Re: [Qemu-devel] [PATCH] qemu: x86: report lapic version as 0x14 instead of 0x11
On Sat, Mar 01, 2014 at 03:46:22PM +0100, Paolo Bonzini wrote: Il 28/02/2014 20:14, Gabriel L. Somlo ha scritto: Some guests (e.g. 0S X) insist on a minimum lapic version of 0x14. This patch bumps the emulated lapic version to 0x14 to accomodate that. Signed-off-by: Gabriel L. Somlo so...@cmu.edu Looks good, but you have to make this a qdev property so that older versions keep using version 0x11. After digging around a bit, I think adding a uint8_t lapic_version member to to struct x86_def_t might be even better, as the version is pretty much a property of the on-chip local APIC, not necessarily something best left up to the user... It could default to 0x11 for anything purely virtual (e.g. qemu64, kvm64, etc.) and to the appropriate value for chips with a real-world physical correspondent (core2duo, haswell, westmere, etc) Although, on KVM, it's simply hardcoded to 0x14 rather than exposing to the guest whatever the host CPU's apic version happens to be, or trying to match it to the CPU model: [somlo@foober kvm]$ grep -i version arch/x86/kvm/lapic.c ... /* 14 is the version for Xeon and Pentium 8.4.8*/ #define APIC_VERSION(0x14UL | ((APIC_LVT_NUM - 1) 16)) ... I'd honestly prefer to stick to 0x14 (because it's simple :) ) but if you're sure that's a bad idea, how about the struct x86_def_t rather than qdev ? So far, only OS X seems to even care at all about the version... Thanks, --Gabriel
Re: [Qemu-devel] [PATCH] qemu: x86: report lapic version as 0x14 instead of 0x11
Am 01.03.2014 um 03:14 schrieb Gabriel L. Somlo gso...@gmail.com: Some guests (e.g. 0S X) insist on a minimum lapic version of 0x14. This patch bumps the emulated lapic version to 0x14 to accomodate that. Signed-off-by: Gabriel L. Somlo so...@cmu.edu --- Along with the TCG ioapic polarity fix, this allows me to boot OS X without KVM acceleration. I dug around the Intel docs and searched the Web, and there was nothing there to indicate any difference in functionality between lapic versions 0x11 and 0x14. It appears to me that lapic version is loosely correlated with the generation of the CPU it's attached to, and Apple simply decided to include an extra check that basically says we don't support CPUs older than foo, where the oldest foo they support ships with lapics that were versioned to 0x14 :) For example: http://www.opensource.apple.com/source/xnu/xnu-1456.1.26/osfmk/i386/lapic.c Let me know what you think. Thanks, Gabriel hw/intc/apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/intc/apic.c b/hw/intc/apic.c index 361ae90..67365b7 100644 --- a/hw/intc/apic.c +++ b/hw/intc/apic.c @@ -675,7 +675,7 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr) val = s-id 24; break; case 0x03: /* version */ -val = 0x11 | ((APIC_LVT_NB - 1) 16); /* version 0x11 */ +val = 0x14 | ((APIC_LVT_NB - 1) 16); /* version 0x14 */ Deja vu :). Should we really set this to thd least compatible version or rather to a current one that resembles roughly what we support? Otherwise patches like this will come up for every new osx release. What is the version in Haswell? Alex break; case 0x08: apic_sync_vapic(s, SYNC_FROM_VAPIC); -- 1.8.1.4
Re: [Qemu-devel] [PATCH] qemu: x86: report lapic version as 0x14 instead of 0x11
On Sat, Mar 01, 2014 at 11:44:33AM +0800, Alexander Graf wrote: Deja vu :). Should we really set this to thd least compatible version or rather to a current one that resembles roughly what we support? Otherwise patches like this will come up for every new osx release. What is the version in Haswell? I don't know specifically about Haswell, but OS X has been checking for lapic_version = 14 ever since 10.5, same check, same number. The latest Intel manual I could find (Feb.2014) says (Vol 3A, page 10-11) that the version is contained in bits 0-8 of the version register, and that values are 0x00 for the 82489 discrete apic, and 0x10 - 0x15 for integrated apic, all other values are reserved. So I guess we could make it 0x15 and be done with it for a (hopefully) long time :) --Gabriel
Re: [Qemu-devel] [PATCH] qemu: x86: report lapic version as 0x14 instead of 0x11
Gabriel L. Somlo wrote: On Sat, Mar 01, 2014 at 11:44:33AM +0800, Alexander Graf wrote: Deja vu :). Should we really set this to thd least compatible version or rather to a current one that resembles roughly what we support? Otherwise patches like this will come up for every new osx release. What is the version in Haswell? I don't know specifically about Haswell, but OS X has been checking for lapic_version = 14 ever since 10.5, same check, same number. The latest Intel manual I could find (Feb.2014) says (Vol 3A, page 10-11) that the version is contained in bits 0-8 of the version register, and that values are 0x00 for the 82489 discrete apic, and 0x10 - 0x15 for integrated apic, all other values are reserved. So I guess we could make it 0x15 and be done with it for a (hopefully) long time :) Sounds good to me :) Alex
