Re: [PATCH v3 07/22] ppc/ppc405: QOM'ify CPU
On 8/8/22 19:25, BALATON Zoltan wrote: On Mon, 8 Aug 2022, Peter Maydell wrote: On Mon, 8 Aug 2022 at 18:05, BALATON Zoltan wrote: But the handler we register here just calls cpu_reset which seems to just call the reset method of the CPU object. If we have nothing else to do here do we need to explicitly call cpi_reset like this? Wouldn't the CPU object be reset by qdev when resetting the machine or the soc its in? If we have our own reset method we may call cpu_reset from there to make sure the CPU is in a known state but is this needed when we don't want to do anything else? I don't know how reset handling works but some machines seems to do this and others don't. You do unfortunately need to manually reset the CPU object. This is because the 'automatic' qdev reset only works for devices that hang off a bus (including sysbus devices). This is because it works by having qdev_machine_creation_done() register a reset function which does "reset the sysbus". Resetting a bus resets every device on it. Resetting a device resets every bus it owns. (This means that for instance PCI devices get reset because the PCI controller is on the sysbus, so it gets reset, and it owns the PCI bus, so the PCI bus resets when the controller is reset, and when the PCI bus resets then all the devices on it are reset.) So reset propagates down the bus tree, but it won't reach devices which aren't on that bus tree at all. The most common case of "device which isn't on a bus" is the CPU objects. This is, clearly, a complete mess. I would strongly like to sort it out, but to do that we need (a) a plan for what reset handling ought to look like and (b) a transition plan for getting from here to there without breaking everything in the process. I haven't had the time to focus on that, though it's been nominally on my todo list for years. In the meantime, it's the responsibility of something in architecture specific code to arrange for the CPU objects to be reset -- whether that's done directly by the board, or hidden in some other layer of code, depends on the target arch. For instance for arm it's hidden inside arm_load_kernel() and board models must therefore call that function even if they have no intention of supporting loading Linux kernels. For sparc it's done directly in the board code. OK thanks for the clarification. Both bamboo and sam460ex has a main_cpu_reset function that is registered from the machine init func which calls cpu_reset. Maybe 405 boards could do the same for consistency? It could also be added to the soc object's reset method as that owns the cpu so maybe it may make more sense that way. This is probably also OK as 440 machines don't have soc object yet so it wouldn't cause any problems. I think the ppc4xx_reset func was kept around because ppc4xx_init was also kept but nothing else seems to use it so maybe these should become the soc object's realize and reset methods and drop the current ppc4xx_init and reset funcs? Yes. I have moved the CPU reset routine under ppc405_uc.c and removed ppc4xx_init() which is now unused. Thanks, C.
Re: [PATCH v3 07/22] ppc/ppc405: QOM'ify CPU
On Mon, 8 Aug 2022, Peter Maydell wrote: On Mon, 8 Aug 2022 at 18:05, BALATON Zoltan wrote: But the handler we register here just calls cpu_reset which seems to just call the reset method of the CPU object. If we have nothing else to do here do we need to explicitly call cpi_reset like this? Wouldn't the CPU object be reset by qdev when resetting the machine or the soc its in? If we have our own reset method we may call cpu_reset from there to make sure the CPU is in a known state but is this needed when we don't want to do anything else? I don't know how reset handling works but some machines seems to do this and others don't. You do unfortunately need to manually reset the CPU object. This is because the 'automatic' qdev reset only works for devices that hang off a bus (including sysbus devices). This is because it works by having qdev_machine_creation_done() register a reset function which does "reset the sysbus". Resetting a bus resets every device on it. Resetting a device resets every bus it owns. (This means that for instance PCI devices get reset because the PCI controller is on the sysbus, so it gets reset, and it owns the PCI bus, so the PCI bus resets when the controller is reset, and when the PCI bus resets then all the devices on it are reset.) So reset propagates down the bus tree, but it won't reach devices which aren't on that bus tree at all. The most common case of "device which isn't on a bus" is the CPU objects. This is, clearly, a complete mess. I would strongly like to sort it out, but to do that we need (a) a plan for what reset handling ought to look like and (b) a transition plan for getting from here to there without breaking everything in the process. I haven't had the time to focus on that, though it's been nominally on my todo list for years. In the meantime, it's the responsibility of something in architecture specific code to arrange for the CPU objects to be reset -- whether that's done directly by the board, or hidden in some other layer of code, depends on the target arch. For instance for arm it's hidden inside arm_load_kernel() and board models must therefore call that function even if they have no intention of supporting loading Linux kernels. For sparc it's done directly in the board code. OK thanks for the clarification. Both bamboo and sam460ex has a main_cpu_reset function that is registered from the machine init func which calls cpu_reset. Maybe 405 boards could do the same for consistency? It could also be added to the soc object's reset method as that owns the cpu so maybe it may make more sense that way. This is probably also OK as 440 machines don't have soc object yet so it wouldn't cause any problems. I think the ppc4xx_reset func was kept around because ppc4xx_init was also kept but nothing else seems to use it so maybe these should become the soc object's realize and reset methods and drop the current ppc4xx_init and reset funcs? Regards, BALATON Zoltan
Re: [PATCH v3 07/22] ppc/ppc405: QOM'ify CPU
On Mon, 8 Aug 2022 at 18:05, BALATON Zoltan wrote: > But the handler we register here just calls cpu_reset which seems to just > call the reset method of the CPU object. If we have nothing else to do > here do we need to explicitly call cpi_reset like this? Wouldn't the CPU > object be reset by qdev when resetting the machine or the soc its in? If > we have our own reset method we may call cpu_reset from there to make sure > the CPU is in a known state but is this needed when we don't want to do > anything else? I don't know how reset handling works but some machines > seems to do this and others don't. You do unfortunately need to manually reset the CPU object. This is because the 'automatic' qdev reset only works for devices that hang off a bus (including sysbus devices). This is because it works by having qdev_machine_creation_done() register a reset function which does "reset the sysbus". Resetting a bus resets every device on it. Resetting a device resets every bus it owns. (This means that for instance PCI devices get reset because the PCI controller is on the sysbus, so it gets reset, and it owns the PCI bus, so the PCI bus resets when the controller is reset, and when the PCI bus resets then all the devices on it are reset.) So reset propagates down the bus tree, but it won't reach devices which aren't on that bus tree at all. The most common case of "device which isn't on a bus" is the CPU objects. This is, clearly, a complete mess. I would strongly like to sort it out, but to do that we need (a) a plan for what reset handling ought to look like and (b) a transition plan for getting from here to there without breaking everything in the process. I haven't had the time to focus on that, though it's been nominally on my todo list for years. In the meantime, it's the responsibility of something in architecture specific code to arrange for the CPU objects to be reset -- whether that's done directly by the board, or hidden in some other layer of code, depends on the target arch. For instance for arm it's hidden inside arm_load_kernel() and board models must therefore call that function even if they have no intention of supporting loading Linux kernels. For sparc it's done directly in the board code. thanks -- PMM
Re: [PATCH v3 07/22] ppc/ppc405: QOM'ify CPU
On Mon, 8 Aug 2022, Cédric Le Goater wrote:
On 8/8/22 15:17, BALATON Zoltan wrote:
Patch title is wrong. It should be Embed CPU object in SoC as it's not
QOMifies the CPU just moves it from dinamically allocated to embedded.
On Mon, 8 Aug 2022, Cédric Le Goater wrote:
Drop the use of ppc4xx_init() and duplicate a bit of code related to
clocks in the SoC realize routine. We will clean that up in the
following patches.
Could this be split off into a separate patch? Maybe it would be clearer
that way what's related to stop using ppc4xx_init() (which is needed
because it dinamically allocates CPU) and what's the embedding it in the
soc object.
I'd rather not. It has been painful enough to untangle. Let's keep it that
way. And, this part is further changed in the CPC patch.
OK, if nobody else thinks this should be split I'm OK with it.
ppc_dcr_init() simply allocates default DCR handlers for the CPU. Maybe
this could be done in model initializer of the CPU families needing it.
Reviewed-by: Daniel Henrique Barboza
Signed-off-by: Cédric Le Goater
---
hw/ppc/ppc405.h | 2 +-
include/hw/ppc/ppc4xx.h | 1 +
hw/ppc/ppc405_boards.c | 2 +-
hw/ppc/ppc405_uc.c | 35 +--
hw/ppc/ppc4xx_devs.c | 2 +-
5 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index dc862bc8614c..8cc76cc8b3fe 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -79,7 +79,7 @@ struct Ppc405SoCState {
hwaddr ram_size;
uint32_t sysclk;
- PowerPCCPU *cpu;
+ PowerPCCPU cpu;
DeviceState *uic;
};
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index 980f964b5a91..021376c2d260 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,6 +29,7 @@
#include "exec/memory.h"
/* PowerPC 4xx core initialization */
+void ppc4xx_reset(void *opaque);
PowerPCCPU *ppc4xx_init(const char *cpu_model,
clk_setup_t *cpu_clk, clk_setup_t *tb_clk,
uint32_t sysclk);
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 0b39ff08bd65..5ba12d60bc00 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -313,7 +313,7 @@ static void ppc405_init(MachineState *machine)
/* Load ELF kernel and rootfs.cpio */
} else if (kernel_filename && !machine->firmware) {
- boot_from_kernel(machine, ppc405->soc.cpu);
+ boot_from_kernel(machine, >soc.cpu);
}
}
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index abcc2537140c..fa3853df2233 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1432,22 +1432,36 @@ static void ppc405ep_cpc_init (CPUPPCState *env,
clk_setup_t clk_setup[8],
#endif
}
+static void ppc405_soc_instance_init(Object *obj)
+{
+ Ppc405SoCState *s = PPC405_SOC(obj);
+
+ object_initialize_child(obj, "cpu", >cpu,
+ POWERPC_CPU_TYPE_NAME("405ep"));
+}
+
static void ppc405_soc_realize(DeviceState *dev, Error **errp)
{
Ppc405SoCState *s = PPC405_SOC(dev);
- clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup;
+ clk_setup_t clk_setup[PPC405EP_CLK_NB];
qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4];
CPUPPCState *env;
memset(clk_setup, 0, sizeof(clk_setup));
/* init CPUs */
- s->cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405ep"),
- _setup[PPC405EP_CPU_CLK],
- _clk_setup, s->sysclk);
- env = >cpu->env;
- clk_setup[PPC405EP_CPU_CLK].cb = tlb_clk_setup.cb;
- clk_setup[PPC405EP_CPU_CLK].opaque = tlb_clk_setup.opaque;
+ if (!qdev_realize(DEVICE(>cpu), NULL, errp)) {
+ return;
+ }
+ qemu_register_reset(ppc4xx_reset, >cpu);
+
+ env = >cpu.env;
+
+ clk_setup[PPC405EP_CPU_CLK].cb =
+ ppc_40x_timers_init(env, s->sysclk, PPC_INTERRUPT_PIT);
+ clk_setup[PPC405EP_CPU_CLK].opaque = env;
+
+ ppc_dcr_init(env, NULL, NULL);
/* CPU control */
ppc405ep_cpc_init(env, clk_setup, s->sysclk);
@@ -1464,16 +1478,16 @@ static void ppc405_soc_realize(DeviceState *dev,
Error **errp)
/* Universal interrupt controller */
s->uic = qdev_new(TYPE_PPC_UIC);
- object_property_set_link(OBJECT(s->uic), "cpu", OBJECT(s->cpu),
+ object_property_set_link(OBJECT(s->uic), "cpu", OBJECT(>cpu),
_fatal);
if (!sysbus_realize(SYS_BUS_DEVICE(s->uic), errp)) {
return;
}
sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_INT,
- qdev_get_gpio_in(DEVICE(s->cpu),
PPC40x_INPUT_INT));
+ qdev_get_gpio_in(DEVICE(>cpu),
PPC40x_INPUT_INT));
sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_CINT,
- qdev_get_gpio_in(DEVICE(s->cpu),
PPC40x_INPUT_CINT));
+ qdev_get_gpio_in(DEVICE(>cpu),
PPC40x_INPUT_CINT));
/* SDRAM controller */
/* XXX 405EP has no ECC interrupt */
@@ -1562,6 +1576,7 @@ static const TypeInfo
Re: [PATCH v3 07/22] ppc/ppc405: QOM'ify CPU
On 8/8/22 15:17, BALATON Zoltan wrote:
Patch title is wrong. It should be Embed CPU object in SoC as it's not QOMifies
the CPU just moves it from dinamically allocated to embedded.
On Mon, 8 Aug 2022, Cédric Le Goater wrote:
Drop the use of ppc4xx_init() and duplicate a bit of code related to
clocks in the SoC realize routine. We will clean that up in the
following patches.
Could this be split off into a separate patch? Maybe it would be clearer that
way what's related to stop using ppc4xx_init() (which is needed because it
dinamically allocates CPU) and what's the embedding it in the soc object.
I'd rather not. It has been painful enough to untangle. Let's keep it that
way. And, this part is further changed in the CPC patch.
ppc_dcr_init() simply allocates default DCR handlers for the CPU. Maybe
this could be done in model initializer of the CPU families needing it.
Reviewed-by: Daniel Henrique Barboza
Signed-off-by: Cédric Le Goater
---
hw/ppc/ppc405.h | 2 +-
include/hw/ppc/ppc4xx.h | 1 +
hw/ppc/ppc405_boards.c | 2 +-
hw/ppc/ppc405_uc.c | 35 +--
hw/ppc/ppc4xx_devs.c | 2 +-
5 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index dc862bc8614c..8cc76cc8b3fe 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -79,7 +79,7 @@ struct Ppc405SoCState {
hwaddr ram_size;
uint32_t sysclk;
- PowerPCCPU *cpu;
+ PowerPCCPU cpu;
DeviceState *uic;
};
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index 980f964b5a91..021376c2d260 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,6 +29,7 @@
#include "exec/memory.h"
/* PowerPC 4xx core initialization */
+void ppc4xx_reset(void *opaque);
PowerPCCPU *ppc4xx_init(const char *cpu_model,
clk_setup_t *cpu_clk, clk_setup_t *tb_clk,
uint32_t sysclk);
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 0b39ff08bd65..5ba12d60bc00 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -313,7 +313,7 @@ static void ppc405_init(MachineState *machine)
/* Load ELF kernel and rootfs.cpio */
} else if (kernel_filename && !machine->firmware) {
- boot_from_kernel(machine, ppc405->soc.cpu);
+ boot_from_kernel(machine, >soc.cpu);
}
}
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index abcc2537140c..fa3853df2233 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1432,22 +1432,36 @@ static void ppc405ep_cpc_init (CPUPPCState *env,
clk_setup_t clk_setup[8],
#endif
}
+static void ppc405_soc_instance_init(Object *obj)
+{
+ Ppc405SoCState *s = PPC405_SOC(obj);
+
+ object_initialize_child(obj, "cpu", >cpu,
+ POWERPC_CPU_TYPE_NAME("405ep"));
+}
+
static void ppc405_soc_realize(DeviceState *dev, Error **errp)
{
Ppc405SoCState *s = PPC405_SOC(dev);
- clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup;
+ clk_setup_t clk_setup[PPC405EP_CLK_NB];
qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4];
CPUPPCState *env;
memset(clk_setup, 0, sizeof(clk_setup));
/* init CPUs */
- s->cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405ep"),
- _setup[PPC405EP_CPU_CLK],
- _clk_setup, s->sysclk);
- env = >cpu->env;
- clk_setup[PPC405EP_CPU_CLK].cb = tlb_clk_setup.cb;
- clk_setup[PPC405EP_CPU_CLK].opaque = tlb_clk_setup.opaque;
+ if (!qdev_realize(DEVICE(>cpu), NULL, errp)) {
+ return;
+ }
+ qemu_register_reset(ppc4xx_reset, >cpu);
+
+ env = >cpu.env;
+
+ clk_setup[PPC405EP_CPU_CLK].cb =
+ ppc_40x_timers_init(env, s->sysclk, PPC_INTERRUPT_PIT);
+ clk_setup[PPC405EP_CPU_CLK].opaque = env;
+
+ ppc_dcr_init(env, NULL, NULL);
/* CPU control */
ppc405ep_cpc_init(env, clk_setup, s->sysclk);
@@ -1464,16 +1478,16 @@ static void ppc405_soc_realize(DeviceState *dev, Error
**errp)
/* Universal interrupt controller */
s->uic = qdev_new(TYPE_PPC_UIC);
- object_property_set_link(OBJECT(s->uic), "cpu", OBJECT(s->cpu),
+ object_property_set_link(OBJECT(s->uic), "cpu", OBJECT(>cpu),
_fatal);
if (!sysbus_realize(SYS_BUS_DEVICE(s->uic), errp)) {
return;
}
sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_INT,
- qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_INT));
+ qdev_get_gpio_in(DEVICE(>cpu), PPC40x_INPUT_INT));
sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_CINT,
- qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_CINT));
+ qdev_get_gpio_in(DEVICE(>cpu), PPC40x_INPUT_CINT));
/* SDRAM controller */
/* XXX 405EP has no ECC interrupt */
@@ -1562,6 +1576,7 @@ static const TypeInfo ppc405_types[] = {
.name = TYPE_PPC405_SOC,
.parent = TYPE_DEVICE,
.instance_size =
Re: [PATCH v3 07/22] ppc/ppc405: QOM'ify CPU
Patch title is wrong. It should be Embed CPU object in SoC as it's not
QOMifies the CPU just moves it from dinamically allocated to embedded.
On Mon, 8 Aug 2022, Cédric Le Goater wrote:
Drop the use of ppc4xx_init() and duplicate a bit of code related to
clocks in the SoC realize routine. We will clean that up in the
following patches.
Could this be split off into a separate patch? Maybe it would be clearer
that way what's related to stop using ppc4xx_init() (which is needed
because it dinamically allocates CPU) and what's the embedding it in the
soc object.
ppc_dcr_init() simply allocates default DCR handlers for the CPU. Maybe
this could be done in model initializer of the CPU families needing it.
Reviewed-by: Daniel Henrique Barboza
Signed-off-by: Cédric Le Goater
---
hw/ppc/ppc405.h | 2 +-
include/hw/ppc/ppc4xx.h | 1 +
hw/ppc/ppc405_boards.c | 2 +-
hw/ppc/ppc405_uc.c | 35 +--
hw/ppc/ppc4xx_devs.c| 2 +-
5 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index dc862bc8614c..8cc76cc8b3fe 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -79,7 +79,7 @@ struct Ppc405SoCState {
hwaddr ram_size;
uint32_t sysclk;
-PowerPCCPU *cpu;
+PowerPCCPU cpu;
DeviceState *uic;
};
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index 980f964b5a91..021376c2d260 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,6 +29,7 @@
#include "exec/memory.h"
/* PowerPC 4xx core initialization */
+void ppc4xx_reset(void *opaque);
PowerPCCPU *ppc4xx_init(const char *cpu_model,
clk_setup_t *cpu_clk, clk_setup_t *tb_clk,
uint32_t sysclk);
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 0b39ff08bd65..5ba12d60bc00 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -313,7 +313,7 @@ static void ppc405_init(MachineState *machine)
/* Load ELF kernel and rootfs.cpio */
} else if (kernel_filename && !machine->firmware) {
-boot_from_kernel(machine, ppc405->soc.cpu);
+boot_from_kernel(machine, >soc.cpu);
}
}
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index abcc2537140c..fa3853df2233 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1432,22 +1432,36 @@ static void ppc405ep_cpc_init (CPUPPCState *env,
clk_setup_t clk_setup[8],
#endif
}
+static void ppc405_soc_instance_init(Object *obj)
+{
+Ppc405SoCState *s = PPC405_SOC(obj);
+
+object_initialize_child(obj, "cpu", >cpu,
+POWERPC_CPU_TYPE_NAME("405ep"));
+}
+
static void ppc405_soc_realize(DeviceState *dev, Error **errp)
{
Ppc405SoCState *s = PPC405_SOC(dev);
-clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup;
+clk_setup_t clk_setup[PPC405EP_CLK_NB];
qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4];
CPUPPCState *env;
memset(clk_setup, 0, sizeof(clk_setup));
/* init CPUs */
-s->cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405ep"),
- _setup[PPC405EP_CPU_CLK],
- _clk_setup, s->sysclk);
-env = >cpu->env;
-clk_setup[PPC405EP_CPU_CLK].cb = tlb_clk_setup.cb;
-clk_setup[PPC405EP_CPU_CLK].opaque = tlb_clk_setup.opaque;
+if (!qdev_realize(DEVICE(>cpu), NULL, errp)) {
+return;
+}
+qemu_register_reset(ppc4xx_reset, >cpu);
+
+env = >cpu.env;
+
+clk_setup[PPC405EP_CPU_CLK].cb =
+ppc_40x_timers_init(env, s->sysclk, PPC_INTERRUPT_PIT);
+clk_setup[PPC405EP_CPU_CLK].opaque = env;
+
+ppc_dcr_init(env, NULL, NULL);
/* CPU control */
ppc405ep_cpc_init(env, clk_setup, s->sysclk);
@@ -1464,16 +1478,16 @@ static void ppc405_soc_realize(DeviceState *dev, Error
**errp)
/* Universal interrupt controller */
s->uic = qdev_new(TYPE_PPC_UIC);
-object_property_set_link(OBJECT(s->uic), "cpu", OBJECT(s->cpu),
+object_property_set_link(OBJECT(s->uic), "cpu", OBJECT(>cpu),
_fatal);
if (!sysbus_realize(SYS_BUS_DEVICE(s->uic), errp)) {
return;
}
sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_INT,
- qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_INT));
+ qdev_get_gpio_in(DEVICE(>cpu), PPC40x_INPUT_INT));
sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_CINT,
- qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_CINT));
+ qdev_get_gpio_in(DEVICE(>cpu), PPC40x_INPUT_CINT));
/* SDRAM controller */
/* XXX 405EP has no ECC interrupt */
@@ -1562,6 +1576,7 @@ static const TypeInfo ppc405_types[] = {
.name = TYPE_PPC405_SOC,
.parent = TYPE_DEVICE,
.instance_size = sizeof(Ppc405SoCState),
+.instance_init = ppc405_soc_instance_init,
.class_init = ppc405_soc_class_init,
}
};
diff --git a/hw/ppc/ppc4xx_devs.c
[PATCH v3 07/22] ppc/ppc405: QOM'ify CPU
Drop the use of ppc4xx_init() and duplicate a bit of code related to
clocks in the SoC realize routine. We will clean that up in the
following patches.
ppc_dcr_init() simply allocates default DCR handlers for the CPU. Maybe
this could be done in model initializer of the CPU families needing it.
Reviewed-by: Daniel Henrique Barboza
Signed-off-by: Cédric Le Goater
---
hw/ppc/ppc405.h | 2 +-
include/hw/ppc/ppc4xx.h | 1 +
hw/ppc/ppc405_boards.c | 2 +-
hw/ppc/ppc405_uc.c | 35 +--
hw/ppc/ppc4xx_devs.c| 2 +-
5 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index dc862bc8614c..8cc76cc8b3fe 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -79,7 +79,7 @@ struct Ppc405SoCState {
hwaddr ram_size;
uint32_t sysclk;
-PowerPCCPU *cpu;
+PowerPCCPU cpu;
DeviceState *uic;
};
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index 980f964b5a91..021376c2d260 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,6 +29,7 @@
#include "exec/memory.h"
/* PowerPC 4xx core initialization */
+void ppc4xx_reset(void *opaque);
PowerPCCPU *ppc4xx_init(const char *cpu_model,
clk_setup_t *cpu_clk, clk_setup_t *tb_clk,
uint32_t sysclk);
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 0b39ff08bd65..5ba12d60bc00 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -313,7 +313,7 @@ static void ppc405_init(MachineState *machine)
/* Load ELF kernel and rootfs.cpio */
} else if (kernel_filename && !machine->firmware) {
-boot_from_kernel(machine, ppc405->soc.cpu);
+boot_from_kernel(machine, >soc.cpu);
}
}
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index abcc2537140c..fa3853df2233 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1432,22 +1432,36 @@ static void ppc405ep_cpc_init (CPUPPCState *env,
clk_setup_t clk_setup[8],
#endif
}
+static void ppc405_soc_instance_init(Object *obj)
+{
+Ppc405SoCState *s = PPC405_SOC(obj);
+
+object_initialize_child(obj, "cpu", >cpu,
+POWERPC_CPU_TYPE_NAME("405ep"));
+}
+
static void ppc405_soc_realize(DeviceState *dev, Error **errp)
{
Ppc405SoCState *s = PPC405_SOC(dev);
-clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup;
+clk_setup_t clk_setup[PPC405EP_CLK_NB];
qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4];
CPUPPCState *env;
memset(clk_setup, 0, sizeof(clk_setup));
/* init CPUs */
-s->cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405ep"),
- _setup[PPC405EP_CPU_CLK],
- _clk_setup, s->sysclk);
-env = >cpu->env;
-clk_setup[PPC405EP_CPU_CLK].cb = tlb_clk_setup.cb;
-clk_setup[PPC405EP_CPU_CLK].opaque = tlb_clk_setup.opaque;
+if (!qdev_realize(DEVICE(>cpu), NULL, errp)) {
+return;
+}
+qemu_register_reset(ppc4xx_reset, >cpu);
+
+env = >cpu.env;
+
+clk_setup[PPC405EP_CPU_CLK].cb =
+ppc_40x_timers_init(env, s->sysclk, PPC_INTERRUPT_PIT);
+clk_setup[PPC405EP_CPU_CLK].opaque = env;
+
+ppc_dcr_init(env, NULL, NULL);
/* CPU control */
ppc405ep_cpc_init(env, clk_setup, s->sysclk);
@@ -1464,16 +1478,16 @@ static void ppc405_soc_realize(DeviceState *dev, Error
**errp)
/* Universal interrupt controller */
s->uic = qdev_new(TYPE_PPC_UIC);
-object_property_set_link(OBJECT(s->uic), "cpu", OBJECT(s->cpu),
+object_property_set_link(OBJECT(s->uic), "cpu", OBJECT(>cpu),
_fatal);
if (!sysbus_realize(SYS_BUS_DEVICE(s->uic), errp)) {
return;
}
sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_INT,
- qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_INT));
+ qdev_get_gpio_in(DEVICE(>cpu), PPC40x_INPUT_INT));
sysbus_connect_irq(SYS_BUS_DEVICE(s->uic), PPCUIC_OUTPUT_CINT,
- qdev_get_gpio_in(DEVICE(s->cpu), PPC40x_INPUT_CINT));
+ qdev_get_gpio_in(DEVICE(>cpu), PPC40x_INPUT_CINT));
/* SDRAM controller */
/* XXX 405EP has no ECC interrupt */
@@ -1562,6 +1576,7 @@ static const TypeInfo ppc405_types[] = {
.name = TYPE_PPC405_SOC,
.parent = TYPE_DEVICE,
.instance_size = sizeof(Ppc405SoCState),
+.instance_init = ppc405_soc_instance_init,
.class_init = ppc405_soc_class_init,
}
};
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index 737c0896b4f8..f20098cf417c 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -37,7 +37,7 @@
#include "qapi/error.h"
#include "trace.h"
-static void ppc4xx_reset(void *opaque)
+void ppc4xx_reset(void *opaque)
{
PowerPCCPU *cpu = opaque;
--
2.37.1
