LGTM, but maybe add some more comments about the required locks?
On Thu Jan 23, 2025 at 3:46 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@xxxxxxxxxxxxx>
> Reviewed-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
Reviewed-by: Christoph Schlameuss <schlameuss@xxxxxxxxxxxxx>
> ---
> arch/s390/include/asm/gmap.h | 9 +-
> arch/s390/kvm/Makefile | 2 +-
> arch/s390/kvm/gmap-vsie.c | 139 ++++++++++++++++++++
> 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, 256 insertions(+), 218 deletions(-)
> create mode 100644 arch/s390/kvm/gmap-vsie.c
[...]
> +/**
> + * 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
nit: Called with parent->shadow_lock
> + */
> +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 (!gmap_shadow_valid(sg, asce, edat_level))
> + continue;
> + if (!sg->initialized)
> + return ERR_PTR(-EAGAIN);
> + refcount_inc(&sg->ref_count);
> + return sg;
> + }
> + return NULL;
> +}
[...]
nit: add comment:
Called with vcpu->kvm->srcu and vcpu->arch.gmap->mm in read
> +int __kvm_s390_mprotect_many(struct gmap *gmap, gpa_t gpa, u8 npages, unsigned int prot,
> + unsigned long bits)
> +{
> + unsigned int fault_flag = (prot & PROT_WRITE) ? FAULT_FLAG_WRITE : 0;
> + gpa_t end = gpa + npages * PAGE_SIZE;
> + int rc;
> +
> + for (; gpa < end; gpa = ALIGN(gpa + 1, rc)) {
> + rc = gmap_protect_one(gmap, gpa, prot, bits);
> + if (rc == -EAGAIN) {
> + __kvm_s390_fixup_fault_sync(gmap, gpa, fault_flag);
> + rc = gmap_protect_one(gmap, gpa, prot, bits);
> + }
> + if (rc < 0)
> + return rc;
> + }
> +
> + return 0;
> +}
[...]
