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

From: David Gibson <david@gibson.dropbear.id.au>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: farosas@linux.ibm.com, qemu-devel@nongnu.org,
	Lucas Mateus Martins Araujo e Castro
	<lucas.araujo@eldorado.org.br>,
	fernando.valle@eldorado.org.br, qemu-ppc@nongnu.org,
	"Bruno Larsen \(billionai\)" <bruno.larsen@eldorado.org.br>,
	matheus.ferst@eldorado.org.br, luis.pires@eldorado.org.br
Subject: Re: [PATCH 09/11] include/exec: added functions to the stubs in exec-all.h
Date: Mon, 17 May 2021 13:58:03 +1000	[thread overview]
Message-ID: <YKHpy53AwM8tTy3E@yekko> (raw)
In-Reply-To: <aad60a0c-1f07-24ab-821a-193047e4ad2e@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 2088 bytes --]

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.

> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]