Re: [Qemu-devel] [Qemu-ppc] [PATCH 11/13] pseries: Fixes and enhancements to L1 cache properties

[David Gibson]

On Mon, Dec 17, 2012 at 11:10:12AM +0100, Alexander Graf wrote:
> 
> 
> On 17.12.2012, at 03:32, David Gibson <address@hidden> wrote:
> 
> > On Thu, Dec 13, 2012 at 01:50:25PM +0100, Alexander Graf wrote:
> >> 
> >> On 04.12.2012, at 03:42, David Gibson wrote:
> >> 
> >>> PAPR requires that the device tree's CPU nodes have several properties
> >>> with information about the L1 cache.  We created two of these
> >>> properties, but with incorrect names - "[id]cache-block-size" instead
> >>> of "[id]-cache-block-size" (note the extra hyphen).
> >>> 
> >>> We were also missing some of the required cache properties.  This
> >>> patch adds the [id]-cache-line-size properties (which have the same
> >>> values as the block size properties in all current cases).  We also
> >>> add the [id]-cache-size properties.  The latter requires some extra
> >>> infrastructure in the general target-ppc code to (optionally) set the
> >>> cache sizes for various CPUs.  We obtain the published values either
> >>> from there, or from the host when KVM is in use.
> >>> 
> >>> Signed-off-by: David Gibson <address@hidden>
> >>> ---
> >>> hw/spapr.c                  |   20 ++++++++++++++++++--
> >>> target-ppc/cpu.h            |    1 +
> >>> target-ppc/kvm.c            |   10 ++++++++++
> >>> target-ppc/kvm_ppc.h        |   12 ++++++++++++
> >>> target-ppc/translate_init.c |    4 ++++
> >>> 5 files changed, 45 insertions(+), 2 deletions(-)
> >>> 
> >>> diff --git a/hw/spapr.c b/hw/spapr.c
> >>> index d23aa9d..3bacf2f 100644
> >>> --- a/hw/spapr.c
> >>> +++ b/hw/spapr.c
> >>> @@ -315,6 +315,10 @@ static void *spapr_create_fdt_skel(const char 
> >>> *cpu_model,
> >>>                           0xffffffff, 0xffffffff};
> >>>        uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : 
> >>> TIMEBASE_FREQ;
> >>>        uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 
> >>> 1000000000;
> >>> +        int dcache_size = kvm_enabled() ? kvmppc_get_dcache_size()
> >>> +            : env->l1_dcache_size;
> >>> +        int icache_size = kvm_enabled() ? kvmppc_get_icache_size()
> >>> +            : env->l1_icache_size;
> >> 
> >> By default with KVM we use -cpu host, right? So we already should
> >> get the correct cache sizes for the CPU you're on.
> > 
> > Um.. sort of.  The first problem with that is that I only just added
> > the cache size information to qemu, so only a few CPUs currently
> > populate that information.  Using the host info means we can get the
> > right information even for CPUs that don't yet have cache info in
> > qemu.
> > 
> >> Imagine we would support the compatibility feature where you could
> >> run with -cpu POWER6 on a POWER7 machine. Would exposing the POWER6
> >> cache size rather than the host's make any real difference to the
> >> guest? Or would it work nevertheless?
> > 
> > The second problem is that there may be circumstances where the
> > cache size is altered from the normal size for the cpu.  Running in
> > POWER6 compat mode
> 
> Well, either we want to be compatible or we don't :). If we run with
> -cpu POWER6 we want to generate the same dt as we did on a POWER6
> system itself.

Hrm.  Ok.

So, the remaining difficulty I have with that is that for -cpu HOST we
should still take the cache sizes from the host, but that can't easily
be done because they're only stored in the env, not the cpu_def.

> Look at it from one step ahead. Take POWER8 and POWER7. I want to be
> able to live migrate from a POWER7 system to a POWER8 system. On my
> POWER8 box, the generated dt should look like it did on my POWER7
> box.

So.. this is kind of an aside at this point but POWER6/POWER7/POWER8
compatibility is more complicated than that.  For TCG or KVM-PR, -cpu
POWER6 should be possible, and pretty much identical to a real POWER6.
For KVM HV, the best we can do is POWER7's POWER6 compatibility mode,
which is kinda-similar to POWER6, but still a fair way from identical
(for starters the POWER7 PVR value is still visible).  It should be
close enough to run (PAPR compliant)  OSes that know about POWER6 but
not POWER7 - but only if they use the device tree the way they're
supposed to (which includes amongst other things a special "virtual
PVR" value for this quasi-POWER6).  And as long as they don't use the
performance counters.

As far as (HV) KVM is concerned our plan is to trigger the POWER6
compatibility mode by setting the relevant virtual PVR value in
SET_SREGS.  For qemu it's a bit less clear - we could add a cputable
entry for the virtual PVR - -cpu POWER7_POWER6 or something.  Or
alternatively we could treat the compatibility mode as that - a
special mode bit in the env.  The latter has the advantage that we can
use the PAPR defined CAS system to see what processors the guest OS
advertises support for and automatically switch on the compat mode
when necessary.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson