Re: [PATCH v3 09/15] KVM: s390: move some gmap shadowing functions away from mm/gmap.c

["Christoph Schlameuss" <schlameuss@linux.ibm.com>]

On Fri Jan 17, 2025 at 8:09 PM CET, Claudio Imbrenda wrote:
> Move some gmap shadowing functions from mm/gmap.c to kvm/kvm-s390.c and
> the newly created kvm/gmap-vsie.c
>
> This is a step toward removing gmap from mm.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>  arch/s390/include/asm/gmap.h |   9 +-
>  arch/s390/kvm/Makefile       |   2 +-
>  arch/s390/kvm/gmap-vsie.c    | 142 +++++++++++++++++++++
>  arch/s390/kvm/gmap.h         |  20 +++
>  arch/s390/kvm/kvm-s390.c     |  62 ++++++++-
>  arch/s390/kvm/kvm-s390.h     |   2 +
>  arch/s390/kvm/vsie.c         |   2 +
>  arch/s390/mm/gmap.c          | 238 +++++------------------------------
>  8 files changed, 259 insertions(+), 218 deletions(-)
>  create mode 100644 arch/s390/kvm/gmap-vsie.c
>

[...]

> diff --git a/arch/s390/kvm/gmap-vsie.c b/arch/s390/kvm/gmap-vsie.c
> new file mode 100644
> index 000000000000..90427f114995
> --- /dev/null
> +++ b/arch/s390/kvm/gmap-vsie.c
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Guest memory management for KVM/s390 nested VMs.
> + *
> + * Copyright IBM Corp. 2008, 2020, 2024
> + *
> + *    Author(s): Claudio Imbrenda <imbrenda@linux.ibm.com>
> + *               Martin Schwidefsky <schwidefsky@de.ibm.com>
> + *               David Hildenbrand <david@redhat.com>
> + *               Janosch Frank <frankja@linux.vnet.ibm.com>
> + */
> +
> +#include <linux/compiler.h>
> +#include <linux/kvm.h>
> +#include <linux/kvm_host.h>
> +#include <linux/pgtable.h>
> +#include <linux/pagemap.h>
> +#include <linux/mman.h>
> +
> +#include <asm/lowcore.h>
> +#include <asm/gmap.h>
> +#include <asm/uv.h>
> +
> +#include "kvm-s390.h"
> +#include "gmap.h"
> +
> +/**
> + * gmap_find_shadow - find a specific asce in the list of shadow tables
> + * @parent: pointer to the parent gmap
> + * @asce: ASCE for which the shadow table is created
> + * @edat_level: edat level to be used for the shadow translation
> + *
> + * Returns the pointer to a gmap if a shadow table with the given asce is
> + * already available, ERR_PTR(-EAGAIN) if another one is just being created,
> + * otherwise NULL
> + */
> +static struct gmap *gmap_find_shadow(struct gmap *parent, unsigned long asce,
> +				     int edat_level)
> +{
> +	struct gmap *sg;
> +
> +	list_for_each_entry(sg, &parent->children, list) {
> +		if (sg->orig_asce != asce || sg->edat_level != edat_level ||
> +		    sg->removed)

This is just:

if !gmap_shadow_valid(sg, asce, edat_level)

> +			continue;
> +		if (!sg->initialized)
> +			return ERR_PTR(-EAGAIN);
> +		refcount_inc(&sg->ref_count);
> +		return sg;
> +	}
> +	return NULL;
> +}

[...]

> diff --git a/arch/s390/kvm/gmap.h b/arch/s390/kvm/gmap.h
> index f2b52ce29be3..978f541059f0 100644
> --- a/arch/s390/kvm/gmap.h
> +++ b/arch/s390/kvm/gmap.h
> @@ -13,5 +13,25 @@
>  int gmap_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb);
>  int gmap_convert_to_secure(struct gmap *gmap, unsigned long gaddr);
>  int gmap_destroy_page(struct gmap *gmap, unsigned long gaddr);
> +struct gmap *gmap_shadow(struct gmap *parent, unsigned long asce, int edat_level);
> +
> +/**
> + * gmap_shadow_valid - check if a shadow guest address space matches the
> + *                     given properties and is still valid
> + * @sg: pointer to the shadow guest address space structure
> + * @asce: ASCE for which the shadow table is requested
> + * @edat_level: edat level to be used for the shadow translation
> + *
> + * Returns 1 if the gmap shadow is still valid and matches the given
> + * properties, the caller can continue using it. Returns 0 otherwise, the
> + * caller has to request a new shadow gmap in this case.
> + *
> + */
> +static inline int gmap_shadow_valid(struct gmap *sg, unsigned long asce, int edat_level)
> +{
> +	if (sg->removed)
> +		return 0;
> +	return sg->orig_asce == asce && sg->edat_level == edat_level;

This can simply be a single return:

return !sg->removed && sg->orig_asce == asce && sg->edat_level == edat_level;

> +}
>  
>  #endif
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index b626c87480ed..482f0968abfa 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -4509,6 +4509,63 @@ static bool ibs_enabled(struct kvm_vcpu *vcpu)
>  	return kvm_s390_test_cpuflags(vcpu, CPUSTAT_IBS);
>  }

[...]