On Tue, Dec 18, 2012 at 12:49:02AM +0100, Alexander Graf wrote:
>
> On 18.12.2012, at 00:00, David Gibson wrote:
>
> > On Mon, Dec 17, 2012 at 11:10:12AM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 17.12.2012, at 03:32, David Gibson <da...@gibson.dropbear.id.au> 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 <da...@gibson.dropbear.id.au>
> >>>>> ---
> >>>>> 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.
>
> Can we set a bit somewhere that allows us to do a sanity check
> later? After all, the values coming from the host and the values in
> the populated env really should just be identical for -cpu
> host. Every time they're not, it's simply a bug that needs to be
> reported.
That works. Although it's not obvious where to put the check and
fixup. kvmppc_fixup_cpu() seems like the obvious place, but that's no
good because it's called before the per-cpu-type init function, which
is what populates the expected cachesize values.
--
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
