Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3032 bytes --]

On Wed, Oct 18, 2017 at 01:12:12PM +0200, Igor Mammedov wrote:
> For enabling early cpu to numa node configuration at runtime
> qmp_query_hotpluggable_cpus() should provide a list of available
> cpu slots at early stage, before machine_init() is called and
> the 1st cpu is created, so that mgmt might be able to call it
> and use output to set numa mapping.
> Use MachineClass::possible_cpu_arch_ids() callback to set
> cpu type info, along with the rest of possible cpu properties,
> to let machine define which cpu type* will be used.
> 
> * for SPAPR it will be a spapr core type and for ARM/s390x/x86
>   a respective descendant of CPUClass.
> 
> Move parse_numa_opts() in vl.c after cpu_model is parsed into
> cpu_type so that possible_cpu_arch_ids() would know which
> cpu_type to use during layout initialization.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>   v2:
>      - fix NULL dereference caused by not initialized
>        MachineState::cpu_type at the time parse_numa_opts()
>        were called
> ---
>  include/hw/boards.h        |  2 ++
>  hw/arm/virt.c              |  3 ++-
>  hw/core/machine.c          | 12 ++++++------
>  hw/i386/pc.c               |  4 +++-
>  hw/ppc/spapr.c             | 13 ++++++++-----
>  hw/s390x/s390-virtio-ccw.c |  1 +
>  vl.c                       |  3 +--
>  7 files changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 191a5b3..fa21758 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -80,6 +80,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
>   * CPUArchId:
>   * @arch_id - architecture-dependent CPU ID of present or possible CPU

I know this isn't really in scope for this patch, but is @arch_id here
supposed to have meaning defined by the target, or by the machine?

If it's the machime, it could do with a rename - "arch" means target
to most people (thanks to Linux).

If it's the target, it's kind of bogus, because it doesn't necessarily
have a clear meaning per target - get_arch_id in CPUClass has the same
problem, which is probably one reason it's basically only used by the
x86 code at present.

e.g. for target/ppc, what do we use?  There's the PIR, which is in the
CPU.. but only on some cpu models, not all.  There will generally be
some kind of master PIC id, but there are different PIC models on
different boards.  What goes in the devicetree?  Well only some
machines use devicetree, and they might define the cpu reg 
differently.

Board designs will generally try to make some if not all of those
possible values equal for simplicity, but there's still no real way of
defining a sensible arch_id independent of machine / board.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]