[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field |
Date: |
Thu, 9 Nov 2017 17:53:46 +1100 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Mon, Nov 06, 2017 at 04:02:16PM -0200, Eduardo Habkost wrote:
> On Tue, Oct 31, 2017 at 03:01:14PM +0100, Igor Mammedov wrote:
> > On Thu, 19 Oct 2017 17:31:51 +1100
> > David Gibson <address@hidden> wrote:
> >
> > > 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 <address@hidden>
> > >
> > > Reviewed-by: David Gibson <address@hidden>
> > >
> > > > ---
> > > > 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.
> > I'd say arch_id is machine specific so far, it was introduced when we
> > didn't have CpuInstanceProperties and at that time we considered only
> > vcpus (threads) and doesn't really apply to spapr cores.
> >
> > In general we could do away with arch_id and use CpuInstanceProperties
> > instead, but arch_id also serves aux purpose, it allows machine to
> > pre-calculate(cache) apic-id/mpidr values in one place and then they
> > are/(could be) used by arch in-depended code to build acpi tables.
> > So if we drop arch_id we would need to introduce a machine hook,
> > which would translate CpuInstanceProperties into current arch_id.
>
> I think we need to do a better to job documenting where exactly
> we expect arch_id to be used and how, so people know what it's
> supposed to return.
The trouble with this is I think it's impossible - it doesn't have a
well defined meaning.
> If the only place where it's useful now is ACPI code (is it?),
> should we rename it to something like get_acpi_id()?
--
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
signature.asc
Description: PGP signature
- Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field, Eduardo Habkost, 2017/11/06
- Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field, Cornelia Huck, 2017/11/07
- Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field, David Gibson, 2017/11/09
- Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field, Eduardo Habkost, 2017/11/09
- Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field, Cornelia Huck, 2017/11/10
- Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field, David Hildenbrand, 2017/11/10
- Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field, Eduardo Habkost, 2017/11/10
- Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field, David Hildenbrand, 2017/11/10
- Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field, Igor Mammedov, 2017/11/21
Re: [Qemu-devel] [RFC v2 3/6] possible_cpus: add CPUArchId::type field,
David Gibson <=