[PATCH v7 0/2] xen/riscv: introduce identity mapping

[PATCH v7 0/2] xen/riscv: introduce identity mapping

[Oleksii Kurochko]

The patch series introduces things necessary to implement identity mapping:
  1. Make identity mapping for the entire Xen.
  2. Enable MMU.
  3. Jump to the virtual address world
  4. Remove identity mapping.

Also current patch series introduces the calculation of physical offset before
MMU is enabled as access to physical offset will be calculated wrong after
MMU will be enabled because access to phys_off variable is PC-relative and
in the case when linker address != load address, it will cause MMU fault.

The reason for this patch series can be found here:
https://lore.kernel.org/xen-devel/4e336121-fc0c-b007-bf7b-430352563d55@citrix.com/

---
Changes in V7:
 - use srli instruction to be consistent with slli instruction in turn_on_mmu()
---
Changes in V6:
  - Update calc_phys_offset() after rebase.
  - Refactor turn_on_mmu() and a way how an argument of turn_on_mmu() is
    calculated.
---
Changes in V5:
	- update the algo of identity mapping removing.
	- introduce IDENT_AREA_SIZE.
	- introduce turn_on_mmu() function to enable and switch from 1:1 mapping.
	- fix typo in PGTBL_INITIAL_COUNT define.
	- update the comment above PGTBL_INITIAL_COUNT.
	- update prototype of calc_phys_offset(). now it returns phys_offset.
	- declare phys_offset as static.
	- save returned value of calc_phys_offset to register s2.
---
Changes in V4:
  - drop patch 	[PATCH v3 1/3] xen/riscv: add SPDX tag to config.h as it was
    merged to staging
  - remove definition of ARRAY_SIZE and ROUNDUP as <xen/macors.h> was introduced where these macros are located now.
	- update definition of PGTBL_INITIAL_COUNT
	- update the commit message for patch 'xen/riscv: introduce identity mapping'
	- update the comments in head.S
  - update the algo of identity mapping removing 
---
Changes in V3:
 - Update the patch series message.
 - The following patches were merged to staging so droped from the patch series:
   * xen/riscv: add .sbss section to .bss
   * xen/riscv: introduce reset_stack() function
   * xen/riscv: move extern of cpu0_boot_stack to header
   * xen/riscv: add SPDX tags
 - move save/restore of a0/a1 registers from patch 4 to patch 2 ( numbers are
   from the previous patch series version )
 - add SPDX tag in config.h
 - update definition of PGTBL_INITIAL_COUNT taking into account identity mapping.
 - refactor remove_identity_mapping() function.
 - add explanatory comments in xen.lds.S and mm.c.
---
Changes in V2:
 - update the patch series message.
 - drop patches from the previous version of the patch series:
   * xen/riscv: add __ASSEMBLY__ guards". ( merged )
   * xen/riscv: make sure that identity mapping isn't bigger then page size
     ( entire Xen is 1:1 mapped so there is no need for the checks from the patch )
 - add .sbss.* and put it befor .bss* .
 - move out reset_stack() to .text section.
 - add '__ro_after_init' for phys_offset variable.
 - add '__init' for calc_phys_offset().
 - declaring variable phys_off as non static as it will be used in head.S.
 - update definition of PGTBL_INITIAL_COUNT and the comment above.
 - code style fixes.
 - remove id_addrs array becase entire Xen is mapped.
 - reverse condition for cycle inside remove_identity_mapping().
 - fix page table walk in remove_identity_mapping().
 - save hart_id and dtb_addr before call MMU related C functions
 - use phys_offset variable instead of doing calcultations to get phys offset
   in head.S file. ( it can be easily done as entire Xen is 1:1 mapped now )
 - declare enable_muu() as __init.
 - Update SPDX tags.
 - Add Review-By/Suggested-By for some patches.
 - code style fixes.

Oleksii Kurochko (2):
  xen/riscv: introduce function for physical offset calculation
  xen/riscv: introduce identity mapping

 xen/arch/riscv/include/asm/acpi.h   |   6 ++
 xen/arch/riscv/include/asm/config.h |   2 +
 xen/arch/riscv/include/asm/mm.h     |   7 +-
 xen/arch/riscv/mm.c                 | 109 +++++++++++++++++-----------
 xen/arch/riscv/riscv64/head.S       |  44 +++++++++++
 xen/arch/riscv/setup.c              |  14 +---
 xen/arch/riscv/xen.lds.S            |  11 +++
 7 files changed, 136 insertions(+), 57 deletions(-)
 create mode 100644 xen/arch/riscv/include/asm/acpi.h

-- 
2.41.0

[PATCH v7 1/2] xen/riscv: introduce function for physical offset calculation

[Oleksii Kurochko]

The function was introduced to calculate and save physical
offset before MMU is enabled because access to start() is
PC-relative and in case of linker_addr != load_addr it will
result in incorrect value in phys_offset.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V7:
 - nothing changed. only rebase
---
Changes in V6:
 - After 9380f06fe8("xen: Drop the (almost) unused extern start[]")
   an update of calc_phys_offset() was needed:
   - change start to _start.
   - add local volatile variable load_start to get properly Xens' load address.
     Otherwise compile thinks that (start - XEN_VIRT_START) is equal to 0.
---
Changes in V5:
 - update prototype of calc_phys_offset(). now it returns phys_offset.
 - declare phys_offset as static.
 - save returned value of calc_phys_offset to register s2.
---
Changes in V4:
 - update the comment messages in head.S related to save/restore of a0/a1 regs.
---
Changes in V3:
 - save/restore of a0/a1 registers before C first function call.
---
Changes in V2:
  - add __ro_after_init for phys_offset variable.
  - remove double blank lines.
  - add __init for calc_phys_offset().
  - update the commit message.
  - declaring variable phys_off as non static as it will be used in head.S.
---
 xen/arch/riscv/include/asm/mm.h |  2 ++
 xen/arch/riscv/mm.c             | 21 ++++++++++++++++++---
 xen/arch/riscv/riscv64/head.S   | 14 ++++++++++++++
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index 5e3ac5cde3..7b94cbadd7 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -15,4 +15,6 @@ void setup_initial_pagetables(void);
 void enable_mmu(void);
 void cont_after_mmu_is_enabled(void);
 
+unsigned long calc_phys_offset(void);
+
 #endif /* _ASM_RISCV_MM_H */
diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index 99ed5e9623..a73f135a3c 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
+#include <xen/cache.h>
 #include <xen/compiler.h>
 #include <xen/init.h>
 #include <xen/kernel.h>
@@ -19,9 +20,10 @@ struct mmu_desc {
     pte_t *pgtbl_base;
 };
 
-#define PHYS_OFFSET ((unsigned long)_start - XEN_VIRT_START)
-#define LOAD_TO_LINK(addr) ((addr) - PHYS_OFFSET)
-#define LINK_TO_LOAD(addr) ((addr) + PHYS_OFFSET)
+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)
 
 /*
  * It is expected that Xen won't be more then 2 MB.
@@ -273,3 +275,16 @@ void __init noreturn noinline enable_mmu()
     switch_stack_and_jump((unsigned long)cpu0_boot_stack + STACK_SIZE,
                           cont_after_mmu_is_enabled);
 }
+
+/*
+ * 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
+ * will have an incorrect value
+ */
+unsigned long __init calc_phys_offset(void)
+{
+    volatile unsigned long load_start = (unsigned long)_start;
+
+    phys_offset = load_start - XEN_VIRT_START;
+    return phys_offset;
+}
diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
index 2c0304646a..ae194bb099 100644
--- a/xen/arch/riscv/riscv64/head.S
+++ b/xen/arch/riscv/riscv64/head.S
@@ -29,6 +29,20 @@ ENTRY(start)
 
         jal     reset_stack
 
+        /*
+         * save hart_id ( bootcpu_id ) and dtb_base as a0 and a1 register can
+         * be used by C code
+         */
+        mv      s0, a0
+        mv      s1, a1
+
+        jal     calc_phys_offset
+        mv      s2, a0
+
+        /* restore hart_id ( bootcpu_id ) and dtb address */
+        mv      a0, s0
+        mv      a1, s1
+
         tail    start_xen
 
         .section .text, "ax", %progbits
-- 
2.41.0

[PATCH v7 2/2] xen/riscv: introduce identity mapping

[Oleksii Kurochko]

The way how switch to virtual address was implemented in the
commit e66003e7be ("xen/riscv: introduce setup_initial_pages")
isn't safe enough as:
* enable_mmu() depends on hooking all exceptions
  and pagefault.
* Any exception other than pagefault, or not taking a pagefault
  causes it to malfunction, which means you will fail to boot
  depending on where Xen was loaded into memory.

Instead of the proposed way of switching to virtual addresses was
decided to use identity mapping for area which constains needed code
to switch from identity mapping and after switching to virtual addresses,
identity mapping is removed from page-tables in the following way:
search for top-most page table entry and remove it.

Fixes: e66003e7be ("xen/riscv: introduce setup_initial_pages")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in V7:
 - use srli instruction to be consistent with slli instruction in turn_on_mmu()
---
Changes in V6:
 - t2 register was renamed to t1 as t1 isn't used anymore in
   turn_on_mmu().
 - fold li, sll instructions to slli.
 - use a0 directly to calculate an argument of turn_on_mmu() instead of t0.
---
Changes in V5:
 - update the algo of identity mapping removing.
 - introduce IDENT_AREA_SIZE.
 - introduce turn_on_mmu() function to enable and switch from 1:1 mapping.
 - fix typo in PGTBL_INITIAL_COUNT define.
 - update the comment above PGTBL_INITIAL_COUNT.
 - update the commit message.
---
Changes in V4:
 - remove definition of ARRAY_SIZE and ROUNDUP as <xen/macors.h> was introduced where these macros are located now.
 - update definition of PGTBL_INITIAL_COUNT
 - update the commit message
 - update the algo of identity mapping removing
---
Changes in V3:
 - remove unrelated to the patch changes ( SPDX tags in config.h ).
 - update definition of PGTBL_INITIAL_COUNT taking into account identity mapping.
 - refactor remove_identity_mapping() function.
 - add explanatory comments in xen.lds.S and mm.c.
 - update commit message.
 - move save/restore of a0/a1 registers to [PATCH v2 2/3] xen/riscv: introduce
   function for physical offset calculation.
---
Changes in V2:
  - update definition of PGTBL_INITIAL_COUNT and the comment above.
  - code style fixes.
  - 1:1 mapping for entire Xen.
  - remove id_addrs array becase entire Xen is mapped.
  - reverse condition for cycle inside remove_identity_mapping().
  - fix page table walk in remove_identity_mapping().
  - update the commit message.
  - add Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
  - save hart_id and dtb_addr before call MMU related C functions.
  - use phys_offset variable instead of doing calcultations to get phys offset
    in head.S file. ( it can be easily done as entire Xen is 1:1 mapped )
  - declare enable_muu() as __init.
---
 xen/arch/riscv/include/asm/acpi.h   |  6 ++
 xen/arch/riscv/include/asm/config.h |  2 +
 xen/arch/riscv/include/asm/mm.h     |  5 +-
 xen/arch/riscv/mm.c                 | 90 +++++++++++++++--------------
 xen/arch/riscv/riscv64/head.S       | 30 ++++++++++
 xen/arch/riscv/setup.c              | 14 +----
 xen/arch/riscv/xen.lds.S            | 11 ++++
 7 files changed, 103 insertions(+), 55 deletions(-)
 create mode 100644 xen/arch/riscv/include/asm/acpi.h

diff --git a/xen/arch/riscv/include/asm/acpi.h b/xen/arch/riscv/include/asm/acpi.h
new file mode 100644
index 0000000000..3aef993d81
--- /dev/null
+++ b/xen/arch/riscv/include/asm/acpi.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __ASM_RISCV_ACPI_H
+#define __ASM_RISCV_ACPI_H
+
+#endif /* __ASM_RISCV_ACPI_H */
diff --git a/xen/arch/riscv/include/asm/config.h b/xen/arch/riscv/include/asm/config.h
index fa90ae0898..f0544c6a20 100644
--- a/xen/arch/riscv/include/asm/config.h
+++ b/xen/arch/riscv/include/asm/config.h
@@ -95,6 +95,8 @@
 #define RV_STAGE1_MODE SATP_MODE_SV32
 #endif
 
+#define IDENT_AREA_SIZE 64
+
 #endif /* __RISCV_CONFIG_H__ */
 /*
  * Local variables:
diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index 7b94cbadd7..07c7a0abba 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -13,8 +13,11 @@ extern unsigned char cpu0_boot_stack[];
 void setup_initial_pagetables(void);
 
 void enable_mmu(void);
-void cont_after_mmu_is_enabled(void);
+
+void remove_identity_mapping(void);
 
 unsigned long calc_phys_offset(void);
 
+void turn_on_mmu(unsigned long ra);
+
 #endif /* _ASM_RISCV_MM_H */
diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index a73f135a3c..053f043a3d 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -4,6 +4,7 @@
 #include <xen/compiler.h>
 #include <xen/init.h>
 #include <xen/kernel.h>
+#include <xen/macros.h>
 #include <xen/pfn.h>
 
 #include <asm/early_printk.h>
@@ -35,8 +36,11 @@ static unsigned long __ro_after_init phys_offset;
  *
  * It might be needed one more page table in case when Xen load address
  * isn't 2 MB aligned.
+ *
+ * CONFIG_PAGING_LEVELS page tables are needed for the identity mapping,
+ * except that the root page table is shared with the initial mapping
  */
-#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) + 1)
+#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) * 2 + 1)
 
 pte_t __section(".bss.page_aligned") __aligned(PAGE_SIZE)
 stage1_pgtbl_root[PAGETABLE_ENTRIES];
@@ -75,6 +79,7 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
     unsigned int index;
     pte_t *pgtbl;
     unsigned long page_addr;
+    bool is_identity_mapping = map_start == pa_start;
 
     if ( (unsigned long)_start % XEN_PT_LEVEL_SIZE(0) )
     {
@@ -108,16 +113,18 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
             {
                 unsigned long paddr = (page_addr - map_start) + pa_start;
                 unsigned int permissions = PTE_LEAF_DEFAULT;
+                unsigned long addr = is_identity_mapping
+                                     ? page_addr : LINK_TO_LOAD(page_addr);
                 pte_t pte_to_be_written;
 
                 index = pt_index(0, page_addr);
 
-                if ( is_kernel_text(LINK_TO_LOAD(page_addr)) ||
-                     is_kernel_inittext(LINK_TO_LOAD(page_addr)) )
-                    permissions =
-                        PTE_EXECUTABLE | PTE_READABLE | PTE_VALID;
+                if ( is_kernel_text(addr) ||
+                     is_kernel_inittext(addr) )
+                        permissions =
+                            PTE_EXECUTABLE | PTE_READABLE | PTE_VALID;
 
-                if ( is_kernel_rodata(LINK_TO_LOAD(page_addr)) )
+                if ( is_kernel_rodata(addr) )
                     permissions = PTE_READABLE | PTE_VALID;
 
                 pte_to_be_written = paddr_to_pte(paddr, permissions);
@@ -211,6 +218,13 @@ void __init setup_initial_pagetables(void)
     unsigned long linker_start  = LOAD_TO_LINK(load_start);
     unsigned long linker_end    = LOAD_TO_LINK(load_end);
 
+    unsigned long ident_start;
+    unsigned long ident_end;
+
+    /*
+     * If the overlapping check will be removed then remove_identity_mapping()
+     * logic should be updated.
+     */
     if ( (linker_start != load_start) &&
          (linker_start <= load_end) && (load_start <= linker_end) )
     {
@@ -232,48 +246,40 @@ void __init setup_initial_pagetables(void)
                           linker_start,
                           linker_end,
                           load_start);
+
+    if ( linker_start == load_start )
+        return;
+
+    ident_start = (unsigned long)turn_on_mmu & XEN_PT_LEVEL_MAP_MASK(0);
+    ident_end = ident_start + PAGE_SIZE;
+
+    setup_initial_mapping(&mmu_desc,
+                          ident_start,
+                          ident_end,
+                          ident_start);
 }
 
-void __init noreturn noinline enable_mmu()
+void __init remove_identity_mapping(void)
 {
-    /*
-     * Calculate a linker time address of the mmu_is_enabled
-     * label and update CSR_STVEC with it.
-     * MMU is configured in a way where linker addresses are mapped
-     * on load addresses so in a case when linker addresses are not equal
-     * to load addresses, after MMU is enabled, it will cause
-     * an exception and jump to linker time addresses.
-     * Otherwise if load addresses are equal to linker addresses the code
-     * after mmu_is_enabled label will be executed without exception.
-     */
-    csr_write(CSR_STVEC, LOAD_TO_LINK((unsigned long)&&mmu_is_enabled));
-
-    /* Ensure page table writes precede loading the SATP */
-    sfence_vma();
+    unsigned int i;
+    pte_t *pgtbl;
+    unsigned int index, xen_index;
+    unsigned long ident_start =
+        LINK_TO_LOAD(turn_on_mmu) & XEN_PT_LEVEL_MAP_MASK(0);
 
-    /* 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);
+    for ( pgtbl = stage1_pgtbl_root, i = CONFIG_PAGING_LEVELS; i; i-- )
+    {
+        index = pt_index(i - 1, ident_start);
+        xen_index = pt_index(i - 1, XEN_VIRT_START);
 
-    asm volatile ( ".p2align 2" );
- mmu_is_enabled:
-    /*
-     * Stack should be re-inited as:
-     * 1. Right now an address of the stack is relative to load time
-     *    addresses what will cause an issue in case of load start address
-     *    isn't equal to linker start address.
-     * 2. Addresses in stack are all load time relative which can be an
-     *    issue in case when load start address isn't equal to linker
-     *    start address.
-     *
-     * We can't return to the caller because the stack was reseted
-     * and it may have stash some variable on the stack.
-     * Jump to a brand new function as the stack was reseted
-     */
+        if ( index != xen_index )
+        {
+            pgtbl[index].pte = 0;
+            break;
+        }
 
-    switch_stack_and_jump((unsigned long)cpu0_boot_stack + STACK_SIZE,
-                          cont_after_mmu_is_enabled);
+        pgtbl = (pte_t *)LOAD_TO_LINK(pte_to_paddr(pgtbl[index]));
+    }
 }
 
 /*
diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
index ae194bb099..b6ce2894ab 100644
--- a/xen/arch/riscv/riscv64/head.S
+++ b/xen/arch/riscv/riscv64/head.S
@@ -39,6 +39,22 @@ ENTRY(start)
         jal     calc_phys_offset
         mv      s2, a0
 
+        jal     setup_initial_pagetables
+
+        /* Calculate proper VA after jump from 1:1 mapping */
+        la      a0, .L_primary_switched
+        sub     a0, a0, s2
+
+        jal     turn_on_mmu
+
+.L_primary_switched:
+        /*
+         * cpu0_boot_stack address is 1:1 mapping related so it should be
+         * recalculated after jump from 1:1 mapping world as 1:1 mapping
+         * will be removed soon in start_xen().
+         */
+        jal     reset_stack
+
         /* restore hart_id ( bootcpu_id ) and dtb address */
         mv      a0, s0
         mv      a1, s1
@@ -54,3 +70,17 @@ ENTRY(reset_stack)
 
         ret
 
+        .section .text.ident, "ax", %progbits
+
+ENTRY(turn_on_mmu)
+        sfence.vma
+
+        li      t0, RV_STAGE1_MODE
+        slli    t0, t0, SATP_MODE_SHIFT
+
+        la      t1, stage1_pgtbl_root
+        srli    t1, t1, PAGE_SHIFT
+        or      t1, t1, t0
+        csrw    CSR_SATP, t1
+
+        jr      a0
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index dde8fb898b..6593f601c1 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -13,20 +13,10 @@ unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
 void __init noreturn start_xen(unsigned long bootcpu_id,
                                paddr_t dtb_addr)
 {
-    early_printk("Hello from C env\n");
-
-    setup_initial_pagetables();
-
-    enable_mmu();
-
-    for ( ;; )
-        asm volatile ("wfi");
+    remove_identity_mapping();
 
-    unreachable();
-}
+    early_printk("Hello from C env\n");
 
-void __init noreturn cont_after_mmu_is_enabled(void)
-{
     early_printk("All set up\n");
 
     for ( ;; )
diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index 9064852173..3fa7db3bf9 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -31,6 +31,11 @@ SECTIONS
         *(.text.*)
 #endif
 
+        . = ALIGN(IDENT_AREA_SIZE);
+        _ident_start = .;
+        *(.text.ident)
+        _ident_end = .;
+
         *(.fixup)
         *(.gnu.warning)
         . = ALIGN(POINTER_ALIGN);
@@ -173,4 +178,10 @@ ASSERT(IS_ALIGNED(__bss_end,        POINTER_ALIGN), "__bss_end is misaligned")
 ASSERT(!SIZEOF(.got),      ".got non-empty")
 ASSERT(!SIZEOF(.got.plt),  ".got.plt non-empty")
 
+/*
+ * Changing the size of Xen binary can require an update of
+ * PGTBL_INITIAL_COUNT.
+ */
 ASSERT(_end - _start <= MB(2), "Xen too large for early-boot assumptions")
+
+ASSERT(_ident_end - _ident_start <= IDENT_AREA_SIZE, "identity region is too big");
-- 
2.41.0

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

[Oleksii]

Hello Alistair and Bobby,

Could you please review this patch series when you have a moment? Your
insights would be greatly appreciated.

Thanks in advance.

~ Oleksii

On Tue, 2023-08-08 at 18:14 +0300, Oleksii Kurochko wrote:
> The patch series introduces things necessary to implement identity
> mapping:
>   1. Make identity mapping for the entire Xen.
>   2. Enable MMU.
>   3. Jump to the virtual address world
>   4. Remove identity mapping.
> 
> Also current patch series introduces the calculation of physical
> offset before
> MMU is enabled as access to physical offset will be calculated wrong
> after
> MMU will be enabled because access to phys_off variable is PC-
> relative and
> in the case when linker address != load address, it will cause MMU
> fault.
> 
> The reason for this patch series can be found here:
> https://lore.kernel.org/xen-devel/4e336121-fc0c-b007-bf7b-430352563d55@citrix.com/
> 
> ---
> Changes in V7:
>  - use srli instruction to be consistent with slli instruction in
> turn_on_mmu()
> ---
> Changes in V6:
>   - Update calc_phys_offset() after rebase.
>   - Refactor turn_on_mmu() and a way how an argument of turn_on_mmu()
> is
>     calculated.
> ---
> Changes in V5:
>         - update the algo of identity mapping removing.
>         - introduce IDENT_AREA_SIZE.
>         - introduce turn_on_mmu() function to enable and switch from
> 1:1 mapping.
>         - fix typo in PGTBL_INITIAL_COUNT define.
>         - update the comment above PGTBL_INITIAL_COUNT.
>         - update prototype of calc_phys_offset(). now it returns
> phys_offset.
>         - declare phys_offset as static.
>         - save returned value of calc_phys_offset to register s2.
> ---
> Changes in V4:
>   - drop patch  [PATCH v3 1/3] xen/riscv: add SPDX tag to config.h as
> it was
>     merged to staging
>   - remove definition of ARRAY_SIZE and ROUNDUP as <xen/macors.h> was
> introduced where these macros are located now.
>         - update definition of PGTBL_INITIAL_COUNT
>         - update the commit message for patch 'xen/riscv: introduce
> identity mapping'
>         - update the comments in head.S
>   - update the algo of identity mapping removing 
> ---
> Changes in V3:
>  - Update the patch series message.
>  - The following patches were merged to staging so droped from the
> patch series:
>    * xen/riscv: add .sbss section to .bss
>    * xen/riscv: introduce reset_stack() function
>    * xen/riscv: move extern of cpu0_boot_stack to header
>    * xen/riscv: add SPDX tags
>  - move save/restore of a0/a1 registers from patch 4 to patch 2 (
> numbers are
>    from the previous patch series version )
>  - add SPDX tag in config.h
>  - update definition of PGTBL_INITIAL_COUNT taking into account
> identity mapping.
>  - refactor remove_identity_mapping() function.
>  - add explanatory comments in xen.lds.S and mm.c.
> ---
> Changes in V2:
>  - update the patch series message.
>  - drop patches from the previous version of the patch series:
>    * xen/riscv: add __ASSEMBLY__ guards". ( merged )
>    * xen/riscv: make sure that identity mapping isn't bigger then
> page size
>      ( entire Xen is 1:1 mapped so there is no need for the checks
> from the patch )
>  - add .sbss.* and put it befor .bss* .
>  - move out reset_stack() to .text section.
>  - add '__ro_after_init' for phys_offset variable.
>  - add '__init' for calc_phys_offset().
>  - declaring variable phys_off as non static as it will be used in
> head.S.
>  - update definition of PGTBL_INITIAL_COUNT and the comment above.
>  - code style fixes.
>  - remove id_addrs array becase entire Xen is mapped.
>  - reverse condition for cycle inside remove_identity_mapping().
>  - fix page table walk in remove_identity_mapping().
>  - save hart_id and dtb_addr before call MMU related C functions
>  - use phys_offset variable instead of doing calcultations to get
> phys offset
>    in head.S file. ( it can be easily done as entire Xen is 1:1
> mapped now )
>  - declare enable_muu() as __init.
>  - Update SPDX tags.
>  - Add Review-By/Suggested-By for some patches.
>  - code style fixes.
> 
> Oleksii Kurochko (2):
>   xen/riscv: introduce function for physical offset calculation
>   xen/riscv: introduce identity mapping
> 
>  xen/arch/riscv/include/asm/acpi.h   |   6 ++
>  xen/arch/riscv/include/asm/config.h |   2 +
>  xen/arch/riscv/include/asm/mm.h     |   7 +-
>  xen/arch/riscv/mm.c                 | 109 +++++++++++++++++---------
> --
>  xen/arch/riscv/riscv64/head.S       |  44 +++++++++++
>  xen/arch/riscv/setup.c              |  14 +---
>  xen/arch/riscv/xen.lds.S            |  11 +++
>  7 files changed, 136 insertions(+), 57 deletions(-)
>  create mode 100644 xen/arch/riscv/include/asm/acpi.h
> 

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

[Oleksii]

Hello Bobby and Alistair,

Could you kindly take a moment to examine this series of patches?
Your input would be highly valued.

Thanks in advance.

~ Oleksii

On Tue, 2023-08-08 at 18:14 +0300, Oleksii Kurochko wrote:
> The patch series introduces things necessary to implement identity
> mapping:
>   1. Make identity mapping for the entire Xen.
>   2. Enable MMU.
>   3. Jump to the virtual address world
>   4. Remove identity mapping.
> 
> Also current patch series introduces the calculation of physical
> offset before
> MMU is enabled as access to physical offset will be calculated wrong
> after
> MMU will be enabled because access to phys_off variable is PC-
> relative and
> in the case when linker address != load address, it will cause MMU
> fault.
> 
> The reason for this patch series can be found here:
> https://lore.kernel.org/xen-devel/4e336121-fc0c-b007-bf7b-430352563d55@citrix.com/
> 
> ---
> Changes in V7:
>  - use srli instruction to be consistent with slli instruction in
> turn_on_mmu()
> ---
> Changes in V6:
>   - Update calc_phys_offset() after rebase.
>   - Refactor turn_on_mmu() and a way how an argument of turn_on_mmu()
> is
>     calculated.
> ---
> Changes in V5:
>         - update the algo of identity mapping removing.
>         - introduce IDENT_AREA_SIZE.
>         - introduce turn_on_mmu() function to enable and switch from
> 1:1 mapping.
>         - fix typo in PGTBL_INITIAL_COUNT define.
>         - update the comment above PGTBL_INITIAL_COUNT.
>         - update prototype of calc_phys_offset(). now it returns
> phys_offset.
>         - declare phys_offset as static.
>         - save returned value of calc_phys_offset to register s2.
> ---
> Changes in V4:
>   - drop patch  [PATCH v3 1/3] xen/riscv: add SPDX tag to config.h as
> it was
>     merged to staging
>   - remove definition of ARRAY_SIZE and ROUNDUP as <xen/macors.h> was
> introduced where these macros are located now.
>         - update definition of PGTBL_INITIAL_COUNT
>         - update the commit message for patch 'xen/riscv: introduce
> identity mapping'
>         - update the comments in head.S
>   - update the algo of identity mapping removing 
> ---
> Changes in V3:
>  - Update the patch series message.
>  - The following patches were merged to staging so droped from the
> patch series:
>    * xen/riscv: add .sbss section to .bss
>    * xen/riscv: introduce reset_stack() function
>    * xen/riscv: move extern of cpu0_boot_stack to header
>    * xen/riscv: add SPDX tags
>  - move save/restore of a0/a1 registers from patch 4 to patch 2 (
> numbers are
>    from the previous patch series version )
>  - add SPDX tag in config.h
>  - update definition of PGTBL_INITIAL_COUNT taking into account
> identity mapping.
>  - refactor remove_identity_mapping() function.
>  - add explanatory comments in xen.lds.S and mm.c.
> ---
> Changes in V2:
>  - update the patch series message.
>  - drop patches from the previous version of the patch series:
>    * xen/riscv: add __ASSEMBLY__ guards". ( merged )
>    * xen/riscv: make sure that identity mapping isn't bigger then
> page size
>      ( entire Xen is 1:1 mapped so there is no need for the checks
> from the patch )
>  - add .sbss.* and put it befor .bss* .
>  - move out reset_stack() to .text section.
>  - add '__ro_after_init' for phys_offset variable.
>  - add '__init' for calc_phys_offset().
>  - declaring variable phys_off as non static as it will be used in
> head.S.
>  - update definition of PGTBL_INITIAL_COUNT and the comment above.
>  - code style fixes.
>  - remove id_addrs array becase entire Xen is mapped.
>  - reverse condition for cycle inside remove_identity_mapping().
>  - fix page table walk in remove_identity_mapping().
>  - save hart_id and dtb_addr before call MMU related C functions
>  - use phys_offset variable instead of doing calcultations to get
> phys offset
>    in head.S file. ( it can be easily done as entire Xen is 1:1
> mapped now )
>  - declare enable_muu() as __init.
>  - Update SPDX tags.
>  - Add Review-By/Suggested-By for some patches.
>  - code style fixes.
> 
> Oleksii Kurochko (2):
>   xen/riscv: introduce function for physical offset calculation
>   xen/riscv: introduce identity mapping
> 
>  xen/arch/riscv/include/asm/acpi.h   |   6 ++
>  xen/arch/riscv/include/asm/config.h |   2 +
>  xen/arch/riscv/include/asm/mm.h     |   7 +-
>  xen/arch/riscv/mm.c                 | 109 +++++++++++++++++---------
> --
>  xen/arch/riscv/riscv64/head.S       |  44 +++++++++++
>  xen/arch/riscv/setup.c              |  14 +---
>  xen/arch/riscv/xen.lds.S            |  11 +++
>  7 files changed, 136 insertions(+), 57 deletions(-)
>  create mode 100644 xen/arch/riscv/include/asm/acpi.h
> 

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

[Julien Grall]

Hi,

On 08/08/2023 16:14, Oleksii Kurochko wrote:
> The patch series introduces things necessary to implement identity mapping:
>    1. Make identity mapping for the entire Xen.
>    2. Enable MMU.
>    3. Jump to the virtual address world
>    4. Remove identity mapping.
> 
> Also current patch series introduces the calculation of physical offset before
> MMU is enabled as access to physical offset will be calculated wrong after
> MMU will be enabled because access to phys_off variable is PC-relative and
> in the case when linker address != load address, it will cause MMU fault.
> 
> The reason for this patch series can be found here:
> https://lore.kernel.org/xen-devel/4e336121-fc0c-b007-bf7b-430352563d55@citrix.com/

During the last community call it was pointed out that this is missing a 
formal ack. Now that we have no RISC-V maintainers, this would fall to 
the REST.

@Jan, you reviewed the previous version of the series. Would you be able 
to provide a formal ack?

Cheers,

-- 
Julien Grall

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

[Jan Beulich]

On 08.08.2023 17:14, Oleksii Kurochko wrote:
> The patch series introduces things necessary to implement identity mapping:
>   1. Make identity mapping for the entire Xen.
>   2. Enable MMU.
>   3. Jump to the virtual address world
>   4. Remove identity mapping.
> 
> Also current patch series introduces the calculation of physical offset before
> MMU is enabled as access to physical offset will be calculated wrong after
> MMU will be enabled because access to phys_off variable is PC-relative and
> in the case when linker address != load address, it will cause MMU fault.
> 
> The reason for this patch series can be found here:
> https://lore.kernel.org/xen-devel/4e336121-fc0c-b007-bf7b-430352563d55@citrix.com/
> 
> ---
> Changes in V7:
>  - use srli instruction to be consistent with slli instruction in turn_on_mmu()
> ---
> Changes in V6:
>   - Update calc_phys_offset() after rebase.
>   - Refactor turn_on_mmu() and a way how an argument of turn_on_mmu() is
>     calculated.
> ---
> Changes in V5:
> 	- update the algo of identity mapping removing.
> 	- introduce IDENT_AREA_SIZE.
> 	- introduce turn_on_mmu() function to enable and switch from 1:1 mapping.
> 	- fix typo in PGTBL_INITIAL_COUNT define.
> 	- update the comment above PGTBL_INITIAL_COUNT.
> 	- update prototype of calc_phys_offset(). now it returns phys_offset.
> 	- declare phys_offset as static.
> 	- save returned value of calc_phys_offset to register s2.
> ---
> Changes in V4:
>   - drop patch 	[PATCH v3 1/3] xen/riscv: add SPDX tag to config.h as it was
>     merged to staging
>   - remove definition of ARRAY_SIZE and ROUNDUP as <xen/macors.h> was introduced where these macros are located now.
> 	- update definition of PGTBL_INITIAL_COUNT
> 	- update the commit message for patch 'xen/riscv: introduce identity mapping'
> 	- update the comments in head.S
>   - update the algo of identity mapping removing 
> ---
> Changes in V3:
>  - Update the patch series message.
>  - The following patches were merged to staging so droped from the patch series:
>    * xen/riscv: add .sbss section to .bss
>    * xen/riscv: introduce reset_stack() function
>    * xen/riscv: move extern of cpu0_boot_stack to header
>    * xen/riscv: add SPDX tags
>  - move save/restore of a0/a1 registers from patch 4 to patch 2 ( numbers are
>    from the previous patch series version )
>  - add SPDX tag in config.h
>  - update definition of PGTBL_INITIAL_COUNT taking into account identity mapping.
>  - refactor remove_identity_mapping() function.
>  - add explanatory comments in xen.lds.S and mm.c.
> ---
> Changes in V2:
>  - update the patch series message.
>  - drop patches from the previous version of the patch series:
>    * xen/riscv: add __ASSEMBLY__ guards". ( merged )
>    * xen/riscv: make sure that identity mapping isn't bigger then page size
>      ( entire Xen is 1:1 mapped so there is no need for the checks from the patch )
>  - add .sbss.* and put it befor .bss* .
>  - move out reset_stack() to .text section.
>  - add '__ro_after_init' for phys_offset variable.
>  - add '__init' for calc_phys_offset().
>  - declaring variable phys_off as non static as it will be used in head.S.
>  - update definition of PGTBL_INITIAL_COUNT and the comment above.
>  - code style fixes.
>  - remove id_addrs array becase entire Xen is mapped.
>  - reverse condition for cycle inside remove_identity_mapping().
>  - fix page table walk in remove_identity_mapping().
>  - save hart_id and dtb_addr before call MMU related C functions
>  - use phys_offset variable instead of doing calcultations to get phys offset
>    in head.S file. ( it can be easily done as entire Xen is 1:1 mapped now )
>  - declare enable_muu() as __init.
>  - Update SPDX tags.
>  - Add Review-By/Suggested-By for some patches.
>  - code style fixes.
> 
> Oleksii Kurochko (2):
>   xen/riscv: introduce function for physical offset calculation
>   xen/riscv: introduce identity mapping

Acked-by: Jan Beulich <jbeulich@suse.com>

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

[Oleksii]

On Thu, 2023-10-19 at 12:32 +0200, Jan Beulich wrote:
> On 08.08.2023 17:14, Oleksii Kurochko wrote:
> > The patch series introduces things necessary to implement identity
> > mapping:
> >   1. Make identity mapping for the entire Xen.
> >   2. Enable MMU.
> >   3. Jump to the virtual address world
> >   4. Remove identity mapping.
> > 
> > Also current patch series introduces the calculation of physical
> > offset before
> > MMU is enabled as access to physical offset will be calculated
> > wrong after
> > MMU will be enabled because access to phys_off variable is PC-
> > relative and
> > in the case when linker address != load address, it will cause MMU
> > fault.
> > 
> > The reason for this patch series can be found here:
> > https://lore.kernel.org/xen-devel/4e336121-fc0c-b007-bf7b-430352563d55@citrix.com/
> > 
> > ---
> > Changes in V7:
> >  - use srli instruction to be consistent with slli instruction in
> > turn_on_mmu()
> > ---
> > Changes in V6:
> >   - Update calc_phys_offset() after rebase.
> >   - Refactor turn_on_mmu() and a way how an argument of
> > turn_on_mmu() is
> >     calculated.
> > ---
> > Changes in V5:
> >         - update the algo of identity mapping removing.
> >         - introduce IDENT_AREA_SIZE.
> >         - introduce turn_on_mmu() function to enable and switch
> > from 1:1 mapping.
> >         - fix typo in PGTBL_INITIAL_COUNT define.
> >         - update the comment above PGTBL_INITIAL_COUNT.
> >         - update prototype of calc_phys_offset(). now it returns
> > phys_offset.
> >         - declare phys_offset as static.
> >         - save returned value of calc_phys_offset to register s2.
> > ---
> > Changes in V4:
> >   - drop patch  [PATCH v3 1/3] xen/riscv: add SPDX tag to config.h
> > as it was
> >     merged to staging
> >   - remove definition of ARRAY_SIZE and ROUNDUP as <xen/macors.h>
> > was introduced where these macros are located now.
> >         - update definition of PGTBL_INITIAL_COUNT
> >         - update the commit message for patch 'xen/riscv: introduce
> > identity mapping'
> >         - update the comments in head.S
> >   - update the algo of identity mapping removing 
> > ---
> > Changes in V3:
> >  - Update the patch series message.
> >  - The following patches were merged to staging so droped from the
> > patch series:
> >    * xen/riscv: add .sbss section to .bss
> >    * xen/riscv: introduce reset_stack() function
> >    * xen/riscv: move extern of cpu0_boot_stack to header
> >    * xen/riscv: add SPDX tags
> >  - move save/restore of a0/a1 registers from patch 4 to patch 2 (
> > numbers are
> >    from the previous patch series version )
> >  - add SPDX tag in config.h
> >  - update definition of PGTBL_INITIAL_COUNT taking into account
> > identity mapping.
> >  - refactor remove_identity_mapping() function.
> >  - add explanatory comments in xen.lds.S and mm.c.
> > ---
> > Changes in V2:
> >  - update the patch series message.
> >  - drop patches from the previous version of the patch series:
> >    * xen/riscv: add __ASSEMBLY__ guards". ( merged )
> >    * xen/riscv: make sure that identity mapping isn't bigger then
> > page size
> >      ( entire Xen is 1:1 mapped so there is no need for the checks
> > from the patch )
> >  - add .sbss.* and put it befor .bss* .
> >  - move out reset_stack() to .text section.
> >  - add '__ro_after_init' for phys_offset variable.
> >  - add '__init' for calc_phys_offset().
> >  - declaring variable phys_off as non static as it will be used in
> > head.S.
> >  - update definition of PGTBL_INITIAL_COUNT and the comment above.
> >  - code style fixes.
> >  - remove id_addrs array becase entire Xen is mapped.
> >  - reverse condition for cycle inside remove_identity_mapping().
> >  - fix page table walk in remove_identity_mapping().
> >  - save hart_id and dtb_addr before call MMU related C functions
> >  - use phys_offset variable instead of doing calcultations to get
> > phys offset
> >    in head.S file. ( it can be easily done as entire Xen is 1:1
> > mapped now )
> >  - declare enable_muu() as __init.
> >  - Update SPDX tags.
> >  - Add Review-By/Suggested-By for some patches.
> >  - code style fixes.
> > 
> > Oleksii Kurochko (2):
> >   xen/riscv: introduce function for physical offset calculation
> >   xen/riscv: introduce identity mapping
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
Thanks for Ack.

Should be the patch series re-send with Ack after the end of code
freeze?

~ Oleksii

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

[Jan Beulich]

On 20.10.2023 15:52, Oleksii wrote:
> On Thu, 2023-10-19 at 12:32 +0200, Jan Beulich wrote:
>> On 08.08.2023 17:14, Oleksii Kurochko wrote:
>>> Oleksii Kurochko (2):
>>>   xen/riscv: introduce function for physical offset calculation
>>>   xen/riscv: introduce identity mapping
>>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
> Thanks for Ack.
> 
> Should be the patch series re-send with Ack after the end of code
> freeze?

If what was sent still applies, I don't think there's a need.

Jan

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

[Oleksii]

On Fri, 2023-10-20 at 15:53 +0200, Jan Beulich wrote:
> On 20.10.2023 15:52, Oleksii wrote:
> > On Thu, 2023-10-19 at 12:32 +0200, Jan Beulich wrote:
> > > On 08.08.2023 17:14, Oleksii Kurochko wrote:
> > > > Oleksii Kurochko (2):
> > > >   xen/riscv: introduce function for physical offset calculation
> > > >   xen/riscv: introduce identity mapping
> > > 
> > > Acked-by: Jan Beulich <jbeulich@suse.com>
> > Thanks for Ack.
> > 
> > Should be the patch series re-send with Ack after the end of code
> > freeze?
> 
> If what was sent still applies, I don't think there's a need.
I pulled staging and rebased the identity mapping patch series. All is
still applied and worked.

~ Oleksii