Re: [Qemu-ppc] Cache sizes and device tree

[David Gibson]

On Mon, Nov 19, 2012 at 11:38:35PM +0100, Alexander Graf wrote:
> 
> On 19.11.2012, at 23:05, David Gibson wrote:
> 
> > On Mon, Nov 19, 2012 at 12:32:51PM +0100, Alexander Graf wrote:
> >> 
> >> On 16.11.2012, at 04:36, David Gibson wrote:
> >> 
> >>> Hi Alex,
> >>> 
> >>> At present, pseries doesn't set the i-cache-size and d-cache-size
> >>> properties on the device tree CPU nodes.  That's a bit of a pain,
> >>> because it means the guest kernel then doesn't provide some files in
> >>> /sys and that means that lscpu bombs out with an error.
> >>> 
> >>> So the question is what should we put in there.  For KVM it's
> >>> straightforward enough for us to find the host L1 cache sizes and put
> >>> those in the guest device tree.  However to make it work for full emu
> >>> as well, we'll need to add this cache size information to the qemu cpu
> >>> table somewhere.
> >> 
> >> Well, cache size should be modeled pretty much the same way as cache
> >> line size, right? Today, for cache line size, we just set the
> >> respective values in the cpu init functions.
> >> 
> >>> In practice, I think we need a way to add it incrementally - since I
> >>> really don't want to have to look up and confirm the L1 cache size for
> >>> every one of the hundreds of supported CPU models in order to make an
> >>> initial patch.
> >> 
> >> Sure. Just add a check if (env->cache_size_l1) and only set the dt
> >> property in that case ;). For KVM. override the env value similar to
> >> how we do it for the timebase.
> > 
> > Erm.. I'm not sure which timebase override you're referring to.
> 
> I am referring to kvmppc_get_tbfreq() :).

Hrm.  I was hoping not to have to explicitly test for kvm for this in
the machine, as is necessary for kvmppc_get_tbfreq).

> >>> I'm thinking it should probably go into ppc_def_t.  That will need a
> >>> new DEF macro which includes the cache sizes, and then there will be a
> >>> somewhat ugly but manageable transition to the new macro as we add
> >>> cache sizes to various CPUs.  But then I'm not entirely sure how to
> >>> get to the information when we're building the devtree, since there
> >>> isn't actually an obvious link from the CPUPPCState to the ppc_def_t
> >>> it was created from.
> >> 
> >> I think just shoving it into env should be enough, no? Do we have
> >> any cache information that goes beyond vcpu level, so that we can
> >> not populate it in env?
> > 
> > Well.. I was thinking ppc_def_t because there's an obvious place to
> > retrieve the host value in the kvm case - kvmppc_host_cpu_def().  It's
> > not so clear to me where we would do that if it's only in env.
> 
> Well, usually everything in env gets populated by the init
> function. Our current -cpu host implementation doesn't allow us to
> override the cpu init function. We should allow it to do so.
> 
> Instead of calling the base_spec init function, we should call a
> host init function which calls the base_spec init function and then
> overrides env variables based on what we want to override, like
> cache size.

Hm, ok.  I could do it from kvmppc_fixup_cpu().  Does that seem
reasonable?

-- 
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