Re: [Qemu-devel] [0/8] RFC: target-ppc: Start disentangling different MMU types

[Andreas Färber]

Am 12.02.2013 03:00, schrieb David Gibson:
> The target-ppc code supports CPUs with a number of different MMU
> types: there's both the 32-bit and 64-bit versions of the "classic"
> hash page table based powerpc mmu and there's also the BookE and 40x
> MMUs.
> 
> Currently handling of all these has a roughly shared path in
> mmu_helper.c.  Although code sharing is usually a good idea, in this
> case the MMUs really aren't similar enough for this to be useful.
> Instead it results in checking and behaving differently at many, many
> different points in the path leading to an unreadable tangle of code.
> 
> This patch series is a first step to cleaning this up, moving to a
> model where we have a single switch on the MMU family at the top-level
> entry points, then have a simpler, clearer separate code path for each
> MMU type.  More specifically, it disentangles the path for the 64-bit
> classic hash MMU into its own new file.  The other MMU types keep the
> existing code (minus 64-bit hash specific conditionals) for now.
> Disentangling those as well would be a good idea, but would be best
> done by someone with more resources to test those other platforms than
> I have.
> 
> For now, the resulting 64-bit hash path retains the same structure as
> the original shared code, just the obvious conditionals on other mmu
> types are removed.  This path is fairly ugly in itself, but cleaning
> that up is a later step, now much simpler without the other MMU types
> to deal with at the same time.

Some general comments: The idea of the ongoing QOM work just sent out is
to change the hierarchy from:

Object
- DeviceState
  - CPUState
    - PowerPCCPU
      - 970 vX.Y
      - POWER7 vX.Y
      ...

to:

Object
- DeviceState
  - CPUState
    - PowerPCCPU
      - 970 family
        - 970 vX.Y
      - POWER7 family
        - POWER7 vX.Y
      ...

PowerPCCPUClass is expected to grow methods overridden per family or
model. I.e., where sensible the class should serve as indirection for
which MMU/... implementation to choose rather than ifs or #ifdefs or
<prefix>_family glue sprinkled througout code.

As reminded repeatedly, please do not introduce new static or global
helper functions using CPUPPCState, use PowerPCCPU instead.

If you introduce global functions, please make them unique by using ppc_
prefix for arbitrary functions and ppc_cpu_ for functions taking
PowerPCCPU as first argument.

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg