[Xen-devel] [PATCH MM-PART3 v3 0/9] xen/arm: Provide a generic function to update Xen PT

[Xen-devel] [PATCH MM-PART3 v3 0/9] xen/arm: Provide a generic function to update Xen PT

[Julien Grall]

Hi all,

This is the third part of the boot/memory rework for Xen on Arm. At the
moment, the update to Xen PT is scattered all around mm.c. This makes
difficult to rework Xen memory layout or even ensuring we are following the
Arm Arm properly (and we are not so far!).

This part contains code to provide a generic function to update Xen PT.
While I could have started from scratch, I decided to base the new function
on create_xen_entries() (now renamed xen_pt_update()). This makes slightly
easier to follow the changes.

In this series, the new generic function will only support 3rd-level update
and cannot be used in early boot (i.e because xenheap is not initialized).
This will be extended in follow-up series to allow more use within mm.c.

There are probably some optimization possible around the TLBs flush. I haven't
looked at it so far.

The last two patches of this series is to show how existing callers can be
converted. There are more conversion to come in follow-up series.

For convenience, I provided a branch with all the patches applied base
on staging.

git://xenbits.xen.org/people/julieng/xen-unstable.git branch mm/part3/v3

Cheers,

Julien Grall (9):
  xen/arm: Rework HSCTLR_BASE
  xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE
  xen/arm: mm: Sanity check any update of Xen page tables
  xen/arm: mm: Rework xen_pt_update_entry to avoid use xenmap_operation
  xen/arm: mm: Remove enum xenmap_operation
  xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables
  xen/arm: mm: Rework Xen page-tables walk during update
  xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap()
  xen/arm: mm: Remove set_pte_flags_on_range()

 xen/arch/arm/arm32/head.S       |  12 +-
 xen/arch/arm/arm64/head.S       |  10 +-
 xen/arch/arm/mm.c               | 390 +++++++++++++++++++++++++++-------------
 xen/include/asm-arm/page.h      |   9 +-
 xen/include/asm-arm/processor.h |  56 +++++-
 5 files changed, 332 insertions(+), 145 deletions(-)

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH MM-PART3 v3 1/9] xen/arm: Rework HSCTLR_BASE

[Julien Grall]

The current value of HSCTLR_BASE for Arm64 is pretty wrong. It would
actually turn on SCTLR_EL2.nAA (bit 6) on hardware implementing
ARMv8.4-LSE.

Furthermore, the documentation of what is cleared/set in SCTLR_EL2 is
also not correct and looks like to be a verbatim copy from Arm32.

HSCTLR_BASE is replaced with a bunch of per-architecture new defines
helping to understand better what is the initial value for
SCTLR_EL2/HSCTLR.

Note the defines *_CLEAR are only used to check the state of each bits
are known.

Lastly, the documentation is dropped from arm{32,64}/head.S as it would
be pretty easy to get out-of-sync with the definitions.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii.anisov@epam.com>

---
    Note this patch was part of MM-PART2 before. As this was the only
    patch of the series not merged, it is now part of MM-PART3.

    Changes in v3:
        - Add comment on top of HSCTLR_CLEAR/SCTLR_CLEAR to explain that
        it is only used one time at pre-processing time
        - Fix typo in the commit message
        - Add Andrii's reviewed-by

    Changes in v2:
        - Use BIT(..., UL) instead of _BITUL
---
 xen/arch/arm/arm32/head.S       | 12 +--------
 xen/arch/arm/arm64/head.S       | 10 +-------
 xen/include/asm-arm/processor.h | 56 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 57 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 5f817d473e..18ded49a04 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -224,17 +224,7 @@ cpu_init_done:
         ldr   r0, =(TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0))
         mcr   CP32(r0, HTCR)
 
-        /*
-         * Set up the HSCTLR:
-         * Exceptions in LE ARM,
-         * Low-latency IRQs disabled,
-         * Write-implies-XN disabled (for now),
-         * D-cache disabled (for now),
-         * I-cache enabled,
-         * Alignment checking enabled,
-         * MMU translation disabled (for now).
-         */
-        ldr   r0, =(HSCTLR_BASE|SCTLR_Axx_ELx_A)
+        ldr   r0, =HSCTLR_SET
         mcr   CP32(r0, HSCTLR)
 
         /*
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index ddd3a33108..08094a273e 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -352,15 +352,7 @@ skip_bss:
 
         msr   tcr_el2, x0
 
-        /* Set up the SCTLR_EL2:
-         * Exceptions in LE ARM,
-         * Low-latency IRQs disabled,
-         * Write-implies-XN disabled (for now),
-         * D-cache disabled (for now),
-         * I-cache enabled,
-         * Alignment checking disabled,
-         * MMU translation disabled (for now). */
-        ldr   x0, =(HSCTLR_BASE)
+        ldr   x0, =SCTLR_EL2_SET
         msr   SCTLR_EL2, x0
 
         /* Ensure that any exceptions encountered at EL2
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index bbcba061ca..e9d2ae2715 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -127,6 +127,9 @@
 #define SCTLR_A32_ELx_TE    BIT(30, UL)
 #define SCTLR_A32_ELx_FI    BIT(21, UL)
 
+/* Common bits for SCTLR_ELx for Arm64 */
+#define SCTLR_A64_ELx_SA    BIT(3, UL)
+
 /* Common bits for SCTLR_ELx on all architectures */
 #define SCTLR_Axx_ELx_EE    BIT(25, UL)
 #define SCTLR_Axx_ELx_WXN   BIT(19, UL)
@@ -135,7 +138,58 @@
 #define SCTLR_Axx_ELx_A     BIT(1, UL)
 #define SCTLR_Axx_ELx_M     BIT(0, UL)
 
-#define HSCTLR_BASE     _AC(0x30c51878,U)
+#ifdef CONFIG_ARM_32
+
+#define HSCTLR_RES1     (BIT( 3, UL) | BIT( 4, UL) | BIT( 5, UL) |\
+                         BIT( 6, UL) | BIT(11, UL) | BIT(16, UL) |\
+                         BIT(18, UL) | BIT(22, UL) | BIT(23, UL) |\
+                         BIT(28, UL) | BIT(29, UL))
+
+#define HSCTLR_RES0     (BIT(7, UL)  | BIT(8, UL)  | BIT(9, UL)  | BIT(10, UL) |\
+                         BIT(13, UL) | BIT(14, UL) | BIT(15, UL) | BIT(17, UL) |\
+                         BIT(20, UL) | BIT(24, UL) | BIT(26, UL) | BIT(27, UL) |\
+                         BIT(31, UL))
+
+/* Initial value for HSCTLR */
+#define HSCTLR_SET      (HSCTLR_RES1    | SCTLR_Axx_ELx_A   | SCTLR_Axx_ELx_I)
+
+/* Only used a pre-processing time... */
+#define HSCTLR_CLEAR    (HSCTLR_RES0        | SCTLR_Axx_ELx_M   |\
+                         SCTLR_Axx_ELx_C    | SCTLR_Axx_ELx_WXN |\
+                         SCTLR_A32_ELx_FI   | SCTLR_Axx_ELx_EE  |\
+                         SCTLR_A32_ELx_TE)
+
+#if (HSCTLR_SET ^ HSCTLR_CLEAR) != 0xffffffffU
+#error "Inconsistent HSCTLR set/clear bits"
+#endif
+
+#else
+
+#define SCTLR_EL2_RES1  (BIT( 4, UL) | BIT( 5, UL) | BIT(11, UL) |\
+                         BIT(16, UL) | BIT(18, UL) | BIT(22, UL) |\
+                         BIT(23, UL) | BIT(28, UL) | BIT(29, UL))
+
+#define SCTLR_EL2_RES0  (BIT( 6, UL) | BIT( 7, UL) | BIT( 8, UL) |\
+                         BIT( 9, UL) | BIT(10, UL) | BIT(13, UL) |\
+                         BIT(14, UL) | BIT(15, UL) | BIT(17, UL) |\
+                         BIT(20, UL) | BIT(21, UL) | BIT(24, UL) |\
+                         BIT(26, UL) | BIT(27, UL) | BIT(30, UL) |\
+                         BIT(31, UL) | (0xffffffffULL << 32))
+
+/* Initial value for SCTLR_EL2 */
+#define SCTLR_EL2_SET   (SCTLR_EL2_RES1     | SCTLR_A64_ELx_SA  |\
+                         SCTLR_Axx_ELx_I)
+
+/* Only used a pre-processing time... */
+#define SCTLR_EL2_CLEAR (SCTLR_EL2_RES0     | SCTLR_Axx_ELx_M   |\
+                         SCTLR_Axx_ELx_A    | SCTLR_Axx_ELx_C   |\
+                         SCTLR_Axx_ELx_WXN  | SCTLR_Axx_ELx_EE)
+
+#if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffffUL
+#error "Inconsistent SCTLR_EL2 set/clear bits"
+#endif
+
+#endif
 
 /* HCR Hyp Configuration Register */
 #define HCR_RW          (_AC(1,UL)<<31) /* Register Width, ARM64 only */
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH MM-PART3 v3 2/9] xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE

[Julien Grall]

At the moment, the flags are not enough to describe what kind of update
will done on the VA range. They need to be used in conjunction with the
enum xenmap_operation.

It would be more convenient to have all the information for the update
in a single place.

Two new flags are added to remove the relience on xenmap_operation:
    - _PAGE_PRESENT: Indicate whether we are adding/removing the mapping
    - _PAGE_POPULATE: Indicate whether we only populate page-tables

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v3:
        - Clarify the description of the new flags

    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c          | 2 +-
 xen/include/asm-arm/page.h | 9 +++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 23e9565ddc..b13d9adf40 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1056,7 +1056,7 @@ int map_pages_to_xen(unsigned long virt,
 
 int populate_pt_range(unsigned long virt, unsigned long nr_mfns)
 {
-    return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, 0);
+    return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, _PAGE_POPULATE);
 }
 
 int destroy_xen_mappings(unsigned long v, unsigned long e)
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 2bcdb0f1a5..37e1d9aadb 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -76,6 +76,8 @@
  *
  * [0:2] Memory Attribute Index
  * [3:4] Permission flags
+ * [5]   Page present
+ * [6]   Only populate page tables
  */
 #define PAGE_AI_MASK(x) ((x) & 0x7U)
 
@@ -86,12 +88,15 @@
 #define PAGE_XN_MASK(x) (((x) >> _PAGE_XN_BIT) & 0x1U)
 #define PAGE_RO_MASK(x) (((x) >> _PAGE_RO_BIT) & 0x1U)
 
+#define _PAGE_PRESENT    (1U << 5)
+#define _PAGE_POPULATE   (1U << 6)
+
 /*
  * _PAGE_DEVICE and _PAGE_NORMAL are convenience defines. They are not
  * meant to be used outside of this header.
  */
-#define _PAGE_DEVICE    _PAGE_XN
-#define _PAGE_NORMAL    MT_NORMAL
+#define _PAGE_DEVICE    (_PAGE_XN|_PAGE_PRESENT)
+#define _PAGE_NORMAL    (MT_NORMAL|_PAGE_PRESENT)
 
 #define PAGE_HYPERVISOR_RO      (_PAGE_NORMAL|_PAGE_RO|_PAGE_XN)
 #define PAGE_HYPERVISOR_RX      (_PAGE_NORMAL|_PAGE_RO)
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH MM-PART3 v3 3/9] xen/arm: mm: Sanity check any update of Xen page tables

[Julien Grall]

The code handling Xen PT update has quite a few restrictions on what it
can do. This is not a bad thing as it keeps the code simple.

There are already a few checks scattered in current page table handling.
However they are not sufficient as they could still allow to
modify/remove entry with contiguous bit set.

The checks are divided in two sets:
    - per entry check: They are gathered in a new function that will
    check whether an update is valid based on the flags passed and the
    current value of an entry.
    - global check: They are sanity check on xen_pt_update() parameters.

Additionally to contiguous check, we also now check that the caller is
not trying to modify the memory attributes of an entry.

Lastly, it was probably a bit over the top to forbid removing an
invalid mapping. This could just be ignored. The new behavior will be
helpful in future changes.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v3:
        - Only allow modification on valid entry

    Changes in v2:
        - Correctly detect the removal of a page
        - Fix ASSERT on flags in the else case
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 104 insertions(+), 18 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b13d9adf40..dcf041578b 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -50,6 +50,19 @@
 #undef mfn_to_virt
 #define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn))
 
+#ifdef NDEBUG
+static inline void
+__attribute__ ((__format__ (__printf__, 1, 2)))
+mm_printk(const char *fmt, ...) {}
+#else
+#define mm_printk(fmt, args...)             \
+    do                                      \
+    {                                       \
+        dprintk(XENLOG_ERR, fmt, ## args);  \
+        WARN();                             \
+    } while (0);
+#endif
+
 #define DEFINE_PAGE_TABLES(name, nr)                    \
 lpae_t __aligned(PAGE_SIZE) name[LPAE_ENTRIES * (nr)]
 
@@ -941,12 +954,81 @@ enum xenmap_operation {
     RESERVE
 };
 
+/* Sanity check of the entry */
+static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
+{
+    /* Sanity check when modifying a page. */
+    if ( (flags & _PAGE_PRESENT) && mfn_eq(mfn, INVALID_MFN) )
+    {
+        /* We don't allow modifying an invalid entry. */
+        if ( !lpae_is_valid(entry) )
+        {
+            mm_printk("Modifying invalid entry is not allowed.\n");
+            return false;
+        }
+
+        /* We don't allow changing memory attributes. */
+        if ( entry.pt.ai != PAGE_AI_MASK(flags) )
+        {
+            mm_printk("Modifying memory attributes is not allowed (0x%x -> 0x%x).\n",
+                      entry.pt.ai, PAGE_AI_MASK(flags));
+            return false;
+        }
+
+        /* We don't allow modifying entry with contiguous bit set. */
+        if ( entry.pt.contig )
+        {
+            mm_printk("Modifying entry with contiguous bit set is not allowed.\n");
+            return false;
+        }
+    }
+    /* Sanity check when inserting a page */
+    else if ( flags & _PAGE_PRESENT )
+    {
+        /* We should be here with a valid MFN. */
+        ASSERT(!mfn_eq(mfn, INVALID_MFN));
+
+        /* We don't allow replacing any valid entry. */
+        if ( lpae_is_valid(entry) )
+        {
+            mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
+                      mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
+            return false;
+        }
+    }
+    /* Sanity check when removing a page. */
+    else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) == 0 )
+    {
+        /* We should be here with an invalid MFN. */
+        ASSERT(mfn_eq(mfn, INVALID_MFN));
+
+        /* We don't allow removing page with contiguous bit set. */
+        if ( entry.pt.contig )
+        {
+            mm_printk("Removing entry with contiguous bit set is not allowed.\n");
+            return false;
+        }
+    }
+    /* Sanity check when populating the page-table. No check so far. */
+    else
+    {
+        ASSERT(flags & _PAGE_POPULATE);
+        /* We should be here with an invalid MFN */
+        ASSERT(mfn_eq(mfn, INVALID_MFN));
+    }
+
+    return true;
+}
+
 static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
                                mfn_t mfn, unsigned int flags)
 {
     lpae_t pte, *entry;
     lpae_t *third = NULL;
 
+    /* _PAGE_POPULATE and _PAGE_PRESENT should never be set together. */
+    ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) != (_PAGE_POPULATE|_PAGE_PRESENT));
+
     entry = &xen_second[second_linear_offset(addr)];
     if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
     {
@@ -962,15 +1044,12 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
     third = mfn_to_virt(lpae_get_mfn(*entry));
     entry = &third[third_table_offset(addr)];
 
+    if ( !xen_pt_check_entry(*entry, mfn, flags) )
+        return -EINVAL;
+
     switch ( op ) {
         case INSERT:
         case RESERVE:
-            if ( lpae_is_valid(*entry) )
-            {
-                printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
-                       __func__, addr, mfn_x(mfn));
-                return -EINVAL;
-            }
             if ( op == RESERVE )
                 break;
             pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
@@ -982,12 +1061,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
             break;
         case MODIFY:
         case REMOVE:
-            if ( !lpae_is_valid(*entry) )
-            {
-                printk("%s: trying to %s a non-existing mapping addr=%lx\n",
-                       __func__, op == REMOVE ? "remove" : "modify", addr);
-                return -EINVAL;
-            }
             if ( op == REMOVE )
                 pte.bits = 0;
             else
@@ -995,12 +1068,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
                 pte = *entry;
                 pte.pt.ro = PAGE_RO_MASK(flags);
                 pte.pt.xn = PAGE_XN_MASK(flags);
-                if ( !pte.pt.ro && !pte.pt.xn )
-                {
-                    printk("%s: Incorrect combination for addr=%lx\n",
-                           __func__, addr);
-                    return -EINVAL;
-                }
             }
             write_pte(entry, pte);
             break;
@@ -1022,6 +1089,25 @@ static int xen_pt_update(enum xenmap_operation op,
     int rc = 0;
     unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
 
+    /*
+     * The hardware was configured to forbid mapping both writeable and
+     * executable.
+     * When modifying/creating mapping (i.e _PAGE_PRESENT is set),
+     * prevent any update if this happen.
+     */
+    if ( (flags & _PAGE_PRESENT) && !PAGE_RO_MASK(flags) &&
+         !PAGE_XN_MASK(flags) )
+    {
+        mm_printk("Mappings should not be both Writeable and Executable.\n");
+        return -EINVAL;
+    }
+
+    if ( !IS_ALIGNED(virt, PAGE_SIZE) )
+    {
+        mm_printk("The virtual address is not aligned to the page-size.\n");
+        return -EINVAL;
+    }
+
     spin_lock(&xen_pt_lock);
 
     for( ; addr < addr_end; addr += PAGE_SIZE )
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH MM-PART3 v3 4/9] xen/arm: mm: Rework xen_pt_update_entry to avoid use xenmap_operation

[Julien Grall]

With the newly introduced flags, it is now possible to know how the page
will be updated through the flags.

All the use of xenmap_operation are now replaced with the flags. At the
same time, validity check are now removed as they are gathered in
xen_pt_check_entry().

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v3:
        - Add Stefano's reviewed-by

    Changes in v2:
        - Fix typo in the commit message
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c | 47 +++++++++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index dcf041578b..b2b8bd3dc6 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1047,34 +1047,33 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
     if ( !xen_pt_check_entry(*entry, mfn, flags) )
         return -EINVAL;
 
-    switch ( op ) {
-        case INSERT:
-        case RESERVE:
-            if ( op == RESERVE )
-                break;
+    /* If we are only populating page-table, then we are done. */
+    if ( flags & _PAGE_POPULATE )
+        return 0;
+
+    /* We are removing the page */
+    if ( !(flags & _PAGE_PRESENT) )
+        memset(&pte, 0x00, sizeof(pte));
+    else
+    {
+        /* We are inserting a mapping => Create new pte. */
+        if ( !mfn_eq(mfn, INVALID_MFN) )
+        {
             pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
-            pte.pt.ro = PAGE_RO_MASK(flags);
-            pte.pt.xn = PAGE_XN_MASK(flags);
-            BUG_ON(!pte.pt.ro && !pte.pt.xn);
+
+            /* Third level entries set pte.pt.table = 1 */
             pte.pt.table = 1;
-            write_pte(entry, pte);
-            break;
-        case MODIFY:
-        case REMOVE:
-            if ( op == REMOVE )
-                pte.bits = 0;
-            else
-            {
-                pte = *entry;
-                pte.pt.ro = PAGE_RO_MASK(flags);
-                pte.pt.xn = PAGE_XN_MASK(flags);
-            }
-            write_pte(entry, pte);
-            break;
-        default:
-            BUG();
+        }
+        else /* We are updating the permission => Copy the current pte. */
+            pte = *entry;
+
+        /* Set permission */
+        pte.pt.ro = PAGE_RO_MASK(flags);
+        pte.pt.xn = PAGE_XN_MASK(flags);
     }
 
+    write_pte(entry, pte);
+
     return 0;
 }
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH MM-PART3 v3 5/9] xen/arm: mm: Remove enum xenmap_operation

[Julien Grall]

The enum xenmap_operation is not used anymore. So remove it.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v3:
        - Add Stefano's acked-by

    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b2b8bd3dc6..3f6d0e29d5 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -947,13 +947,6 @@ static int create_xen_table(lpae_t *entry)
     return 0;
 }
 
-enum xenmap_operation {
-    INSERT,
-    REMOVE,
-    MODIFY,
-    RESERVE
-};
-
 /* Sanity check of the entry */
 static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
 {
@@ -1020,8 +1013,8 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
     return true;
 }
 
-static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
-                               mfn_t mfn, unsigned int flags)
+static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
+                               unsigned int flags)
 {
     lpae_t pte, *entry;
     lpae_t *third = NULL;
@@ -1079,8 +1072,7 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
 
 static DEFINE_SPINLOCK(xen_pt_lock);
 
-static int xen_pt_update(enum xenmap_operation op,
-                         unsigned long virt,
+static int xen_pt_update(unsigned long virt,
                          mfn_t mfn,
                          unsigned long nr_mfns,
                          unsigned int flags)
@@ -1111,7 +1103,7 @@ static int xen_pt_update(enum xenmap_operation op,
 
     for( ; addr < addr_end; addr += PAGE_SIZE )
     {
-        rc = xen_pt_update_entry(op, addr, mfn, flags);
+        rc = xen_pt_update_entry(addr, mfn, flags);
         if ( rc )
             break;
 
@@ -1136,24 +1128,24 @@ int map_pages_to_xen(unsigned long virt,
                      unsigned long nr_mfns,
                      unsigned int flags)
 {
-    return xen_pt_update(INSERT, virt, mfn, nr_mfns, flags);
+    return xen_pt_update(virt, mfn, nr_mfns, flags);
 }
 
 int populate_pt_range(unsigned long virt, unsigned long nr_mfns)
 {
-    return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, _PAGE_POPULATE);
+    return xen_pt_update(virt, INVALID_MFN, nr_mfns, _PAGE_POPULATE);
 }
 
 int destroy_xen_mappings(unsigned long v, unsigned long e)
 {
     ASSERT(v <= e);
-    return xen_pt_update(REMOVE, v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
+    return xen_pt_update(v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
 }
 
 int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
 {
     ASSERT(s <= e);
-    return xen_pt_update(MODIFY, s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
+    return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
 }
 
 enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH MM-PART3 v3 6/9] xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables

[Julien Grall]

Currently, the virtual address of the 3rd level page-tables is obtained
using mfn_to_virt().

On Arm32, mfn_to_virt can only work on xenheap page. While in theory
all the page-tables updated will reside in xenheap, in practice the
page-tables covering Xen memory (e.g xen_mapping) is part of Xen binary.

Furthermore, a follow-up change will update xen_pt_update_entry() to
walk all the levels and therefore be more generic. Some of the
page-tables will also part of Xen memory and therefore will not be
reachable using mfn_to_virt().

The easiest way to reach those pages is to use {, un}map_domain_page().
While on arm32 this means an extra mapping in the normal cases, this is not
very important as xen page-tables are not updated often.

In order to allow future change in the way Xen page-tables are mapped,
two new helpers are introduced to map/unmap the page-tables.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v3:
        - Fix typo in the commit message
        - Add Stefano's acked-by

    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 3f6d0e29d5..c3dd2c08ba 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -947,6 +947,16 @@ static int create_xen_table(lpae_t *entry)
     return 0;
 }
 
+static lpae_t *xen_map_table(mfn_t mfn)
+{
+    return map_domain_page(mfn);
+}
+
+static void xen_unmap_table(const lpae_t *table)
+{
+    unmap_domain_page(table);
+}
+
 /* Sanity check of the entry */
 static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
 {
@@ -1016,6 +1026,7 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
 static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
                                unsigned int flags)
 {
+    int rc;
     lpae_t pte, *entry;
     lpae_t *third = NULL;
 
@@ -1034,15 +1045,17 @@ static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
 
     BUG_ON(!lpae_is_valid(*entry));
 
-    third = mfn_to_virt(lpae_get_mfn(*entry));
+    third = xen_map_table(lpae_get_mfn(*entry));
     entry = &third[third_table_offset(addr)];
 
+    rc = -EINVAL;
     if ( !xen_pt_check_entry(*entry, mfn, flags) )
-        return -EINVAL;
+        goto out;
 
     /* If we are only populating page-table, then we are done. */
+    rc = 0;
     if ( flags & _PAGE_POPULATE )
-        return 0;
+        goto out;
 
     /* We are removing the page */
     if ( !(flags & _PAGE_PRESENT) )
@@ -1067,7 +1080,12 @@ static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
 
     write_pte(entry, pte);
 
-    return 0;
+    rc = 0;
+
+out:
+    xen_unmap_table(third);
+
+    return rc;
 }
 
 static DEFINE_SPINLOCK(xen_pt_lock);
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH MM-PART3 v3 7/9] xen/arm: mm: Rework Xen page-tables walk during update

[Julien Grall]

Currently, xen_pt_update_entry() is only able to update the region covered
by xen_second (i.e 0 to 0x7fffffff).

Because of the restriction we end to have multiple functions in mm.c
modifying the page-tables differently.

Furthermore, we never walked the page-tables fully. This means that any
change in the layout may requires major rewrite of the page-tables code.

Lastly, we have been quite lucky that no one ever tried to pass an address
outside this range because it would have blown-up.

xen_pt_update_entry() is reworked to walk over the page-tables every
time. The logic has been borrowed from arch/arm/p2m.c and contain some
limitations for the time being:
    - Superpage cannot be shattered
    - Only level 3 (i.e 4KB) can be done

Note that the parameter 'addr' has been renamed to 'virt' to make clear
we are dealing with a virtual address.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v3:
        - Remove an ASSERT()
        - Add Stefano's acked-by

    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 104 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index c3dd2c08ba..028fbd38ad 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -957,6 +957,51 @@ static void xen_unmap_table(const lpae_t *table)
     unmap_domain_page(table);
 }
 
+#define XEN_TABLE_MAP_FAILED 0
+#define XEN_TABLE_SUPER_PAGE 1
+#define XEN_TABLE_NORMAL_PAGE 2
+
+/*
+ * Take the currently mapped table, find the corresponding entry,
+ * and map the next table, if available.
+ *
+ * The read_only parameters indicates whether intermediate tables should
+ * be allocated when not present.
+ *
+ * Return values:
+ *  XEN_TABLE_MAP_FAILED: Either read_only was set and the entry
+ *  was empty, or allocating a new page failed.
+ *  XEN_TABLE_NORMAL_PAGE: next level mapped normally
+ *  XEN_TABLE_SUPER_PAGE: The next entry points to a superpage.
+ */
+static int xen_pt_next_level(bool read_only, unsigned int level,
+                             lpae_t **table, unsigned int offset)
+{
+    lpae_t *entry;
+    int ret;
+
+    entry = *table + offset;
+
+    if ( !lpae_is_valid(*entry) )
+    {
+        if ( read_only )
+            return XEN_TABLE_MAP_FAILED;
+
+        ret = create_xen_table(entry);
+        if ( ret )
+            return XEN_TABLE_MAP_FAILED;
+    }
+
+    /* The function xen_pt_next_level is never called at the 3rd level */
+    if ( lpae_is_mapping(*entry, level) )
+        return XEN_TABLE_SUPER_PAGE;
+
+    xen_unmap_table(*table);
+    *table = xen_map_table(lpae_get_mfn(*entry));
+
+    return XEN_TABLE_NORMAL_PAGE;
+}
+
 /* Sanity check of the entry */
 static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
 {
@@ -1023,30 +1068,65 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
     return true;
 }
 
-static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
-                               unsigned int flags)
+static int xen_pt_update_entry(mfn_t root, unsigned long virt,
+                               mfn_t mfn, unsigned int flags)
 {
     int rc;
+    unsigned int level;
+    /* We only support 4KB mapping (i.e level 3) for now */
+    unsigned int target = 3;
+    lpae_t *table;
+    /*
+     * The intermediate page tables are read-only when the MFN is not valid
+     * and we are not populating page table.
+     * This means we either modify permissions or remove an entry.
+     */
+    bool read_only = mfn_eq(mfn, INVALID_MFN) && !(flags & _PAGE_POPULATE);
     lpae_t pte, *entry;
-    lpae_t *third = NULL;
+
+    /* convenience aliases */
+    DECLARE_OFFSETS(offsets, (paddr_t)virt);
 
     /* _PAGE_POPULATE and _PAGE_PRESENT should never be set together. */
     ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) != (_PAGE_POPULATE|_PAGE_PRESENT));
 
-    entry = &xen_second[second_linear_offset(addr)];
-    if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
+    table = xen_map_table(root);
+    for ( level = HYP_PT_ROOT_LEVEL; level < target; level++ )
     {
-        int rc = create_xen_table(entry);
-        if ( rc < 0 ) {
-            printk("%s: L2 failed\n", __func__);
-            return rc;
+        rc = xen_pt_next_level(read_only, level, &table, offsets[level]);
+        if ( rc == XEN_TABLE_MAP_FAILED )
+        {
+            /*
+             * We are here because xen_pt_next_level has failed to map
+             * the intermediate page table (e.g the table does not exist
+             * and the pt is read-only). It is a valid case when
+             * removing a mapping as it may not exist in the page table.
+             * In this case, just ignore it.
+             */
+            if ( flags & (_PAGE_PRESENT|_PAGE_POPULATE) )
+            {
+                mm_printk("%s: Unable to map level %u\n", __func__, level);
+                rc = -ENOENT;
+                goto out;
+            }
+            else
+            {
+                rc = 0;
+                goto out;
+            }
         }
+        else if ( rc != XEN_TABLE_NORMAL_PAGE )
+            break;
     }
 
-    BUG_ON(!lpae_is_valid(*entry));
+    if ( level != target )
+    {
+        mm_printk("%s: Shattering superpage is not supported\n", __func__);
+        rc = -EOPNOTSUPP;
+        goto out;
+    }
 
-    third = xen_map_table(lpae_get_mfn(*entry));
-    entry = &third[third_table_offset(addr)];
+    entry = table + offsets[level];
 
     rc = -EINVAL;
     if ( !xen_pt_check_entry(*entry, mfn, flags) )
@@ -1083,7 +1163,7 @@ static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
     rc = 0;
 
 out:
-    xen_unmap_table(third);
+    xen_unmap_table(table);
 
     return rc;
 }
@@ -1099,6 +1179,15 @@ static int xen_pt_update(unsigned long virt,
     unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
 
     /*
+     * For arm32, page-tables are different on each CPUs. Yet, they share
+     * some common mappings. It is assumed that only common mappings
+     * will be modified with this function.
+     *
+     * XXX: Add a check.
+     */
+    const mfn_t root = virt_to_mfn(THIS_CPU_PGTABLE);
+
+    /*
      * The hardware was configured to forbid mapping both writeable and
      * executable.
      * When modifying/creating mapping (i.e _PAGE_PRESENT is set),
@@ -1119,9 +1208,9 @@ static int xen_pt_update(unsigned long virt,
 
     spin_lock(&xen_pt_lock);
 
-    for( ; addr < addr_end; addr += PAGE_SIZE )
+    for ( ; addr < addr_end; addr += PAGE_SIZE )
     {
-        rc = xen_pt_update_entry(addr, mfn, flags);
+        rc = xen_pt_update_entry(root, addr, mfn, flags);
         if ( rc )
             break;
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH MM-PART3 v3 8/9] xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap()

[Julien Grall]

{set, clear}_fixmap() are currently open-coding update to the Xen
page-tables. This can be avoided by using the generic helpers
map_pages_to_xen() and destroy_xen_mappings().

Both function are not meant to fail for fixmap, hence the BUG_ON()
checking the return.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v3:
        - Add Stefano's acked-by

    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 028fbd38ad..46bc3d8075 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -348,19 +348,19 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned attr)
 /* Map a 4k page in a fixmap entry */
 void set_fixmap(unsigned map, mfn_t mfn, unsigned int flags)
 {
-    lpae_t pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
-    pte.pt.table = 1; /* 4k mappings always have this bit set */
-    pte.pt.xn = 1;
-    write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
-    flush_xen_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
+    int res;
+
+    res = map_pages_to_xen(FIXMAP_ADDR(map), mfn, 1, flags);
+    BUG_ON(res != 0);
 }
 
 /* Remove a mapping from a fixmap entry */
 void clear_fixmap(unsigned map)
 {
-    lpae_t pte = {0};
-    write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
-    flush_xen_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
+    int res;
+
+    res = destroy_xen_mappings(FIXMAP_ADDR(map), FIXMAP_ADDR(map) + PAGE_SIZE);
+    BUG_ON(res != 0);
 }
 
 /* Create Xen's mappings of memory.
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH MM-PART3 v3 9/9] xen/arm: mm: Remove set_pte_flags_on_range()

[Julien Grall]

set_pte_flags_on_range() is yet another function that will open-code
update to a specific range in the Xen page-tables. It can be completely
dropped by using either modify_xen_mappings() or destroy_xen_mappings().

Note that modify_xen_mappings() will keep the field 'pxn' cleared for
the all the cases. This is because the field is RES0 for the stage-1
hypervisor as only a single VA range is supported (see D5.4.5 in
DDI0487D.b).

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v3:
        - Update commit message to explain why the field 'pxn' is
        now cleared.

    Changes in v2:
        - Add missing newline in panic
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c | 58 ++++++++++---------------------------------------------
 1 file changed, 10 insertions(+), 48 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 46bc3d8075..35dc1f7e71 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1255,52 +1255,6 @@ int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
     return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
 }
 
-enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
-static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg)
-{
-    lpae_t pte;
-    int i;
-
-    ASSERT(is_kernel(p) && is_kernel(p + l));
-
-    /* Can only guard in page granularity */
-    ASSERT(!((unsigned long) p & ~PAGE_MASK));
-    ASSERT(!(l & ~PAGE_MASK));
-
-    for ( i = (p - _start) / PAGE_SIZE; 
-          i < (p + l - _start) / PAGE_SIZE; 
-          i++ )
-    {
-        pte = xen_xenmap[i];
-        switch ( mg )
-        {
-        case mg_clear:
-            pte.pt.valid = 0;
-            break;
-        case mg_ro:
-            pte.pt.valid = 1;
-            pte.pt.pxn = 1;
-            pte.pt.xn = 1;
-            pte.pt.ro = 1;
-            break;
-        case mg_rw:
-            pte.pt.valid = 1;
-            pte.pt.pxn = 1;
-            pte.pt.xn = 1;
-            pte.pt.ro = 0;
-            break;
-        case mg_rx:
-            pte.pt.valid = 1;
-            pte.pt.pxn = 0;
-            pte.pt.xn = 0;
-            pte.pt.ro = 1;
-            break;
-        }
-        write_pte(xen_xenmap + i, pte);
-    }
-    flush_xen_tlb_local();
-}
-
 /* Release all __init and __initdata ranges to be reused */
 void free_init_memory(void)
 {
@@ -1309,8 +1263,12 @@ void free_init_memory(void)
     uint32_t insn;
     unsigned int i, nr = len / sizeof(insn);
     uint32_t *p;
+    int rc;
 
-    set_pte_flags_on_range(__init_begin, len, mg_rw);
+    rc = modify_xen_mappings((unsigned long)__init_begin,
+                             (unsigned long)__init_end, PAGE_HYPERVISOR_RW);
+    if ( rc )
+        panic("Unable to map RW the init section (rc = %d)\n", rc);
 
     /*
      * From now on, init will not be used for execution anymore,
@@ -1328,7 +1286,11 @@ void free_init_memory(void)
     for ( i = 0; i < nr; i++ )
         *(p + i) = insn;
 
-    set_pte_flags_on_range(__init_begin, len, mg_clear);
+    rc = destroy_xen_mappings((unsigned long)__init_begin,
+                              (unsigned long)__init_end);
+    if ( rc )
+        panic("Unable to remove the init section (rc = %d)\n", rc);
+
     init_domheap_pages(pa, pa + len);
     printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
 }
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART3 v3 9/9] xen/arm: mm: Remove set_pte_flags_on_range()

[Stefano Stabellini]

On Fri, 14 Jun 2019, Julien Grall wrote:
> set_pte_flags_on_range() is yet another function that will open-code
> update to a specific range in the Xen page-tables. It can be completely
> dropped by using either modify_xen_mappings() or destroy_xen_mappings().
> 
> Note that modify_xen_mappings() will keep the field 'pxn' cleared for
> the all the cases. This is because the field is RES0 for the stage-1
> hypervisor as only a single VA range is supported (see D5.4.5 in
> DDI0487D.b).
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v3:
>         - Update commit message to explain why the field 'pxn' is
>         now cleared.
> 
>     Changes in v2:
>         - Add missing newline in panic
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/mm.c | 58 ++++++++++---------------------------------------------
>  1 file changed, 10 insertions(+), 48 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 46bc3d8075..35dc1f7e71 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1255,52 +1255,6 @@ int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
>      return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
>  }
>  
> -enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
> -static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg)
> -{
> -    lpae_t pte;
> -    int i;
> -
> -    ASSERT(is_kernel(p) && is_kernel(p + l));
> -
> -    /* Can only guard in page granularity */
> -    ASSERT(!((unsigned long) p & ~PAGE_MASK));
> -    ASSERT(!(l & ~PAGE_MASK));
> -
> -    for ( i = (p - _start) / PAGE_SIZE; 
> -          i < (p + l - _start) / PAGE_SIZE; 
> -          i++ )
> -    {
> -        pte = xen_xenmap[i];
> -        switch ( mg )
> -        {
> -        case mg_clear:
> -            pte.pt.valid = 0;
> -            break;
> -        case mg_ro:
> -            pte.pt.valid = 1;
> -            pte.pt.pxn = 1;
> -            pte.pt.xn = 1;
> -            pte.pt.ro = 1;
> -            break;
> -        case mg_rw:
> -            pte.pt.valid = 1;
> -            pte.pt.pxn = 1;
> -            pte.pt.xn = 1;
> -            pte.pt.ro = 0;
> -            break;
> -        case mg_rx:
> -            pte.pt.valid = 1;
> -            pte.pt.pxn = 0;
> -            pte.pt.xn = 0;
> -            pte.pt.ro = 1;
> -            break;
> -        }
> -        write_pte(xen_xenmap + i, pte);
> -    }
> -    flush_xen_tlb_local();
> -}
> -
>  /* Release all __init and __initdata ranges to be reused */
>  void free_init_memory(void)
>  {
> @@ -1309,8 +1263,12 @@ void free_init_memory(void)
>      uint32_t insn;
>      unsigned int i, nr = len / sizeof(insn);
>      uint32_t *p;
> +    int rc;
>  
> -    set_pte_flags_on_range(__init_begin, len, mg_rw);
> +    rc = modify_xen_mappings((unsigned long)__init_begin,
> +                             (unsigned long)__init_end, PAGE_HYPERVISOR_RW);
> +    if ( rc )
> +        panic("Unable to map RW the init section (rc = %d)\n", rc);
>  
>      /*
>       * From now on, init will not be used for execution anymore,
> @@ -1328,7 +1286,11 @@ void free_init_memory(void)
>      for ( i = 0; i < nr; i++ )
>          *(p + i) = insn;
>  
> -    set_pte_flags_on_range(__init_begin, len, mg_clear);
> +    rc = destroy_xen_mappings((unsigned long)__init_begin,
> +                              (unsigned long)__init_end);
> +    if ( rc )
> +        panic("Unable to remove the init section (rc = %d)\n", rc);
> +
>      init_domheap_pages(pa, pa + len);
>      printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
>  }
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART3 v3 6/9] xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables

[Stefano Stabellini]

On Fri, 14 Jun 2019, Julien Grall wrote:
> Currently, the virtual address of the 3rd level page-tables is obtained
> using mfn_to_virt().
> 
> On Arm32, mfn_to_virt can only work on xenheap page. While in theory
> all the page-tables updated will reside in xenheap, in practice the
> page-tables covering Xen memory (e.g xen_mapping) is part of Xen binary.
> 
> Furthermore, a follow-up change will update xen_pt_update_entry() to
> walk all the levels and therefore be more generic. Some of the
> page-tables will also part of Xen memory and therefore will not be
> reachable using mfn_to_virt().
> 
> The easiest way to reach those pages is to use {, un}map_domain_page().
> While on arm32 this means an extra mapping in the normal cases, this is not
> very important as xen page-tables are not updated often.
> 
> In order to allow future change in the way Xen page-tables are mapped,
> two new helpers are introduced to map/unmap the page-tables.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
> 
> ---
>     Changes in v3:
>         - Fix typo in the commit message
>         - Add Stefano's acked-by

It didn't stick, so:

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

:-)


> 
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/mm.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 3f6d0e29d5..c3dd2c08ba 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -947,6 +947,16 @@ static int create_xen_table(lpae_t *entry)
>      return 0;
>  }
>  
> +static lpae_t *xen_map_table(mfn_t mfn)
> +{
> +    return map_domain_page(mfn);
> +}
> +
> +static void xen_unmap_table(const lpae_t *table)
> +{
> +    unmap_domain_page(table);
> +}
> +
>  /* Sanity check of the entry */
>  static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
>  {
> @@ -1016,6 +1026,7 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
>  static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
>                                 unsigned int flags)
>  {
> +    int rc;
>      lpae_t pte, *entry;
>      lpae_t *third = NULL;
>  
> @@ -1034,15 +1045,17 @@ static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
>  
>      BUG_ON(!lpae_is_valid(*entry));
>  
> -    third = mfn_to_virt(lpae_get_mfn(*entry));
> +    third = xen_map_table(lpae_get_mfn(*entry));
>      entry = &third[third_table_offset(addr)];
>  
> +    rc = -EINVAL;
>      if ( !xen_pt_check_entry(*entry, mfn, flags) )
> -        return -EINVAL;
> +        goto out;
>  
>      /* If we are only populating page-table, then we are done. */
> +    rc = 0;
>      if ( flags & _PAGE_POPULATE )
> -        return 0;
> +        goto out;
>  
>      /* We are removing the page */
>      if ( !(flags & _PAGE_PRESENT) )
> @@ -1067,7 +1080,12 @@ static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
>  
>      write_pte(entry, pte);
>  
> -    return 0;
> +    rc = 0;
> +
> +out:
> +    xen_unmap_table(third);
> +
> +    return rc;
>  }
>  
>  static DEFINE_SPINLOCK(xen_pt_lock);
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART3 v3 3/9] xen/arm: mm: Sanity check any update of Xen page tables

[Stefano Stabellini]

On Fri, 14 Jun 2019, Julien Grall wrote:
> The code handling Xen PT update has quite a few restrictions on what it
> can do. This is not a bad thing as it keeps the code simple.
> 
> There are already a few checks scattered in current page table handling.
> However they are not sufficient as they could still allow to
> modify/remove entry with contiguous bit set.
> 
> The checks are divided in two sets:
>     - per entry check: They are gathered in a new function that will
>     check whether an update is valid based on the flags passed and the
>     current value of an entry.
>     - global check: They are sanity check on xen_pt_update() parameters.
> 
> Additionally to contiguous check, we also now check that the caller is
> not trying to modify the memory attributes of an entry.
> 
> Lastly, it was probably a bit over the top to forbid removing an
> invalid mapping. This could just be ignored. The new behavior will be
> helpful in future changes.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v3:
>         - Only allow modification on valid entry
> 
>     Changes in v2:
>         - Correctly detect the removal of a page
>         - Fix ASSERT on flags in the else case
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/mm.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 104 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index b13d9adf40..dcf041578b 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -50,6 +50,19 @@
>  #undef mfn_to_virt
>  #define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn))
>  
> +#ifdef NDEBUG
> +static inline void
> +__attribute__ ((__format__ (__printf__, 1, 2)))
> +mm_printk(const char *fmt, ...) {}
> +#else
> +#define mm_printk(fmt, args...)             \
> +    do                                      \
> +    {                                       \
> +        dprintk(XENLOG_ERR, fmt, ## args);  \
> +        WARN();                             \
> +    } while (0);
> +#endif
> +
>  #define DEFINE_PAGE_TABLES(name, nr)                    \
>  lpae_t __aligned(PAGE_SIZE) name[LPAE_ENTRIES * (nr)]
>  
> @@ -941,12 +954,81 @@ enum xenmap_operation {
>      RESERVE
>  };
>  
> +/* Sanity check of the entry */
> +static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
> +{
> +    /* Sanity check when modifying a page. */
> +    if ( (flags & _PAGE_PRESENT) && mfn_eq(mfn, INVALID_MFN) )
> +    {
> +        /* We don't allow modifying an invalid entry. */
> +        if ( !lpae_is_valid(entry) )
> +        {
> +            mm_printk("Modifying invalid entry is not allowed.\n");
> +            return false;
> +        }
> +
> +        /* We don't allow changing memory attributes. */
> +        if ( entry.pt.ai != PAGE_AI_MASK(flags) )
> +        {
> +            mm_printk("Modifying memory attributes is not allowed (0x%x -> 0x%x).\n",
> +                      entry.pt.ai, PAGE_AI_MASK(flags));
> +            return false;
> +        }
> +
> +        /* We don't allow modifying entry with contiguous bit set. */
> +        if ( entry.pt.contig )
> +        {
> +            mm_printk("Modifying entry with contiguous bit set is not allowed.\n");
> +            return false;
> +        }
> +    }
> +    /* Sanity check when inserting a page */
> +    else if ( flags & _PAGE_PRESENT )
> +    {
> +        /* We should be here with a valid MFN. */
> +        ASSERT(!mfn_eq(mfn, INVALID_MFN));
> +
> +        /* We don't allow replacing any valid entry. */
> +        if ( lpae_is_valid(entry) )
> +        {
> +            mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
> +                      mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
> +            return false;
> +        }
> +    }
> +    /* Sanity check when removing a page. */
> +    else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) == 0 )
> +    {
> +        /* We should be here with an invalid MFN. */
> +        ASSERT(mfn_eq(mfn, INVALID_MFN));
> +
> +        /* We don't allow removing page with contiguous bit set. */
> +        if ( entry.pt.contig )
> +        {
> +            mm_printk("Removing entry with contiguous bit set is not allowed.\n");
> +            return false;
> +        }
> +    }
> +    /* Sanity check when populating the page-table. No check so far. */
> +    else
> +    {
> +        ASSERT(flags & _PAGE_POPULATE);
> +        /* We should be here with an invalid MFN */
> +        ASSERT(mfn_eq(mfn, INVALID_MFN));
> +    }
> +
> +    return true;
> +}
> +
>  static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
>                                 mfn_t mfn, unsigned int flags)
>  {
>      lpae_t pte, *entry;
>      lpae_t *third = NULL;
>  
> +    /* _PAGE_POPULATE and _PAGE_PRESENT should never be set together. */
> +    ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) != (_PAGE_POPULATE|_PAGE_PRESENT));
> +
>      entry = &xen_second[second_linear_offset(addr)];
>      if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
>      {
> @@ -962,15 +1044,12 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
>      third = mfn_to_virt(lpae_get_mfn(*entry));
>      entry = &third[third_table_offset(addr)];
>  
> +    if ( !xen_pt_check_entry(*entry, mfn, flags) )
> +        return -EINVAL;
> +
>      switch ( op ) {
>          case INSERT:
>          case RESERVE:
> -            if ( lpae_is_valid(*entry) )
> -            {
> -                printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
> -                       __func__, addr, mfn_x(mfn));
> -                return -EINVAL;
> -            }
>              if ( op == RESERVE )
>                  break;
>              pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
> @@ -982,12 +1061,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
>              break;
>          case MODIFY:
>          case REMOVE:
> -            if ( !lpae_is_valid(*entry) )
> -            {
> -                printk("%s: trying to %s a non-existing mapping addr=%lx\n",
> -                       __func__, op == REMOVE ? "remove" : "modify", addr);
> -                return -EINVAL;
> -            }
>              if ( op == REMOVE )
>                  pte.bits = 0;
>              else
> @@ -995,12 +1068,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
>                  pte = *entry;
>                  pte.pt.ro = PAGE_RO_MASK(flags);
>                  pte.pt.xn = PAGE_XN_MASK(flags);
> -                if ( !pte.pt.ro && !pte.pt.xn )
> -                {
> -                    printk("%s: Incorrect combination for addr=%lx\n",
> -                           __func__, addr);
> -                    return -EINVAL;
> -                }
>              }
>              write_pte(entry, pte);
>              break;
> @@ -1022,6 +1089,25 @@ static int xen_pt_update(enum xenmap_operation op,
>      int rc = 0;
>      unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
>  
> +    /*
> +     * The hardware was configured to forbid mapping both writeable and
> +     * executable.
> +     * When modifying/creating mapping (i.e _PAGE_PRESENT is set),
> +     * prevent any update if this happen.
> +     */
> +    if ( (flags & _PAGE_PRESENT) && !PAGE_RO_MASK(flags) &&
> +         !PAGE_XN_MASK(flags) )
> +    {
> +        mm_printk("Mappings should not be both Writeable and Executable.\n");
> +        return -EINVAL;
> +    }
> +
> +    if ( !IS_ALIGNED(virt, PAGE_SIZE) )
> +    {
> +        mm_printk("The virtual address is not aligned to the page-size.\n");
> +        return -EINVAL;
> +    }
> +
>      spin_lock(&xen_pt_lock);
>  
>      for( ; addr < addr_end; addr += PAGE_SIZE )
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART3 v3 2/9] xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE

[Stefano Stabellini]

On Fri, 14 Jun 2019, Julien Grall wrote:
> At the moment, the flags are not enough to describe what kind of update
> will done on the VA range. They need to be used in conjunction with the
> enum xenmap_operation.
> 
> It would be more convenient to have all the information for the update
> in a single place.
> 
> Two new flags are added to remove the relience on xenmap_operation:
>     - _PAGE_PRESENT: Indicate whether we are adding/removing the mapping
>     - _PAGE_POPULATE: Indicate whether we only populate page-tables
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v3:
>         - Clarify the description of the new flags
> 
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/mm.c          | 2 +-
>  xen/include/asm-arm/page.h | 9 +++++++--
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 23e9565ddc..b13d9adf40 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1056,7 +1056,7 @@ int map_pages_to_xen(unsigned long virt,
>  
>  int populate_pt_range(unsigned long virt, unsigned long nr_mfns)
>  {
> -    return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, 0);
> +    return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, _PAGE_POPULATE);
>  }
>  
>  int destroy_xen_mappings(unsigned long v, unsigned long e)
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index 2bcdb0f1a5..37e1d9aadb 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -76,6 +76,8 @@
>   *
>   * [0:2] Memory Attribute Index
>   * [3:4] Permission flags
> + * [5]   Page present
> + * [6]   Only populate page tables
>   */
>  #define PAGE_AI_MASK(x) ((x) & 0x7U)
>  
> @@ -86,12 +88,15 @@
>  #define PAGE_XN_MASK(x) (((x) >> _PAGE_XN_BIT) & 0x1U)
>  #define PAGE_RO_MASK(x) (((x) >> _PAGE_RO_BIT) & 0x1U)
>  
> +#define _PAGE_PRESENT    (1U << 5)
> +#define _PAGE_POPULATE   (1U << 6)
> +
>  /*
>   * _PAGE_DEVICE and _PAGE_NORMAL are convenience defines. They are not
>   * meant to be used outside of this header.
>   */
> -#define _PAGE_DEVICE    _PAGE_XN
> -#define _PAGE_NORMAL    MT_NORMAL
> +#define _PAGE_DEVICE    (_PAGE_XN|_PAGE_PRESENT)
> +#define _PAGE_NORMAL    (MT_NORMAL|_PAGE_PRESENT)
>  
>  #define PAGE_HYPERVISOR_RO      (_PAGE_NORMAL|_PAGE_RO|_PAGE_XN)
>  #define PAGE_HYPERVISOR_RX      (_PAGE_NORMAL|_PAGE_RO)
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART3 v3 1/9] xen/arm: Rework HSCTLR_BASE

[Stefano Stabellini]

On Fri, 14 Jun 2019, Julien Grall wrote:
> The current value of HSCTLR_BASE for Arm64 is pretty wrong. It would
> actually turn on SCTLR_EL2.nAA (bit 6) on hardware implementing
> ARMv8.4-LSE.
> 
> Furthermore, the documentation of what is cleared/set in SCTLR_EL2 is
> also not correct and looks like to be a verbatim copy from Arm32.
> 
> HSCTLR_BASE is replaced with a bunch of per-architecture new defines
> helping to understand better what is the initial value for
> SCTLR_EL2/HSCTLR.
> 
> Note the defines *_CLEAR are only used to check the state of each bits
> are known.
> 
> Lastly, the documentation is dropped from arm{32,64}/head.S as it would
> be pretty easy to get out-of-sync with the definitions.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii.anisov@epam.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Note this patch was part of MM-PART2 before. As this was the only
>     patch of the series not merged, it is now part of MM-PART3.
> 
>     Changes in v3:
>         - Add comment on top of HSCTLR_CLEAR/SCTLR_CLEAR to explain that
>         it is only used one time at pre-processing time
>         - Fix typo in the commit message
>         - Add Andrii's reviewed-by
> 
>     Changes in v2:
>         - Use BIT(..., UL) instead of _BITUL
> ---
>  xen/arch/arm/arm32/head.S       | 12 +--------
>  xen/arch/arm/arm64/head.S       | 10 +-------
>  xen/include/asm-arm/processor.h | 56 ++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 57 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 5f817d473e..18ded49a04 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -224,17 +224,7 @@ cpu_init_done:
>          ldr   r0, =(TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(0))
>          mcr   CP32(r0, HTCR)
>  
> -        /*
> -         * Set up the HSCTLR:
> -         * Exceptions in LE ARM,
> -         * Low-latency IRQs disabled,
> -         * Write-implies-XN disabled (for now),
> -         * D-cache disabled (for now),
> -         * I-cache enabled,
> -         * Alignment checking enabled,
> -         * MMU translation disabled (for now).
> -         */
> -        ldr   r0, =(HSCTLR_BASE|SCTLR_Axx_ELx_A)
> +        ldr   r0, =HSCTLR_SET
>          mcr   CP32(r0, HSCTLR)
>  
>          /*
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index ddd3a33108..08094a273e 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -352,15 +352,7 @@ skip_bss:
>  
>          msr   tcr_el2, x0
>  
> -        /* Set up the SCTLR_EL2:
> -         * Exceptions in LE ARM,
> -         * Low-latency IRQs disabled,
> -         * Write-implies-XN disabled (for now),
> -         * D-cache disabled (for now),
> -         * I-cache enabled,
> -         * Alignment checking disabled,
> -         * MMU translation disabled (for now). */
> -        ldr   x0, =(HSCTLR_BASE)
> +        ldr   x0, =SCTLR_EL2_SET
>          msr   SCTLR_EL2, x0
>  
>          /* Ensure that any exceptions encountered at EL2
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index bbcba061ca..e9d2ae2715 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -127,6 +127,9 @@
>  #define SCTLR_A32_ELx_TE    BIT(30, UL)
>  #define SCTLR_A32_ELx_FI    BIT(21, UL)
>  
> +/* Common bits for SCTLR_ELx for Arm64 */
> +#define SCTLR_A64_ELx_SA    BIT(3, UL)
> +
>  /* Common bits for SCTLR_ELx on all architectures */
>  #define SCTLR_Axx_ELx_EE    BIT(25, UL)
>  #define SCTLR_Axx_ELx_WXN   BIT(19, UL)
> @@ -135,7 +138,58 @@
>  #define SCTLR_Axx_ELx_A     BIT(1, UL)
>  #define SCTLR_Axx_ELx_M     BIT(0, UL)
>  
> -#define HSCTLR_BASE     _AC(0x30c51878,U)
> +#ifdef CONFIG_ARM_32
> +
> +#define HSCTLR_RES1     (BIT( 3, UL) | BIT( 4, UL) | BIT( 5, UL) |\
> +                         BIT( 6, UL) | BIT(11, UL) | BIT(16, UL) |\
> +                         BIT(18, UL) | BIT(22, UL) | BIT(23, UL) |\
> +                         BIT(28, UL) | BIT(29, UL))
> +
> +#define HSCTLR_RES0     (BIT(7, UL)  | BIT(8, UL)  | BIT(9, UL)  | BIT(10, UL) |\
> +                         BIT(13, UL) | BIT(14, UL) | BIT(15, UL) | BIT(17, UL) |\
> +                         BIT(20, UL) | BIT(24, UL) | BIT(26, UL) | BIT(27, UL) |\
> +                         BIT(31, UL))
> +
> +/* Initial value for HSCTLR */
> +#define HSCTLR_SET      (HSCTLR_RES1    | SCTLR_Axx_ELx_A   | SCTLR_Axx_ELx_I)
> +
> +/* Only used a pre-processing time... */
> +#define HSCTLR_CLEAR    (HSCTLR_RES0        | SCTLR_Axx_ELx_M   |\
> +                         SCTLR_Axx_ELx_C    | SCTLR_Axx_ELx_WXN |\
> +                         SCTLR_A32_ELx_FI   | SCTLR_Axx_ELx_EE  |\
> +                         SCTLR_A32_ELx_TE)
> +
> +#if (HSCTLR_SET ^ HSCTLR_CLEAR) != 0xffffffffU
> +#error "Inconsistent HSCTLR set/clear bits"
> +#endif
> +
> +#else
> +
> +#define SCTLR_EL2_RES1  (BIT( 4, UL) | BIT( 5, UL) | BIT(11, UL) |\
> +                         BIT(16, UL) | BIT(18, UL) | BIT(22, UL) |\
> +                         BIT(23, UL) | BIT(28, UL) | BIT(29, UL))
> +
> +#define SCTLR_EL2_RES0  (BIT( 6, UL) | BIT( 7, UL) | BIT( 8, UL) |\
> +                         BIT( 9, UL) | BIT(10, UL) | BIT(13, UL) |\
> +                         BIT(14, UL) | BIT(15, UL) | BIT(17, UL) |\
> +                         BIT(20, UL) | BIT(21, UL) | BIT(24, UL) |\
> +                         BIT(26, UL) | BIT(27, UL) | BIT(30, UL) |\
> +                         BIT(31, UL) | (0xffffffffULL << 32))
> +
> +/* Initial value for SCTLR_EL2 */
> +#define SCTLR_EL2_SET   (SCTLR_EL2_RES1     | SCTLR_A64_ELx_SA  |\
> +                         SCTLR_Axx_ELx_I)
> +
> +/* Only used a pre-processing time... */
> +#define SCTLR_EL2_CLEAR (SCTLR_EL2_RES0     | SCTLR_Axx_ELx_M   |\
> +                         SCTLR_Axx_ELx_A    | SCTLR_Axx_ELx_C   |\
> +                         SCTLR_Axx_ELx_WXN  | SCTLR_Axx_ELx_EE)
> +
> +#if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffffUL
> +#error "Inconsistent SCTLR_EL2 set/clear bits"
> +#endif
> +
> +#endif
>  
>  /* HCR Hyp Configuration Register */
>  #define HCR_RW          (_AC(1,UL)<<31) /* Register Width, ARM64 only */
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART3 v3 6/9] xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables

[Julien Grall]

Hi Stefano,

On 6/14/19 10:00 PM, Stefano Stabellini wrote:
> On Fri, 14 Jun 2019, Julien Grall wrote:
>> Currently, the virtual address of the 3rd level page-tables is obtained
>> using mfn_to_virt().
>>
>> On Arm32, mfn_to_virt can only work on xenheap page. While in theory
>> all the page-tables updated will reside in xenheap, in practice the
>> page-tables covering Xen memory (e.g xen_mapping) is part of Xen binary.
>>
>> Furthermore, a follow-up change will update xen_pt_update_entry() to
>> walk all the levels and therefore be more generic. Some of the
>> page-tables will also part of Xen memory and therefore will not be
>> reachable using mfn_to_virt().
>>
>> The easiest way to reach those pages is to use {, un}map_domain_page().
>> While on arm32 this means an extra mapping in the normal cases, this is not
>> very important as xen page-tables are not updated often.
>>
>> In order to allow future change in the way Xen page-tables are mapped,
>> two new helpers are introduced to map/unmap the page-tables.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
>>
>> ---
>>      Changes in v3:
>>          - Fix typo in the commit message
>>          - Add Stefano's acked-by
> 
> It didn't stick, so:
> 
> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> :-)

Whoops yes. Thankfully I didn't add an ack by mistake this time :).

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART3 v3 0/9] xen/arm: Provide a generic function to update Xen PT

[Julien Grall]

Hi,

On 6/14/19 6:51 PM, Julien Grall wrote:
> Julien Grall (9):
>    xen/arm: Rework HSCTLR_BASE
>    xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE
>    xen/arm: mm: Sanity check any update of Xen page tables
>    xen/arm: mm: Rework xen_pt_update_entry to avoid use xenmap_operation
>    xen/arm: mm: Remove enum xenmap_operation
>    xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables
>    xen/arm: mm: Rework Xen page-tables walk during update
>    xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap()
>    xen/arm: mm: Remove set_pte_flags_on_range()

They are all committed now. Thank you for the review!

I am still working on the follow-up series. I am hoping to send more 
before XDS.

Cheers,

> 
>   xen/arch/arm/arm32/head.S       |  12 +-
>   xen/arch/arm/arm64/head.S       |  10 +-
>   xen/arch/arm/mm.c               | 390 +++++++++++++++++++++++++++-------------
>   xen/include/asm-arm/page.h      |   9 +-
>   xen/include/asm-arm/processor.h |  56 +++++-
>   5 files changed, 332 insertions(+), 145 deletions(-)
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel