Re: [PATCH v2 12/15] KVM: s390: move gmap_shadow_pgt_lookup() into kvm

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Mon, 20 Jan 2025 14:47:55 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 1/16/25 12:33 PM, Claudio Imbrenda wrote:
> > Move gmap_shadow_pgt_lookup() from mm/gmap.c into kvm/gaccess.c .
> > 
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > ---
> >   arch/s390/include/asm/gmap.h |  3 +--
> >   arch/s390/kvm/gaccess.c      | 40 +++++++++++++++++++++++++++++++
> >   arch/s390/kvm/gmap.h         |  2 ++
> >   arch/s390/mm/gmap.c          | 46 ++----------------------------------
> >   4 files changed, 45 insertions(+), 46 deletions(-)  
> 
> [...]
> 
> >   
> > +/**
> > + * gmap_shadow_pgt_lookup - find a shadow page table  
> 
> The other two VSIE functions in gaccess.c have the kvm_s390_shadow 
> prefix but this one is only static anyway and hence we could just drop 
> the gmap_ from the name.

fair enough, will fix

> 
> > + * @sg: pointer to the shadow guest address space structure
> > + * @saddr: the address in the shadow aguest address space
> > + * @pgt: parent gmap address of the page table to get shadowed
> > + * @dat_protection: if the pgtable is marked as protected by dat
> > + * @fake: pgt references contiguous guest memory block, not a pgtable
> > + *
> > + * Returns 0 if the shadow page table was found and -EAGAIN if the page
> > + * table was not found.
> > + *
> > + * Called with sg->mm->mmap_lock in read.
> > + */
> > +static int gmap_shadow_pgt_lookup(struct gmap *sg, unsigned long saddr, unsigned long *pgt,
> > +				  int *dat_protection, int *fake)
> > +{
> > +	unsigned long *table;
> > +	struct page *page;
> > +	int rc;
> > +
> > +	spin_lock(&sg->guest_table_lock);
> > +	table = gmap_table_walk(sg, saddr, 1); /* get segment pointer */  
> 
> I'd be happy if you could introduce an enum for the level argument in a 
> future series.

in upcoming series, the gmap table walk will be completely
rewritten, and yes, it will have enums

> 
> > +	if (table && !(*table & _SEGMENT_ENTRY_INVALID)) {
> > +		/* Shadow page tables are full pages (pte+pgste) */
> > +		page = pfn_to_page(*table >> PAGE_SHIFT);
> > +		*pgt = page->index & ~GMAP_SHADOW_FAKE_TABLE;
> > +		*dat_protection = !!(*table & _SEGMENT_ENTRY_PROTECT);
> > +		*fake = !!(page->index & GMAP_SHADOW_FAKE_TABLE);
> > +		rc = 0;
> > +	} else  {
> > +		rc = -EAGAIN;
> > +	}
> > +	spin_unlock(&sg->guest_table_lock);
> > +	return rc;
> > +}
> > +
> >