[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-ppc] [0/8] RFC: target-ppc: Start disentangling d
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [Qemu-ppc] [0/8] RFC: target-ppc: Start disentangling different MMU types |
Date: |
Wed, 13 Feb 2013 21:06:03 +0000 |
On Tue, Feb 12, 2013 at 10:40 AM, Andreas Färber <address@hidden> wrote:
> 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.
I'd also add that separating the MMU functions could help if one day
we introduce CPU model specific TCG memory access helpers.
>
> 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
>
- Re: [Qemu-devel] [Qemu-ppc] [PATCH 6/8] target-ppc: Rework get_physical_address(), (continued)
[Qemu-devel] [PATCH 8/8] target-ppc Disentangle ppc64 hash mmu path for cpu_ppc_handle_mmu_fault, David Gibson, 2013/02/11
[Qemu-devel] [PATCH 4/8] target-ppc: Disentangle 64-bit version of find_pte(), David Gibson, 2013/02/11
[Qemu-devel] [PATCH 2/8] target-ppc: Move SLB handling into a mmu-hash64.c, David Gibson, 2013/02/11
[Qemu-devel] [PATCH 5/8] target-ppc: Disentangle 64-bit version of get_segment(), David Gibson, 2013/02/11
Re: [Qemu-devel] [0/8] RFC: target-ppc: Start disentangling different MMU types, Andreas Färber, 2013/02/12
Re: [Qemu-devel] [0/8] RFC: target-ppc: Start disentangling different MMU types, Alexander Graf, 2013/02/22