| Message ID | 1456117285-22273-5-git-send-email-bharata@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Am 22.02.2016 um 06:01 schrieb Bharata B Rao: > sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is > performed in the granularity of CPU core device by setting the "realized" > property of this device to "true". When hotplugged, CPU core creates CPU > thread devices. > > TODO: Right now allows for only homogeneous configurations as we depend > on global smp_threads and machine->cpu_model. > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > --- > hw/ppc/Makefile.objs | 1 + > hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++ > 3 files changed, 77 insertions(+) > create mode 100644 hw/ppc/spapr_cpu_package.c > create mode 100644 include/hw/ppc/spapr_cpu_package.h > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > index c1ffc77..3000982 100644 > --- a/hw/ppc/Makefile.objs > +++ b/hw/ppc/Makefile.objs > @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o > obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o > obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o > obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o > +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) > obj-y += spapr_pci_vfio.o > endif > diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c > new file mode 100644 > index 0000000..3120a16 > --- /dev/null > +++ b/hw/ppc/spapr_cpu_package.c > @@ -0,0 +1,50 @@ > +/* > + * sPAPR CPU package device, acts as container of CPU thread devices. > + * > + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > +#include "hw/cpu/package.h" > +#include "hw/ppc/spapr_cpu_package.h" > +#include "hw/boards.h" > +#include <sysemu/cpus.h> > +#include "qemu/error-report.h" > + > +static void spapr_cpu_package_instance_init(Object *obj) > +{ > + int i; > + CPUState *cpu; > + MachineState *machine = MACHINE(qdev_get_machine()); > + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj); > + > + /* Create as many CPU threads as specified in the topology */ > + for (i = 0; i < smp_threads; i++) { > + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model); No, no, no. This is horribly violating QOM design. Please compare the x86 RFC. Andreas
On Mon, Feb 22, 2016 at 07:44:40AM +0100, Andreas Färber wrote: > Am 22.02.2016 um 06:01 schrieb Bharata B Rao: > > sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is > > performed in the granularity of CPU core device by setting the "realized" > > property of this device to "true". When hotplugged, CPU core creates CPU > > thread devices. > > > > TODO: Right now allows for only homogeneous configurations as we depend > > on global smp_threads and machine->cpu_model. > > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > > --- > > hw/ppc/Makefile.objs | 1 + > > hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++ > > include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++ > > 3 files changed, 77 insertions(+) > > create mode 100644 hw/ppc/spapr_cpu_package.c > > create mode 100644 include/hw/ppc/spapr_cpu_package.h > > > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > > index c1ffc77..3000982 100644 > > --- a/hw/ppc/Makefile.objs > > +++ b/hw/ppc/Makefile.objs > > @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o > > obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o > > obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o > > obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o > > +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o > > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) > > obj-y += spapr_pci_vfio.o > > endif > > diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c > > new file mode 100644 > > index 0000000..3120a16 > > --- /dev/null > > +++ b/hw/ppc/spapr_cpu_package.c > > @@ -0,0 +1,50 @@ > > +/* > > + * sPAPR CPU package device, acts as container of CPU thread devices. > > + * > > + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com> > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > > + * See the COPYING file in the top-level directory. > > + */ > > +#include "hw/cpu/package.h" > > +#include "hw/ppc/spapr_cpu_package.h" > > +#include "hw/boards.h" > > +#include <sysemu/cpus.h> > > +#include "qemu/error-report.h" > > + > > +static void spapr_cpu_package_instance_init(Object *obj) > > +{ > > + int i; > > + CPUState *cpu; > > + MachineState *machine = MACHINE(qdev_get_machine()); > > + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj); > > + > > + /* Create as many CPU threads as specified in the topology */ > > + for (i = 0; i < smp_threads; i++) { > > + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model); > > No, no, no. This is horribly violating QOM design. Ok.. why? There does not, to me, seem to be any remotely easily discoverable means of finding out what QOM design principles are. It also would have been nice if you weighed in on my RFC this code is based on. > Please compare the x86 RFC. Where do I find this? From all I could tell there just seemed to be a lot of spinning of wheels on the cpu hotplug stuff, which is why I made the cpu-packages proposal to try and move things forward. There's been a lot of "don't do it like that" but precious little "here's how you _should_ do it" that actually addresses the needs of the various platforms.
On Mon, Feb 22, 2016 at 07:44:40AM +0100, Andreas Färber wrote: > Am 22.02.2016 um 06:01 schrieb Bharata B Rao: > > sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is > > performed in the granularity of CPU core device by setting the "realized" > > property of this device to "true". When hotplugged, CPU core creates CPU > > thread devices. > > > > TODO: Right now allows for only homogeneous configurations as we depend > > on global smp_threads and machine->cpu_model. > > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > > --- > > hw/ppc/Makefile.objs | 1 + > > hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++ > > include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++ > > 3 files changed, 77 insertions(+) > > create mode 100644 hw/ppc/spapr_cpu_package.c > > create mode 100644 include/hw/ppc/spapr_cpu_package.h > > > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > > index c1ffc77..3000982 100644 > > --- a/hw/ppc/Makefile.objs > > +++ b/hw/ppc/Makefile.objs > > @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o > > obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o > > obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o > > obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o > > +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o > > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) > > obj-y += spapr_pci_vfio.o > > endif > > diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c > > new file mode 100644 > > index 0000000..3120a16 > > --- /dev/null > > +++ b/hw/ppc/spapr_cpu_package.c > > @@ -0,0 +1,50 @@ > > +/* > > + * sPAPR CPU package device, acts as container of CPU thread devices. > > + * > > + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com> > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > > + * See the COPYING file in the top-level directory. > > + */ > > +#include "hw/cpu/package.h" > > +#include "hw/ppc/spapr_cpu_package.h" > > +#include "hw/boards.h" > > +#include <sysemu/cpus.h> > > +#include "qemu/error-report.h" > > + > > +static void spapr_cpu_package_instance_init(Object *obj) > > +{ > > + int i; > > + CPUState *cpu; > > + MachineState *machine = MACHINE(qdev_get_machine()); > > + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj); > > + > > + /* Create as many CPU threads as specified in the topology */ > > + for (i = 0; i < smp_threads; i++) { > > + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model); > > No, no, no. This is horribly violating QOM design. > > Please compare the x86 RFC. Are you referring to the in-place initialization of child objects like that you did in socket based x86 hotplug RFC last year ? Regards, Bharata.
Am 22.02.2016 um 09:05 schrieb Bharata B Rao: > On Mon, Feb 22, 2016 at 07:44:40AM +0100, Andreas Färber wrote: >> Am 22.02.2016 um 06:01 schrieb Bharata B Rao: >>> sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is >>> performed in the granularity of CPU core device by setting the "realized" >>> property of this device to "true". When hotplugged, CPU core creates CPU >>> thread devices. >>> >>> TODO: Right now allows for only homogeneous configurations as we depend >>> on global smp_threads and machine->cpu_model. >>> >>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> >>> --- >>> hw/ppc/Makefile.objs | 1 + >>> hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++ >>> include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++ >>> 3 files changed, 77 insertions(+) >>> create mode 100644 hw/ppc/spapr_cpu_package.c >>> create mode 100644 include/hw/ppc/spapr_cpu_package.h >>> >>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs >>> index c1ffc77..3000982 100644 >>> --- a/hw/ppc/Makefile.objs >>> +++ b/hw/ppc/Makefile.objs >>> @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o >>> obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o >>> obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o >>> obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o >>> +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o >>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) >>> obj-y += spapr_pci_vfio.o >>> endif >>> diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c >>> new file mode 100644 >>> index 0000000..3120a16 >>> --- /dev/null >>> +++ b/hw/ppc/spapr_cpu_package.c >>> @@ -0,0 +1,50 @@ >>> +/* >>> + * sPAPR CPU package device, acts as container of CPU thread devices. >>> + * >>> + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com> >>> + * >>> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >>> + * See the COPYING file in the top-level directory. >>> + */ >>> +#include "hw/cpu/package.h" >>> +#include "hw/ppc/spapr_cpu_package.h" >>> +#include "hw/boards.h" >>> +#include <sysemu/cpus.h> >>> +#include "qemu/error-report.h" >>> + >>> +static void spapr_cpu_package_instance_init(Object *obj) >>> +{ >>> + int i; >>> + CPUState *cpu; >>> + MachineState *machine = MACHINE(qdev_get_machine()); >>> + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj); >>> + >>> + /* Create as many CPU threads as specified in the topology */ >>> + for (i = 0; i < smp_threads; i++) { >>> + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model); >> >> No, no, no. This is horribly violating QOM design. >> >> Please compare the x86 RFC. > > Are you referring to the in-place initialization of child objects like > that you did in socket based x86 hotplug RFC last year ? Yes, in-place object_initialize() is one of two options. The other is using object_initialize() with g_try_malloc0() from property setters, like Alexey did for xics I think. But mainly the idea is to not bend your QOM types to reuse the legacy glue but to use true QOM concepts in the design of those new objects. If there is some big hurdle I'm missing, please say so openly. Regards, Andreas
Am 22.02.2016 um 08:47 schrieb David Gibson: > On Mon, Feb 22, 2016 at 07:44:40AM +0100, Andreas Färber wrote: >> Am 22.02.2016 um 06:01 schrieb Bharata B Rao: >>> sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is >>> performed in the granularity of CPU core device by setting the "realized" >>> property of this device to "true". When hotplugged, CPU core creates CPU >>> thread devices. >>> >>> TODO: Right now allows for only homogeneous configurations as we depend >>> on global smp_threads and machine->cpu_model. >>> >>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> >>> --- >>> hw/ppc/Makefile.objs | 1 + >>> hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++ >>> include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++ >>> 3 files changed, 77 insertions(+) >>> create mode 100644 hw/ppc/spapr_cpu_package.c >>> create mode 100644 include/hw/ppc/spapr_cpu_package.h >>> >>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs >>> index c1ffc77..3000982 100644 >>> --- a/hw/ppc/Makefile.objs >>> +++ b/hw/ppc/Makefile.objs >>> @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o >>> obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o >>> obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o >>> obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o >>> +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o >>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) >>> obj-y += spapr_pci_vfio.o >>> endif >>> diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c >>> new file mode 100644 >>> index 0000000..3120a16 >>> --- /dev/null >>> +++ b/hw/ppc/spapr_cpu_package.c >>> @@ -0,0 +1,50 @@ >>> +/* >>> + * sPAPR CPU package device, acts as container of CPU thread devices. >>> + * >>> + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com> >>> + * >>> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >>> + * See the COPYING file in the top-level directory. >>> + */ >>> +#include "hw/cpu/package.h" >>> +#include "hw/ppc/spapr_cpu_package.h" >>> +#include "hw/boards.h" >>> +#include <sysemu/cpus.h> >>> +#include "qemu/error-report.h" >>> + >>> +static void spapr_cpu_package_instance_init(Object *obj) >>> +{ >>> + int i; >>> + CPUState *cpu; >>> + MachineState *machine = MACHINE(qdev_get_machine()); >>> + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj); >>> + >>> + /* Create as many CPU threads as specified in the topology */ >>> + for (i = 0; i < smp_threads; i++) { >>> + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model); >> >> No, no, no. This is horribly violating QOM design. > > Ok.. why? There does not, to me, seem to be any remotely easily > discoverable means of finding out what QOM design principles are. > > It also would have been nice if you weighed in on my RFC this code is > based on. It would've been nice had you joined our KVM call prompted by _your_ suggestions (which again you could've discussed at KVM Forum where I had a session specifically for discussing this topic), but you didn't. I did discuss several aspects on the call and in the recorded session and will not write it by email again. Feel free to put it on the call agenda again. I told Bharata that you can't have a base type that suddenly mutates its topology, therefore we discussed the possibility of having a QOM _interface_, not a base type as apparently done here. We deliberately do not have multi-inheritence in QOM; there's not just ppc in the world. My time for reading list mails is limited. Make it easy for me to understand what the difference is between your package and my core and why instead of reusing my posted cpu-core type you need to propose your own cpu-package type. In one point you are right, we keep going in circles and sometimes backwards rather than forward here. Andreas
On Mon, Feb 22, 2016 at 04:58:46PM +0100, Andreas Färber wrote: > Am 22.02.2016 um 08:47 schrieb David Gibson: > > On Mon, Feb 22, 2016 at 07:44:40AM +0100, Andreas Färber wrote: > >> Am 22.02.2016 um 06:01 schrieb Bharata B Rao: > >>> sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is > >>> performed in the granularity of CPU core device by setting the "realized" > >>> property of this device to "true". When hotplugged, CPU core creates CPU > >>> thread devices. > >>> > >>> TODO: Right now allows for only homogeneous configurations as we depend > >>> on global smp_threads and machine->cpu_model. > >>> > >>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > >>> --- > >>> hw/ppc/Makefile.objs | 1 + > >>> hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++ > >>> include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++ > >>> 3 files changed, 77 insertions(+) > >>> create mode 100644 hw/ppc/spapr_cpu_package.c > >>> create mode 100644 include/hw/ppc/spapr_cpu_package.h > >>> > >>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > >>> index c1ffc77..3000982 100644 > >>> --- a/hw/ppc/Makefile.objs > >>> +++ b/hw/ppc/Makefile.objs > >>> @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o > >>> obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o > >>> obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o > >>> obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o > >>> +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o > >>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) > >>> obj-y += spapr_pci_vfio.o > >>> endif > >>> diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c > >>> new file mode 100644 > >>> index 0000000..3120a16 > >>> --- /dev/null > >>> +++ b/hw/ppc/spapr_cpu_package.c > >>> @@ -0,0 +1,50 @@ > >>> +/* > >>> + * sPAPR CPU package device, acts as container of CPU thread devices. > >>> + * > >>> + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com> > >>> + * > >>> + * This work is licensed under the terms of the GNU GPL, version 2 or later. > >>> + * See the COPYING file in the top-level directory. > >>> + */ > >>> +#include "hw/cpu/package.h" > >>> +#include "hw/ppc/spapr_cpu_package.h" > >>> +#include "hw/boards.h" > >>> +#include <sysemu/cpus.h> > >>> +#include "qemu/error-report.h" > >>> + > >>> +static void spapr_cpu_package_instance_init(Object *obj) > >>> +{ > >>> + int i; > >>> + CPUState *cpu; > >>> + MachineState *machine = MACHINE(qdev_get_machine()); > >>> + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj); > >>> + > >>> + /* Create as many CPU threads as specified in the topology */ > >>> + for (i = 0; i < smp_threads; i++) { > >>> + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model); > >> > >> No, no, no. This is horribly violating QOM design. > > > > Ok.. why? There does not, to me, seem to be any remotely easily > > discoverable means of finding out what QOM design principles are. > > > > It also would have been nice if you weighed in on my RFC this code is > > based on. So, first, apologies for my grumpy tone. This is just one of a number of frustrations I've had in the last few days, putting me in an uncharitable frame of mind. > It would've been nice had you joined our KVM call prompted by _your_ > suggestions Ok, a) I was not aware that this call was prompted by my suggestions. I got some vague second hand notices about it; as I do about KVM calls from time to time, which I ignored, as usual, because b) the KVM call is at stupid o'clock for me. > (which again you could've discussed at KVM Forum where I had > a session specifically for discussing this topic), but you didn't. At KVM Forum, cpu hotplug wasn't yet on my radar. > I did discuss several aspects on the call and in the recorded session > and will not write it by email again. Feel free to put it on the call > agenda again. > > I told Bharata that you can't have a base type that suddenly mutates its > topology, Ok, I'm not entirely sure what you mean by that. Do you mean the fact that the core object constructs its own sub-objects? > therefore we discussed the possibility of having a QOM > _interface_, not a base type as apparently done here. We deliberately do > not have multi-inheritence in QOM; there's not just ppc in the world. So, in other discussions I've realised the package needs to be an interface, rather than a type. But the patch your objecting to here implements the spapr-core subtype. If that were implementing rather than inheriting "cpu-package" I don't see that it would really change the logic here. Like you (I think) I do dislike the use of machine->cpu_type and machine->cpu_model. I just haven't figured out enough QOM to know how to avoid them. What is the QOM equivalent of, essentially, arguments to a constructor? > My time for reading list mails is limited. Make it easy for me to > understand what the difference is between your package and my core and > why instead of reusing my posted cpu-core type you need to propose your Because my time for reading the list is also limited, so I haven't seen your cpu-core posting. I did see Bharata's earlier core based stuff which didn't seem to go over well either. > own cpu-package type. In one point you are right, we keep going in > circles and sometimes backwards rather than forward here. But, also, from what I can tell of those parts of the discussion I have seen, locking the hotplug at core level doesn't seem to work. It seems to create an awful backwards compatibility tangle for x86 which has existing thread-level hotplug, and I expect it would cause problems when we encounter a platform with socket level hotplug only (this seems very plausible for something modelling physical hotplug). The idea of cpu-package was to abstract away the granularity of cpu hotplug from a fixed level of the socket/core/thread heirarchy, while still making that granularity easy to discover for management.
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs index c1ffc77..3000982 100644 --- a/hw/ppc/Makefile.objs +++ b/hw/ppc/Makefile.objs @@ -4,6 +4,7 @@ obj-y += ppc.o ppc_booke.o obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o +obj-$(CONFIG_PSERIES) += spapr_cpu_package.o ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) obj-y += spapr_pci_vfio.o endif diff --git a/hw/ppc/spapr_cpu_package.c b/hw/ppc/spapr_cpu_package.c new file mode 100644 index 0000000..3120a16 --- /dev/null +++ b/hw/ppc/spapr_cpu_package.c @@ -0,0 +1,50 @@ +/* + * sPAPR CPU package device, acts as container of CPU thread devices. + * + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#include "hw/cpu/package.h" +#include "hw/ppc/spapr_cpu_package.h" +#include "hw/boards.h" +#include <sysemu/cpus.h> +#include "qemu/error-report.h" + +static void spapr_cpu_package_instance_init(Object *obj) +{ + int i; + CPUState *cpu; + MachineState *machine = MACHINE(qdev_get_machine()); + sPAPRCPUPackage *package = SPAPR_CPU_PACKAGE(obj); + + /* Create as many CPU threads as specified in the topology */ + for (i = 0; i < smp_threads; i++) { + cpu = cpu_generic_init(machine->cpu_type, machine->cpu_model); + if (!cpu) { + error_setg(&error_fatal, "Unable to find CPU definition: %s\n", + machine->cpu_model); + } + object_property_add_child(obj, "thread[*]", OBJECT(cpu), &error_fatal); + object_unref(OBJECT(cpu)); + DEVICE(OBJECT(cpu))->hotplugged = true; + if (!i) { + package->thread0 = POWERPC_CPU(cpu); + } + } +} + +static const TypeInfo spapr_cpu_package_type_info = { + .name = TYPE_SPAPR_CPU_PACKAGE, + .parent = TYPE_CPU_PACKAGE, + .instance_init = spapr_cpu_package_instance_init, + .instance_size = sizeof(sPAPRCPUPackage), +}; + +static void spapr_cpu_package_register_types(void) +{ + type_register_static(&spapr_cpu_package_type_info); +} + +type_init(spapr_cpu_package_register_types) diff --git a/include/hw/ppc/spapr_cpu_package.h b/include/hw/ppc/spapr_cpu_package.h new file mode 100644 index 0000000..547dbc1 --- /dev/null +++ b/include/hw/ppc/spapr_cpu_package.h @@ -0,0 +1,26 @@ +/* + * sPAPR CPU package device. + * + * Copyright (C) 2016 Bharata B Rao <bharata@linux.vnet.ibm.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#ifndef HW_SPAPR_CPU_PACKAGE_H +#define HW_SPAPR_CPU_PACKAGE_H + +#include "hw/qdev.h" + +#define TYPE_SPAPR_CPU_PACKAGE "spapr-cpu-package" +#define SPAPR_CPU_PACKAGE(obj) \ + OBJECT_CHECK(sPAPRCPUPackage, (obj), TYPE_SPAPR_CPU_PACKAGE) + +typedef struct sPAPRCPUPackage { + /*< private >*/ + DeviceState parent_obj; + + /*< public >*/ + PowerPCCPU *thread0; +} sPAPRCPUPackage; + +#endif
sPAPR CPU core device is a container of CPU thread devices. CPU hotplug is performed in the granularity of CPU core device by setting the "realized" property of this device to "true". When hotplugged, CPU core creates CPU thread devices. TODO: Right now allows for only homogeneous configurations as we depend on global smp_threads and machine->cpu_model. Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> --- hw/ppc/Makefile.objs | 1 + hw/ppc/spapr_cpu_package.c | 50 ++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr_cpu_package.h | 26 ++++++++++++++++++++ 3 files changed, 77 insertions(+) create mode 100644 hw/ppc/spapr_cpu_package.c create mode 100644 include/hw/ppc/spapr_cpu_package.h