Re: [Qemu-devel] [PATCH for-1.4 00/12] target-i386: Fix APIC-ID-based topology (v4)

[Eduardo Habkost]

On Fri, Jan 18, 2013 at 02:54:41PM +0800, li guang wrote:
> 在 2013-01-17四的 18:59 -0200，Eduardo Habkost写道：
> > I am hoping to get this bug fixed in 1.4. I didn't get much feedback on the 
> > RFC
> > I sent last week, though.
> > 
> > Igor argued that APIC ID should be set by the board and not by the CPU 
> > itself,
> 
> per Intel's SPEC, seems APIC ID really based on design of board.
> (refer to Intel® 64 and IA-32 Architectures
>     Software Developer’s Manual
>                 Volume 3 (3A, 3B & 3C):
>              System Programming Guide
>      chapter 10.4.6)
> but, actually, it maybe meaningless for emulation.
> after go though your patches,
> I can't capture the purpose you do a topology map between 
> APIC ID and cpu_index, (sorry for that)
> can you help to clear that?

See the documents mentioned on PATCH 11/12:

+/* This file implements the APIC-ID-based CPU topology enumeration logic,
+ * documented at the following document:
+ *   Intel® 64 Architecture Processor Topology Enumeration
+ *   
http://software.intel.com/en-us/articles/intel-64-architecture-processor-topology-enumeration/
+ *
+ * This code should be compatible with AMD's "Extended Method" described at:
+ *   AMD CPUID Specification (Publication #25481)
+ *   Section 3: Multiple Core Calcuation
+ * as long as:
+ *  nr_threads is set to 1;
+ *  OFFSET_IDX is assumed to be 0;
+ *  CPUID Fn8000_0008_ECX[ApicIdCoreIdSize[3:0]] is set to apicid_core_width().
+ */

If we don't generate the APIC IDs properly, identification of CPU
sockets/cores/threads is broken.

e.g. today -smp 12,cores=3,threads=2 currently ends up exposing 4 cores on the
first socket, and 2 cores in the second one, because the APIC IDs are generated
sequentially instead of being based on package/core/thread IDs.

> 
> > but I am not doing that because:
> >  - I want to keep the bug fix simple and isolated as we are past soft freeze
> >  - I believe the creator of the CPU object shouldn't be forced to provide 
> > the
> >    APIC ID, so the APIC ID is not unnecessarily exposed on the CPU hotplug
> >    device_add interface in the future
> >  - The APIC ID _is_ set by the CPU itself (because each CPU package may have
> >    multiple core/threads, and each core/thread has a different APIC ID). 
> > What
> >    needs to be provided by the board to the CPU package in the future is the
> >    package ID and the bit width of the core/thread IDs.
> > 
> > Git tree for reference:
> >   git://github.com/ehabkost/qemu-hacks.git apicid-topology.v5
> >   https://github.com/ehabkost/qemu-hacks/tree/apicid-topology.v5
> > 
> > Eduardo Habkost (12):
> >   kvm: Add fake KVM_FEATURE_CLOCKSOURCE_STABLE_BIT for builds withou
> >     KVM
> >   target-i386: Don't set any KVM flag by default if KVM is disabled
> >   pc: Reverse pc_init_pci() compatibility logic
> >   kvm: Create kvm_arch_vcpu_id() function
> >   target-i386: kvm: Set vcpu_id to APIC ID instead of CPU index
> >   fw_cfg: Remove FW_CFG_MAX_CPUS from fw_cfg_init()
> >   target-i386/cpu: Introduce apic_id_for_cpu() function
> >   cpus.h: Make constant smp_cores/smp_threads available on *-user
> >   pc: Set fw_cfg data based on APIC ID calculation
> >   tests: Support target-specific unit tests
> >   target-i386: Topology & APIC ID utility functions
> >   pc: Generate APIC IDs according to CPU topology
> > 
> >  hw/fw_cfg.c            |   1 -
> >  hw/pc.c                |  44 +++++++++++++---
> >  hw/pc_piix.c           |  26 +++++++---
> >  hw/ppc_newworld.c      |   1 +
> >  hw/ppc_oldworld.c      |   1 +
> >  hw/sun4m.c             |   3 ++
> >  hw/sun4u.c             |   1 +
> >  include/sysemu/cpus.h  |   7 +++
> >  include/sysemu/kvm.h   |   4 ++
> >  kvm-all.c              |   2 +-
> >  target-i386/cpu.c      |  52 +++++++++++++++----
> >  target-i386/cpu.h      |   5 +-
> >  target-i386/kvm.c      |   6 +++
> >  target-i386/topology.h | 133 
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  target-ppc/kvm.c       |   5 ++
> >  target-s390x/kvm.c     |   5 ++
> >  tests/.gitignore       |   1 +
> >  tests/Makefile         |  21 +++++++-
> >  tests/test-x86-cpuid.c | 101 +++++++++++++++++++++++++++++++++++++
> >  19 files changed, 391 insertions(+), 28 deletions(-)
> >  create mode 100644 target-i386/topology.h
> >  create mode 100644 tests/test-x86-cpuid.c
> > 
> 
> -- 
> regards!
> li guang
> 
> 

-- 
Eduardo