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

Claudio Imbrenda
2025-01-20T13:54:53Z

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;
> > +}
> > +
> >