Skip to content

Commit 2d505c2

Browse files
author
Claudio Imbrenda
committed
KVM: s390: vsie: Fix unshadowing logic
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. Opportunistically add a check in gmap_protect_rmap() to make sure it won't be called with level == TABLE_TYPE_PAGE_TABLE. Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Fixes: a2c17f9 ("KVM: s390: New gmap code") Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
1 parent 4df4b7c commit 2d505c2

5 files changed

Lines changed: 63 additions & 5 deletions

File tree

arch/s390/kvm/dat.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ static int dat_split_ste(struct kvm_s390_mmu_cache *mc, union pmd *pmdp, gfn_t g
267267
/* No need to take locks as the page table is not installed yet. */
268268
pgste_init.prefix_notif = old.s.fc1.prefix_notif;
269269
pgste_init.vsie_notif = old.s.fc1.vsie_notif;
270+
pgste_init.vsie_gmem = old.s.fc1.vsie_notif;
270271
pgste_init.pcl = uses_skeys && init.h.i;
271272
dat_init_pgstes(pt, pgste_init.val);
272273
} else {

arch/s390/kvm/dat.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,8 @@ union pgste {
145145
unsigned long cmma_d : 1; /* Dirty flag for CMMA bits */
146146
unsigned long prefix_notif : 1; /* Guest prefix invalidation notification */
147147
unsigned long vsie_notif : 1; /* Referenced in a shadow table */
148-
unsigned long : 5;
148+
unsigned long vsie_gmem : 1; /* Contains nested guest memory */
149+
unsigned long : 4;
149150
unsigned long : 8;
150151
};
151152
struct {

arch/s390/kvm/gaccess.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,6 +1445,7 @@ static int _do_shadow_pte(struct gmap *sg, gpa_t raddr, union pte *ptep_h, union
14451445
} else {
14461446
pgste = _gmap_ptep_xchg(sg->parent, ptep_h, newpte, pgste, f->gfn, false);
14471447
pgste.vsie_notif = 1;
1448+
pgste.vsie_gmem = 1;
14481449
}
14491450
pgste_set_unlock(ptep_h, pgste);
14501451
if (rc)

arch/s390/kvm/gmap.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1031,7 +1031,8 @@ int gmap_protect_rmap(struct kvm_s390_mmu_cache *mc, struct gmap *sg, gfn_t p_gf
10311031
union pte pte;
10321032
int flags, rc;
10331033

1034-
KVM_BUG_ON(!is_shadow(sg), sg->kvm);
1034+
if (KVM_BUG_ON(!is_shadow(sg) || level <= TABLE_TYPE_PAGE_TABLE, sg->kvm))
1035+
return -EINVAL;
10351036
lockdep_assert_held(&sg->parent->children_lock);
10361037

10371038
flags = DAT_WALK_SPLIT_ALLOC | (uses_skeys(sg->parent) ? DAT_WALK_USES_SKEYS : 0);

arch/s390/kvm/gmap.h

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,36 @@ static inline bool gmap_unmap_prefix(struct gmap *gmap, gfn_t gfn, gfn_t end)
167167
return _gmap_unmap_prefix(gmap, gfn, end, false);
168168
}
169169

170+
/**
171+
* pte_needs_unshadow() -- Check if the pte operations triggers unshadowing.
172+
* @oldpte: the previous value for the guest pte.
173+
* @newpte: the new pte being set.
174+
* @pgste: the pgste for the pte entry.
175+
*
176+
* If the pgste.vsie_notif bit is not set, return false: the page is not
177+
* involved in vsie and thus should not trigger an unshadow operation.
178+
*
179+
* If the pgste.vsie_gmem bit is set, this pte represents shadowed guest
180+
* memory. The access rights on g3's memory should be synchronized with g1's
181+
* and g2's. Therefore unshadowing is triggered if the new and old pte
182+
* differ in protection, or if the new pte is invalid.
183+
*
184+
* If the pgste.vsie_gmem bit is not set, this pte maps the g2 dat tables
185+
* for g3. If the entry becomes writable or absent, it becomes impossible to
186+
* guarantee that the shadow mapping will match g2's mapping. In that case,
187+
* trigger an unshadow event.
188+
*
189+
* Return: true if an unshadow event should be triggered, otherwise false.
190+
*/
191+
static inline bool pte_needs_unshadow(union pte oldpte, union pte newpte, union pgste pgste)
192+
{
193+
if (!pgste.vsie_notif)
194+
return false;
195+
if (pgste.vsie_gmem)
196+
return (oldpte.h.p != newpte.h.p) || newpte.h.i;
197+
return !newpte.h.p || !newpte.s.pr;
198+
}
199+
170200
static inline union pgste _gmap_ptep_xchg(struct gmap *gmap, union pte *ptep, union pte newpte,
171201
union pgste pgste, gfn_t gfn, bool needs_lock)
172202
{
@@ -180,8 +210,9 @@ static inline union pgste _gmap_ptep_xchg(struct gmap *gmap, union pte *ptep, un
180210
pgste.prefix_notif = 0;
181211
gmap_unmap_prefix(gmap, gfn, gfn + 1);
182212
}
183-
if (pgste.vsie_notif && (ptep->h.p != newpte.h.p || newpte.h.i)) {
213+
if (pte_needs_unshadow(*ptep, newpte, pgste)) {
184214
pgste.vsie_notif = 0;
215+
pgste.vsie_gmem = 0;
185216
if (needs_lock)
186217
gmap_handle_vsie_unshadow_event(gmap, gfn);
187218
else
@@ -198,6 +229,30 @@ static inline union pgste gmap_ptep_xchg(struct gmap *gmap, union pte *ptep, uni
198229
return _gmap_ptep_xchg(gmap, ptep, newpte, pgste, gfn, true);
199230
}
200231

232+
/**
233+
* crste_needs_unshadow() -- Check if the crste operations triggers unshadowing.
234+
* @oldcrste: the previous value for the crste.
235+
* @newcrste: the new value for the crste.
236+
*
237+
* If the old crste did not have the vsie_notif bit set, return false: the
238+
* page is not involved in vsie and thus should not trigger an unshadow
239+
* operation. Conversely, if the bit is set, it can only be g3 memory, since
240+
* dat tables are never mapped using large pages.
241+
*
242+
* Similar to the pgste.vsie_gmem case of pte_needs_unshadow(), if the
243+
* protection bit is changing or the new page is invalid, trigger an
244+
* unshadow event. Also trigger an unshadow event if the new crste does not
245+
* have the vsie_notif bit set.
246+
*
247+
* Return: true if an unshadow event should be triggered, otherwise false.
248+
*/
249+
static inline bool crste_needs_unshadow(union crste oldcrste, union crste newcrste)
250+
{
251+
if (!oldcrste.s.fc1.vsie_notif)
252+
return false;
253+
return (newcrste.h.p != oldcrste.h.p) || newcrste.h.i || !newcrste.s.fc1.vsie_notif;
254+
}
255+
201256
static inline bool __must_check _gmap_crstep_xchg_atomic(struct gmap *gmap, union crste *crstep,
202257
union crste oldcrste, union crste newcrste,
203258
gfn_t gfn, bool needs_lock)
@@ -216,8 +271,7 @@ static inline bool __must_check _gmap_crstep_xchg_atomic(struct gmap *gmap, unio
216271
newcrste.s.fc1.prefix_notif = 0;
217272
gmap_unmap_prefix(gmap, gfn, gfn + align);
218273
}
219-
if (crste_leaf(oldcrste) && oldcrste.s.fc1.vsie_notif &&
220-
(newcrste.h.p || newcrste.h.i || !newcrste.s.fc1.vsie_notif)) {
274+
if (crste_leaf(oldcrste) && crste_needs_unshadow(oldcrste, newcrste)) {
221275
newcrste.s.fc1.vsie_notif = 0;
222276
if (needs_lock)
223277
gmap_handle_vsie_unshadow_event(gmap, gfn);

0 commit comments

Comments
 (0)