diff mbox series

[v2,2/4] x86/mm: skip super-page alignment checks for non-present entries

Message ID 20241108113144.83637-3-roger.pau@citrix.com (mailing list archive)
State Superseded
Headers show
Series x86/mm: miscellaneous fixes | expand

Commit Message

Roger Pau Monné Nov. 8, 2024, 11:31 a.m. UTC 
INVALID_MFN is ~0, so by it having all bits as 1s it doesn't fulfill the
super-page address alignment checks for L3 and L2 entries.  Skip the alignment
checks if the new entry is a non-present one.

This fixes a regression introduced by 0b6b51a69f4d, where the switch from 0 to
INVALID_MFN caused all super-pages to be shattered when attempting to remove
mappings by passing INVALID_MFN instead of 0.

Fixes: 0b6b51a69f4d ('xen/mm: Switch map_pages_to_xen to use MFN typesafe')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
 - Detect non-present entries from the flags contents rather than checking the
   mfn parameter.
---
 xen/arch/x86/mm.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

Comments

Jan Beulich Nov. 11, 2024, 8:59 a.m. UTC | #1 
On 08.11.2024 12:31, Roger Pau Monne wrote:
> @@ -5544,7 +5553,8 @@ int map_pages_to_xen(
>   check_l3:
>          if ( cpu_has_page1gb &&
>               (flags == PAGE_HYPERVISOR) &&
> -             ((nr_mfns == 0) || IS_L3E_ALIGNED(virt, mfn)) )
> +             ((nr_mfns == 0) || !(flags & _PAGE_PRESENT) ||
> +              IS_L3E_ALIGNED(virt, mfn)) )
>          {
>              unsigned long base_mfn;
>              const l2_pgentry_t *l2t;

With the "flags == PAGE_HYPERVISOR" check I don't think any change is needed
here? With this dropped (again: uncertain whether to offer making the adjustment
while committing)
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 8afb63c855b9..64b8054891da 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5232,9 +5232,17 @@  int map_pages_to_xen(
     }                                          \
 } while (0)
 
-/* Check if a (virt, mfn) tuple is aligned for a given slot level. */
-#define IS_LnE_ALIGNED(v, m, n) \
-    IS_ALIGNED(PFN_DOWN(v) | mfn_x(m), (1UL << (PAGETABLE_ORDER * (n - 1))) - 1)
+/*
+ * Check if a (virt, mfn) tuple is aligned for a given slot level. m must not
+ * be INVALID_MFN, since alignment is only relevant for present entries.
+ */
+#define IS_LnE_ALIGNED(v, m, n) ({                              \
+    mfn_t m_ = m;                                               \
+                                                                \
+    ASSERT(!mfn_eq(m_, INVALID_MFN));                           \
+    IS_ALIGNED(PFN_DOWN(v) | mfn_x(m_),                         \
+               (1UL << (PAGETABLE_ORDER * (n - 1))) - 1);       \
+})
 #define IS_L2E_ALIGNED(v, m) IS_LnE_ALIGNED(v, m, 2)
 #define IS_L3E_ALIGNED(v, m) IS_LnE_ALIGNED(v, m, 3)
 
@@ -5255,7 +5263,8 @@  int map_pages_to_xen(
         L3T_LOCK(current_l3page);
         ol3e = *pl3e;
 
-        if ( cpu_has_page1gb && IS_L3E_ALIGNED(virt, mfn) &&
+        if ( cpu_has_page1gb &&
+             (!(flags & _PAGE_PRESENT) || IS_L3E_ALIGNED(virt, mfn)) &&
              nr_mfns >= (1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)) &&
              !(flags & (_PAGE_PAT | MAP_SMALL_PAGES)) )
         {
@@ -5374,7 +5383,7 @@  int map_pages_to_xen(
         if ( !pl2e )
             goto out;
 
-        if ( IS_L2E_ALIGNED(virt, mfn) &&
+        if ( (!(flags & _PAGE_PRESENT) || IS_L2E_ALIGNED(virt, mfn)) &&
              (nr_mfns >= (1u << PAGETABLE_ORDER)) &&
              !(flags & (_PAGE_PAT|MAP_SMALL_PAGES)) )
         {
@@ -5544,7 +5553,8 @@  int map_pages_to_xen(
  check_l3:
         if ( cpu_has_page1gb &&
              (flags == PAGE_HYPERVISOR) &&
-             ((nr_mfns == 0) || IS_L3E_ALIGNED(virt, mfn)) )
+             ((nr_mfns == 0) || !(flags & _PAGE_PRESENT) ||
+              IS_L3E_ALIGNED(virt, mfn)) )
         {
             unsigned long base_mfn;
             const l2_pgentry_t *l2t;