Re: [RFC PATCH v2 1/2] target/ppc: Moved functions out of mmu-hash64

[David Gibson <david@gibson.dropbear.id.au>]

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

On Wed, May 05, 2021 at 02:30:35PM -0300, Lucas Mateus Martins Araujo e Castro wrote:
> 
> On 03/05/2021 01:24, David Gibson wrote:
> > On Fri, Apr 30, 2021 at 03:40:46PM -0300, Lucas Mateus Castro (alqotel) wrote:
> > > The functions ppc_store_lpcr, ppc_hash64_filter_pagesizes and
> > > ppc_hash64_unmap_hptes have been moved to mmu-misc.h since they are
> > > not needed in a !TCG context and mmu-hash64 should not be compiled
> > > in such situation.
> > > 
> > > ppc_store_lpcr and ppc_hash64_filter_pagesizes are used by multiple
> > > functions, while ppc_hash64_unmap_hptes is used by rehash_hpt (in
> > > spapr_hcall.c).
> > Hmm.. looking at it, ppc_store_lpcr() (and helper_store_lpcr()) don't
> > really belong in this file at all.  The LPCR has some things related
> > to the hash MMU, but plenty of others that don't.  So, maybe
> > misc_helper.c?  That might have to be moved again, since misc_helper
> > itself should probably mostly not be used for !TCG.  But.. one thing
> > at a time.
> 
> I tested here and compiling misc_helper.c with disable-tcg it's kind of
> complicated and it would require many changes in it, so for this patch
> just move it there and deal with it in a later patch?

Yes, sounds reasonable.

> 
> > AFAICT the only user of ppc_hash64_filter_pagesizes() is in
> > spapr_caps.c.  For now you can just move it next to the caller, it's
> > debatable whether it belongs more to PAPR or MMU code.
> 
> Also I'm assuming the prototype should also be moved from
> "target/ppc/mmu-hash64.h" to "include/hw/ppc/spapr.h" (or some other
> spapr_*.h file), or should it be left in the original file?

Well, if you put it next to the caller you can make it static and
remove the prototype entirely.

> 
> > ppc_hash64_unmap_hptes() is definitely TCG only and should stay where
> > it is.  The call from rehash_hpt() can be solved because rehash_hpt()
> > itself is TCG only.  I've already suggested splitting the TCG (well,
> > softmmu) only things out from spapr_hcall.c, so it might simplify
> > things to tackle that first.
> > 
> 

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