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