[PATCH v2 0/7] Unflattening and relocation of host device tree

[PATCH v2 0/7] Unflattening and relocation of host device tree

[Oleksii Kurochko]

The current patch series introduces the relocation of the host device tree file
to free up low memory and also it is expected that discard_initial_modules()
will be called sooner or later, it will discard the FDT boot module,
and remove_early_mappings() will destroy the early mappings.

In addition to relocation, unflattening is introduced to create the tree of
struct device_node for the host device tree.

To implement this, several things have been introduced:
 - destroy_xen_mappings() function, which removes page mappings from Xen's
   page tables. This is necessary for clear_fixmap().
 - {set,clear}_fixmap() functions to manage mappings in the fixmap region,
   which are expected to be used in copy_from_paddr() to copy the FDT to Xen's
   heap.
 - Introduce new config CONFIG_QEMU which is going to be used to cover changes
   connected to QEMU virtual board. It will be used during introduction of stubs
   for clean_and_invalidate_dcache_va_range() and clean_dcache_va_range(),
   which are expected to be used in copy_from_paddr() and flush_page_to_ram(),
   which in turn are expected to be used during the call to xvmalloc_array() in
   relocate_fdt().
   In case of QEMU cached related functions are implemented as returning 0 as
   QEMU doesn't model cache ( and so CMO extensions ). For others cases, it is
   introduced as -ENOSUPP as h/w could support CMO extension ( or hardware
   specific insertions ) and it will need to update implementation of the
   mentioned functions.
 - The introduction of copy_from_paddr() to copy the FDT to an address
   allocated in Xen's heap.

---
Changes in v2:
 - Update the cover letter.
 - Introduce new patch with aligning of upper bounds in the layout table in
   config.h with the definitions below which are inclusive.
 - Other changes please look at the specific patch.
---

Oleksii Kurochko (7):
  xen/riscv: update layout table in config.h
  xen/riscv: add destroy_xen_mappings() to remove mappings in Xen page
    tables
  xen/riscv: reorder includes in asm/page.h alphabetically
  xen/riscv: add {set,clear}_fixmap() functions for managing fixmap
    entries
  xen/riscv: implement data and instruction cache operations
  xen/riscv: implement prereq for DTB relocation
  xen/riscv: relocating and unflattening host device tree

 xen/arch/riscv/Kconfig                  |  2 +
 xen/arch/riscv/configs/tiny64_defconfig |  1 +
 xen/arch/riscv/include/asm/config.h     | 12 +++---
 xen/arch/riscv/include/asm/fixmap.h     |  5 +++
 xen/arch/riscv/include/asm/mm.h         |  8 +++-
 xen/arch/riscv/include/asm/page.h       | 34 +++++++++++++--
 xen/arch/riscv/include/asm/setup.h      |  4 ++
 xen/arch/riscv/mm.c                     |  6 ---
 xen/arch/riscv/platforms/Kconfig        |  5 +++
 xen/arch/riscv/pt.c                     | 25 +++++++++++
 xen/arch/riscv/setup.c                  | 57 ++++++++++++++++++++++++-
 11 files changed, 139 insertions(+), 20 deletions(-)
 create mode 100644 xen/arch/riscv/platforms/Kconfig

-- 
2.47.1

[PATCH v2 1/7] xen/riscv: update layout table in config.h

[Oleksii Kurochko]

Make all upper bounds (end addresses) for areas inclusive to align
with the corresponding definitions.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v2:
 - new patch
---
 xen/arch/riscv/include/asm/config.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/riscv/include/asm/config.h b/xen/arch/riscv/include/asm/config.h
index 4954677aff..826e5c7172 100644
--- a/xen/arch/riscv/include/asm/config.h
+++ b/xen/arch/riscv/include/asm/config.h
@@ -41,17 +41,17 @@
  * Start addr          | End addr         | Slot       | area description
  * ============================================================================
  *                   .....                 L2 511          Unused
- *  0xffffffffc0a00000  0xffffffffc0c00000 L2 511          Fixmap
+ *  0xffffffffc0a00000  0xffffffffc0bfffff L2 511          Fixmap
  *                   ..... ( 2 MB gap )
- *  0xffffffffc0400000  0xffffffffc0800000 L2 511          FDT
+ *  0xffffffffc0400000  0xffffffffc07fffff L2 511          FDT
  *                   ..... ( 2 MB gap )
- *  0xffffffffc0000000  0xffffffffc0200000 L2 511          Xen
+ *  0xffffffffc0000000  0xffffffffc01fffff L2 511          Xen
  *                   .....                 L2 510          Unused
- *  0x3200000000        0x7f40000000       L2 200-509      Direct map
+ *  0x3200000000        0x7f7fffffff       L2 200-509      Direct map
  *                   .....                 L2 199          Unused
- *  0x30c0000000        0x31c0000000       L2 195-198      Frametable
+ *  0x30c0000000        0x31bfffffff       L2 195-198      Frametable
  *                   .....                 L2 194          Unused
- *  0x3040000000        0x3080000000       L2 193          VMAP
+ *  0x3040000000        0x307fffffff       L2 193          VMAP
  *                   .....                 L2 0-192        Unused
 #elif RV_STAGE1_MODE == SATP_MODE_SV48
  * Memory layout is the same as for SV39 in terms of slots, so only start and
-- 
2.47.1

Re: [PATCH v2 1/7] xen/riscv: update layout table in config.h

[Jan Beulich]

On 11.12.2024 18:27, Oleksii Kurochko wrote:
> Make all upper bounds (end addresses) for areas inclusive to align
> with the corresponding definitions.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

I expected this would be straightforward to ack, but ...

> --- a/xen/arch/riscv/include/asm/config.h
> +++ b/xen/arch/riscv/include/asm/config.h
> @@ -41,17 +41,17 @@
>   * Start addr          | End addr         | Slot       | area description
>   * ============================================================================
>   *                   .....                 L2 511          Unused
> - *  0xffffffffc0a00000  0xffffffffc0c00000 L2 511          Fixmap
> + *  0xffffffffc0a00000  0xffffffffc0bfffff L2 511          Fixmap
>   *                   ..... ( 2 MB gap )
> - *  0xffffffffc0400000  0xffffffffc0800000 L2 511          FDT
> + *  0xffffffffc0400000  0xffffffffc07fffff L2 511          FDT
>   *                   ..... ( 2 MB gap )
> - *  0xffffffffc0000000  0xffffffffc0200000 L2 511          Xen
> + *  0xffffffffc0000000  0xffffffffc01fffff L2 511          Xen
>   *                   .....                 L2 510          Unused
> - *  0x3200000000        0x7f40000000       L2 200-509      Direct map
> + *  0x3200000000        0x7f7fffffff       L2 200-509      Direct map

... this isn't just an adjustment by -1. If the old table entry was wrong,
the description wants to say so. Else an adjustment to the number is needed
here.

Jan

[PATCH v2 2/7] xen/riscv: add destroy_xen_mappings() to remove mappings in Xen page tables

[Oleksii Kurochko]

Introduce the destroy_xen_mappings() function, which removes page
mappings in Xen's page tables between a start address s and an end
address e.
The function ensures that both s and e are page-aligned
and verifies that the start address is less than or equal to the end
address before calling pt_update() to invalidate the mappings.
The pt_update() function is called with INVALID_MFN and PTE_VALID=0
in the flags, which tell pt_update() to remove mapping. No additional
ASSERT() is required to check these arguments, as they are hardcoded in
the call to pt_update() within destroy_xen_mappings().

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in V2:
 - Drop ASSERT(s <= e).
 - Update implementation of destroy_xen_mappings() to avoid calling of
   pt_update() when start_addr >= end_addr and return -EINVAL.
 - Add Acked-by: Jan Beulich <jbeulich@suse.com>.
---
 xen/arch/riscv/mm.c | 6 ------
 xen/arch/riscv/pt.c | 8 ++++++++
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index 9359dc7f33..f2bf279bac 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -360,12 +360,6 @@ int xenmem_add_to_physmap_one(struct domain *d, unsigned int space,
     return 0;
 }
 
-int destroy_xen_mappings(unsigned long s, unsigned long e)
-{
-    BUG_ON("unimplemented");
-    return -1;
-}
-
 void share_xen_page_with_guest(struct page_info *page, struct domain *d,
                                enum XENSHARE_flags flags)
 {
diff --git a/xen/arch/riscv/pt.c b/xen/arch/riscv/pt.c
index d62aceb36c..86bd9ea613 100644
--- a/xen/arch/riscv/pt.c
+++ b/xen/arch/riscv/pt.c
@@ -421,6 +421,14 @@ int map_pages_to_xen(unsigned long virt,
     return pt_update(virt, mfn, nr_mfns, flags);
 }
 
+int destroy_xen_mappings(unsigned long s, unsigned long e)
+{
+    ASSERT(IS_ALIGNED(s, PAGE_SIZE));
+    ASSERT(IS_ALIGNED(e, PAGE_SIZE));
+
+    return ( s < e ) ? pt_update(s, INVALID_MFN, PFN_DOWN(e - s), 0) : -EINVAL;
+}
+
 int __init populate_pt_range(unsigned long virt, unsigned long nr_mfns)
 {
     return pt_update(virt, INVALID_MFN, nr_mfns, PTE_POPULATE);
-- 
2.47.1

Re: [PATCH v2 2/7] xen/riscv: add destroy_xen_mappings() to remove mappings in Xen page tables

[Jan Beulich]

On 11.12.2024 18:27, Oleksii Kurochko wrote:
> Introduce the destroy_xen_mappings() function, which removes page
> mappings in Xen's page tables between a start address s and an end
> address e.
> The function ensures that both s and e are page-aligned
> and verifies that the start address is less than or equal to the end
> address before calling pt_update() to invalidate the mappings.
> The pt_update() function is called with INVALID_MFN and PTE_VALID=0
> in the flags, which tell pt_update() to remove mapping. No additional
> ASSERT() is required to check these arguments, as they are hardcoded in
> the call to pt_update() within destroy_xen_mappings().
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

Apparently I just shouldn't provide advance acks, when ...

> --- a/xen/arch/riscv/pt.c
> +++ b/xen/arch/riscv/pt.c
> @@ -421,6 +421,14 @@ int map_pages_to_xen(unsigned long virt,
>      return pt_update(virt, mfn, nr_mfns, flags);
>  }
>  
> +int destroy_xen_mappings(unsigned long s, unsigned long e)
> +{
> +    ASSERT(IS_ALIGNED(s, PAGE_SIZE));
> +    ASSERT(IS_ALIGNED(e, PAGE_SIZE));
> +
> +    return ( s < e ) ? pt_update(s, INVALID_MFN, PFN_DOWN(e - s), 0) : -EINVAL;

... then you introduce basic style violations like the excess blanks here.

Jan

Re: [PATCH v2 2/7] xen/riscv: add destroy_xen_mappings() to remove mappings in Xen page tables

[Oleksii Kurochko]

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]


On 12/12/24 12:43 PM, Jan Beulich wrote:
> On 11.12.2024 18:27, Oleksii Kurochko wrote:
>> Introduce the destroy_xen_mappings() function, which removes page
>> mappings in Xen's page tables between a start address s and an end
>> address e.
>> The function ensures that both s and e are page-aligned
>> and verifies that the start address is less than or equal to the end
>> address before calling pt_update() to invalidate the mappings.
>> The pt_update() function is called with INVALID_MFN and PTE_VALID=0
>> in the flags, which tell pt_update() to remove mapping. No additional
>> ASSERT() is required to check these arguments, as they are hardcoded in
>> the call to pt_update() within destroy_xen_mappings().
>>
>> Signed-off-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
>> Acked-by: Jan Beulich<jbeulich@suse.com>
> Apparently I just shouldn't provide advance acks, when ...
>
>> --- a/xen/arch/riscv/pt.c
>> +++ b/xen/arch/riscv/pt.c
>> @@ -421,6 +421,14 @@ int map_pages_to_xen(unsigned long virt,
>>       return pt_update(virt, mfn, nr_mfns, flags);
>>   }
>>   
>> +int destroy_xen_mappings(unsigned long s, unsigned long e)
>> +{
>> +    ASSERT(IS_ALIGNED(s, PAGE_SIZE));
>> +    ASSERT(IS_ALIGNED(e, PAGE_SIZE));
>> +
>> +    return ( s < e ) ? pt_update(s, INVALID_MFN, PFN_DOWN(e - s), 0) : -EINVAL;
> ... then you introduce basic style violations like the excess blanks here.

I thought that I could consider ternary operator as `if` which requires spaces around condition.

~ Oleksii

[-- Attachment #2: Type: text/html, Size: 2288 bytes --]

[PATCH v2 3/7] xen/riscv: reorder includes in asm/page.h alphabetically

[Oleksii Kurochko]

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in V2:
 - Add Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/riscv/include/asm/page.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/riscv/include/asm/page.h b/xen/arch/riscv/include/asm/page.h
index 91b1194b55..bf3f75e85d 100644
--- a/xen/arch/riscv/include/asm/page.h
+++ b/xen/arch/riscv/include/asm/page.h
@@ -5,8 +5,8 @@
 
 #ifndef __ASSEMBLY__
 
-#include <xen/const.h>
 #include <xen/bug.h>
+#include <xen/const.h>
 #include <xen/types.h>
 
 #include <asm/atomic.h>
-- 
2.47.1

[PATCH v2 4/7] xen/riscv: add {set,clear}_fixmap() functions for managing fixmap entries

[Oleksii Kurochko]

Introduce set_fixmap() and clear_fixmap() functions to manage mappings
in the fixmap region. The set_fixmap() function maps a 4k page ( as only L0
is expected to be updated; look at setup_fixmap_mappings() ) at a specified
fixmap entry using map_pages_to_xen(), while clear_fixmap() removes the
mapping from a fixmap entry by calling destroy_xen_mappings().

Both functions ensure that the operations succeed by asserting that their
respective calls (map_pages_to_xen() and destroy_xen_mappings()) return 0.
`BUG()` is added to trigger a failure if any issues occur during
the mapping or unmapping process.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in V2:
 - Update the commit message.
 - drop local variables from {set, clear}_fixmap(); use if (... != 0) + BUG().
---
 xen/arch/riscv/include/asm/fixmap.h |  5 +++++
 xen/arch/riscv/pt.c                 | 17 +++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/xen/arch/riscv/include/asm/fixmap.h b/xen/arch/riscv/include/asm/fixmap.h
index 818c8ce07b..e399a15f53 100644
--- a/xen/arch/riscv/include/asm/fixmap.h
+++ b/xen/arch/riscv/include/asm/fixmap.h
@@ -32,6 +32,11 @@
  */
 extern pte_t xen_fixmap[];
 
+/* Map a page in a fixmap entry */
+void set_fixmap(unsigned int map, mfn_t mfn, unsigned int flags);
+/* Remove a mapping from a fixmap entry */
+void clear_fixmap(unsigned int map);
+
 #define fix_to_virt(slot) ((void *)FIXMAP_ADDR(slot))
 
 static inline unsigned int virt_to_fix(vaddr_t vaddr)
diff --git a/xen/arch/riscv/pt.c b/xen/arch/riscv/pt.c
index 86bd9ea613..3407fda937 100644
--- a/xen/arch/riscv/pt.c
+++ b/xen/arch/riscv/pt.c
@@ -8,6 +8,7 @@
 #include <xen/pmap.h>
 #include <xen/spinlock.h>
 
+#include <asm/fixmap.h>
 #include <asm/flushtlb.h>
 #include <asm/page.h>
 
@@ -433,3 +434,19 @@ int __init populate_pt_range(unsigned long virt, unsigned long nr_mfns)
 {
     return pt_update(virt, INVALID_MFN, nr_mfns, PTE_POPULATE);
 }
+
+/* Map a 4k page in a fixmap entry */
+void set_fixmap(unsigned int map, mfn_t mfn, unsigned int flags)
+{
+    if ( map_pages_to_xen(FIXMAP_ADDR(map), mfn, 1, flags | PTE_SMALL) != 0 )
+        BUG();
+}
+
+/* Remove a mapping from a fixmap entry */
+void clear_fixmap(unsigned int map)
+{
+    if ( destroy_xen_mappings(
+            FIXMAP_ADDR(map),
+            FIXMAP_ADDR(map) + PAGE_SIZE) != 0 )
+        BUG();
+}
-- 
2.47.1

Re: [PATCH v2 4/7] xen/riscv: add {set,clear}_fixmap() functions for managing fixmap entries

[Jan Beulich]

On 11.12.2024 18:27, Oleksii Kurochko wrote:
> @@ -433,3 +434,19 @@ int __init populate_pt_range(unsigned long virt, unsigned long nr_mfns)
>  {
>      return pt_update(virt, INVALID_MFN, nr_mfns, PTE_POPULATE);
>  }
> +
> +/* Map a 4k page in a fixmap entry */
> +void set_fixmap(unsigned int map, mfn_t mfn, unsigned int flags)
> +{
> +    if ( map_pages_to_xen(FIXMAP_ADDR(map), mfn, 1, flags | PTE_SMALL) != 0 )
> +        BUG();
> +}
> +
> +/* Remove a mapping from a fixmap entry */
> +void clear_fixmap(unsigned int map)
> +{
> +    if ( destroy_xen_mappings(
> +            FIXMAP_ADDR(map),
> +            FIXMAP_ADDR(map) + PAGE_SIZE) != 0 )

There are multiple options of how to indent such wrapped lines in function
invocations, but this isn't one of them.

    if ( destroy_xen_mappings(
             FIXMAP_ADDR(map),
             FIXMAP_ADDR(map) + PAGE_SIZE) != 0 )

(arguments offset by 4 from the function name, which may not be a multiple
of 4 from the start of the line) or

    if ( destroy_xen_mappings(FIXMAP_ADDR(map),
                              FIXMAP_ADDR(map) + PAGE_SIZE) != 0 )

Jan

[PATCH v2 5/7] xen/riscv: implement data and instruction cache operations

[Oleksii Kurochko]

Implement following cache operations:
- clean_and_invalidate_dcache_va_range()
- clean_dcache_va_range()
- invalidate_icache()

The first two functions may require support for the CMO (Cache Management
Operations) extension and/or hardware-specific instructions.
Currently, only QEMU is supported, which does not model cache behavior.
Therefore, clean_and_invalidate_dcache_va_range() and clean_dcache_va_range()
are implemented to simply return 0. For other cases, -ENOTSUPP is returned.
If hardware supports CMO or hardware-specific instructions,
these functions should be updated accordingly. To support current
implementation of these function CONFIG_QEMU is introduced.

invalidate_icache() is implemented using fence.i instruction as
mentioned in the unpriv spec:
  The FENCE.I instruction was designed to support a wide variety of
  implementations. A simple implementation can flush the local instruction
  cache and the instruction pipeline when the FENCE.I is executed.
  A more complex implementation might snoop the instruction (data) cache
  on every data (instruction) cache miss, or use an inclusive unified
  private L2 cache to invalidate lines from the primary instruction cache
  when they are being written by a local store instruction.
  If instruction and data caches are kept coherent in this way, or if the
  memory system consists of only uncached RAMs, then just the fetch pipeline
  needs to be flushed at a FENCE.I.
The FENCE.I instruction requires the presence of the Zifencei extension,
which might not always be available. However, Xen uses the RV64G ISA, which
guarantees the presence of the Zifencei extension. According to the
unprivileged ISA specification (version 20240411):
  One goal of the RISC-V project is that it be used as a stable software
  development target. For this purpose, we define a combination of a base ISA
  (RV32I or RV64I) plus selected standard extensions (IMAFD, Zicsr, Zifencei)
  as a "general-purpose" ISA, and we use the abbreviation G for the
  IMAFDZicsr_Zifencei combination of instruction-set extensions.

Set CONFIG_QEMU=y in tiny64_defconfig to have proper implemtation of
clean_and_invalidate_dcache_va_range() and clean_dcache_va_range() for CI.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
 - Update the commit message and subject:
   - drop information about HAS_CMO;
   - add information about Zifencei extension;
 - Introdce platforms directory and CONFIG_QEMU; update implementation of
   data/instruction cache operations as returning 0 for CONFIG_QEMU and for
   others - return -ENOTSUPP.
 - Drop HAS_CMO config.
---
 xen/arch/riscv/Kconfig                  |  2 ++
 xen/arch/riscv/configs/tiny64_defconfig |  1 +
 xen/arch/riscv/include/asm/page.h       | 21 ++++++++++++++++++++-
 xen/arch/riscv/platforms/Kconfig        |  5 +++++
 4 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 xen/arch/riscv/platforms/Kconfig

diff --git a/xen/arch/riscv/Kconfig b/xen/arch/riscv/Kconfig
index 1858004676..00f329054c 100644
--- a/xen/arch/riscv/Kconfig
+++ b/xen/arch/riscv/Kconfig
@@ -52,6 +52,8 @@ config RISCV_ISA_C
 
 endmenu
 
+source "arch/riscv/platforms/Kconfig"
+
 source "common/Kconfig"
 
 source "drivers/Kconfig"
diff --git a/xen/arch/riscv/configs/tiny64_defconfig b/xen/arch/riscv/configs/tiny64_defconfig
index fc7a04872f..47076e357c 100644
--- a/xen/arch/riscv/configs/tiny64_defconfig
+++ b/xen/arch/riscv/configs/tiny64_defconfig
@@ -10,3 +10,4 @@ CONFIG_RISCV_64=y
 CONFIG_DEBUG=y
 CONFIG_DEBUG_INFO=y
 CONFIG_EXPERT=y
+CONFIG_QEMU=y
diff --git a/xen/arch/riscv/include/asm/page.h b/xen/arch/riscv/include/asm/page.h
index bf3f75e85d..54c6fe6515 100644
--- a/xen/arch/riscv/include/asm/page.h
+++ b/xen/arch/riscv/include/asm/page.h
@@ -7,6 +7,7 @@
 
 #include <xen/bug.h>
 #include <xen/const.h>
+#include <xen/errno.h>
 #include <xen/types.h>
 
 #include <asm/atomic.h>
@@ -148,9 +149,27 @@ static inline bool pte_is_mapping(pte_t p)
     return (p.pte & PTE_VALID) && (p.pte & PTE_ACCESS_MASK);
 }
 
+static inline int clean_and_invalidate_dcache_va_range(const void *p, unsigned long size)
+{
+#ifdef CONFIG_QEMU
+    return 0;
+#else
+    return -EOPNOTSUPP;
+#endif
+}
+
+static inline int clean_dcache_va_range(const void *p, unsigned long size)
+{
+#ifdef CONFIG_QEMU
+    return 0;
+#else
+    return -EOPNOTSUPP;
+#endif
+}
+
 static inline void invalidate_icache(void)
 {
-    BUG_ON("unimplemented");
+    asm volatile ( "fence.i" ::: "memory" );
 }
 
 #define clear_page(page) memset((void *)(page), 0, PAGE_SIZE)
diff --git a/xen/arch/riscv/platforms/Kconfig b/xen/arch/riscv/platforms/Kconfig
new file mode 100644
index 0000000000..30ed938d52
--- /dev/null
+++ b/xen/arch/riscv/platforms/Kconfig
@@ -0,0 +1,5 @@
+config QEMU
+	bool "QEMU aarch virt machine support"
+	depends on RISCV_64
+	help
+	  Enable all the required drivers for QEMU riscv64 virt emulated machine.
-- 
2.47.1

Re: [PATCH v2 5/7] xen/riscv: implement data and instruction cache operations

[Jan Beulich]

On 11.12.2024 18:27, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/include/asm/page.h
> +++ b/xen/arch/riscv/include/asm/page.h
> @@ -7,6 +7,7 @@
>  
>  #include <xen/bug.h>
>  #include <xen/const.h>
> +#include <xen/errno.h>
>  #include <xen/types.h>
>  
>  #include <asm/atomic.h>
> @@ -148,9 +149,27 @@ static inline bool pte_is_mapping(pte_t p)
>      return (p.pte & PTE_VALID) && (p.pte & PTE_ACCESS_MASK);
>  }
>  
> +static inline int clean_and_invalidate_dcache_va_range(const void *p, unsigned long size)
> +{
> +#ifdef CONFIG_QEMU
> +    return 0;
> +#else
> +    return -EOPNOTSUPP;
> +#endif
> +}
> +
> +static inline int clean_dcache_va_range(const void *p, unsigned long size)
> +{
> +#ifdef CONFIG_QEMU
> +    return 0;
> +#else
> +    return -EOPNOTSUPP;
> +#endif
> +}

So testing on real hardware will then effectively become impossible, until
someone goes and implements these?

> --- /dev/null
> +++ b/xen/arch/riscv/platforms/Kconfig
> @@ -0,0 +1,5 @@
> +config QEMU
> +	bool "QEMU aarch virt machine support"
> +	depends on RISCV_64

I understand Arm has it like this, but: Is QEMU really a sufficiently non-
ambiguous name to use? Is the RISCV_64 dependency really needed?

> +	help
> +	  Enable all the required drivers for QEMU riscv64 virt emulated machine.

This line looks to be slightly too long now (after you apparently unwrapped
what Arm has).

Jan

Re: [PATCH v2 5/7] xen/riscv: implement data and instruction cache operations

[Oleksii Kurochko]

[-- Attachment #1: Type: text/plain, Size: 3713 bytes --]

On 12/16/24 3:23 PM, Jan Beulich wrote:
> On 11.12.2024 18:27, Oleksii Kurochko wrote:
>> --- a/xen/arch/riscv/include/asm/page.h
>> +++ b/xen/arch/riscv/include/asm/page.h
>> @@ -7,6 +7,7 @@
>>   
>>   #include <xen/bug.h>
>>   #include <xen/const.h>
>> +#include <xen/errno.h>
>>   #include <xen/types.h>
>>   
>>   #include <asm/atomic.h>
>> @@ -148,9 +149,27 @@ static inline bool pte_is_mapping(pte_t p)
>>       return (p.pte & PTE_VALID) && (p.pte & PTE_ACCESS_MASK);
>>   }
>>   
>> +static inline int clean_and_invalidate_dcache_va_range(const void *p, unsigned long size)
>> +{
>> +#ifdef CONFIG_QEMU
>> +    return 0;
>> +#else
>> +    return -EOPNOTSUPP;
>> +#endif
>> +}
>> +
>> +static inline int clean_dcache_va_range(const void *p, unsigned long size)
>> +{
>> +#ifdef CONFIG_QEMU
>> +    return 0;
>> +#else
>> +    return -EOPNOTSUPP;
>> +#endif
>> +}
> So testing on real hardware will then effectively become impossible, until
> someone goes and implements these?

Yes...

I am not sure what better we can do. It seems like it will be the best one to check if CMO
extensions is supported and use instructions for this extensions to implement these functions as they
are in the specification and not expected to be changed.

But I want to back a little bit later to this implemntation as this not issue for QEMU as it doesn't model cache and
h/w on which I can ask to run Xen has IO cache coherency so for it will be needed just to add a new config
and implementation will still be 'return 0'. ( I thought to introduce instead of CONFIG_QEMU something like
CONFIG_HAS_CACHE_COHERENCY )

And also in the spec it is mentioned:
```
This suggests that RISC-V platforms prefer to support full 
cache-coherent I/O, but it isn't actually mandatory.
As a result, the PMBT and CMO extensions aren't mandatory either, 
meaning that some platforms might not
have instructions to properly flush, clean, or invalidate the cache.
``` Based on that I also think to implement that in the following way:
```
     #ifdef CONFIG_QEMU
     static inline int plat_clean_and_invalidate_dcache_va_range() { return 0; }
   static inline int plat_clean_dcache_va_range() { return 0; }
   #else /* !CONFIG_QEMU */
     static inline void plat_clean_and_invalidate_dcache_va_range()
   {
     printk_once("%s: should it be implemented for your platform?\n", __func__);
     return 0;
   }

   static inline void plat_clean_dcache_va_range()
   {
     printk_once("%s: should it be implemented for your platform?\n", __func__);
     return 0;
   }
   #endif

   static inline int clean_and_invalidate_dcache_va_range(const void *p, unsigned long size)
   {
       return plat_clean_and_invalidate_dcache_va_range();
   }
....
```
So we will have a notification for others none-QEMU platforms notification that probably some
changes are required.

>
>> --- /dev/null
>> +++ b/xen/arch/riscv/platforms/Kconfig
>> @@ -0,0 +1,5 @@
>> +config QEMU
>> +	bool "QEMU aarch virt machine support"
>> +	depends on RISCV_64
> I understand Arm has it like this, but: Is QEMU really a sufficiently non-
> ambiguous name to use?

Yes, it sounds good to me to have such naming for the platform which are running on top of QEMU.

The other option I thought about it is to use CONFIG_VIRT_PLATFORM.

>   Is the RISCV_64 dependency really needed?

At the moment , we are testing only RISCV_64 so not to have none-verified config I added RISCV_64 but

probably it is too much and `depends on` should be dropped at all.

Thanks.

~ Oleksii

>
>> +	help
>> +	  Enable all the required drivers for QEMU riscv64 virt emulated machine.
> This line looks to be slightly too long now (after you apparently unwrapped
> what Arm has).
>
> Jan

[-- Attachment #2: Type: text/html, Size: 11937 bytes --]

Re: [PATCH v2 5/7] xen/riscv: implement data and instruction cache operations

[Jan Beulich]

On 16.12.2024 18:40, Oleksii Kurochko wrote:
> On 12/16/24 3:23 PM, Jan Beulich wrote:
>> On 11.12.2024 18:27, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/include/asm/page.h
>>> +++ b/xen/arch/riscv/include/asm/page.h
>>> @@ -7,6 +7,7 @@
>>>   
>>>   #include <xen/bug.h>
>>>   #include <xen/const.h>
>>> +#include <xen/errno.h>
>>>   #include <xen/types.h>
>>>   
>>>   #include <asm/atomic.h>
>>> @@ -148,9 +149,27 @@ static inline bool pte_is_mapping(pte_t p)
>>>       return (p.pte & PTE_VALID) && (p.pte & PTE_ACCESS_MASK);
>>>   }
>>>   
>>> +static inline int clean_and_invalidate_dcache_va_range(const void *p, unsigned long size)
>>> +{
>>> +#ifdef CONFIG_QEMU
>>> +    return 0;
>>> +#else
>>> +    return -EOPNOTSUPP;
>>> +#endif
>>> +}
>>> +
>>> +static inline int clean_dcache_va_range(const void *p, unsigned long size)
>>> +{
>>> +#ifdef CONFIG_QEMU
>>> +    return 0;
>>> +#else
>>> +    return -EOPNOTSUPP;
>>> +#endif
>>> +}
>> So testing on real hardware will then effectively become impossible, until
>> someone goes and implements these?
> 
> Yes...
> 
> I am not sure what better we can do. It seems like it will be the best one to check if CMO
> extensions is supported and use instructions for this extensions to implement these functions as they
> are in the specification and not expected to be changed.

Yes, using CMO when available is certainly the route to go. The main
question there is what the behavior ought to be when CMO is unavailable.

> But I want to back a little bit later to this implemntation as this not issue for QEMU as it doesn't model cache and
> h/w on which I can ask to run Xen has IO cache coherency so for it will be needed just to add a new config
> and implementation will still be 'return 0'. ( I thought to introduce instead of CONFIG_QEMU something like
> CONFIG_HAS_CACHE_COHERENCY )
> 
> And also in the spec it is mentioned:
> ```
> This suggests that RISC-V platforms prefer to support full 
> cache-coherent I/O, but it isn't actually mandatory.
> As a result, the PMBT and CMO extensions aren't mandatory either, 
> meaning that some platforms might not
> have instructions to properly flush, clean, or invalidate the cache.
> ``` Based on that I also think to implement that in the following way:
> ```
>      #ifdef CONFIG_QEMU
>      static inline int plat_clean_and_invalidate_dcache_va_range() { return 0; }
>    static inline int plat_clean_dcache_va_range() { return 0; }
>    #else /* !CONFIG_QEMU */
>      static inline void plat_clean_and_invalidate_dcache_va_range()
>    {
>      printk_once("%s: should it be implemented for your platform?\n", __func__);
>      return 0;
>    }
> 
>    static inline void plat_clean_dcache_va_range()
>    {
>      printk_once("%s: should it be implemented for your platform?\n", __func__);
>      return 0;
>    }
>    #endif
> 
>    static inline int clean_and_invalidate_dcache_va_range(const void *p, unsigned long size)
>    {
>        return plat_clean_and_invalidate_dcache_va_range();
>    }
> ....
> ```
> So we will have a notification for others none-QEMU platforms notification that probably some
> changes are required.

Yet failing to get cache management right can easily result in data corruption.
I don't think a on-time printk() is appropriate to handle the lack of respective
implementation. At least not anymore once RISC-V leaves "experimental" status.

>>> --- /dev/null
>>> +++ b/xen/arch/riscv/platforms/Kconfig
>>> @@ -0,0 +1,5 @@
>>> +config QEMU
>>> +	bool "QEMU aarch virt machine support"
>>> +	depends on RISCV_64
>> I understand Arm has it like this, but: Is QEMU really a sufficiently non-
>> ambiguous name to use?
> 
> Yes, it sounds good to me to have such naming for the platform which are running on top of QEMU.
> 
> The other option I thought about it is to use CONFIG_VIRT_PLATFORM.

I don't think QEMU should be fully omitted from the name. Nor do I think that
you can generally infer from "virtual platform" that caches aren't modeled.
What I was rather getting at is to perhaps add some qualifier to QEMU, e.g.
QEMU_PLATFORM.

Jan

Re: [PATCH v2 5/7] xen/riscv: implement data and instruction cache operations

[Oleksii Kurochko]

[-- Attachment #1: Type: text/plain, Size: 4790 bytes --]


On 12/17/24 9:32 AM, Jan Beulich wrote:
> On 16.12.2024 18:40, Oleksii Kurochko wrote:
>> On 12/16/24 3:23 PM, Jan Beulich wrote:
>>> On 11.12.2024 18:27, Oleksii Kurochko wrote:
>>>> --- a/xen/arch/riscv/include/asm/page.h
>>>> +++ b/xen/arch/riscv/include/asm/page.h
>>>> @@ -7,6 +7,7 @@
>>>>    
>>>>    #include <xen/bug.h>
>>>>    #include <xen/const.h>
>>>> +#include <xen/errno.h>
>>>>    #include <xen/types.h>
>>>>    
>>>>    #include <asm/atomic.h>
>>>> @@ -148,9 +149,27 @@ static inline bool pte_is_mapping(pte_t p)
>>>>        return (p.pte & PTE_VALID) && (p.pte & PTE_ACCESS_MASK);
>>>>    }
>>>>    
>>>> +static inline int clean_and_invalidate_dcache_va_range(const void *p, unsigned long size)
>>>> +{
>>>> +#ifdef CONFIG_QEMU
>>>> +    return 0;
>>>> +#else
>>>> +    return -EOPNOTSUPP;
>>>> +#endif
>>>> +}
>>>> +
>>>> +static inline int clean_dcache_va_range(const void *p, unsigned long size)
>>>> +{
>>>> +#ifdef CONFIG_QEMU
>>>> +    return 0;
>>>> +#else
>>>> +    return -EOPNOTSUPP;
>>>> +#endif
>>>> +}
>>> So testing on real hardware will then effectively become impossible, until
>>> someone goes and implements these?
>> Yes...
>>
>> I am not sure what better we can do. It seems like it will be the best one to check if CMO
>> extensions is supported and use instructions for this extensions to implement these functions as they
>> are in the specification and not expected to be changed.
> Yes, using CMO when available is certainly the route to go. The main
> question there is what the behavior ought to be when CMO is unavailable.

If CMO ( or SoC specific extension for cache operation ) isn't available then IMO it means that memory is
coherent and nothing specific should be done in the mentioned above functions what means returning 0 should
be fine. Then implementation of these functions could look like:
```

static inline int <.....>(....)
{
#if !defined(CONFIG_QEMU)

    #warning should implementation of <....>  be updated? 

#endif

    return 0; 

}
```

Or just to be sure that user see the message change #warning -> #error.

~ Oleksii

>
>> But I want to back a little bit later to this implemntation as this not issue for QEMU as it doesn't model cache and
>> h/w on which I can ask to run Xen has IO cache coherency so for it will be needed just to add a new config
>> and implementation will still be 'return 0'. ( I thought to introduce instead of CONFIG_QEMU something like
>> CONFIG_HAS_CACHE_COHERENCY )
>>
>> And also in the spec it is mentioned:
>> ```
>> This suggests that RISC-V platforms prefer to support full
>> cache-coherent I/O, but it isn't actually mandatory.
>> As a result, the PMBT and CMO extensions aren't mandatory either,
>> meaning that some platforms might not
>> have instructions to properly flush, clean, or invalidate the cache.
>> ``` Based on that I also think to implement that in the following way:
>> ```
>>       #ifdef CONFIG_QEMU
>>       static inline int plat_clean_and_invalidate_dcache_va_range() { return 0; }
>>     static inline int plat_clean_dcache_va_range() { return 0; }
>>     #else /* !CONFIG_QEMU */
>>       static inline void plat_clean_and_invalidate_dcache_va_range()
>>     {
>>       printk_once("%s: should it be implemented for your platform?\n", __func__);
>>       return 0;
>>     }
>>
>>     static inline void plat_clean_dcache_va_range()
>>     {
>>       printk_once("%s: should it be implemented for your platform?\n", __func__);
>>       return 0;
>>     }
>>     #endif
>>
>>     static inline int clean_and_invalidate_dcache_va_range(const void *p, unsigned long size)
>>     {
>>         return plat_clean_and_invalidate_dcache_va_range();
>>     }
>> ....
>> ```
>> So we will have a notification for others none-QEMU platforms notification that probably some
>> changes are required.
> Yet failing to get cache management right can easily result in data corruption.
> I don't think a on-time printk() is appropriate to handle the lack of respective
> implementation. At least not anymore once RISC-V leaves "experimental" status.
>
>>>> --- /dev/null
>>>> +++ b/xen/arch/riscv/platforms/Kconfig
>>>> @@ -0,0 +1,5 @@
>>>> +config QEMU
>>>> +	bool "QEMU aarch virt machine support"
>>>> +	depends on RISCV_64
>>> I understand Arm has it like this, but: Is QEMU really a sufficiently non-
>>> ambiguous name to use?
>> Yes, it sounds good to me to have such naming for the platform which are running on top of QEMU.
>>
>> The other option I thought about it is to use CONFIG_VIRT_PLATFORM.
> I don't think QEMU should be fully omitted from the name. Nor do I think that
> you can generally infer from "virtual platform" that caches aren't modeled.
> What I was rather getting at is to perhaps add some qualifier to QEMU, e.g.
> QEMU_PLATFORM.
>
> Jan

[-- Attachment #2: Type: text/html, Size: 6073 bytes --]

Re: [PATCH v2 5/7] xen/riscv: implement data and instruction cache operations

[Jan Beulich]

On 17.12.2024 11:37, Oleksii Kurochko wrote:
> 
> On 12/17/24 9:32 AM, Jan Beulich wrote:
>> On 16.12.2024 18:40, Oleksii Kurochko wrote:
>>> On 12/16/24 3:23 PM, Jan Beulich wrote:
>>>> On 11.12.2024 18:27, Oleksii Kurochko wrote:
>>>>> --- a/xen/arch/riscv/include/asm/page.h
>>>>> +++ b/xen/arch/riscv/include/asm/page.h
>>>>> @@ -7,6 +7,7 @@
>>>>>    
>>>>>    #include <xen/bug.h>
>>>>>    #include <xen/const.h>
>>>>> +#include <xen/errno.h>
>>>>>    #include <xen/types.h>
>>>>>    
>>>>>    #include <asm/atomic.h>
>>>>> @@ -148,9 +149,27 @@ static inline bool pte_is_mapping(pte_t p)
>>>>>        return (p.pte & PTE_VALID) && (p.pte & PTE_ACCESS_MASK);
>>>>>    }
>>>>>    
>>>>> +static inline int clean_and_invalidate_dcache_va_range(const void *p, unsigned long size)
>>>>> +{
>>>>> +#ifdef CONFIG_QEMU
>>>>> +    return 0;
>>>>> +#else
>>>>> +    return -EOPNOTSUPP;
>>>>> +#endif
>>>>> +}
>>>>> +
>>>>> +static inline int clean_dcache_va_range(const void *p, unsigned long size)
>>>>> +{
>>>>> +#ifdef CONFIG_QEMU
>>>>> +    return 0;
>>>>> +#else
>>>>> +    return -EOPNOTSUPP;
>>>>> +#endif
>>>>> +}
>>>> So testing on real hardware will then effectively become impossible, until
>>>> someone goes and implements these?
>>> Yes...
>>>
>>> I am not sure what better we can do. It seems like it will be the best one to check if CMO
>>> extensions is supported and use instructions for this extensions to implement these functions as they
>>> are in the specification and not expected to be changed.
>> Yes, using CMO when available is certainly the route to go. The main
>> question there is what the behavior ought to be when CMO is unavailable.
> 
> If CMO ( or SoC specific extension for cache operation ) isn't available then IMO it means that memory is
> coherent and nothing specific should be done in the mentioned above functions what means returning 0 should
> be fine.

Hmm, imo that would be a pretty bold assumption.

Jan

[PATCH v2 6/7] xen/riscv: implement prereq for DTB relocation

[Oleksii Kurochko]

DTB relocatin in Xen heap requires the following functions which are
introduced in current patch:
- xvmalloc_array()
- copy_from_paddr()

For internal use of xvmalloc, the functions flush_page_to_ram() and
virt_to_page() are introduced. virt_to_page() is also required for
free_xenheap_pages().

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
 - Drop variable directmap_virt_end.
 - Update ASSERT in virt_to_page() to use DIRECTMAP_VIRT_END instead of
   variable directmap_virt_end.
 - Declare local varibale v as const inside flush_page_to_ram().
 - Declare copy_from_paddr() in riscv/setup.h as in the future it will be
   used for copying kernel in guest memory.
 - Code style updates for copy_from_paddr().
 - Make l and s variable in copy_from_paddr() as the virables' initializers.
 - Drop call of clean_dcache_va_range(dst, l) in copy_from_paddr() as the
   necessiry of it is Arm-specific:
     After memcpy'ing the kernel in guest memory Arm need to flush the dcache
     to make sure that the data actually reaches the memory before we start
     executing guest code with caches disabled.
   RISC-V has caches always enabled thereby there is no such issue for RISC-V.
 - Make local variable src in copy_from_paddr() as const.
 - Update the commit message and subject: drop information of relocate_fdt()
   introduction and rename it to "prereq for DTB relocation".
 - Add BUG_ON() inside flush_page_to_ram() to check the return value of
   clean_and_invalidate_dcache_va_range().
 - Move relocate_fdt() introduction to the next patch to make this patch
   compilable.
---
 xen/arch/riscv/include/asm/mm.h    |  8 ++++++--
 xen/arch/riscv/include/asm/page.h  | 11 +++++++++--
 xen/arch/riscv/include/asm/setup.h |  4 ++++
 xen/arch/riscv/setup.c             | 26 ++++++++++++++++++++++++++
 4 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index 699ed23f0d..292aa48fc1 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -8,6 +8,7 @@
 #include <xen/const.h>
 #include <xen/mm-frame.h>
 #include <xen/pdx.h>
+#include <xen/pfn.h>
 #include <xen/types.h>
 
 #include <asm/page-bits.h>
@@ -148,8 +149,11 @@ static inline void *page_to_virt(const struct page_info *pg)
 /* Convert between Xen-heap virtual addresses and page-info structures. */
 static inline struct page_info *virt_to_page(const void *v)
 {
-    BUG_ON("unimplemented");
-    return NULL;
+    unsigned long va = (unsigned long)v;
+
+    ASSERT((va >= DIRECTMAP_VIRT_START) && (va <= DIRECTMAP_VIRT_END));
+
+    return frametable_virt_start + PFN_DOWN(va - directmap_virt_start);
 }
 
 /*
diff --git a/xen/arch/riscv/include/asm/page.h b/xen/arch/riscv/include/asm/page.h
index 54c6fe6515..fbb35a6673 100644
--- a/xen/arch/riscv/include/asm/page.h
+++ b/xen/arch/riscv/include/asm/page.h
@@ -7,6 +7,7 @@
 
 #include <xen/bug.h>
 #include <xen/const.h>
+#include <xen/domain_page.h>
 #include <xen/errno.h>
 #include <xen/types.h>
 
@@ -175,10 +176,16 @@ static inline void invalidate_icache(void)
 #define clear_page(page) memset((void *)(page), 0, PAGE_SIZE)
 #define copy_page(dp, sp) memcpy(dp, sp, PAGE_SIZE)
 
-/* TODO: Flush the dcache for an entire page. */
 static inline void flush_page_to_ram(unsigned long mfn, bool sync_icache)
 {
-    BUG_ON("unimplemented");
+    const void *v = map_domain_page(_mfn(mfn));
+
+    BUG_ON(clean_and_invalidate_dcache_va_range(v, PAGE_SIZE));
+
+    unmap_domain_page(v);
+
+    if ( sync_icache )
+        invalidate_icache();
 }
 
 /* Write a pagetable entry. */
diff --git a/xen/arch/riscv/include/asm/setup.h b/xen/arch/riscv/include/asm/setup.h
index 844a2f0ef1..c9d69cdf51 100644
--- a/xen/arch/riscv/include/asm/setup.h
+++ b/xen/arch/riscv/include/asm/setup.h
@@ -3,10 +3,14 @@
 #ifndef ASM__RISCV__SETUP_H
 #define ASM__RISCV__SETUP_H
 
+#include <xen/types.h>
+
 #define max_init_domid (0)
 
 void setup_mm(void);
 
+void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
+
 #endif /* ASM__RISCV__SETUP_H */
 
 /*
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index 9680332fee..bea3f27c4d 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -12,6 +12,7 @@
 #include <public/version.h>
 
 #include <asm/early_printk.h>
+#include <asm/fixmap.h>
 #include <asm/sbi.h>
 #include <asm/setup.h>
 #include <asm/smp.h>
@@ -26,6 +27,31 @@ void arch_get_xen_caps(xen_capabilities_info_t *info)
 unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
     __aligned(STACK_SIZE);
 
+/**
+ * copy_from_paddr - copy data from a physical address
+ * @dst: destination virtual address
+ * @paddr: source physical address
+ * @len: length to copy
+ */
+void __init copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
+{
+    const void *src = (void *)FIXMAP_ADDR(FIX_MISC);
+
+    while ( len )
+    {
+        unsigned long s = paddr & (PAGE_SIZE - 1);
+        unsigned long l = min(PAGE_SIZE - s, len);
+
+        set_fixmap(FIX_MISC, maddr_to_mfn(paddr), PAGE_HYPERVISOR_RW);
+        memcpy(dst, src + s, l);
+        clear_fixmap(FIX_MISC);
+
+        paddr += l;
+        dst += l;
+        len -= l;
+    }
+}
+
 void __init noreturn start_xen(unsigned long bootcpu_id,
                                paddr_t dtb_addr)
 {
-- 
2.47.1

Re: [PATCH v2 6/7] xen/riscv: implement prereq for DTB relocation

[Jan Beulich]

On 11.12.2024 18:27, Oleksii Kurochko wrote:
> @@ -175,10 +176,16 @@ static inline void invalidate_icache(void)
>  #define clear_page(page) memset((void *)(page), 0, PAGE_SIZE)
>  #define copy_page(dp, sp) memcpy(dp, sp, PAGE_SIZE)
>  
> -/* TODO: Flush the dcache for an entire page. */
>  static inline void flush_page_to_ram(unsigned long mfn, bool sync_icache)
>  {
> -    BUG_ON("unimplemented");
> +    const void *v = map_domain_page(_mfn(mfn));
> +
> +    BUG_ON(clean_and_invalidate_dcache_va_range(v, PAGE_SIZE));

Please avoid expressions with side effects in BUG_ON(). This wants to be

    if ( clean_and_invalidate_dcache_va_range(v, PAGE_SIZE) )
        BUG();

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

Jan

[PATCH v2 7/7] xen/riscv: relocating and unflattening host device tree

[Oleksii Kurochko]

Introduce relocate_fdt() and call it to relocate FDT to Xen heap
instead of using early mapping as it is expected that discard_initial_modules()
( is supposed to call in the future ) discards the FDT boot module and
remove_early_mappings() destroys the early mapping.

Unflatten a device tree, creating the tree of struct device_node.
It also fills the "name" and "type" pointers of the nodes so the normal
device-tree walking functions can be used.

Set device_tree_flattened to NULL in the case when acpi_disabled is
equal to false.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
 - Move introduction of relocate_fdt() to the current patch with the following
   change:
    - use xvmalloc() instead of xmalloc_bytes() in relocate_fdt();
 - Drop the check of returned fdt_size from boot_fdt_info() to be in sync
   with Arm and boot_fdt_info() will panic anyway if something wrong with
   DTB.
 - Update the commit message.
---
 xen/arch/riscv/setup.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index bea3f27c4d..fb6bbba684 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
+#include <xen/acpi.h>
 #include <xen/bug.h>
 #include <xen/bootfdt.h>
 #include <xen/compile.h>
@@ -8,6 +9,7 @@
 #include <xen/mm.h>
 #include <xen/shutdown.h>
 #include <xen/vmap.h>
+#include <xen/xvmalloc.h>
 
 #include <public/version.h>
 
@@ -52,10 +54,24 @@ void __init copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
     }
 }
 
+/* Relocate the FDT in Xen heap */
+static void * __init relocate_fdt(paddr_t dtb_paddr, size_t dtb_size)
+{
+    void *fdt = xvmalloc_array(uint8_t, dtb_size);
+
+    if ( !fdt )
+        panic("Unable to allocate memory for relocating the Device-Tree.\n");
+
+    copy_from_paddr(fdt, dtb_paddr, dtb_size);
+
+    return fdt;
+}
+
 void __init noreturn start_xen(unsigned long bootcpu_id,
                                paddr_t dtb_addr)
 {
     const char *cmdline;
+    size_t fdt_size;
 
     remove_identity_mapping();
 
@@ -80,8 +96,7 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
                           _end - _start, false) )
         panic("Failed to add BOOTMOD_XEN\n");
 
-    if ( !boot_fdt_info(device_tree_flattened, dtb_addr) )
-        BUG();
+    fdt_size = boot_fdt_info(device_tree_flattened, dtb_addr);
 
     cmdline = boot_fdt_cmdline(device_tree_flattened);
     printk("Command line: %s\n", cmdline);
@@ -99,6 +114,18 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
      */
     system_state = SYS_STATE_boot;
 
+    if ( acpi_disabled )
+    {
+        printk("Booting using Device Tree\n");
+        device_tree_flattened = relocate_fdt(dtb_addr, fdt_size);
+        dt_unflatten_host_device_tree();
+    }
+    else
+    {
+        device_tree_flattened = NULL;
+        panic("Booting using ACPI isn't supported\n");
+    }
+
     printk("All set up\n");
 
     machine_halt();
-- 
2.47.1

Re: [PATCH v2 7/7] xen/riscv: relocating and unflattening host device tree

[Jan Beulich]

On 11.12.2024 18:27, Oleksii Kurochko wrote:
> Introduce relocate_fdt() and call it to relocate FDT to Xen heap
> instead of using early mapping as it is expected that discard_initial_modules()
> ( is supposed to call in the future ) discards the FDT boot module and
> remove_early_mappings() destroys the early mapping.
> 
> Unflatten a device tree, creating the tree of struct device_node.
> It also fills the "name" and "type" pointers of the nodes so the normal
> device-tree walking functions can be used.
> 
> Set device_tree_flattened to NULL in the case when acpi_disabled is
> equal to false.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

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