Am 23.02.2013 um 09:28 schrieb David Gibson <[email protected]>:
> On Fri, Feb 22, 2013 at 05:15:22PM +0100, Alexander Graf wrote: >> >> 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. > > The common helpers are all gone again by the end of the series, so is > there really any point? Oh, really? I must have missed the bit where you remove / unexport them then ;). > >> - 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 :). > > Sigh. If I must. > >> - 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. > > Ok. I think that works best as an extension to the series, rather > than actually changing any of the existing patches. Sure, if it makes life easier for you you can certainly append the qomification to the series. Alex > > -- > 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
