qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 05/22] ppc/ppc405: Introduce a PPC405 SoC


From: Cédric Le Goater
Subject: Re: [PATCH v3 05/22] ppc/ppc405: Introduce a PPC405 SoC
Date: Mon, 8 Aug 2022 15:51:12 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0

On 8/8/22 14:43, BALATON Zoltan wrote:
On Mon, 8 Aug 2022, Cédric Le Goater wrote:
It is an initial model to start QOMification of the PPC405 board.
QOM'ified devices will be reintroduced one by one. Start with the
memory regions, which name prefix is changed to "ppc405".

Also, initialize only one RAM bank. The second bank is a dummy one
(zero size) which is here to match the hard coded number of banks in
ppc405ep_init().

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/ppc/ppc405.h        | 16 ++++++++++++++++
hw/ppc/ppc405_boards.c | 23 ++++++++++++-----------
hw/ppc/ppc405_uc.c     | 40 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index 83f156f585c8..66dc21cdfed8 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -25,6 +25,7 @@
#ifndef PPC405_H
#define PPC405_H

+#include "qom/object.h"
#include "hw/ppc/ppc4xx.h"

#define PPC405EP_SDRAM_BASE 0x00000000
@@ -62,6 +63,21 @@ struct ppc4xx_bd_info_t {
    uint32_t bi_iic_fast[2];
};

+#define TYPE_PPC405_SOC "ppc405-soc"
+OBJECT_DECLARE_SIMPLE_TYPE(Ppc405SoCState, PPC405_SOC);
+
+struct Ppc405SoCState {
+    /* Private */
+    DeviceState parent_obj;
+
+    /* Public */
+    MemoryRegion ram_banks[2];
+    hwaddr ram_bases[2], ram_sizes[2];
+
+    MemoryRegion *dram_mr;
+    hwaddr ram_size;
+};
+
/* PowerPC 405 core */
ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, ram_addr_t ram_size);

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index c6fa559b03d9..1dc5065fcc1d 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -57,6 +57,8 @@ struct Ppc405MachineState {
    /* Private */
    MachineState parent_obj;
    /* Public */
+
+    Ppc405SoCState soc;
};

/*****************************************************************************/
@@ -232,11 +234,10 @@ static void boot_from_kernel(MachineState *machine, 
PowerPCCPU *cpu)

static void ppc405_init(MachineState *machine)
{
+    Ppc405MachineState *ppc405 = PPC405_MACHINE(machine);
    MachineClass *mc = MACHINE_GET_CLASS(machine);
    const char *kernel_filename = machine->kernel_filename;
    PowerPCCPU *cpu;
-    MemoryRegion *ram_memories = g_new(MemoryRegion, 2);
-    hwaddr ram_bases[2], ram_sizes[2];
    MemoryRegion *sysmem = get_system_memory();
    DeviceState *uicdev;

@@ -247,16 +248,16 @@ static void ppc405_init(MachineState *machine)
        exit(EXIT_FAILURE);
    }

-    /* XXX: fix this */
-    memory_region_init_alias(&ram_memories[0], NULL, "ef405ep.ram.alias",
-                             machine->ram, 0, machine->ram_size);
-    ram_bases[0] = 0;
-    ram_sizes[0] = machine->ram_size;
-    memory_region_init(&ram_memories[1], NULL, "ef405ep.ram1", 0);
-    ram_bases[1] = 0x00000000;
-    ram_sizes[1] = 0x00000000;
+    object_initialize_child(OBJECT(machine), "soc", &ppc405->soc,
+                            TYPE_PPC405_SOC);
+    object_property_set_uint(OBJECT(&ppc405->soc), "ram-size",
+                             machine->ram_size, &error_fatal);
+    object_property_set_link(OBJECT(&ppc405->soc), "dram",
+                             OBJECT(machine->ram), &error_abort);
+    qdev_realize(DEVICE(&ppc405->soc), NULL, &error_abort);

-    cpu = ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
+    cpu = ppc405ep_init(sysmem, ppc405->soc.ram_banks, ppc405->soc.ram_bases,
+                        ppc405->soc.ram_sizes,
                        33333333, &uicdev, kernel_filename == NULL ? 0 : 1);

    /* allocate and load BIOS */
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index d6420c88d3a6..adadb3a0ae08 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -30,6 +30,7 @@
#include "hw/ppc/ppc.h"
#include "hw/i2c/ppc4xx_i2c.h"
#include "hw/irq.h"
+#include "hw/qdev-properties.h"
#include "ppc405.h"
#include "hw/char/serial.h"
#include "qemu/timer.h"
@@ -1530,3 +1531,42 @@ PowerPCCPU *ppc405ep_init(MemoryRegion 
*address_space_mem,

    return cpu;
}
+
+static void ppc405_soc_realize(DeviceState *dev, Error **errp)
+{
+    Ppc405SoCState *s = PPC405_SOC(dev);
+
+    /* Initialize only one bank */
+    s->ram_bases[0] = 0;
+    s->ram_sizes[0] = s->ram_size;
+    memory_region_init_alias(&s->ram_banks[0], OBJECT(s),
+                             "ppc405.sdram0", s->dram_mr,
+                             s->ram_bases[0], s->ram_sizes[0]);
+}
+
+static Property ppc405_soc_properties[] = {
+    DEFINE_PROP_LINK("dram", Ppc405SoCState, dram_mr, TYPE_MEMORY_REGION,
+                     MemoryRegion *),
+    DEFINE_PROP_UINT64("ram-size", Ppc405SoCState, ram_size, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};

I'm not sure why we need to duplicate these in the soc if they are always the same as machine->ram and machine->ram_size.

There are machines with multi SoCs. PowerNV can have 16.

But that's not the reason here, we pass the dram memory region to the SoC
for controllers that might need it for memory transactions, typically DMAs.
In this case, it's the SDRAM controller which creates slices of RAM for
each available RAM bank.

Is it theoretically possible to have a soc where it uses ram that's not the whole ram?

Yes.

If not then you could just uose machine which is accessible either as current_machine or qdev_get_machine()

These are QEMU globals. We should avoid using them in device models.

(also the parent of out soc but we don't know how to get that). If setting ram 
this way
is still desired do we separately need to set its size or we could use memory_region_size() instead?

Let's keep SDRAM modeling for the next patchset.

+
+static void ppc405_soc_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = ppc405_soc_realize;
+    dc->user_creatable = false;

May need a comment explaining why user_creatable = false. (Also for all other 
similar lines in other patches, I won't repeat this there.)

TYPE_PPC405_SOC devices can not be created from the QEMU command line.
it doesn't make sense. That's what 'dc->user_creatable = false' means.

Thanks,

C.


Regards,
BALATON Zoltan

+    device_class_set_props(dc, ppc405_soc_properties);
+}
+
+static const TypeInfo ppc405_types[] = {
+    {
+        .name           = TYPE_PPC405_SOC,
+        .parent         = TYPE_DEVICE,
+        .instance_size  = sizeof(Ppc405SoCState),
+        .class_init     = ppc405_soc_class_init,
+    }
+};
+
+DEFINE_TYPES(ppc405_types)





reply via email to

[Prev in Thread] Current Thread [Next in Thread]