Re: [Qemu-devel] [PATCH] qemu: x86: report lapic version as 0x14 instead of 0x11

[Michael S. Tsirkin]

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 <address@hidden>
> > 
> > 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:
> 
> 
> address@hidden 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