Re: [PATCH v2 3/5] KVM: s390: vsie: Fix unshadowing logic

[Claudio Imbrenda]

On Mon, 18 May 2026 13:28:44 +0200
Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:

> Am 12.05.26 um 19:52 schrieb Claudio Imbrenda:
> > In some cases (i.e. under extreme memory pressure on the host),
> > attempting to shadow memory will result in the same memory being
> > unshadowed, causing a loop.
> > 
> > Add a PGSTE bit to distinguish between shadowed memory and shadowed DAT
> > tables, fix the unshadowing logic in _gmap_ptep_xchg() to prevent
> > unnecessary unshadowing and perform better checks.
> > 
> > Also fix the unshadowing logic in _gmap_crstep_xchg_atomic() which did
> > not unshadow properly when the large page would become unprotected.
> > 
> > Opportunistilcally add a check in gmap_protect_rmap() to make sure it  
>                ^
>    Opportunistically

oops

> 
> [...]
> > +static inline bool pte_needs_unshadow(union pte oldpte, union pte newpte, union pgste pgste)
> > +{
> > +	if (!pgste.vsie_notif)
> > +		return false;
> > +	if (pgste.vsie_gmem)
> > +		return (oldpte.h.p != newpte.h.p) || newpte.h.i;
> > +	return !newpte.h.p || !newpte.s.pr;
> > +}
> > +  
> 
> The newpte.s.pr part is new and seems not related to the vsie_gmem change
> but I cannot find anything about this in the patch description. And I dont

it's part of "fix unshadowing logic"

> understand this part. So either some explanation here, in the patch description or
> a comment maybe?

I think a comment explaining what's going on is the best option

> 
> >   static inline union pgste _gmap_ptep_xchg(struct gmap *gmap, union pte *ptep, union pte newpte,
> >   					  union pgste pgste, gfn_t gfn, bool needs_lock)
> >   {
> > @@ -180,8 +189,9 @@ static inline union pgste _gmap_ptep_xchg(struct gmap *gmap, union pte *ptep, un
> >   		pgste.prefix_notif = 0;
> >   		gmap_unmap_prefix(gmap, gfn, gfn + 1);
> >   	}
> > -	if (pgste.vsie_notif && (ptep->h.p != newpte.h.p || newpte.h.i)) {
> > +	if (pte_needs_unshadow(*ptep, newpte, pgste)) {
> >   		pgste.vsie_notif = 0;
> > +		pgste.vsie_gmem = 0;
> >   		if (needs_lock)
> >   			gmap_handle_vsie_unshadow_event(gmap, gfn);
> >   		else  
> [...]