On Mon, 2025-09-01 at 09:25 +0200, Cédric Le Goater wrote:
> On 8/26/25 22:17, Glenn Miles wrote:
> > Adds a test machine for the IBM PPE42 processor, including a
> > DEC, FIT, WDT and 1MB of ram.
> > 
> > The purpose of this machine is only to provide a generic platform
> > for testing instructions of the recently  added PPE42 processor
> > model which is used extensively in the IBM Power9, Power10 and
> > future Power server processors.
> > 
> > Signed-off-by: Glenn Miles <[email protected]>
> > ---
> > 
> > Changes from previous version
> > - Added ppe42_machine.c to MAINTAINERS file with self as maintainer
> > 
> >   MAINTAINERS            |  6 ++++
> >   hw/ppc/Kconfig         |  9 ++++++
> >   hw/ppc/meson.build     |  2 ++
> >   hw/ppc/ppc_booke.c     |  7 ++++-
> >   hw/ppc/ppe42_machine.c | 69 ++++++++++++++++++++++++++++++++++++++++++
> >   include/hw/ppc/ppc.h   |  1 +
> >   6 files changed, 93 insertions(+), 1 deletion(-)
> >   create mode 100644 hw/ppc/ppe42_machine.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index a07086ed76..52fa303e0a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1531,6 +1531,12 @@ F: include/hw/pci-host/grackle.h
> >   F: pc-bios/qemu_vga.ndrv
> >   F: tests/functional/test_ppc_mac.py
> >   
> > +PPE42
> > +M: Glenn Miles <[email protected]>
> > +L: [email protected]
> > +S: Odd Fixes
> > +F: hw/ppc/ppe42_machine.c
> > +
> >   PReP
> >   M: Hervé Poussineau <[email protected]>
> >   L: [email protected]
> > diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> > index ced6bbc740..3fdea5919c 100644
> > --- a/hw/ppc/Kconfig
> > +++ b/hw/ppc/Kconfig
> > @@ -44,6 +44,15 @@ config POWERNV
> >       select SSI_M25P80
> >       select PNV_SPI
> >   
> > +config PPC405
> > +    bool
> > +    default y
> > +    depends on PPC
> > +    select M48T59
> > +    select PFLASH_CFI02
> > +    select PPC4XX
> > +    select SERIAL
> > +
> >   config PPC440
> >       bool
> >       default y
> > diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> > index 9893f8adeb..170b90ae7d 100644
> > --- a/hw/ppc/meson.build
> > +++ b/hw/ppc/meson.build
> > @@ -57,6 +57,8 @@ ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
> >     'pnv_n1_chiplet.c',
> >   ))
> >   # PowerPC 4xx boards
> > +ppc_ss.add(when: 'CONFIG_PPC405', if_true: files(
> > +  'ppe42_machine.c'))
> >   ppc_ss.add(when: 'CONFIG_PPC440', if_true: files(
> >     'ppc440_bamboo.c',
> >     'ppc440_uc.c'))
> > diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c
> > index 3872ae2822..13403a56b1 100644
> > --- a/hw/ppc/ppc_booke.c
> > +++ b/hw/ppc/ppc_booke.c
> > @@ -352,7 +352,12 @@ void ppc_booke_timers_init(PowerPCCPU *cpu, uint32_t 
> > freq, uint32_t flags)
> >       booke_timer = g_new0(booke_timer_t, 1);
> >   
> >       cpu->env.tb_env = tb_env;
> > -    tb_env->flags = flags | PPC_TIMER_BOOKE | PPC_DECR_ZERO_TRIGGERED;
> > +    if (flags & PPC_TIMER_PPE) {
> 
> PPC_TIMER_PPE definition should be introduced in its own patch.
> 
Ok, will do.

> 
> > +        /* PPE's use a modified version of the booke behavior */
> > +        tb_env->flags = flags | PPC_DECR_UNDERFLOW_TRIGGERED;
> > +    } else {
> > +        tb_env->flags = flags | PPC_TIMER_BOOKE | PPC_DECR_ZERO_TRIGGERED;
> > +    }
> >   
> >       tb_env->tb_freq    = freq;
> >       tb_env->decr_freq  = freq;
> > diff --git a/hw/ppc/ppe42_machine.c b/hw/ppc/ppe42_machine.c
> > new file mode 100644
> > index 0000000000..0bc295da28
> > --- /dev/null
> > +++ b/hw/ppc/ppe42_machine.c
> > @@ -0,0 +1,69 @@
> > +
> > +/*
> > + * Test Machine for the IBM PPE42 processor
> > + *
> > + * Copyright (c) 2025, IBM Corporation.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/units.h"
> > +#include "qemu/error-report.h"
> > +#include "system/address-spaces.h"
> > +#include "hw/boards.h"
> > +#include "hw/ppc/ppc.h"
> > +#include "system/system.h"
> > +#include "system/reset.h"
> > +#include "system/kvm.h"
> > +
> > +static void main_cpu_reset(void *opaque)
> > +{
> > +    PowerPCCPU *cpu = opaque;
> > +
> > +    cpu_reset(CPU(cpu));
> 
> There are no register settings ? Just asking
> 

Admittedly, I did not do much digging into the reset case.  I'll look
into it.

> > +}
> > +
> > +static void ppe42_machine_init(MachineState *machine)
> > +{
> > +    PowerPCCPU *cpu;
> > +    CPUPPCState *env;
> > +
> > +    if (kvm_enabled()) {
> > +        error_report("machine %s does not support the KVM accelerator",
> > +                     MACHINE_GET_CLASS(machine)->name);
> > +        exit(EXIT_FAILURE);
> > +    }
> > +
> > +    /* init CPU */
> > +    cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
> 
> I would introduce a specific MachineState for the ppe42 Machine:
> 
>    struct Ppe42MachineState {
>      /* Private */
>      MachineState parent_obj;
>      /* Public */
> 
>      PowerPCCPU cpu;
>    };
> 
> and use qdev_realize() too.
> 
> 

Sure, no problem.

> > +    env = &cpu->env;
> > +    if (PPC_INPUT(env) != PPC_FLAGS_INPUT_PPE42) {
> > +        error_report("Incompatible CPU, only PPE42 bus supported");
> 
> Can't we define valid_cpu_types instead ?
> 

Ah, I was not aware of that option.  Thanks, I'll give that a try.

> > +        exit(1);
> > +    }
> > +
> > +    qemu_register_reset(main_cpu_reset, cpu);
> > +
> > +    /* This sets the decrementer timebase */
> > +    ppc_booke_timers_init(cpu, 37500000, PPC_TIMER_PPE);
> > +
> > +    /* RAM */
> > +    if (machine->ram_size > 2 * GiB) {
> 
> 2GB RAM ? really ?
> 

Hmm, not sure what I was thinking there.  Since the ram starts at
address 0xfff80000, I'll change that to 512K.

Thanks for taking a look!

Glenn

> > +        error_report("RAM size more than 2 GiB is not supported");
> > +        exit(1);
> > +    }
> > +    memory_region_add_subregion(get_system_memory(), 0xfff80000, 
> > machine->ram);
> > +}
> > +
> > +
> > +static void ppe42_machine_class_init(MachineClass *mc)
> > +{
> > +    mc->desc = "PPE42 Test Machine";
> > +    mc->init = ppe42_machine_init;
> > +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("PPE42XM");
> > +    mc->default_ram_id = "ram";
> > +    mc->default_ram_size = 1 * MiB;
> > +}
> > +
> > +DEFINE_MACHINE("ppe42_machine", ppe42_machine_class_init)
> > diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
> > index 8a14d623f8..cb51d704c6 100644
> > --- a/include/hw/ppc/ppc.h
> > +++ b/include/hw/ppc/ppc.h
> > @@ -52,6 +52,7 @@ struct ppc_tb_t {
> >   #define PPC_DECR_UNDERFLOW_LEVEL     (1 << 4) /* Decr interrupt active 
> > when
> >                                                  * the most significant bit 
> > is 1.
> >                                                  */
> > +#define PPC_TIMER_PPE                (1 << 5) /* Enable PPE support */
> >   
> >   uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t 
> > tb_offset);
> >   void cpu_ppc_tb_init(CPUPPCState *env, uint32_t freq);


Reply via email to