Re: [PATCH 03/18] ppc/pnv: Add a pointer from PnvChip to PnvMachineState

[Cédric Le Goater]

On 7/11/24 16:18, Nicholas Piggin wrote:
> This helps avoid qdev_get_machine() calls.

I see two uses of this pointer in pnv_core_realize() :

    ...
    PnvMachineState *pnv = pc->chip->pnv_machine;
    ....
    pc->big_core = pnv->big_core;
    pc->lpar_per_core = pc->chip->pnv_machine->lpar_per_core;

Instead of this (ugly) machine pointer, why not add "lpar-per-core"
and "big-core" properties to PnvCore and simply set them in
pnv_chip_core_realize() ? I would prefer this solution.


Thanks,

C.



> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   include/hw/ppc/pnv_chip.h | 2 ++ 
>   hw/ppc/pnv.c              | 3 ++-
>   2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> index 4eaa7d3999..decfbc0ff7 100644
> --- a/include/hw/ppc/pnv_chip.h
> +++ b/include/hw/ppc/pnv_chip.h
> @@ -23,6 +23,8 @@ struct PnvChip {
>       SysBusDevice parent_obj;
>   
>       /*< public >*/
> +    PnvMachineState *pnv_machine;
> +
>       uint32_t     chip_id;
>       uint64_t     ram_start;
>       uint64_t     ram_size;
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 7878fed43c..3bcf11984c 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -2202,6 +2202,7 @@ static void pnv_chip_core_realize(PnvChip *chip, Error 
> **errp)
>           return;
>       }
>   
> +    chip->pnv_machine = pnv;
>       chip->cores = g_new0(PnvCore *, chip->nr_cores);
>   
>       for (i = 0, core_hwid = 0; (core_hwid < sizeof(chip->cores_mask) * 8)
> @@ -2614,7 +2615,7 @@ static void pnv_cpu_do_nmi(PnvChip *chip, PowerPCCPU 
> *cpu, void *opaque)
>   
>   static void pnv_nmi(NMIState *n, int cpu_index, Error **errp)
>   {
> -    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> +    PnvMachineState *pnv = PNV_MACHINE(n);
>       int i;
>   
>       for (i = 0; i < pnv->num_chips; i++) {