Commit 77fbbbd2 authored by Takuya Yoshikawa's avatar Takuya Yoshikawa Committed by Paolo Bonzini

KVM: x86: MMU: Consolidate BUG_ON checks for reverse-mapped sptes

At some call sites of rmap_get_first() and rmap_get_next(), BUG_ON is
placed right after the call to detect unrelated sptes which must not be
found in the reverse-mapping list.

Move this check in rmap_get_first/next() so that all call sites, not
just the users of the for_each_rmap_spte() macro, will be checked the
same way.

One thing to keep in mind is that kvm_mmu_unlink_parents() also uses
rmap_get_first() to handle parent sptes.  The change will not break it
because parent sptes are present, at least until drop_parent_pte()
actually unlinks them, and not mmio-sptes.
Signed-off-by: default avatarTakuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent afd28fe1
...@@ -203,10 +203,10 @@ Shadow pages contain the following information: ...@@ -203,10 +203,10 @@ Shadow pages contain the following information:
page cannot be destroyed. See role.invalid. page cannot be destroyed. See role.invalid.
parent_ptes: parent_ptes:
The reverse mapping for the pte/ptes pointing at this page's spt. If The reverse mapping for the pte/ptes pointing at this page's spt. If
parent_ptes bit 0 is zero, only one spte points at this pages and parent_ptes bit 0 is zero, only one spte points at this page and
parent_ptes points at this single spte, otherwise, there exists multiple parent_ptes points at this single spte, otherwise, there exists multiple
sptes pointing at this page and (parent_ptes & ~0x1) points at a data sptes pointing at this page and (parent_ptes & ~0x1) points at a data
structure with a list of parent_ptes. structure with a list of parent sptes.
unsync: unsync:
If true, then the translations in this page may not match the guest's If true, then the translations in this page may not match the guest's
translation. This is equivalent to the state of the tlb when a pte is translation. This is equivalent to the state of the tlb when a pte is
......
...@@ -1098,17 +1098,23 @@ struct rmap_iterator { ...@@ -1098,17 +1098,23 @@ struct rmap_iterator {
static u64 *rmap_get_first(struct kvm_rmap_head *rmap_head, static u64 *rmap_get_first(struct kvm_rmap_head *rmap_head,
struct rmap_iterator *iter) struct rmap_iterator *iter)
{ {
u64 *sptep;
if (!rmap_head->val) if (!rmap_head->val)
return NULL; return NULL;
if (!(rmap_head->val & 1)) { if (!(rmap_head->val & 1)) {
iter->desc = NULL; iter->desc = NULL;
return (u64 *)rmap_head->val; sptep = (u64 *)rmap_head->val;
goto out;
} }
iter->desc = (struct pte_list_desc *)(rmap_head->val & ~1ul); iter->desc = (struct pte_list_desc *)(rmap_head->val & ~1ul);
iter->pos = 0; iter->pos = 0;
return iter->desc->sptes[iter->pos]; sptep = iter->desc->sptes[iter->pos];
out:
BUG_ON(!is_shadow_present_pte(*sptep));
return sptep;
} }
/* /*
...@@ -1118,14 +1124,14 @@ static u64 *rmap_get_first(struct kvm_rmap_head *rmap_head, ...@@ -1118,14 +1124,14 @@ static u64 *rmap_get_first(struct kvm_rmap_head *rmap_head,
*/ */
static u64 *rmap_get_next(struct rmap_iterator *iter) static u64 *rmap_get_next(struct rmap_iterator *iter)
{ {
if (iter->desc) {
if (iter->pos < PTE_LIST_EXT - 1) {
u64 *sptep; u64 *sptep;
if (iter->desc) {
if (iter->pos < PTE_LIST_EXT - 1) {
++iter->pos; ++iter->pos;
sptep = iter->desc->sptes[iter->pos]; sptep = iter->desc->sptes[iter->pos];
if (sptep) if (sptep)
return sptep; goto out;
} }
iter->desc = iter->desc->more; iter->desc = iter->desc->more;
...@@ -1133,17 +1139,20 @@ static u64 *rmap_get_next(struct rmap_iterator *iter) ...@@ -1133,17 +1139,20 @@ static u64 *rmap_get_next(struct rmap_iterator *iter)
if (iter->desc) { if (iter->desc) {
iter->pos = 0; iter->pos = 0;
/* desc->sptes[0] cannot be NULL */ /* desc->sptes[0] cannot be NULL */
return iter->desc->sptes[iter->pos]; sptep = iter->desc->sptes[iter->pos];
goto out;
} }
} }
return NULL; return NULL;
out:
BUG_ON(!is_shadow_present_pte(*sptep));
return sptep;
} }
#define for_each_rmap_spte(_rmap_head_, _iter_, _spte_) \ #define for_each_rmap_spte(_rmap_head_, _iter_, _spte_) \
for (_spte_ = rmap_get_first(_rmap_head_, _iter_); \ for (_spte_ = rmap_get_first(_rmap_head_, _iter_); \
_spte_ && ({BUG_ON(!is_shadow_present_pte(*_spte_)); 1;}); \ _spte_; _spte_ = rmap_get_next(_iter_))
_spte_ = rmap_get_next(_iter_))
static void drop_spte(struct kvm *kvm, u64 *sptep) static void drop_spte(struct kvm *kvm, u64 *sptep)
{ {
...@@ -1358,7 +1367,6 @@ static bool kvm_zap_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head) ...@@ -1358,7 +1367,6 @@ static bool kvm_zap_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head)
bool flush = false; bool flush = false;
while ((sptep = rmap_get_first(rmap_head, &iter))) { while ((sptep = rmap_get_first(rmap_head, &iter))) {
BUG_ON(!(*sptep & PT_PRESENT_MASK));
rmap_printk("%s: spte %p %llx.\n", __func__, sptep, *sptep); rmap_printk("%s: spte %p %llx.\n", __func__, sptep, *sptep);
drop_spte(kvm, sptep); drop_spte(kvm, sptep);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment