Re: [Qemu-devel] [PATCH] target-ppc: Add POWER8E_v2.1 CPU model.

[Alexey Kardashevskiy]

On 07/09/2015 08:03 PM, Andrea Bolognani wrote:
> On Thu, 2015-07-09 at 12:35 +1000, Alexey Kardashevskiy wrote:
> > > Does "all versions of POWER8" include things like POWER8E,
> > > POWER8NVL
> > > and "POWER8 DD1", as one of the variations is known in the kernel
> > > source? Can we safely migrate guests eg. from a POWER8 v1.0 host to
> > > a POWER8E v2.1 host?
> >
> > Yes. afaik the only difference between POWER8 and POWER8E is how many
> > cores
> > are packed into an actual chip.
>
> If I'm reading the kernel source[1] correctly, there are actually
> subtle
> differences other than the number of cores:
>
>    #define CPU_FTRS_POWER8 (/* Bunch of features here */)
>    #define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)


POWER8E was the first family of POWER8. Some revisions of POWER8E have this 
bug. The bug is that if we migrate from a good POWER8 to POWER8 with this 
bug, perf counters will stop working as the source kernel did not detect 
broken CPU and did not enable a workaround. We do not really know how many 
of this chips are out there (not many I believe) and how many have this 
bug. And also I guess that more people will be annoyed by inability to 
migrate rather than by broken perf.

I'd leave migration enabled but it is worth mentioning somewhere in user's 
guide that if the user migrates a guest to POWER8E with the specific PVR, 
perf might break.


>    #define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)

This bug appears in POWER8E and POWER8 1.0 ("DD1"). The bug is that when a 
CPU thread wakes up after nap because of doorbell message, the doorbell 
interrupt is not pending. Guests cannot do nap themselves (they use H_CEDE 
hypercall for this), when a thread wakes up, it wakes in the host context 
so there is nothing to handle in the guest, it is purely for KVM to workaround.




> Whether or not these differences will cause issues, I have no idea :)
>
> > >   From libvirt's point of view, it would be nice to be able to
> > > identify
> > > as "POWER8" anything that looks like it, by matching the host's PVR
> > > agains a numer of known PVRs (with relative bitmask). Ideally, if
> > > the
> > > host can be identified as POWER8, that's the only CPU model libvirt
> > > should advertise...
> >
> > I have a very little idea about libvirt here. QEMU considers
> > everything
> > with 0x004dxxxx and 0x004bxxxx as POWER8 (ppc_pvr_match_power8()
> > helper)
> > and supports migration between these.
>
> Looking again at the kernel source[2]:
>
>    {   /* Power8E */
>        .pvr_mask               = 0xffff0000,
>        .pvr_value              = 0x004b0000,
>        .cpu_name               = "POWER8E (raw)",
>        .cpu_features           = CPU_FTRS_POWER8E,
>        .cpu_user_features      = COMMON_USER_POWER8,
>        .cpu_user_features2     = COMMON_USER2_POWER8,
>        .mmu_features           = MMU_FTRS_POWER8,
>        .icache_bsize           = 128,
>        .dcache_bsize           = 128,
>        .num_pmcs               = 6,
>        .pmc_type               = PPC_PMC_IBM,
>        .oprofile_cpu_type      = "ppc64/power8",
>        .oprofile_type          = PPC_OPROFILE_INVALID,
>        .cpu_setup              = __setup_cpu_power8,
>        .cpu_restore            = __restore_cpu_power8,
>        .flush_tlb              = __flush_tlb_power8,
>        .machine_check_early    = __machine_check_early_realmode_p8,
>        .platform               = "power8",
>    },
>    {   /* Power8NVL */
>        .pvr_mask               = 0xffff0000,
>        .pvr_value              = 0x004c0000,
>        .cpu_name               = "POWER8NVL (raw)",
>        .cpu_features           = CPU_FTRS_POWER8,
>        /* Same as above */
>    },
>    {   /* Power8 DD1: Does not support doorbell IPIs */
>        .pvr_mask               = 0xffffff00,
>        .pvr_value              = 0x004d0100,
>        .cpu_name               = "POWER8 (raw)",
>        .cpu_features           = CPU_FTRS_POWER8_DD1,
>        /* Same as above */
>    },
>    {   /* Power8 */
>        .pvr_mask               = 0xffff0000,
>        .pvr_value              = 0x004d0000,
>        .cpu_name               = "POWER8 (raw)",
>        .cpu_features           = CPU_FTRS_POWER8,
>        /* Same as above */
>    },
>
> So the PVR matching, as done currently in QEMU, will include POWER8DD1
> but exclude POWER8NVL, which according to to commit ddee09c0 and the
> code
> above is absolutely identical to POWER8.


Yeah, we have to add 0x004c0000 into the POWER8 family as well, I'll 
doublecheck.


> libvirt currently considers the guest CPU model to be compatible with
> the host CPU model if both have the same PVR, which is clearly far from
> optimal.
>
> If we can rely on the above CPU families, as identified by pvr_value
> and pvr_mask, to behave exactly the same for our purposes[3], then we
> can change libvirt to perform the same kind of PVR matching as QEMU and
> report to the user that the guest CPU model POWER8 is compatible with
> all of the above host CPU models.
>
> > I am adding Shiva to the coversation, he might enlighen us how this
> > is
> > solved by powerkvm's libvirt.
>
> Sure, the more the merrier :)
>
> Cheers.
>
>
> [1] arch/powerpc/include/asm/cputable.h
> [2] arch/powerpc/kernel/cputable.c
> [3] eg. a guest running on a POWER8E host can be migrated to a
>      POWER8DD1 host, despite the fact that the former has
> CPU_FTR_PMAO_BUG
>      and the latter lacks CPU_FTR_DBELL.


--
Alexey