@@ -102,6 +102,9 @@ static pte_t *map_table(mfn_t mfn)
static void unmap_table(const pte_t *table)
{
+ if ( !table )
+ return;
+
/*
* During early boot, map_table() will not use map_domain_page()
* but the PMAP.
@@ -245,14 +248,21 @@ pte_t pt_walk(vaddr_t va, unsigned int *pte_level)
return pte;
}
-/* Update an entry at the level @target. */
+/*
+ * Update an entry at the level @target.
+ *
+ * If `target` == CONFIG_PAGING_LEVELS, the search will continue until
+ * a leaf node is found.
+ * Otherwise, the page table entry will be searched at the requested
+ * `target` level.
+ * For an example of why this might be needed, see the comment in
+ * pt_update() before pt_update_entry() is called.
+ */
static int pt_update_entry(mfn_t root, vaddr_t virt,
- mfn_t mfn, unsigned int target,
+ mfn_t mfn, unsigned int *target,
unsigned int flags)
{
int rc;
- unsigned int level = HYP_PT_ROOT_LEVEL;
- pte_t *table;
/*
* The intermediate page table shouldn't be allocated when MFN isn't
* valid and we are not populating page table.
@@ -263,43 +273,50 @@ static int pt_update_entry(mfn_t root, vaddr_t virt,
* combinations of (mfn, flags).
*/
bool alloc_tbl = !mfn_eq(mfn, INVALID_MFN) || (flags & PTE_POPULATE);
- pte_t pte, *entry;
-
- /* convenience aliases */
- DECLARE_OFFSETS(offsets, virt);
+ pte_t pte, *ptep = NULL;
- table = map_table(root);
- for ( ; level > target; level-- )
+ if ( *target == CONFIG_PAGING_LEVELS )
+ ptep = _pt_walk(virt, target);
+ else
{
- rc = pt_next_level(alloc_tbl, &table, offsets[level]);
- if ( rc == XEN_TABLE_MAP_NOMEM )
+ pte_t *table;
+ unsigned int level = HYP_PT_ROOT_LEVEL;
+ /* Convenience aliases */
+ DECLARE_OFFSETS(offsets, virt);
+
+ table = map_table(root);
+ for ( ; level > *target; level-- )
{
- rc = -ENOMEM;
- goto out;
+ rc = pt_next_level(alloc_tbl, &table, offsets[level]);
+ if ( rc == XEN_TABLE_MAP_NOMEM )
+ {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ if ( rc == XEN_TABLE_MAP_NONE )
+ {
+ rc = 0;
+ goto out;
+ }
+
+ if ( rc != XEN_TABLE_NORMAL )
+ break;
}
- if ( rc == XEN_TABLE_MAP_NONE )
+ if ( level != *target )
{
- rc = 0;
+ dprintk(XENLOG_ERR,
+ "%s: Shattering superpage is not supported\n", __func__);
+ rc = -EOPNOTSUPP;
goto out;
}
- if ( rc != XEN_TABLE_NORMAL )
- break;
- }
-
- if ( level != target )
- {
- dprintk(XENLOG_ERR,
- "%s: Shattering superpage is not supported\n", __func__);
- rc = -EOPNOTSUPP;
- goto out;
+ ptep = table + offsets[level];
}
- entry = table + offsets[level];
-
rc = -EINVAL;
- if ( !pt_check_entry(*entry, mfn, flags) )
+ if ( !pt_check_entry(*ptep, mfn, flags) )
goto out;
/* We are removing the page */
@@ -316,7 +333,7 @@ static int pt_update_entry(mfn_t root, vaddr_t virt,
pte = pte_from_mfn(mfn, PTE_VALID);
else /* We are updating the permission => Copy the current pte. */
{
- pte = *entry;
+ pte = *ptep;
pte.pte &= ~PTE_ACCESS_MASK;
}
@@ -324,12 +341,12 @@ static int pt_update_entry(mfn_t root, vaddr_t virt,
pte.pte |= (flags & PTE_ACCESS_MASK) | PTE_ACCESSED | PTE_DIRTY;
}
- write_pte(entry, pte);
+ write_pte(ptep, pte);
rc = 0;
out:
- unmap_table(table);
+ unmap_table(ptep);
return rc;
}
@@ -422,17 +439,40 @@ static int pt_update(vaddr_t virt, mfn_t mfn,
while ( left )
{
- unsigned int order, level;
-
- level = pt_mapping_level(vfn, mfn, left, flags);
- order = XEN_PT_LEVEL_ORDER(level);
+ unsigned int order, level = CONFIG_PAGING_LEVELS;
- ASSERT(left >= BIT(order, UL));
+ /*
+ * In the case when modifying or destroying a mapping, it is necessary
+ * to search until a leaf node is found, instead of searching for
+ * a page table entry based on the precalculated `level` and `order`
+ * (look at pt_update()).
+ * This is because when `mfn` == INVALID_MFN, the `mask`(in
+ * pt_mapping_level()) will take into account only `vfn`, which could
+ * accidentally return an incorrect level, leading to the discovery of
+ * an incorrect page table entry.
+ *
+ * For example, if `vfn` is page table level 1 aligned, but it was
+ * mapped as page table level 0, then pt_mapping_level() will return
+ * `level` = 1, since only `vfn` (which is page table level 1 aligned)
+ * is taken into account when `mfn` == INVALID_MFN
+ * (look at pt_mapping_level()).
+ *
+ * To force searching until a leaf node is found is necessary to have
+ * `level` == CONFIG_PAGING_LEVELS which is a default value for
+ * `level`.
+ *
+ * For other cases (when a mapping is not being modified or destroyed),
+ * pt_mapping_level() should be used.
+ */
+ if ( !mfn_eq(mfn, INVALID_MFN) || (flags & PTE_POPULATE) )
+ level = pt_mapping_level(vfn, mfn, left, flags);
- rc = pt_update_entry(root, vfn << PAGE_SHIFT, mfn, level, flags);
+ rc = pt_update_entry(root, vfn << PAGE_SHIFT, mfn, &level, flags);
if ( rc )
break;
+ order = XEN_PT_LEVEL_ORDER(level);
+
vfn += 1UL << order;
if ( !mfn_eq(mfn, INVALID_MFN) )
mfn = mfn_add(mfn, 1UL << order);
When pt_update() is called with arguments (..., INVALID_MFN, ..., 0 or 1), it indicates that a mapping is being destroyed/modifyed. In the case when modifying or destroying a mapping, it is necessary to search until a leaf node is found, instead of searching for a page table entry based on the precalculated `level` and `order`(look at pt_update()). This is because when `mfn` == INVALID_MFN, the `mask` (in pt_mapping_level()) will take into account only `vfn`, which could accidentally return an incorrect level, leading to the discovery of an incorrect page table entry. For example, if `vfn` is page table level 1 aligned, but it was mapped as page table level 0, then pt_mapping_level() will return `level` = 1, since only `vfn` (which is page table level 1 aligned) is taken into account when `mfn` == INVALID_MFN (look at pt_mapping_level()). Fixes: c2f1ded524 ("xen/riscv: page table handling") Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in v5: - Rename *entry to *ptep in pt_update_entry(). - Fix code style issue in the comment. - Move NULL check of pointer to `table` inside unmap_table and then drop it in pt_update_entry(). --- Changes in v4: - Move defintion of local variable table inside `else` case as it is used only there. - Change unmap_table(table) to unmap_table(entry) for unifying both cases when _pt_walk() is used and when pte is seached on the specified level. - Initialize local variable `entry` to avoid compilation error caused by uninitialized variable. --- Changes in v3: - Drop ASSERT() for order as it isn't needed anymore. - Drop PTE_LEAF_SEARCH and use instead level=CONFIG_PAGING_LEVELS; refactor connected code correspondingly. - Calculate order once. - Drop initializer for local variable order. - Drop BUG_ON(!pte_is_mapping(*entry)) for the case when leaf searching happens as there is a similar check in pt_check_entry(). Look at pt.c:41 and pt.c:75. --- Changes in v2: - Introduce PTE_LEAF_SEARCH to tell page table update operation to walk down to wherever the leaf entry is. - Use introduced PTE_LEAF_SEARCH to not searching pte_t entry twice. - Update the commit message. --- xen/arch/riscv/pt.c | 116 +++++++++++++++++++++++++++++--------------- 1 file changed, 78 insertions(+), 38 deletions(-)