[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [0/8] RFC: target-ppc: Start disentangling different MMU
From: |
Alexander Graf |
Subject: |
Re: [Qemu-ppc] [0/8] RFC: target-ppc: Start disentangling different MMU types |
Date: |
Fri, 22 Feb 2013 17:15:22 +0100 |
On 12.02.2013, at 03:00, David Gibson wrote:
> 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.
Looks like a nice and long overdue cleanup. A few points I'd like to see
addressed:
- Instead of exporting common helper functions, put them into a new
mmu-hash.h as static inline. That way things can get optimized a lot better.
- Please extract the 32bit hash functions into a separate file as well.
You're touching all that code anyways, so I'm sure you can test it :).
- QOM'ify things please :). Just make your mmu type specific function calls
function pointers in the class and invoke them from there instead of the
switchery.
Alex