Re: [PATCH v2 09/20] KVM: s390: KVM page table management functions: clear and replace

Claudio Imbrenda
2025-09-11T13:19:41Z

On Thu, 11 Sep 2025 14:57:40 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 9/10/25 8:07 PM, Claudio Imbrenda wrote:
> > Add page table management functions to be used for KVM guest (gmap)
> > page tables.
> >
> > This patch adds functions to clear, replace or exchange DAT table
> > entries.
> >
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > ---
> > arch/s390/kvm/dat.c | 120 ++++++++++++++++++++++++++++++++++++++++++++
> > arch/s390/kvm/dat.h | 40 +++++++++++++++
> > 2 files changed, 160 insertions(+)
> >
> > diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
> > index 326be78adcda..f26e3579bd77 100644
> > --- a/arch/s390/kvm/dat.c
> > +++ b/arch/s390/kvm/dat.c
> > @@ -89,3 +89,123 @@ void dat_free_level(struct crst_table *table, bool owns_ptes)
> > }
> > dat_free_crst(table);
> > }
> > +
> > +/**
> > + * dat_crstep_xchg - exchange a guest CRST with another
> > + * @crstep: pointer to the CRST entry
> > + * @new: replacement entry
> > + * @gfn: the affected guest address
> > + * @asce: the ASCE of the address space
> > + *
> > + * This function is assumed to be called with the guest_table_lock
> > + * held.
> > + */
> > +void dat_crstep_xchg(union crste *crstep, union crste new, gfn_t gfn, union asce asce)
> > +{
> > + if (crstep->h.i) {
> > + WRITE_ONCE(*crstep, new);
> > + return;
> > + } else if (cpu_has_edat2()) {
> > + crdte_crste(crstep, *crstep, new, gfn, asce);
> > + return;
> > + }
> > +
> > + if (machine_has_tlb_guest())
> > + idte_crste(crstep, gfn, IDTE_GUEST_ASCE, asce, IDTE_GLOBAL);
> > + else if (cpu_has_idte())
> > + idte_crste(crstep, gfn, 0, NULL_ASCE, IDTE_GLOBAL);
> > + else
> > + csp_invalidate_crste(crstep);
>
> I'm wondering if we can make stfle 3 (DTE) a requirement for KVM or
> Linux as a whole since it was introduced with z990 AFAIK.

AFAIK we don't support machines older than z10 anyway

but in that case we can only get rid of csp_invalidate_crste(), which
is not much.

I can remove it, if you really think it's ugly

>
> > + WRITE_ONCE(*crstep, new);
> > +}
> > +
> > +/**
> > + * dat_crstep_xchg_atomic - exchange a gmap pmd with another
> > + * @crstep: pointer to the crste entry
> > + * @old: expected old value
> > + * @new: replacement entry
> > + * @gfn: the affected guest address
> > + * @asce: the asce of the address space
> > + *
> > + * This function should only be called on invalid crstes, or on crstes with
> > + * FC = 1, as that guarantees the presence of CSPG.
> > + *
> > + * Return: true if the exchange was successful.
> > + */
> > +bool dat_crstep_xchg_atomic(union crste *crstep, union crste old, union crste new, gfn_t gfn,
> > + union asce asce)
> > +{
> > + if (old.h.i)
> > + return arch_try_cmpxchg((long *)crstep, &old.val, new.val);
> > + if (cpu_has_edat2())
> > + return crdte_crste(crstep, old, new, gfn, asce);
> > + if (cpu_has_idte())
> > + return cspg_crste(crstep, old, new);
> > +
> > + WARN_ONCE(1, "Machine does not have CSPG and DAT table was not invalid.");
> > + return false;
> > +}