Re: [PATCH v1 4/5] KVM: s390: refactor and split some gmap helpers

[Nina Schoetterl-Glausch <nsg@linux.ibm.com>]

On Wed, 2025-05-21 at 17:41 +0200, Claudio Imbrenda wrote:
> On Wed, 21 May 2025 17:30:00 +0200
> Nina Schoetterl-Glausch <nsg@linux.ibm.com> wrote:
> 
> > On Wed, 2025-05-21 at 17:19 +0200, Claudio Imbrenda wrote:
> > > On Wed, 21 May 2025 16:55:18 +0200
> > > Nina Schoetterl-Glausch <nsg@linux.ibm.com> wrote:
> > >   
> > > > On Wed, 2025-05-14 at 18:38 +0200, Claudio Imbrenda wrote:  
> > > > > Refactor some gmap functions; move the implementation into a separate
> > > > > file with only helper functions. The new helper functions work on vm
> > > > > addresses, leaving all gmap logic in the gmap functions, which mostly
> > > > > become just wrappers.
> > > > > 
> > > > > The whole gmap handling is going to be moved inside KVM soon, but the
> > > > > helper functions need to touch core mm functions, and thus need to
> > > > > stay in the core of kernel.
> > > > > 
> > > > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > > > > ---
> > > > >  MAINTAINERS                          |   2 +
> > > > >  arch/s390/include/asm/gmap_helpers.h |  18 ++
> > > > >  arch/s390/kvm/diag.c                 |  11 +-
> > > > >  arch/s390/kvm/kvm-s390.c             |   3 +-
> > > > >  arch/s390/mm/Makefile                |   2 +
> > > > >  arch/s390/mm/gmap.c                  |  46 ++---
> > > > >  arch/s390/mm/gmap_helpers.c          | 266 +++++++++++++++++++++++++++
> > > > >  7 files changed, 307 insertions(+), 41 deletions(-)
> > > > >  create mode 100644 arch/s390/include/asm/gmap_helpers.h
> > > > >  create mode 100644 arch/s390/mm/gmap_helpers.c
> > > > >     
> > [...]
> > 
> > > > > +void __gmap_helper_zap_one(struct mm_struct *mm, unsigned long vmaddr)    
> > > > 
> > > > __gmap_helper_zap_mapping_pte ?  
> > > 
> > > but I'm not taking a pte as parameter  
> > 
> > The pte being zapped is the one mapping vmaddr, right?
> 
> I don't know, _pte kinda sounds to me as the function would be taking a
> pte as parameter

__gmap_helper_zap_pte_mapping_addr ?

IMO __gmap_helper_zap_one is rather vague. Zap one? Which one?

[...]

> > > 
> > > > The stuff below is from arch/s390/mm/gmap.c right?
> > > > Are you going to delete it from there?  
> > > 
> > > not in this series, but the next series will remove mm/gmap.c altogether  
> > 
> > Can't you do it with this one?
> 
> if you mean removing mm/gmap.c, no. I would need to push the whole gmap
> rewrite series, which is not ready yet.
> 
> if you mean removing the redundant functions... I guess I could

The latter. I think that would be cleaner.
> 
> > 
> > 
> > [...]

-- 
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294