| Message ID | 20220330125639.201937-1-damien.hedde@greensocs.com |
|---|---|
| Headers | show |
| Series | user-creatable cpu clusters | expand |
Ping ! It would be nice to have some rough feedback about this series. I plan to submit it in 2 sub-series (riscv-array in a separate part) but would like to know first if this direction looks ok to you ? Note that I'm a bit confused by the terminology. Should a "cluster" as in "machine topology's cluster" be a 1-1 match with a gdb inferior ? Thanks, Damien On 3/30/22 14:56, Damien Hedde wrote: > Hi, > > This series add devices to be able to user-create (coldplug) cpu > clusters. The existing cpu cluster dictates how cpus are exposed > in gdb, but it does not handle the cpu objects creation. This series > adds a new device to handle both issues and adds support for two > architectures: arm and riscv. > > Please look at patches 2 and 3 for more details about the new device. > > Last part concerning the riscv is rfc as I do non-backward compatible > updates. I'm not sure what migration (or other) constraints we have > on these machines and I probably need to make some changes to cope > with them. > > This series almost deprecates the cpu-cluster type as all uses > but one are replaced. > > It is organized as follows: > > + Patches 1 to 7 adds a new base device to replace cpu-cluster > > + Patches 8 and 9 adds an arm specific version and replace existing > clusters in the xlnx-zynqmp machine. > > + patches 10 to 17 updates the riscv_array. It was already used to > create cpus but was not a cpu cluster. > > Thanks for any comments, > -- > Damien > > Damien Hedde (18): > define MAX_CLUSTERS in cpu.h instead of cluster.h > hw/cpu/cpus: introduce _cpus_ device > hw/cpu/cpus: prepare to handle cpu clusters > hw/cpu/cluster: make _cpu-cluster_ a subclass of _cpus_ > gdbstub: deal with _cpus_ object instead of _cpu-cluster_ > hw/cpu/cluster: remove cluster_id now that gdbstub is updated > hw/cpu/cpus: add a common start-powered-off property > hw/arm/arm_cpus: add arm_cpus device > hw/arm/xlnx-zynqmp: convert cpu clusters to arm_cpus > hw/riscv/riscv_hart: prepare transition to cpus > hw/riscv: prepare riscv_hart transition to cpus > hw/riscv/virt: prepare riscv_hart transition to cpus > hw/riscv/spike: prepare riscv_hart transition to cpus > hw/riscv/riscv_hart: use cpus as base class > hw/riscv/sifive_uµchip_pfsoc: apply riscv_hart_array update > hw/riscv: update remaining machines due to riscv_hart_array update > hw/riscv/riscv_hart: remove temporary features > add myself as reviewer of the newly added _cpus_ > > include/hw/arm/arm_cpus.h | 45 +++++++ > include/hw/arm/xlnx-zynqmp.h | 8 +- > include/hw/core/cpu.h | 6 + > include/hw/cpu/cluster.h | 26 ++-- > include/hw/cpu/cpus.h | 93 ++++++++++++++ > include/hw/riscv/microchip_pfsoc.h | 2 - > include/hw/riscv/riscv_hart.h | 25 +++- > include/hw/riscv/sifive_u.h | 2 - > gdbstub.c | 12 +- > hw/arm/arm_cpus.c | 63 ++++++++++ > hw/arm/xlnx-zynqmp.c | 121 +++++++----------- > hw/cpu/cluster.c | 53 ++++---- > hw/cpu/cpus.c | 195 +++++++++++++++++++++++++++++ > hw/riscv/boot.c | 2 +- > hw/riscv/microchip_pfsoc.c | 28 +---- > hw/riscv/opentitan.c | 4 +- > hw/riscv/riscv_hart.c | 44 ++----- > hw/riscv/shakti_c.c | 4 +- > hw/riscv/sifive_e.c | 4 +- > hw/riscv/sifive_u.c | 31 ++--- > hw/riscv/spike.c | 18 +-- > hw/riscv/virt.c | 79 +++++++----- > MAINTAINERS | 3 + > hw/arm/meson.build | 1 + > hw/cpu/meson.build | 2 +- > 25 files changed, 612 insertions(+), 259 deletions(-) > create mode 100644 include/hw/arm/arm_cpus.h > create mode 100644 include/hw/cpu/cpus.h > create mode 100644 hw/arm/arm_cpus.c > create mode 100644 hw/cpu/cpus.c >
On Wed, 30 Mar 2022 at 13:56, Damien Hedde <damien.hedde@greensocs.com> wrote: > > Hi, > > This series add devices to be able to user-create (coldplug) cpu > clusters. The existing cpu cluster dictates how cpus are exposed > in gdb, but it does not handle the cpu objects creation. This series > adds a new device to handle both issues and adds support for two > architectures: arm and riscv. > > Please look at patches 2 and 3 for more details about the new device. > > Last part concerning the riscv is rfc as I do non-backward compatible > updates. I'm not sure what migration (or other) constraints we have > on these machines and I probably need to make some changes to cope > with them. > > This series almost deprecates the cpu-cluster type as all uses > but one are replaced. I don't think we should have two different concepts which are "a group of CPUs". It means we wind up with two different ways to do something (which we have too many examples of already), only one of which gets to use the nicer, more obvious name. The stated motivation is to allow user creation of CPU clusters, but I'm not sure how this would work -- CPUs need a lot of things wiring up like interrupt lines and memory regions, which you can't do on the command line anyway. Do you have an example of what the new code lets you do? thanks -- PMM
On 4/21/22 17:51, Peter Maydell wrote: > On Wed, 30 Mar 2022 at 13:56, Damien Hedde <damien.hedde@greensocs.com> wrote: >> >> Hi, >> >> This series add devices to be able to user-create (coldplug) cpu >> clusters. The existing cpu cluster dictates how cpus are exposed >> in gdb, but it does not handle the cpu objects creation. This series >> adds a new device to handle both issues and adds support for two >> architectures: arm and riscv. >> >> Please look at patches 2 and 3 for more details about the new device. >> >> Last part concerning the riscv is rfc as I do non-backward compatible >> updates. I'm not sure what migration (or other) constraints we have >> on these machines and I probably need to make some changes to cope >> with them. >> >> This series almost deprecates the cpu-cluster type as all uses >> but one are replaced. > > I don't think we should have two different concepts which > are "a group of CPUs". It means we wind up with two different > ways to do something (which we have too many examples of > already), only one of which gets to use the nicer, more obvious > name. I was not sure that I could merge the cpu-cluster into the new object and agree having both is not ideal. I can 1. delete the last cpu-cluster usecase and the object type 2. manage to update in place the cpu-cluster instead of adding the new type. In the end, the object will be the same. I am also confused by the "cluster" naming. The original cpu-cluster is used only to set cluster_index of cpus which is used in the gdbstub to group cpus in inferiors. So it is just a container to expose cpus in a specific gdb inferior. But in most machines (which don't use explicit cpu-clusters) this cluster[_index] has nothing to do with the topology's cluster. In practice, topology's cluster is not a 1-1 match of gdb inferior in gsbtub ("cpu-cluster") and I'm not sure what to do with that... I'm happy to keep the "cpu-cluster" if everyone feel the same. > > The stated motivation is to allow user creation of CPU clusters, > but I'm not sure how this would work -- CPUs need a lot of > things wiring up like interrupt lines and memory regions, which > you can't do on the command line anyway. Do you have an example > of what the new code lets you do? This simplify the place where cpu-cluster was used (see patch 9 for example) as you don't need to create both the cpus and the cluster. But mostly the goal is to provide cpu cold plug functionality with the qapi interface (not the cli interface). With this series + my other series adding commands to cold-plug devices and memory-map sysbus devices, I reproduced the arm virt board starting from the none machine. Interrupts are wired using qom_set. Thanks, -- Damien