relocate_fdt() relocates 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.

To implement that the following things are introduced as they are called
by internals of xmalloc_bytes() which is used in relocate_fdt():
1. As RISC-V may have non-coherent access for RAM ( f.e., in case
   of non-coherent IO devices ) flush_page_to_ram() is implemented
   to ensure that cache and RAM are consistent for such platforms.
2. copy_from_paddr() to copy FDT from a physical address to allocated
   by xmalloc_bytes() in Xen heap.
3. virt_to_page() to convert virtual address to page. Also introduce
   directmap_virt_end to check that VA argument of virt_to_page() is
   inside directmap region.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/include/asm/mm.h   | 10 ++++++--
 xen/arch/riscv/include/asm/page.h | 10 ++++++--
 xen/arch/riscv/mm.c               |  3 +++
 xen/arch/riscv/setup.c            | 41 +++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index 699ed23f0d..25fd38531f 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -8,11 +8,13 @@
 #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>
 
 extern vaddr_t directmap_virt_start;
+extern vaddr_t directmap_virt_end;
 
 #define pfn_to_paddr(pfn) ((paddr_t)(pfn) << PAGE_SHIFT)
 #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
@@ -148,8 +150,12 @@ 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);
+    ASSERT(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 0f297141d3..c245a4273f 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>
 
@@ -172,10 +173,15 @@ 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");
+    void *v = map_domain_page(_mfn(mfn));
+
+    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/mm.c b/xen/arch/riscv/mm.c
index f2bf279bac..c614d547a6 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -419,6 +419,7 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
 }
 
 vaddr_t __ro_after_init directmap_virt_start = DIRECTMAP_VIRT_START;
+vaddr_t __ro_after_init directmap_virt_end;
 
 struct page_info *__ro_after_init frametable_virt_start = frame_table;
 
@@ -556,6 +557,8 @@ void __init setup_mm(void)
         setup_directmap_mappings(PFN_DOWN(bank_start), PFN_DOWN(bank_size));
     }
 
+    directmap_virt_end = directmap_virt_start + ram_end - 1;
+
     setup_frametable_mappings(ram_start, ram_end);
     max_page = PFN_DOWN(ram_end);
 }
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index 9680332fee..ff667260ec 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,46 @@ 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)
+{
+    void *src = (void *)FIXMAP_ADDR(FIX_MISC);
+
+    while (len) {
+        unsigned long l, s;
+
+        s = paddr & (PAGE_SIZE - 1);
+        l = min(PAGE_SIZE - s, len);
+
+        set_fixmap(FIX_MISC, maddr_to_mfn(paddr), PAGE_HYPERVISOR_RW);
+        memcpy(dst, src + s, l);
+        clean_dcache_va_range(dst, l);
+        clear_fixmap(FIX_MISC);
+
+        paddr += l;
+        dst += l;
+        len -= l;
+    }
+}
+
+/* Relocate the FDT in Xen heap */
+static void * __init relocate_fdt(paddr_t dtb_paddr, size_t dtb_size)
+{
+    void *fdt = xmalloc_bytes(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)
 {
-- 
2.47.0

On 27.11.2024 13:50, Oleksii Kurochko wrote:
> relocate_fdt() relocates 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.
> 
> To implement that the following things are introduced as they are called
> by internals of xmalloc_bytes() which is used in relocate_fdt():
> 1. As RISC-V may have non-coherent access for RAM ( f.e., in case
>    of non-coherent IO devices ) flush_page_to_ram() is implemented
>    to ensure that cache and RAM are consistent for such platforms.

This is a detail of the page allocator, yes. It can then be viewed as also
a detail of xmalloc() et al, but I consider the wording a little misleading.

> 2. copy_from_paddr() to copy FDT from a physical address to allocated
>    by xmalloc_bytes() in Xen heap.

This doesn't look to be related to the internals of xmalloc() et al.

> 3. virt_to_page() to convert virtual address to page. Also introduce
>    directmap_virt_end to check that VA argument of virt_to_page() is
>    inside directmap region.

This is a need of free_xenheap_pages(), yes; see remark on point 1.

> @@ -148,8 +150,12 @@ 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);
> +    ASSERT(va <= directmap_virt_end);

Why the difference compared to virt_to_maddr()?

Also recall my comment on one of your earlier series, regarding inclusive
vs exclusive ranges. Can that please be sorted properly as a prereq, to
avoid extending the inconsistency?

> @@ -172,10 +173,15 @@ 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");
> +    void *v = map_domain_page(_mfn(mfn));

const void *?

> +    clean_and_invalidate_dcache_va_range(v, PAGE_SIZE);
> +    unmap_domain_page(v);
> +
> +    if ( sync_icache )
> +        invalidate_icache();
>  }
>  
>  /* Write a pagetable entry. */
> --- a/xen/arch/riscv/mm.c
> +++ b/xen/arch/riscv/mm.c
> @@ -419,6 +419,7 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
>  }
>  
>  vaddr_t __ro_after_init directmap_virt_start = DIRECTMAP_VIRT_START;
> +vaddr_t __ro_after_init directmap_virt_end;

If the variable is needed (see above) it pretty certainly wants an
initializer, too.

> @@ -26,6 +27,46 @@ 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)

Without a declaration in a header this function ought to be static.

> +{
> +    void *src = (void *)FIXMAP_ADDR(FIX_MISC);

const void *

> +    while (len) {

Nit: Style.

> +        unsigned long l, s;
> +
> +        s = paddr & (PAGE_SIZE - 1);
> +        l = min(PAGE_SIZE - s, len);

Make these the variables' initializers?

> +        set_fixmap(FIX_MISC, maddr_to_mfn(paddr), PAGE_HYPERVISOR_RW);
> +        memcpy(dst, src + s, l);
> +        clean_dcache_va_range(dst, l);

Why is this necessary here? You're copying to plain RAM that Xen alone
is using.

> +/* Relocate the FDT in Xen heap */
> +static void * __init relocate_fdt(paddr_t dtb_paddr, size_t dtb_size)

This function having no caller will - aiui - mean build breakage at
this point of the series.

> +{
> +    void *fdt = xmalloc_bytes(dtb_size);

New code ought to be using xvmalloc() et al. Unless there's a firm
reason not to.

Jan

On 12/9/24 4:00 PM, Jan Beulich wrote:
> On 27.11.2024 13:50, Oleksii Kurochko wrote:
>> relocate_fdt() relocates 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.
>>
>> To implement that the following things are introduced as they are called
>> by internals of xmalloc_bytes() which is used in relocate_fdt():
>> 1. As RISC-V may have non-coherent access for RAM ( f.e., in case
>>     of non-coherent IO devices ) flush_page_to_ram() is implemented
>>     to ensure that cache and RAM are consistent for such platforms.
> This is a detail of the page allocator, yes. It can then be viewed as also
> a detail of xmalloc() et al, but I consider the wording a little misleading.
>
>> 2. copy_from_paddr() to copy FDT from a physical address to allocated
>>     by xmalloc_bytes() in Xen heap.
> This doesn't look to be related to the internals of xmalloc() et al.
>
>> 3. virt_to_page() to convert virtual address to page. Also introduce
>>     directmap_virt_end to check that VA argument of virt_to_page() is
>>     inside directmap region.
> This is a need of free_xenheap_pages(), yes; see remark on point 1.

Actually I faced the usage of virt_to_page() in xmalloc_whole_page():
```
   static void *xmalloc_whole_pages(unsigned long size, unsigned long align)
   {
     ...
     PFN_ORDER(virt_to_page(res)) = PFN_UP(size);
     /* Check that there was no truncation: */
     ASSERT(PFN_ORDER(virt_to_page(res)) == PFN_UP(size));

     return res;
   }
```
which is called from xmalloc().

Do we need a second paragraph of the commit message at all? Or it is just obvious if
flush_page_to_ram(), virt_to_page() and copy_from_paddr() are introduces that they are needed for
relocate_fdt()?

Or perhaps rephrasing in the following way would be enough?
```
For internal use of|xmalloc|, the functions|flush_page_to_ram()| and|virt_to_page()| are introduced.
|virt_to_page()| is also required for|free_xenheap_pages()|. These additions are used to support
|xmalloc|, which is utilized within|relocate_fdt()|. Additionally,|copy_from_paddr()| is introduced
for use in|relocate_fdt()|.
```

>> @@ -148,8 +150,12 @@ 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);
>> +    ASSERT(va <= directmap_virt_end);
> Why the difference compared to virt_to_maddr()?

It is just a mistake as `directmap_virt_end` is directmap_virt_start-relative but `v` is DIRECTMAP_VIRT_START-relative.
The check should be following:
   ASSERT((va >= DIRECTMAP_VIRT_START) && (va <= DIRECTMAP_VIRT_END));
and then directmap_virt_end should be dropped at all.

>
> Also recall my comment on one of your earlier series, regarding inclusive
> vs exclusive ranges. Can that please be sorted properly as a prereq, to
> avoid extending the inconsistency?

Yes, I remember that and at the moment everything ( DIRECTMAP_VIRT_END, FRAMETABLE_VIRT_END )
is following "inclusive" way. Considering that you remind me that could you please tell me more time
what I am missing?

>> +        set_fixmap(FIX_MISC, maddr_to_mfn(paddr), PAGE_HYPERVISOR_RW);
>> +        memcpy(dst, src + s, l);
>> +        clean_dcache_va_range(dst, l);
> Why is this necessary here? You're copying to plain RAM that Xen alone
> is using.

It is Arm specific:
```
commit c60209d77e2c02de110ca0fdaa2582ef4e53d8fd
Author: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
Date:   Mon Jan 21 12:40:31 2013 +0000

     xen/arm: flush dcache after memcpy'ing the kernel image
     
     After memcpy'ing the kernel in guest memory we need to flush the dcache
     to make sure that the data actually reaches the memory before we start
     executing guest code with caches disabled.
     
     copy_from_paddr is the function that does the copy, so add a
     flush_xen_dcache_va_range there.
```
I wanted to put copy_from_paddr() to some common place at the end but in RISC-V cache is always enabled
( I don't see an instruction in the spec for disable/enable cache ) so this issue isn't present for RISC-V
and clean_dcache_va_range() should/could be dropped.

>> +/* Relocate the FDT in Xen heap */
>> +static void * __init relocate_fdt(paddr_t dtb_paddr, size_t dtb_size)
> This function having no caller will - aiui - mean build breakage at
> this point of the series.

Yes, it should be a problem, missed that. Then I have to merge it with the next one patch.

Thanks.

~ Oleksii

On 10.12.2024 16:20, Oleksii Kurochko wrote:
> On 12/9/24 4:00 PM, Jan Beulich wrote:
>> On 27.11.2024 13:50, Oleksii Kurochko wrote:
>>> relocate_fdt() relocates 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.
>>>
>>> To implement that the following things are introduced as they are called
>>> by internals of xmalloc_bytes() which is used in relocate_fdt():
>>> 1. As RISC-V may have non-coherent access for RAM ( f.e., in case
>>>     of non-coherent IO devices ) flush_page_to_ram() is implemented
>>>     to ensure that cache and RAM are consistent for such platforms.
>> This is a detail of the page allocator, yes. It can then be viewed as also
>> a detail of xmalloc() et al, but I consider the wording a little misleading.
>>
>>> 2. copy_from_paddr() to copy FDT from a physical address to allocated
>>>     by xmalloc_bytes() in Xen heap.
>> This doesn't look to be related to the internals of xmalloc() et al.
>>
>>> 3. virt_to_page() to convert virtual address to page. Also introduce
>>>     directmap_virt_end to check that VA argument of virt_to_page() is
>>>     inside directmap region.
>> This is a need of free_xenheap_pages(), yes; see remark on point 1.
> 
> Actually I faced the usage of virt_to_page() in xmalloc_whole_page():
> ```
>    static void *xmalloc_whole_pages(unsigned long size, unsigned long align)
>    {
>      ...
>      PFN_ORDER(virt_to_page(res)) = PFN_UP(size);
>      /* Check that there was no truncation: */
>      ASSERT(PFN_ORDER(virt_to_page(res)) == PFN_UP(size));
> 
>      return res;
>    }
> ```
> which is called from xmalloc().
> 
> Do we need a second paragraph of the commit message at all? Or it is just obvious if
> flush_page_to_ram(), virt_to_page() and copy_from_paddr() are introduces that they are needed for
> relocate_fdt()?
> 
> Or perhaps rephrasing in the following way would be enough?
> ```
> For internal use of|xmalloc|, the functions|flush_page_to_ram()| and|virt_to_page()| are introduced.
> |virt_to_page()| is also required for|free_xenheap_pages()|. These additions are used to support
> |xmalloc|, which is utilized within|relocate_fdt()|. Additionally,|copy_from_paddr()| is introduced
> for use in|relocate_fdt()|.
> ```

I think that would do.

>> Also recall my comment on one of your earlier series, regarding inclusive
>> vs exclusive ranges. Can that please be sorted properly as a prereq, to
>> avoid extending the inconsistency?
> 
> Yes, I remember that and at the moment everything ( DIRECTMAP_VIRT_END, FRAMETABLE_VIRT_END )
> is following "inclusive" way. Considering that you remind me that could you please tell me more time
> what I am missing?

First the table azt the top of config.h uses all exclusive upper bounds.
And then DIRECTMAP_SIZE's definition assumes DIRECTMAP_SLOT_END would be
exclusive, when it's inclusive.

>>> +        set_fixmap(FIX_MISC, maddr_to_mfn(paddr), PAGE_HYPERVISOR_RW);
>>> +        memcpy(dst, src + s, l);
>>> +        clean_dcache_va_range(dst, l);
>> Why is this necessary here? You're copying to plain RAM that Xen alone
>> is using.
> 
> It is Arm specific:
> ```
> commit c60209d77e2c02de110ca0fdaa2582ef4e53d8fd
> Author: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
> Date:   Mon Jan 21 12:40:31 2013 +0000
> 
>      xen/arm: flush dcache after memcpy'ing the kernel image
>      
>      After memcpy'ing the kernel in guest memory we need to flush the dcache
>      to make sure that the data actually reaches the memory before we start
>      executing guest code with caches disabled.
>      
>      copy_from_paddr is the function that does the copy, so add a
>      flush_xen_dcache_va_range there.
> ```
> I wanted to put copy_from_paddr() to some common place at the end but in RISC-V cache is always enabled
> ( I don't see an instruction in the spec for disable/enable cache ) so this issue isn't present for RISC-V
> and clean_dcache_va_range() should/could be dropped.

That plus there's no kernel in sight just yet.

Jan

On 12/10/24 5:20 PM, Jan Beulich wrote:
>>> Also recall my comment on one of your earlier series, regarding inclusive
>>> vs exclusive ranges. Can that please be sorted properly as a prereq, to
>>> avoid extending the inconsistency?
>> Yes, I remember that and at the moment everything ( DIRECTMAP_VIRT_END, FRAMETABLE_VIRT_END )
>> is following "inclusive" way. Considering that you remind me that could you please tell me more time
>> what I am missing?
> First the table azt the top of config.h uses all exclusive upper bounds.
> And then DIRECTMAP_SIZE's definition assumes DIRECTMAP_SLOT_END would be
> exclusive, when it's inclusive.

Really missed to update the tale on the top of config.h.

But it seems to me like any *_SIZE will be defined in exclusive way by its nature, doesn't it?
For example, size of directmap is (509-200+1)<<30 = 0x7F80000000 and it is not really (
0x7F80000000 - 1 ) = 7F7FFFFFFF.

I prefer to have DIRECTMAP_{SIZE,VIRT_END} defined as now:
   #define DIRECTMAP_SIZE          (SLOTN(DIRECTMAP_SLOT_END + 1) - SLOTN(DIRECTMAP_SLOT_START))
   #define DIRECTMAP_VIRT_END      (DIRECTMAP_VIRT_START + DIRECTMAP_SIZE - 1)
( of course with making upper bounds inclusive in the table on the top of config.h )

>
>>>> +        set_fixmap(FIX_MISC, maddr_to_mfn(paddr), PAGE_HYPERVISOR_RW);
>>>> +        memcpy(dst, src + s, l);
>>>> +        clean_dcache_va_range(dst, l);
>>> Why is this necessary here? You're copying to plain RAM that Xen alone
>>> is using.
>> It is Arm specific:
>> ```
>> commit c60209d77e2c02de110ca0fdaa2582ef4e53d8fd
>> Author: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>> Date:   Mon Jan 21 12:40:31 2013 +0000
>>
>>       xen/arm: flush dcache after memcpy'ing the kernel image
>>       
>>       After memcpy'ing the kernel in guest memory we need to flush the dcache
>>       to make sure that the data actually reaches the memory before we start
>>       executing guest code with caches disabled.
>>       
>>       copy_from_paddr is the function that does the copy, so add a
>>       flush_xen_dcache_va_range there.
>> ```
>> I wanted to put copy_from_paddr() to some common place at the end but in RISC-V cache is always enabled
>> ( I don't see an instruction in the spec for disable/enable cache ) so this issue isn't present for RISC-V
>> and clean_dcache_va_range() should/could be dropped.
> That plus there's no kernel in sight just yet.

( clarification ) will it change something if kernel will be loaded now? It seems even if we are copying kernel in guest
memory we still don't need to flush the dcache as cache is enabled and cache coherence protocol will do a work automatically.

~ Oleksii

On 11.12.2024 11:26, Oleksii Kurochko wrote:
> On 12/10/24 5:20 PM, Jan Beulich wrote:
>>>> Also recall my comment on one of your earlier series, regarding inclusive
>>>> vs exclusive ranges. Can that please be sorted properly as a prereq, to
>>>> avoid extending the inconsistency?
>>> Yes, I remember that and at the moment everything ( DIRECTMAP_VIRT_END, FRAMETABLE_VIRT_END )
>>> is following "inclusive" way. Considering that you remind me that could you please tell me more time
>>> what I am missing?
>> First the table azt the top of config.h uses all exclusive upper bounds.
>> And then DIRECTMAP_SIZE's definition assumes DIRECTMAP_SLOT_END would be
>> exclusive, when it's inclusive.
> 
> Really missed to update the tale on the top of config.h.
> 
> But it seems to me like any *_SIZE will be defined in exclusive way by its nature, doesn't it?

Of course. I'm not even sure "size" can be reasonably qualified as "exclusive" or
"inclusive".

> For example, size of directmap is (509-200+1)<<30 = 0x7F80000000 and it is not really (
> 0x7F80000000 - 1 ) = 7F7FFFFFFF.
> 
> I prefer to have DIRECTMAP_{SIZE,VIRT_END} defined as now:
>    #define DIRECTMAP_SIZE          (SLOTN(DIRECTMAP_SLOT_END + 1) - SLOTN(DIRECTMAP_SLOT_START))
>    #define DIRECTMAP_VIRT_END      (DIRECTMAP_VIRT_START + DIRECTMAP_SIZE - 1)
> ( of course with making upper bounds inclusive in the table on the top of config.h )

Right.

>>>>> +        set_fixmap(FIX_MISC, maddr_to_mfn(paddr), PAGE_HYPERVISOR_RW);
>>>>> +        memcpy(dst, src + s, l);
>>>>> +        clean_dcache_va_range(dst, l);
>>>> Why is this necessary here? You're copying to plain RAM that Xen alone
>>>> is using.
>>> It is Arm specific:
>>> ```
>>> commit c60209d77e2c02de110ca0fdaa2582ef4e53d8fd
>>> Author: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>>> Date:   Mon Jan 21 12:40:31 2013 +0000
>>>
>>>       xen/arm: flush dcache after memcpy'ing the kernel image
>>>       
>>>       After memcpy'ing the kernel in guest memory we need to flush the dcache
>>>       to make sure that the data actually reaches the memory before we start
>>>       executing guest code with caches disabled.
>>>       
>>>       copy_from_paddr is the function that does the copy, so add a
>>>       flush_xen_dcache_va_range there.
>>> ```
>>> I wanted to put copy_from_paddr() to some common place at the end but in RISC-V cache is always enabled
>>> ( I don't see an instruction in the spec for disable/enable cache ) so this issue isn't present for RISC-V
>>> and clean_dcache_va_range() should/could be dropped.
>> That plus there's no kernel in sight just yet.
> 
> ( clarification ) will it change something if kernel will be loaded now? It seems even if we are copying kernel in guest
> memory we still don't need to flush the dcache as cache is enabled and cache coherence protocol will do a work automatically.

Correct. My point merely was that there are two reasons this isn't needed, each
of which is by itself sufficient to justify omitting that call.

Jan