Re: [PATCH v4 2/2] xen/riscv: introduce identity mapping

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

Hi all,

I would like to ask for advice on whether it would be easier, less bug-
provoking ( during identity mapping to remove of whole Xen ) to have a
separate identity section that won't be more than PAGE_SIZE.

Please take a look at the changes below. Comments are welcome.

diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index 7d1a8beba8..ba4af48fc6 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -26,6 +26,8 @@ static unsigned long __ro_after_init phys_offset;
 #define LOAD_TO_LINK(addr) ((unsigned long)(addr) - phys_offset)
 #define LINK_TO_LOAD(addr) ((unsigned long)(addr) + phys_offset)
 
+extern char _ident_start[], _ident_end[];
+
 /*
  * It is expected that Xen won't be more then 2 MB.
  * The check in xen.lds.S guarantees that.
@@ -112,7 +114,9 @@ static void __init setup_initial_mapping(struct
mmu_desc *mmu_desc,
         case 1: /* Level 0 */
             {
                 unsigned long paddr = (page_addr - map_start) +
pa_start;
-                unsigned int permissions = PTE_LEAF_DEFAULT;
+                unsigned int permissions = is_identity_mapping
+                                           ? PTE_LEAF_DEFAULT |
PTE_EXECUTABLE
+                                           : PTE_LEAF_DEFAULT ;
                 unsigned long addr = is_identity_mapping
                                      ? page_addr :
LINK_TO_LOAD(page_addr);
                 pte_t pte_to_be_written;
@@ -248,9 +252,9 @@ void __init setup_initial_pagetables(void)
         return;
 
     setup_initial_mapping(&mmu_desc,
-                          load_start,
-                          load_end,
-                          load_start);
+                          (unsigned long)_ident_start,
+                          (unsigned long)_ident_end,
+                          (unsigned long)_ident_start);
 }
 
 void __init enable_mmu(void)
@@ -264,6 +268,19 @@ void __init enable_mmu(void)
               RV_STAGE1_MODE << SATP_MODE_SHIFT);
 }
 
+void __attribute__((naked)) __section(".ident") turn_on_mmu(unsigned
long ra)
+{
+    /* Ensure page table writes precede loading the SATP */
+    sfence_vma();
+
+    /* Enable the MMU and load the new pagetable for Xen */
+    csr_write(CSR_SATP,
+              PFN_DOWN((unsigned long)stage1_pgtbl_root) |
+              RV_STAGE1_MODE << SATP_MODE_SHIFT);
+
+    asm volatile( "jr %0\n" : : "r"(ra) );
+}
+
 static void __init __remove_identity_mapping(pte_t *pgtbl,
                                              unsigned long load_start,
                                              unsigned int pt_level)
@@ -297,20 +314,42 @@ static void __init
__remove_identity_mapping(pte_t *pgtbl,
 
 void __init remove_identity_mapping(void)
 {
-    unsigned long load_start = LINK_TO_LOAD(_start);
+    unsigned int i;
+    pte_t *pgtbl;
+    unsigned int index, xen_index;
+    unsigned long ident_start = LINK_TO_LOAD(_ident_start);
 
-    if ( XEN_VIRT_START <= load_start )
+    for ( pgtbl = stage1_pgtbl_root, i = CONFIG_PAGING_LEVELS; i; i--
)
     {
-        early_printk("remove identity mapping algo expects that"
-                     "XEN_VIRT_START > load_start\n");
-        die();
-    }
+        index = pt_index(i - 1, ident_start);
+        xen_index = pt_index(i - 1, XEN_VIRT_START);
 
-    __remove_identity_mapping(stage1_pgtbl_root,
-                              LINK_TO_LOAD(_start),
-                              CONFIG_PAGING_LEVELS - 1);
+        if ( index != xen_index )
+        {
+            pgtbl[index].pte = 0;
+            break;
+        }
+
+        pgtbl = (pte_t *)pte_to_paddr(pgtbl[index]);
+    }
 }
 
+// void __init remove_identity_mapping(void)
+// {
+//     unsigned long load_start = LINK_TO_LOAD(_start);
+
+//     if ( XEN_VIRT_START <= load_start )
+//     {
+//         early_printk("remove identity mapping algo expects that"
+//                      "XEN_VIRT_START > load_start\n");
+//         die();
+//     }
+
+//     __remove_identity_mapping(stage1_pgtbl_root,
+//                               LINK_TO_LOAD(_start),
+//                               CONFIG_PAGING_LEVELS - 1);
+// }
+
 /*
  * calc_phys_offset() should be used before MMU is enabled because
access to
  * start() is PC-relative and in case when load_addr != linker_addr
phys_offset
diff --git a/xen/arch/riscv/riscv64/head.S
b/xen/arch/riscv/riscv64/head.S
index 613e25ea6f..bb529f6a11 100644
--- a/xen/arch/riscv/riscv64/head.S
+++ b/xen/arch/riscv/riscv64/head.S
@@ -41,14 +41,12 @@ ENTRY(start)
 
         jal     setup_initial_pagetables
 
-        jal     enable_mmu
-
         /* Calculate proper VA after jump from 1:1 mapping */
         la      t0, .L_primary_switched
         sub     t0, t0, s2
 
-        /* Jump from 1:1 mapping world */
-        jr      t0
+        mv      a0, t0
+        jal     turn_on_mmu
 
 .L_primary_switched:
         /*
diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index 31ccebadcb..ffa0225332 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -37,6 +37,13 @@ SECTIONS
         _etext = .;             /* End of text section */
     } :text
 
+    .ident : {
+        . = ALIGN(PAGE_SIZE);
+        _ident_start = .;
+        *(.ident)
+        _ident_end = .;
+    } :text
+
     . = ALIGN(PAGE_SIZE);
     .rodata : {
         _srodata = .;          /* Read-only data */
@@ -178,3 +185,5 @@ ASSERT(!SIZEOF(.got.plt),  ".got.plt non-empty")
  * PGTBL_INITIAL_COUNT.
  */
 ASSERT(_end - _start <= MB(2), "Xen too large for early-boot
assumptions")
+
+ASSERT(_ident_end - _ident_start <= KB(4), "Identity section is bigger
then 4Kb")

~ Oleksii