Re: [Qemu-devel] [PATCH for-1.4 qom-cpu 8/9 v6] target-i386: Topology & APIC ID utility functions

[Eduardo Habkost]

On Wed, Jan 23, 2013 at 08:49:58PM +0100, Andreas Färber wrote:
> Am 23.01.2013 18:58, schrieb Eduardo Habkost:
> > This introduces utility functions for the APIC ID calculation, based on:
> >   Intel® 64 Architecture Processor Topology Enumeration
> >   
> > http://software.intel.com/en-us/articles/intel-64-architecture-processor-topology-enumeration/
> > 
> > The code should be also 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().
> > 
> > Unit tests included. The code is still not being used anywhere. It will be 
> > used
> > by the the next patch.
> 
> (I would drop this reference to "next patch" when applying.)
> > 
> > Signed-off-by: Eduardo Habkost <address@hidden>
> [...]
> > diff --git a/tests/Makefile b/tests/Makefile
> > index d86e95a..4b98d4f 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -45,6 +45,10 @@ gcov-files-test-aio-$(CONFIG_WIN32) = aio-win32.c
> >  gcov-files-test-aio-$(CONFIG_POSIX) = aio-posix.c
> >  check-unit-y += tests/test-thread-pool$(EXESUF)
> >  gcov-files-test-thread-pool-y = thread-pool.c
> > +check-unit-y += tests/test-x86-cpuid$(EXESUF)
> > +# all code tested by test-x86-cpuid is inside topology.h,
> > +# so add the test file itself to the gcov list
> > +gcov-files-test-x86-cpuid-y = tests/test-x86-cpuid.c
> >  
> >  check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
> >  
> 
> With patch 7/9 dropped I am more comfortable with the test integration.
> 
> I wonder however whether the gcov line is correct - won't this screw up
> the statistics so that it's better to drop that line and to add
> hw/pc_piix.c or target-i386/cpu.c in 9/9 instead? Blue?

I want to make gcov check for coverage only of topology.h (that's where the
tested code lives). Including test-x86-cpuid.c is the closest I could get to
that[1]. Including pc_piix.c or cpu.c would surely screw up the numbers, as the
tests don't cover any of the pc_piix.c or target-i386/cpu.c code.

[1] If I set gcov-files-test-x86-cpuid-y = target-i386/topology.h, I get:

  GTESTER tests/test-x86-cpuid
  Gcov report for target-i386/topology.h:
  target-i386/topology.gcno:cannot open graph file

It looks like the .gcno file generation is per-object-file, not per-source-file
(gcov-files-*-y being a list of .c files confused me). If that's the case, then
the only valid value for gcov-files-test-x86-cpuid-y is really
tests/test-x86-cpuid.c, because all the tested code is being compiled inside
tests/test-x86-cpuid.o.

-- 
Eduardo