Re: [PATCH 09/11] include/exec: added functions to the stubs in exec-all.h

[David Gibson]

On Mon, May 17, 2021 at 01:59:35PM -0300, Lucas Mateus Martins Araujo e Castro 
wrote:
> 
> On 17/05/2021 00:58, David Gibson wrote:
> > On Thu, May 13, 2021 at 06:44:01PM -0500, Richard Henderson wrote:
> > 65;6401;1c> On 5/13/21 9:03 AM, Lucas Mateus Martins Araujo e Castro wrote:
> > > > tlb_set_page is called by many ppc_hash64_handle_mmu_fault,
> > > > ppc_radix64_handle_mmu_fault and ppc_hash32_handle_mmu_fault, all of
> > > > which from what I've seen are only used inside #if
> > > > defined(CONFIG_SOFTMMU).
> > > tlb_set_page should only be called from one place: ppc_cpu_tlb_fill.  The
> > > other functions should fill in data, much like get_physical_address.
> > > 
> > > 
> > > > So what is the best way to deal with these tlb_set_page calls? Should
> > > > these part of the _handle_mmu_fault functions never be reached or should
> > > > these functions never be called?
> > > There is some duplication between get_physical_address* and
> > > *handle_mmu_fault that should be fixed.
> > > 
> > > What should be happening is that you have one function (per mmu type) that
> > > takes a virtual address and resolves a physical address. This bit of code
> > > should be written so that it is usable by both
> > > CPUClass.get_phys_page_attrs_debug and TCGCPUOps.tlb_fill.  It appears as 
> > > if
> > > ppc_radix64_xlate is the right interface for this.
> > > 
> > > It appears that real mode handling is duplicated between hash64 and 
> > > radix64,
> > > which could be unified.
> > Any common handling between the hash and radix MMUs should go in
> > mmu-book3s-v3.*  That covers common things across the v3 (POWER9 and
> > later) MMUs which includes both hash and radix mode.
> 
> I'm not completely sure how this should be handled, there's a
> get_physical_address in mmu_helper.c but it's a static function and divided
> by processor families instead of MMU types, so get_physical_address_* should
> be a new function?

Sorry, I wasn't clear.  mmu-v3 is only for things specifically common
to the hash64 model (as implemented in mmu-hash64.c) and the radix
model (as implemented in mmu-radix64.c).  Which basically means things
related to the POWER9 MMU which can switch between those modes.

Things common to *more* MMU models (hash32, 4xx, 8xx, BookE, etc.)
which includes most of what's in mmu-helper.c go elsewhere.

> The new get_physical_address_* function would be a mmu-hash(32|64) that do
> something like ppc_radix64_xlate and add a function to mmu-book3s-v3 that
> call either the radix64 or the hash64 function and also handle real mode
> access.
> 
> Also should the tlb_set_page calls in *_handle_mmu_fault be changed to
> ppc_cpu_tlb_fill or the function should themselves fill it?
> 
> > 
> > > You should only call tlb_set_page from TCGCPUOps.tlb_fill, aka
> > > ppc_cpu_tlb_fill.  TCGCPUOps.tlb_fill is obviously TCG only.
> > > 
> > > The version you are looking at here is system emulation specific (sysemu,
> > > !defined(CONFIG_USER_ONLY)).  There is a second version of this function,
> > > with the same signature, that is used for user emulation in the helpfully
> > > named user_only_helper.c.
> > > 
> > > 
> > > r~
> > > 

-- 
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

signature.asc
Description: PGP signature