Re: [PATCH v5 2/5] target/ppc: Add Power11 DD2.0 processor

[Aditya Gupta]

Hi Nick,


On 23/07/24 10:00, Nicholas Piggin wrote:
> > <...snip...>
> >
> > +    { /* POWER11, ISA3.10 */
> > +        .name = "power11",
> > +        .pvr = CPU_POWERPC_LOGICAL_3_10_PLUS,
> Might call that _P11 rather than _PLUS, but I can fold that in my tree.
Sure, makes sense, I can make these changes, and send a v2 soon.
> > <...snip...>
> >
> > +
> > +POWERPC_FAMILY(POWER11)(ObjectClass *oc, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(oc);
> > +    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> > +
> > +    dc->fw_name = "PowerPC,POWER11";
> > +    dc->desc = "POWER11";
> > +    pcc->pvr_match = ppc_pvr_match_power11;
> > +    pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
> > +    pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
> > +    pcc->init_proc = init_proc_POWER10;
> > +    pcc->check_pow = check_pow_nocheck;
> > +    pcc->check_attn = check_attn_hid0_power9;
> > +    pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
> > +    pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
> > +    pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
> > +    pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
> BTW., I still think all these new macros should be named after the exact
> CPU, e.g., all these should be called POWER11 and the differences or
> sameness should be handled in cpu_init.h.

Got it, can create macros for the Power11 things also.


Regarding this:

> +    pcc->check_attn = check_attn_hid0_power9;
> +    pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */

Should I keep them same, or have *_POWER11_* counterparts ?
> I might tweak that and the names a bit locally (e.g., why is one type of
> define called POWERPC_FAMILY_x and another called POWERPC_x_PCC), but
> that's not a big deal and mostly an exercise in bike shed painting. The
> functionality of the patch looks okay.
I am okay if you want to do it, or i can do it in a separate follow up 
patch.
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Thanks for the tag Nick !


- Aditya Gupta

> > +
> > +    pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
> > +    pcc->mmu_model = POWERPC_MMU_3_00;
> > +#if !defined(CONFIG_USER_ONLY)
> > +    /* segment page size remain the same */
> > +    pcc->hash64_opts = &ppc_hash64_opts_POWER7;
> > +    pcc->radix_page_info = &POWER10_radix_page_info;
> > +    pcc->lrg_decr_bits = 56;
> > +#endif
> > +    pcc->excp_model = POWERPC_EXCP_POWER10;
> > +    pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
> > +    pcc->bfd_mach = bfd_mach_ppc64;
> > +    pcc->flags = POWERPC_POWER10_PCC_FLAGS;
> > +    pcc->l1_dcache_size = 0x8000;
> > +    pcc->l1_icache_size = 0x8000;
> > +}
> > +
> >   #if !defined(CONFIG_USER_ONLY)
> >   void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
> >   {