[PATCH v4 00/11] RISCV: enable DOMAIN_BUILD_HELPERS

[PATCH v4 00/11] RISCV: enable DOMAIN_BUILD_HELPERS

[Oleksii Kurochko]

Introduce necessary things to enable DOMAIN_BUILD_HELPERS config for RISC-V.

This patch series is rebased on top of [PATCH v3 0/3] dom0less: various updates
as it could be some merge conflicts depends on which patch series will go first.

CI: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2477740734

---
Changes in v4:
 - Move patch up, closer to other common/Arm related patches:
   - xen/dom0less: rename kernel_zimage_probe() to  kernel_image_probe()
   - xen: move declaration of fw_unreserved_regions() to common header
   - xen: move domain_use_host_layout() to common header
   - xen: rename p2m_ipa_bits to p2m_gpa_bits

 - Merged to upstream/staging:
     xen/riscv: implement get_page_from_gfn()

 - Rebase this patch series on top of [PATCH v3 0/3] dom0less: various updates. 
 - Address the comments from ML.
---
Changes in v3:
 - Address the comments from ML.
---
Changes in v2:
 - Address the comments from ML.
 - Introduce some new patches to make dom0less solution more architecture
   indepenent from terminology point of view.
 - Minor fixes.
---

Oleksii Kurochko (11):
  xen: arm: fix len type for guest copy functions
  xen/dom0less: rename kernel_zimage_probe() to kernel_image_probe()
  xen: move declaration of fw_unreserved_regions() to common header
  xen: introduce domain-layout.h with common domain_use_host_layout()
  xen: rename p2m_ipa_bits to p2m_gpa_bits
  xen/riscv: implement copy_to_guest_phys()
  xen/riscv: add Linux kernel loading support
  xen/riscv: rework G-stage mode handling
  xen/riscv: introduce p2m_gpa_bits
  xen/riscv: add definition of guest RAM banks
  xen/riscv: enable DOMAIN_BUILD_HELPERS

 xen/arch/arm/domain_build.c               |   1 +
 xen/arch/arm/guestcopy.c                  |   6 +-
 xen/arch/arm/include/asm/domain.h         |  14 --
 xen/arch/arm/include/asm/guest_access.h   |   2 +-
 xen/arch/arm/include/asm/p2m.h            |   2 +
 xen/arch/arm/include/asm/setup.h          |   3 -
 xen/arch/arm/kernel.c                     |  48 ++---
 xen/arch/arm/vgic-v3.c                    |   1 +
 xen/arch/riscv/Kconfig                    |   1 +
 xen/arch/riscv/Makefile                   |   3 +
 xen/arch/riscv/dom0less-build.c           |  70 +++++++
 xen/arch/riscv/guestcopy.c                | 116 +++++++++++
 xen/arch/riscv/include/asm/config.h       |  13 ++
 xen/arch/riscv/include/asm/domain.h       |   1 +
 xen/arch/riscv/include/asm/guest-layout.h |  23 ++
 xen/arch/riscv/include/asm/guest_access.h |   7 +
 xen/arch/riscv/include/asm/p2m.h          |  21 +-
 xen/arch/riscv/kernel.c                   | 242 ++++++++++++++++++++++
 xen/arch/riscv/p2m.c                      | 133 ++++++++----
 xen/arch/riscv/vmid.c                     |   2 +-
 xen/common/device-tree/domain-build.c     |   3 +-
 xen/common/device-tree/kernel.c           |   2 +-
 xen/include/public/arch-riscv.h           |   5 +
 xen/include/xen/bootinfo.h                |   4 +
 xen/include/xen/domain-layout.h           |  27 +++
 xen/include/xen/fdt-domain-build.h        |   2 +-
 xen/include/xen/fdt-kernel.h              |  15 +-
 27 files changed, 670 insertions(+), 97 deletions(-)
 create mode 100644 xen/arch/riscv/dom0less-build.c
 create mode 100644 xen/arch/riscv/guestcopy.c
 create mode 100644 xen/arch/riscv/include/asm/guest-layout.h
 create mode 100644 xen/arch/riscv/kernel.c
 create mode 100644 xen/include/xen/domain-layout.h

-- 
2.53.0

[PATCH v4 01/11] xen: arm: fix len type for guest copy functions

[Oleksii Kurochko]

Widen the len argument of copy_to_guest_phys_flush_dcache() and the
copy_to_guest_phys_cb typedef from unsigned int to unsigned long, as
the function can be used to copy large blobs such as the initrd which
may exceed 4GB. Update the internal copy_guest() len argument to
unsigned long accordingly.

Change the type for local variable size in copy_guest() to avoid
compilation error because of type mismatch.

raw_* wrappers above copy_guest() keep returning unsigned long to
avoid type narrowing; it is not an issue for raw_*'s len argument
to remain 'unsigned int' since the assignment to copy_guest()'s wider
unsigned long parameter is safe and there is no raw_* users who
are using a value bigger than what can fit into 'unsigned int'.

Fixes: 2986481b3d9e6 ("xen/arm: guest_copy: Extend the prototype to pass the vCPU")
Fixes: 5302bd490bea7 ("xen/arm: Introduce copy_to_guest_phys_flush_dcache")
Fixes: d07b7369aa65b ("xen/common: dom0less: introduce common domain-build.c")
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v4:
- Add Fixes: tags.
- Add "arm:" prefix to commit subject.
- Add Reviewed-by: Jan Beulich <jbeulich@suse.com>.
- Avoid using of min_t.
---
Changes in v3:
 - Correct the declaration of copy_to_guest_phys_cb() instead of return
   'unsigned int' to be in sync with len argument, the type of len argument
   is changed on 'unsigned long' as initrd could be pretty big and also its
   size is stroed in 'paddr_t' which is 'unsigned long'.
 - Update copy_guest() prototype to avoid trancation bug for len argument.
 - Revert prototype changes for raw_* wrappers above copy_guest() as they
   should keep returning unsigned long to avoid type narrowing; it is not
   an issue for raw_*'s len argument to remain 'unsigned int' since the
   assignment to copy_guest()'s wider unsigned long parameter is safe.
 - Change the type for local variable size in copy_guest() to avoid
   compilation error because of type mismatch.
 - Add Reported-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v2:
 - New patch.
---
---
 xen/arch/arm/guestcopy.c                | 6 +++---
 xen/arch/arm/include/asm/guest_access.h | 2 +-
 xen/include/xen/fdt-domain-build.h      | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index fdb06422b8e9..86f1c9d0e318 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -53,7 +53,7 @@ static struct page_info *translate_get_page(copy_info_t info, uint64_t addr,
     return page;
 }
 
-static unsigned long copy_guest(void *buf, uint64_t addr, unsigned int len,
+static unsigned long copy_guest(void *buf, uint64_t addr, unsigned long len,
                                 copy_info_t info, unsigned int flags)
 {
     /* XXX needs to handle faults */
@@ -65,7 +65,7 @@ static unsigned long copy_guest(void *buf, uint64_t addr, unsigned int len,
     while ( len )
     {
         void *p;
-        unsigned int size = min(len, (unsigned int)PAGE_SIZE - offset);
+        unsigned long size = min(len, PAGE_SIZE + 0UL - offset);
         struct page_info *page;
 
         page = translate_get_page(info, addr, flags & COPY_linear,
@@ -136,7 +136,7 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from,
 unsigned long copy_to_guest_phys_flush_dcache(struct domain *d,
                                               paddr_t gpa,
                                               void *buf,
-                                              unsigned int len)
+                                              unsigned long len)
 {
     return copy_guest(buf, gpa, len, GPA_INFO(d),
                       COPY_to_guest | COPY_ipa | COPY_flush_dcache);
diff --git a/xen/arch/arm/include/asm/guest_access.h b/xen/arch/arm/include/asm/guest_access.h
index 18c88b70d7ec..c13cbec55b65 100644
--- a/xen/arch/arm/include/asm/guest_access.h
+++ b/xen/arch/arm/include/asm/guest_access.h
@@ -14,7 +14,7 @@ unsigned long raw_clear_guest(void *to, unsigned int len);
 unsigned long copy_to_guest_phys_flush_dcache(struct domain *d,
                                               paddr_t gpa,
                                               void *buf,
-                                              unsigned int len);
+                                              unsigned long len);
 
 int access_guest_memory_by_gpa(struct domain *d, paddr_t gpa, void *buf,
                                uint32_t size, bool is_write);
diff --git a/xen/include/xen/fdt-domain-build.h b/xen/include/xen/fdt-domain-build.h
index bc7744270c8f..6ad9e8fd1642 100644
--- a/xen/include/xen/fdt-domain-build.h
+++ b/xen/include/xen/fdt-domain-build.h
@@ -48,7 +48,7 @@ static inline int get_allocation_size(paddr_t size)
 typedef unsigned long (*copy_to_guest_phys_cb)(struct domain *d,
                                                paddr_t gpa,
                                                void *buf,
-                                               unsigned int len);
+                                               unsigned long len);
 
 void initrd_load(struct kernel_info *kinfo,
                  copy_to_guest_phys_cb cb);
-- 
2.53.0

[PATCH v4 02/11] xen/dom0less: rename kernel_zimage_probe() to kernel_image_probe()

[Oleksii Kurochko]

The helper kernel_zimage_probe() is referenced from common code
(xen/common/device-tree/kernel.c), but its name is tied to the zImage
format which is specific to Arm (from architectures supported by Xen).

Other architectures supported by Xen, such as RISC-V, do not use the
zImage format and instead rely on other kernel image types (e.g. Image
or compressed Image variants: Image.gz, etc). Using "zimage" in the
name is therefore misleading in architecture-independent code.

Rename kernel_zimage_probe() to kernel_image_probe() and update the
associated structure field from "zimage" to "image" to reflect that the
code handles generic kernel images rather than the zImage format
specifically.

No functional change intended.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v3-4:
 - Nothing changed. Only rebase.
---
Changes in v2:
 - new patch.
---
---
 xen/arch/arm/kernel.c           | 48 ++++++++++++++++-----------------
 xen/common/device-tree/kernel.c |  2 +-
 xen/include/xen/fdt-kernel.h    |  4 +--
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 9395b5af8745..a5554714cd7b 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -101,8 +101,8 @@ static paddr_t __init kernel_zimage_place(struct kernel_info *info)
     paddr_t load_addr;
 
 #ifdef CONFIG_HAS_DOMAIN_TYPE
-    if ( (info->type == DOMAIN_64BIT) && (info->zimage.start == 0) )
-        return mem->bank[0].start + info->zimage.text_offset;
+    if ( (info->type == DOMAIN_64BIT) && (info->image.start == 0) )
+        return mem->bank[0].start + info->image.text_offset;
 #endif
 
     /*
@@ -111,19 +111,19 @@ static paddr_t __init kernel_zimage_place(struct kernel_info *info)
      * and above 32MiB. Load it as high as possible within these
      * constraints, while also avoiding the DTB.
      */
-    if ( info->zimage.start == 0 )
+    if ( info->image.start == 0 )
     {
         paddr_t load_end;
 
         load_end = mem->bank[0].start + mem->bank[0].size;
         load_end = MIN(mem->bank[0].start + MB(128), load_end);
 
-        load_addr = load_end - info->zimage.len;
+        load_addr = load_end - info->image.len;
         /* Align to 2MB */
         load_addr &= ~((2 << 20) - 1);
     }
     else
-        load_addr = info->zimage.start;
+        load_addr = info->image.start;
 
     return load_addr;
 }
@@ -131,8 +131,8 @@ static paddr_t __init kernel_zimage_place(struct kernel_info *info)
 static void __init kernel_zimage_load(struct kernel_info *info)
 {
     paddr_t load_addr = kernel_zimage_place(info);
-    paddr_t paddr = info->zimage.kernel_addr;
-    paddr_t len = info->zimage.len;
+    paddr_t paddr = info->image.kernel_addr;
+    paddr_t len = info->image.len;
     void *kernel;
     int rc;
 
@@ -215,7 +215,7 @@ int __init kernel_uimage_probe(struct kernel_info *info,
         return -EOPNOTSUPP;
     }
 
-    info->zimage.start = be32_to_cpu(uimage.load);
+    info->image.start = be32_to_cpu(uimage.load);
     info->entry = be32_to_cpu(uimage.ep);
 
     /*
@@ -224,20 +224,20 @@ int __init kernel_uimage_probe(struct kernel_info *info,
      * independent image. That means Xen is free to load such an image at
      * any valid address.
      */
-    if ( info->zimage.start == 0 )
+    if ( info->image.start == 0 )
         printk(XENLOG_INFO
                "No load address provided. Xen will decide where to load it.\n");
     else
         printk(XENLOG_INFO
                "Provided load address: %"PRIpaddr" and entry address: %"PRIpaddr"\n",
-               info->zimage.start, info->entry);
+               info->image.start, info->entry);
 
     /*
      * If the image supports position independent execution, then user cannot
      * provide an entry point as Xen will load such an image at any appropriate
      * memory address. Thus, we need to return error.
      */
-    if ( (info->zimage.start == 0) && (info->entry != 0) )
+    if ( (info->image.start == 0) && (info->entry != 0) )
     {
         printk(XENLOG_ERR
                "Entry point cannot be non zero for PIE image.\n");
@@ -257,13 +257,13 @@ int __init kernel_uimage_probe(struct kernel_info *info,
         if ( rc )
             return rc;
 
-        info->zimage.kernel_addr = mod->start;
-        info->zimage.len = mod->size;
+        info->image.kernel_addr = mod->start;
+        info->image.len = mod->size;
     }
     else
     {
-        info->zimage.kernel_addr = addr + sizeof(uimage);
-        info->zimage.len = len;
+        info->image.kernel_addr = addr + sizeof(uimage);
+        info->image.len = len;
     }
 
     info->load = kernel_zimage_load;
@@ -289,7 +289,7 @@ int __init kernel_uimage_probe(struct kernel_info *info,
      * Thus, Xen uses uimage.load attribute to determine the load address and
      * zimage.text_offset is ignored.
      */
-    info->zimage.text_offset = 0;
+    info->image.text_offset = 0;
 #endif
 
     return 0;
@@ -338,10 +338,10 @@ static int __init kernel_zimage64_probe(struct kernel_info *info,
     if ( (end - start) > size )
         return -EINVAL;
 
-    info->zimage.kernel_addr = addr;
-    info->zimage.len = end - start;
-    info->zimage.text_offset = zimage.text_offset;
-    info->zimage.start = 0;
+    info->image.kernel_addr = addr;
+    info->image.len = end - start;
+    info->image.text_offset = zimage.text_offset;
+    info->image.start = 0;
 
     info->load = kernel_zimage_load;
 
@@ -389,10 +389,10 @@ static int __init kernel_zimage32_probe(struct kernel_info *info,
         }
     }
 
-    info->zimage.kernel_addr = addr;
+    info->image.kernel_addr = addr;
 
-    info->zimage.start = start;
-    info->zimage.len = end - start;
+    info->image.start = start;
+    info->image.len = end - start;
 
     info->load = kernel_zimage_load;
 
@@ -403,7 +403,7 @@ static int __init kernel_zimage32_probe(struct kernel_info *info,
     return 0;
 }
 
-int __init kernel_zimage_probe(struct kernel_info *info, paddr_t addr,
+int __init kernel_image_probe(struct kernel_info *info, paddr_t addr,
                                paddr_t size)
 {
     int rc;
diff --git a/xen/common/device-tree/kernel.c b/xen/common/device-tree/kernel.c
index 28096121a52d..cfa27464f0fc 100644
--- a/xen/common/device-tree/kernel.c
+++ b/xen/common/device-tree/kernel.c
@@ -235,7 +235,7 @@ int __init kernel_probe(struct kernel_info *info,
     if ( rc && rc != -EINVAL )
         return rc;
 
-    rc = kernel_zimage_probe(info, mod->start, mod->size);
+    rc = kernel_image_probe(info, mod->start, mod->size);
 
     return rc;
 }
diff --git a/xen/include/xen/fdt-kernel.h b/xen/include/xen/fdt-kernel.h
index 86a37a13048b..54be77aa191e 100644
--- a/xen/include/xen/fdt-kernel.h
+++ b/xen/include/xen/fdt-kernel.h
@@ -63,7 +63,7 @@ struct kernel_info {
             paddr_t text_offset; /* 64-bit Image only */
 #endif
             paddr_t start; /* Must be 0 for 64-bit Image */
-        } zimage;
+        } image;
     };
 
 #ifdef CONFIG_HAS_DOMAIN_TYPE
@@ -133,7 +133,7 @@ void kernel_load(struct kernel_info *info);
 
 int kernel_decompress(struct boot_module *mod, uint32_t offset);
 
-int kernel_zimage_probe(struct kernel_info *info, paddr_t addr, paddr_t size);
+int kernel_image_probe(struct kernel_info *info, paddr_t addr, paddr_t size);
 
 /*
  * uImage isn't really used nowadays thereby leave kernel_uimage_probe()
-- 
2.53.0

[PATCH v4 03/11] xen: move declaration of fw_unreserved_regions() to common header

[Oleksii Kurochko]

Since the implementation of fw_unreserved_regions() is in common code, move
its declaration to xen/bootinfo.h.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v2-v4:
 - Nothing changed. Only rebase.
---
---
 xen/arch/arm/include/asm/setup.h | 3 ---
 xen/include/xen/bootinfo.h       | 4 ++++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
index 899e33925ca4..0d29b46ea52b 100644
--- a/xen/arch/arm/include/asm/setup.h
+++ b/xen/arch/arm/include/asm/setup.h
@@ -43,9 +43,6 @@ int acpi_make_efi_nodes(void *fdt, struct membank tbl_add[]);
 void create_dom0(void);
 
 void discard_initial_modules(void);
-void fw_unreserved_regions(paddr_t s, paddr_t e,
-                           void (*cb)(paddr_t ps, paddr_t pe),
-                           unsigned int first);
 
 void init_pdx(void);
 void setup_mm(void);
diff --git a/xen/include/xen/bootinfo.h b/xen/include/xen/bootinfo.h
index f834f1957155..dbf492c2e36e 100644
--- a/xen/include/xen/bootinfo.h
+++ b/xen/include/xen/bootinfo.h
@@ -210,4 +210,8 @@ static inline struct membanks *membanks_xzalloc(unsigned int nr,
     return banks;
 }
 
+void fw_unreserved_regions(paddr_t s, paddr_t e,
+                           void (*cb)(paddr_t ps, paddr_t pe),
+                           unsigned int first);
+
 #endif /* XEN_BOOTINFO_H */
-- 
2.53.0

[PATCH v4 04/11] xen: introduce domain-layout.h with common domain_use_host_layout()

[Oleksii Kurochko]

domain_use_host_layout() is not architecture-specific and may be needed
on x86 [1]. Replace the ARM-specific macro in asm/domain.h with a common
static inline in a new dedicated header, xen/domain-layout.h.

xen/domain.h would be the natural home, but placing it there would
require including xen/paging.h (for paging_mode_translate()) and
xen/sched.h (for is_hardware_domain()), which would introduce circular
dependencies. A separate header that callers opt into avoids this.

Adjust the implementation to take paging_mode_translate() into account
so it works correctly for all architectures, including x86. Some extra
details about implementation [2] and [3].

[1] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602161038120.359097@ubuntu-linux-20-04-desktop/
[2] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602271742400.3148344@ubuntu-linux-20-04-desktop/
[3] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602271750190.3148344@ubuntu-linux-20-04-desktop/

Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v4:
 - Update the comment above domain_use_host_layout().
---
Changes in v3:
 - Make argument of domain_use_host_layout() const.
 - Create a separate header to avoid circular heder dependecy and making
   domain_use_host_layour() as static inline.
 - Rework domain_use_host_layout() to be protected by paging_mode_translate().
 - Update the commit message.
---
Changes in v2:
 - Drop ifdef around defintion of domain_use_host_layout() as it
   was suggested generic version. It could be returned back when
   the real use case for it will appear.
 - Add Suggested-by: and update the commit message.
 - Make domain_use_host_layout() function instead of macros to
   avoid ciclular header dependecies. Look at more details in
   the commit message.
---
---
 xen/arch/arm/domain_build.c           |  1 +
 xen/arch/arm/include/asm/domain.h     | 14 --------------
 xen/arch/arm/vgic-v3.c                |  1 +
 xen/common/device-tree/domain-build.c |  1 +
 xen/include/xen/domain-layout.h       | 27 +++++++++++++++++++++++++++
 5 files changed, 30 insertions(+), 14 deletions(-)
 create mode 100644 xen/include/xen/domain-layout.h

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index ad665cd3c045..1efddc60ef0a 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2,6 +2,7 @@
 #include <xen/init.h>
 #include <xen/bootinfo.h>
 #include <xen/compile.h>
+#include <xen/domain-layout.h>
 #include <xen/dom0less-build.h>
 #include <xen/fdt-domain-build.h>
 #include <xen/fdt-kernel.h>
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index b24f02d269be..46a5cdc0c800 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -18,20 +18,6 @@ struct hvm_domain
     uint64_t              params[HVM_NR_PARAMS];
 };
 
-/*
- * Is the domain using the host memory layout?
- *
- * Direct-mapped domain will always have the RAM mapped with GFN == MFN.
- * To avoid any trouble finding space, it is easier to force using the
- * host memory layout.
- *
- * The hardware domain will use the host layout regardless of
- * direct-mapped because some OS may rely on a specific address ranges
- * for the devices.
- */
-#define domain_use_host_layout(d) (is_domain_direct_mapped(d) || \
-                                   is_hardware_domain(d))
-
 struct vtimer {
     struct vcpu *v;
     int irq;
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 77aab5c3c293..77517c303061 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -10,6 +10,7 @@
  */
 
 #include <xen/bitops.h>
+#include <xen/domain-layout.h>
 #include <xen/init.h>
 #include <xen/irq.h>
 #include <xen/lib.h>
diff --git a/xen/common/device-tree/domain-build.c b/xen/common/device-tree/domain-build.c
index c51520ebadf9..6949203dacdc 100644
--- a/xen/common/device-tree/domain-build.c
+++ b/xen/common/device-tree/domain-build.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
 #include <xen/bootinfo.h>
+#include <xen/domain-layout.h>
 #include <xen/fdt-domain-build.h>
 #include <xen/init.h>
 #include <xen/lib.h>
diff --git a/xen/include/xen/domain-layout.h b/xen/include/xen/domain-layout.h
new file mode 100644
index 000000000000..0532a27b44ce
--- /dev/null
+++ b/xen/include/xen/domain-layout.h
@@ -0,0 +1,27 @@
+#ifndef __XEN_DOMAIN_LAYOUT_H__
+#define __XEN_DOMAIN_LAYOUT_H__
+
+#include <xen/domain.h>
+#include <xen/paging.h>
+#include <xen/sched.h>
+
+/*
+ * Is a domain using the host memory layout?
+ *
+ * domain_use_host_layout() is always False for PV domains (including Dom0).
+ *
+ * Direct-mapped domains (autotranslated domains with memory allocated
+ * contiguously and mapped 1:1 so that GFN == MFN) must use the host
+ * memory layout since GFN == MFN by definition.
+ *
+ * The hardware domain will use the host layout (regardless of
+ * direct-mapped) because some OS may rely on specific address ranges
+ * for the devices.
+ */
+static inline bool domain_use_host_layout(const struct domain *d)
+{
+    return paging_mode_translate(d) &&
+           (is_domain_direct_mapped(d) || is_hardware_domain(d));
+}
+
+#endif /* __XEN_DOMAIN_LAYOUT_H__ */
-- 
2.53.0

[PATCH v4 05/11] xen: rename p2m_ipa_bits to p2m_gpa_bits

[Oleksii Kurochko]

The IPA terminology is Arm-specific, so rename p2m_ipa_bits to
p2m_gpa_bits to use architecture-neutral naming in
xen/common/device-tree/ code.

No functional changes.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v4:
 - Nothing changed only rebase.
---
Changes in v3:
 - Introduce #define p2m_gpa_bits p2m_ipa_bits for Arm instead of
   renaming of p2m_ipa_bits to p2m_gpa_bits to keep Arm part of
   changes clearer and keep using Arm-specific terminolgy inside
   Arm code.
---
Changes in v2:
 - New patch.
---
---
 xen/arch/arm/include/asm/p2m.h        | 2 ++
 xen/common/device-tree/domain-build.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/include/asm/p2m.h b/xen/arch/arm/include/asm/p2m.h
index 010ce8c9ebbd..7957dbd96e57 100644
--- a/xen/arch/arm/include/asm/p2m.h
+++ b/xen/arch/arm/include/asm/p2m.h
@@ -14,6 +14,8 @@
 /* Holds the bit size of IPAs in p2m tables.  */
 extern unsigned int p2m_ipa_bits;
 
+#define p2m_gpa_bits p2m_ipa_bits
+
 #define MAX_VMID_8_BIT  (1UL << 8)
 #define MAX_VMID_16_BIT (1UL << 16)
 
diff --git a/xen/common/device-tree/domain-build.c b/xen/common/device-tree/domain-build.c
index 6949203dacdc..2a760b007b57 100644
--- a/xen/common/device-tree/domain-build.c
+++ b/xen/common/device-tree/domain-build.c
@@ -221,7 +221,7 @@ int __init find_unallocated_memory(const struct kernel_info *kinfo,
     }
 
     start = 0;
-    end = (1ULL << p2m_ipa_bits) - 1;
+    end = (1ULL << p2m_gpa_bits) - 1;
     res = rangeset_report_ranges(unalloc_mem, PFN_DOWN(start), PFN_DOWN(end),
                                  cb, free_regions);
     if ( res )
-- 
2.53.0

[PATCH v4 06/11] xen/riscv: implement copy_to_guest_phys()

[Oleksii Kurochko]

Introduce copy_to_guest_phys() for RISC-V, based on the Arm implementation.

Add a generic copy_guest() helper for copying to and from guest physical
(and potentially virtual addresses in the future), and implement
translate_get_page() to translate a guest physical address into a struct
page_info via the domain p2m.

Compared to the Arm code:
- Drop COPY_flush_dcache(), as no such use cases exist on RISC-V.
- Do not implement the linear mapping case, which is currently unused.
- Use PAGE_OFFSET() to initialize the local offset variable in copy_guest().

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v4:
 - Avoid using of min_t. (sync with Arm)
 - Add Acked-by: Jan Beulich <jbeulich@suse.com>.
---
Changes in v3:
 - s/if (page == NULL)/if ( !page ).
 - Drop " - offset" for unmap_domain_page() inside copy_guest() function.
   unmap_domain_page() is expected to mask (or something similar) the page
   offset bits.
 - Change some types for functions as copy_to_guest_phys_cb function pointer
   prototype was changed too in the prev. commit.
---
Changes in v2:
 - Use BIT() instead of open-coding.
 - Rename COPY_ipa to COPY_gpa.
 - Rename COPY_linear to COPY_gva.
 - Use  BUG_ON(linear) instead if (lineer) + BUG_ON.
 - Rename arg liner to gva for translate_get_page().
 - Update translate_get_page() to properly handling write argument.
 - Return unsigned int for copy_guest() and copy_to_guest_phys() as
   len function parameter is only 'unsigned int'.
 - Reformat function arguments for alignment
---
---
 xen/arch/riscv/Makefile                   |   1 +
 xen/arch/riscv/guestcopy.c                | 116 ++++++++++++++++++++++
 xen/arch/riscv/include/asm/guest_access.h |   7 ++
 3 files changed, 124 insertions(+)
 create mode 100644 xen/arch/riscv/guestcopy.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 04f02ad89cba..cfc3fdf7d208 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -4,6 +4,7 @@ obj-y += domain.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 obj-y += entry.o
 obj-y += extable.o
+obj-y += guestcopy.o
 obj-y += imsic.o
 obj-y += intc.o
 obj-y += irq.o
diff --git a/xen/arch/riscv/guestcopy.c b/xen/arch/riscv/guestcopy.c
new file mode 100644
index 000000000000..8a89212e0bea
--- /dev/null
+++ b/xen/arch/riscv/guestcopy.c
@@ -0,0 +1,116 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/domain_page.h>
+#include <xen/page-size.h>
+#include <xen/sched.h>
+#include <xen/string.h>
+
+#include <asm/guest_access.h>
+
+#define COPY_from_guest     0U
+#define COPY_to_guest       BIT(0, U)
+#define COPY_gpa            0U
+#define COPY_gva            BIT(1, U)
+
+typedef union
+{
+    struct
+    {
+        struct vcpu *v;
+    } gva;
+
+    struct
+    {
+        struct domain *d;
+    } gpa;
+} copy_info_t;
+
+#define GVA_INFO(vcpu) ((copy_info_t) { .gva = { vcpu } })
+#define GPA_INFO(domain) ((copy_info_t) { .gpa = { domain } })
+
+static struct page_info *translate_get_page(copy_info_t info, uint64_t addr,
+                                            bool gva, bool write)
+{
+    p2m_type_t p2mt;
+    struct page_info *page;
+
+    /*
+     * Not implemented yet.
+     *
+     * If gva == true, the operation will likely require a struct vcpu
+     * rather than just a struct domain. For this reason copy_info_t is
+     * already passed here instead of only struct domain.
+     */
+    BUG_ON(gva);
+
+    page = get_page_from_gfn(info.gpa.d, paddr_to_pfn(addr), &p2mt, P2M_ALLOC);
+
+    if ( !page )
+        return NULL;
+
+    if ( write ? p2mt != p2m_ram_rw : !p2m_is_ram(p2mt) )
+    {
+        put_page(page);
+        return NULL;
+    }
+
+    return page;
+}
+
+static unsigned long copy_guest(void *buf, uint64_t addr, unsigned long len,
+                                copy_info_t info, unsigned int flags)
+{
+    unsigned int offset = PAGE_OFFSET(addr);
+
+    BUILD_BUG_ON((sizeof(addr)) < sizeof(vaddr_t));
+    BUILD_BUG_ON((sizeof(addr)) < sizeof(paddr_t));
+
+    while ( len )
+    {
+        void *p;
+        unsigned long size = min(len, PAGE_SIZE + 0UL - offset);
+        struct page_info *page;
+
+        page = translate_get_page(info, addr, flags & COPY_gva,
+                                  flags & COPY_to_guest);
+        if ( !page )
+            return len;
+
+        p = __map_domain_page(page);
+        p += offset;
+        if ( flags & COPY_to_guest )
+        {
+            /*
+             * buf will be NULL when the caller request to zero the
+             * guest memory.
+             */
+            if ( buf )
+                memcpy(p, buf, size);
+            else
+                memset(p, 0, size);
+        }
+        else
+            memcpy(buf, p, size);
+
+        unmap_domain_page(p);
+        put_page(page);
+        len -= size;
+        buf += size;
+        addr += size;
+
+        /*
+         * After the first iteration, guest virtual address is correctly
+         * aligned to PAGE_SIZE.
+         */
+        offset = 0;
+    }
+
+    return 0;
+}
+
+unsigned long copy_to_guest_phys(struct domain *d, paddr_t gpa, void *buf,
+                                 unsigned long len)
+{
+    return copy_guest(buf, gpa, len, GPA_INFO(d),
+                      COPY_to_guest | COPY_gpa);
+}
diff --git a/xen/arch/riscv/include/asm/guest_access.h b/xen/arch/riscv/include/asm/guest_access.h
index 7cd51fbbdead..8d679319ded0 100644
--- a/xen/arch/riscv/include/asm/guest_access.h
+++ b/xen/arch/riscv/include/asm/guest_access.h
@@ -2,6 +2,10 @@
 #ifndef ASM__RISCV__GUEST_ACCESS_H
 #define ASM__RISCV__GUEST_ACCESS_H
 
+#include <xen/types.h>
+
+struct domain;
+
 unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len);
 unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len);
 unsigned long raw_clear_guest(void *to, unsigned int len);
@@ -18,6 +22,9 @@ unsigned long raw_clear_guest(void *to, unsigned int len);
 #define guest_handle_okay(hnd, nr) (1)
 #define guest_handle_subrange_okay(hnd, first, last) (1)
 
+unsigned long copy_to_guest_phys(struct domain *d, paddr_t gpa, void *buf,
+                                 unsigned long len);
+
 #endif /* ASM__RISCV__GUEST_ACCESS_H */
 /*
  * Local variables:
-- 
2.53.0

[PATCH v4 07/11] xen/riscv: add Linux kernel loading support

[Oleksii Kurochko]

Introduce support for loading a Linux kernel Image which is got by
uncompressing of Image.gz on RISC-V.

kernel_image_load() and place_modules() currently call panic() on
failure rather than returning an error. This is because the common
kernel_load() in common/device-tree/kernel.c does not expect a
return code. Handling errors gracefully would require a separate
refactor.

The implementation is based on the Xen Arm kernel loading code.

text_offset is available for both 32-bit and 64-bit Image, so fix
that.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v4:
 - Update the patch subject: add "... Linux kernel ...".
 - Make bi variable unsigned. And use while() instead of for().
 - Update the condition which calculates overlapping of kernel with modules.
 - Reject a malformed image before the loop to avoid wrapping
   load_addr + image_size in the per-bankcheck below.
 - Use  mathematical representation to print ranges.
 - Use #error instead of just returning unsupported.
 - Update the comment above stuct image in kernel_image64_probe().
---
---
 xen/arch/riscv/Makefile             |   1 +
 xen/arch/riscv/include/asm/config.h |  13 ++
 xen/arch/riscv/kernel.c             | 242 ++++++++++++++++++++++++++++
 xen/include/xen/fdt-kernel.h        |  11 +-
 4 files changed, 265 insertions(+), 2 deletions(-)
 create mode 100644 xen/arch/riscv/kernel.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index cfc3fdf7d208..eecdcbc76867 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -8,6 +8,7 @@ obj-y += guestcopy.o
 obj-y += imsic.o
 obj-y += intc.o
 obj-y += irq.o
+obj-y += kernel.init.o
 obj-y += mm.o
 obj-y += p2m.o
 obj-y += paging.o
diff --git a/xen/arch/riscv/include/asm/config.h b/xen/arch/riscv/include/asm/config.h
index 0613de008b13..fd69057826e1 100644
--- a/xen/arch/riscv/include/asm/config.h
+++ b/xen/arch/riscv/include/asm/config.h
@@ -151,6 +151,19 @@
 extern unsigned long phys_offset; /* = load_start - XEN_VIRT_START */
 #endif
 
+/*
+ * KERNEL_LOAD_ADDR_ALIGNMENT is defined based on paragraph of
+ * "Kernel location" of boot.rst:
+ * https://docs.kernel.org/arch/riscv/boot.html#kernel-location
+ */
+#if defined(CONFIG_RISCV_32)
+#define KERNEL_LOAD_ADDR_ALIGNMENT MB(4)
+#elif defined(CONFIG_RISCV_64)
+#define KERNEL_LOAD_ADDR_ALIGNMENT MB(2)
+#else
+#error "Define KERNEL_LOAD_ADDR_ALIGNMENT"
+#endif
+
 #endif /* ASM__RISCV__CONFIG_H */
 /*
  * Local variables:
diff --git a/xen/arch/riscv/kernel.c b/xen/arch/riscv/kernel.c
new file mode 100644
index 000000000000..dd79f457732d
--- /dev/null
+++ b/xen/arch/riscv/kernel.c
@@ -0,0 +1,242 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/bug.h>
+#include <xen/compiler.h>
+#include <xen/errno.h>
+#include <xen/fdt-kernel.h>
+#include <xen/guest_access.h>
+#include <xen/init.h>
+#include <xen/libfdt/libfdt.h>
+#include <xen/mm.h>
+#include <xen/types.h>
+#include <xen/vmap.h>
+
+#include <asm/setup.h>
+
+#define IMAGE64_MAGIC_V2 0x05435352 /* Magic number 2, le, "RSC\x05" */
+
+static void __init place_modules(struct kernel_info *info, paddr_t kernbase,
+                                 paddr_t kernend)
+{
+    const struct boot_module *mod = info->bd.initrd;
+    const struct membanks *banks = kernel_info_get_mem_const(info);
+    const paddr_t initrd_len = ROUNDUP(mod ? mod->size : 0,
+                                       KERNEL_LOAD_ADDR_ALIGNMENT);
+    const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt),
+                                    KERNEL_LOAD_ADDR_ALIGNMENT);
+    const paddr_t modsize = initrd_len + dtb_len;
+    unsigned int bi = banks->nr_banks;
+
+    BUG_ON(modsize < initrd_len);
+
+    /*
+     * Place modules as high in RAM as possible, scanning banks from
+     * last to first so that the end of the last bank is preferred.
+     */
+    while ( bi-- > 0 )
+    {
+        const struct membank *bank = &banks->bank[bi];
+        const paddr_t bank_end = bank->start + bank->size;
+        paddr_t modbase;
+
+        if ( modsize > bank->size )
+            continue;
+
+        modbase = ROUNDDOWN(bank_end - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);
+
+        if ( modbase < bank->start )
+            continue;
+
+        /*
+         * If modules would overlap the kernel, try placing them below it.
+         */
+        if ( (modbase < ROUNDUP(kernend, KERNEL_LOAD_ADDR_ALIGNMENT)) &&
+             (modbase + modsize > kernbase) )
+        {
+            modbase = ROUNDDOWN(kernbase - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);
+            if ( modbase < bank->start )
+                continue;
+        }
+
+        info->dtb_paddr = modbase;
+        info->initrd_paddr = modbase + dtb_len;
+
+        return;
+    }
+
+    panic("Unable to find suitable location for dtb+initrd\n");
+}
+
+static paddr_t __init kernel_image_place(struct kernel_info *info)
+{
+    paddr_t load_addr = INVALID_PADDR;
+    uint64_t image_size = info->image.image_size ?: info->image.len;
+    const struct membanks *banks = kernel_info_get_mem_const(info);
+    unsigned int nr_banks = banks->nr_banks;
+    unsigned int bi;
+
+    dprintk(XENLOG_DEBUG, "nr_banks(%u)\n", nr_banks);
+
+    /*
+     * At the moment, RISC-V's Linux kernel should be always position
+     * independent based on "Per-MMU execution" of boot.rst:
+     *   https://docs.kernel.org/arch/riscv/boot.html#pre-mmu-execution
+     *
+     * But just for the case when RISC-V's Linux kernel isn't position
+     * independent it is needed to take load address from
+     * info->image.start.
+     *
+     * If `start` is zero, the Image is position independent.
+     */
+    if ( likely(!info->image.start) )
+    {
+        for ( bi = 0; bi != nr_banks; bi++ )
+        {
+            const struct membank *bank = &banks->bank[bi];
+            paddr_t bank_start = bank->start;
+            /*
+             * According to boot.rst kernel load address should be properly
+             * aligned:
+             *   https://docs.kernel.org/arch/riscv/boot.html#kernel-location
+             *
+             * As Image in this case is PIC we can ignore
+             * info->image.text_offset.
+             */
+            paddr_t aligned_start = ROUNDUP(bank_start, KERNEL_LOAD_ADDR_ALIGNMENT);
+            paddr_t bank_end = bank_start + bank->size;
+            paddr_t bank_size;
+
+            if ( aligned_start > bank_end )
+                continue;
+
+            bank_size = bank_end - aligned_start;
+
+            dprintk(XENLOG_DEBUG, "bank[%u].start=%"PRIpaddr"\n", bi, bank->start);
+
+            if ( image_size <= bank_size )
+            {
+                load_addr = aligned_start;
+                break;
+            }
+        }
+    }
+    else
+    {
+        load_addr = info->image.start + info->image.text_offset;
+
+        WARN_ON(!IS_ALIGNED(load_addr, KERNEL_LOAD_ADDR_ALIGNMENT));
+
+        /*
+         * Reject a malformed image before the loop to avoid wrapping
+         * load_addr + image_size in the per-bank check below by setting
+         * bi = nr_banks.
+         *
+         * image_size covers the kernel from _start (placed at load_addr =
+         * start + text_offset) through _end.  The alignment gap
+         * [start, load_addr) is padding and need not lie within a bank.
+         */
+        bi = image_size <= (paddr_t)-1 - load_addr ? 0 : nr_banks;
+        for ( ; bi != nr_banks; bi++ )
+        {
+            const struct membank *bank = &banks->bank[bi];
+            paddr_t bank_start = bank->start;
+            paddr_t bank_end = bank_start + bank->size;
+
+            if ( (load_addr >= bank_start) &&
+                 (load_addr + image_size <= bank_end) )
+                break;
+        }
+    }
+
+    if ( bi == nr_banks )
+        panic("Failed to place kernel image in any memory bank\n");
+
+    info->entry = load_addr;
+
+    return load_addr;
+}
+
+static void __init kernel_image_load(struct kernel_info *info)
+{
+    int rc;
+    paddr_t load_addr = kernel_image_place(info);
+    paddr_t paddr = info->image.kernel_addr;
+    paddr_t len = info->image.len;
+    paddr_t effective_size = info->image.image_size ?: len;
+    void *kernel;
+
+    place_modules(info, load_addr, load_addr + effective_size);
+
+    printk("Loading Image from %"PRIpaddr" to [%"PRIpaddr",%"PRIpaddr")\n",
+            paddr, load_addr, load_addr + effective_size);
+
+    kernel = ioremap_cache(paddr, len);
+
+    if ( !kernel )
+        panic("Unable to map kernel\n");
+
+    /* Move kernel to proper location in guest phys map */
+    rc = copy_to_guest_phys(info->bd.d, load_addr, kernel, len);
+
+    if ( rc )
+        panic("Unable to copy kernel to proper guest location\n");
+
+    iounmap(kernel);
+}
+
+/* Check if the image is a 64-bit Image */
+static int __init kernel_image64_probe(struct kernel_info *info,
+                                       paddr_t addr, paddr_t size)
+{
+    /* https://www.kernel.org/doc/Documentation/riscv/boot-image-header.rst */
+    struct {
+        uint32_t code0;         /* Executable code */
+        uint32_t code1;         /* Executable code */
+        uint64_t text_offset;   /* Image load offset, little endian */
+        uint64_t image_size;    /* Effective Image size, little endian */
+        uint64_t flags;         /* kernel flags, little endian */
+        uint32_t version;       /* Version of this header */
+        uint32_t res1;          /* Reserved */
+        uint64_t res2;          /* Reserved */
+        uint64_t magic;         /* Deprecated: Magic number, little endian, "RISCV" */
+        uint32_t magic2;        /* Magic number 2, little endian, "RSC\x05" */
+        uint32_t res3;          /* Reserved for PE COFF offset */
+    } image;
+    uint64_t effective_size;
+
+    if ( size < sizeof(image) )
+        return -EINVAL;
+
+    copy_from_paddr(&image, addr, sizeof(image));
+
+    /* Magic v1 is deprecated and may be removed.  Only use v2 */
+    if ( le32_to_cpu(image.magic2) != IMAGE64_MAGIC_V2 )
+        return -EINVAL;
+
+    effective_size = le64_to_cpu(image.image_size);
+
+    if ( effective_size && size > effective_size )
+        return -EINVAL;
+
+    info->image.kernel_addr = addr;
+    /* Actual size in the binary file */
+    info->image.len = size;
+    /* Total memory the kernel occupies at runtime */
+    info->image.image_size = effective_size;
+    info->image.text_offset = le64_to_cpu(image.text_offset);
+    info->image.start = 0;
+
+    info->load = kernel_image_load;
+
+    return 0;
+}
+
+int __init kernel_image_probe(struct kernel_info *info, paddr_t addr,
+                              paddr_t size)
+{
+#ifdef CONFIG_RISCV_64
+    return kernel_image64_probe(info, addr, size);
+#else
+#   error "Only 64-bit RISC-V is supported"
+#endif
+}
diff --git a/xen/include/xen/fdt-kernel.h b/xen/include/xen/fdt-kernel.h
index 54be77aa191e..c69da642eba8 100644
--- a/xen/include/xen/fdt-kernel.h
+++ b/xen/include/xen/fdt-kernel.h
@@ -59,8 +59,15 @@ struct kernel_info {
         struct {
             paddr_t kernel_addr;
             paddr_t len;
-#if defined(CONFIG_ARM_64) || defined(CONFIG_RISCV_64)
-            paddr_t text_offset; /* 64-bit Image only */
+#if defined(CONFIG_ARM_64) || defined(CONFIG_RISCV)
+            /*
+             * ARM: 64-bit Image only.
+             * RISC-V: both 32-bit and 64-bit Images.
+             */
+            paddr_t text_offset;
+#endif
+#if defined(CONFIG_RISCV)
+            uint64_t image_size; /* Effective size of Image */
 #endif
             paddr_t start; /* Must be 0 for 64-bit Image */
         } image;
-- 
2.53.0

[PATCH v4 08/11] xen/riscv: rework G-stage mode handling

[Oleksii Kurochko]

Rework G-stage mode handling to make the selected mode descriptor
reusable outside of p2m initialization, both for filling CPU nodes in
the device tree passed to dom0less guests and for per-domain G-stage
mode selection at domain creation time.

Promote gstage_modes[] from a local __initconst variable inside
gstage_mode_detect() to a file-scope static const array, and convert
max_gstage_mode from an embedded struct (assigned by value) to a global
const pointer into gstage_modes[]. This allows referencing both the mode
identifier and the mode name after init without copying the descriptor.
Remove get_max_supported_mode(); its callers now dereference
max_gstage_mode->mode directly.

Change struct p2m_domain::mode from an embedded gstage_mode_desc to a
const pointer into gstage_modes[], so each domain shares the descriptor
rather than carrying its own copy.

Adjust the gstage_modes[] entries in three ways:
 - Use lowercase names without the "x4" suffix (e.g. "sv39" instead of
   "Sv39x4"). The DT mmu-type binding [1] does not include the suffix,
   so the name can now be passed directly to the guest without
   transformation. The suffix is appended only in the diagnostic
   printk, where it remains informative.
 - Use "none" for Bare mode (HGATP_MODE_OFF) to match the DT binding.
 - Change paging_levels to represent the root page-table level index
   (i.e. total paging levels minus one) rather than the total count.
   P2M_ROOT_LEVEL() now returns the correct VPN index directly, without
   requiring callers to subtract one or use hardcoded offsets.

Add gstage_mode to xen_arch_domainconfig so the toolstack can
request a specific G-stage mode at domain creation time. Introduce
find_gstage_mode() to resolve a mode descriptor by HGATP_MODE_*,
capping the result at max_gstage_mode to prevent requesting a mode
the hardware does not support. Update p2m_init() to accept a
xen_domctl_createdomain pointer and call find_gstage_mode()
instead of hardcoding Sv39x4.

Add arch_parse_dom0less_node() in a new dom0less-build.c to read the
"mmu-type" DT property from a guest domain node and store it in
boot_domain::create_cfg.arch.gstage_mode, falling back to maximum
supported mode when the property is absent.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/riscv/cpus.yaml?h=v6.19-rc3#n82

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v4:
 - Stray blank after * in declaration of find_gstage_mode().
 - Refactor find_gstage_mode(). Now it is find_gstage_mode_by_bits().
 - Add __ro_after_init for static local variable m in p2m_init.
 - s/ char gstage_mode[8];/unsigned char gstage_mode; in struct vcpu_guest_context.
 - s/modes/gstage_modes in p2m.c.
 - Introduce P2M_EXTRA_BITS_AT_LEVEL and re-use P2M_ROOT_EXTRA_BITS to encode magic constant 2.
---
Changes in v3:
 - New patch.
---
---
 xen/arch/riscv/Makefile          |   1 +
 xen/arch/riscv/dom0less-build.c  |  70 ++++++++++++++++++
 xen/arch/riscv/include/asm/p2m.h |  18 +++--
 xen/arch/riscv/p2m.c             | 120 +++++++++++++++++++++----------
 xen/arch/riscv/vmid.c            |   2 +-
 xen/include/public/arch-riscv.h  |   5 ++
 6 files changed, 170 insertions(+), 46 deletions(-)
 create mode 100644 xen/arch/riscv/dom0less-build.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index eecdcbc76867..8f7fd625dddd 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -1,6 +1,7 @@
 obj-y += aplic.o
 obj-y += cpufeature.o
 obj-y += domain.o
+obj-$(CONFIG_DOM0LESS_BOOT) += dom0less-build.init.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 obj-y += entry.o
 obj-y += extable.o
diff --git a/xen/arch/riscv/dom0less-build.c b/xen/arch/riscv/dom0less-build.c
new file mode 100644
index 000000000000..02782d0f9998
--- /dev/null
+++ b/xen/arch/riscv/dom0less-build.c
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/bootfdt.h>
+#include <xen/device_tree.h>
+#include <xen/init.h>
+
+#include <asm/p2m.h>
+
+int __init arch_parse_dom0less_node(struct dt_device_node *node,
+                                    struct boot_domain *bd)
+{
+    const char *mmu_type;
+    unsigned long bits;
+    const char *end;
+
+    if ( dt_property_read_string(node, "mmu-type", &mmu_type) )
+    {
+        dprintk(XENLOG_WARNING, "mmu-type property is missing in guest domain "
+                "node. %s will be used as fallback\n", max_gstage_mode->name);
+
+        bits = P2M_GFN_LEVEL_SHIFT(max_gstage_mode->paging_levels + 1);
+
+        goto out;
+    }
+
+    if ( !strcasecmp(mmu_type, "riscv,none") )
+    {
+        dprintk(XENLOG_ERR, "Bare mode isn't supported by Xen\n");
+
+        return -EOPNOTSUPP;
+    }
+
+    if ( strncasecmp(mmu_type, "riscv,sv", 8) )
+    {
+        dprintk(XENLOG_ERR, "mmu-type value \"%s\" is incorrect\n", mmu_type);
+
+        return -EINVAL;
+    }
+
+    bits = simple_strtoul(mmu_type + 8, &end, 10);
+    if ( (*end != '\0') || (end == mmu_type + 8) )
+    {
+        dprintk(XENLOG_ERR, "mmu-type value \"%s\" is incorrect\n", mmu_type);
+
+        return -EINVAL;
+    }
+
+ out:
+    if ( bits > (UINT8_MAX - P2M_ROOT_EXTRA_BITS) )
+    {
+        dprintk(XENLOG_ERR, "gstage addr bits value overflows uint8\n");
+
+        return -EINVAL;
+    }
+    /*
+     * The correct value of bits will be checked in p2m_init() by call of
+     * find_gstage_mode_by_bits().
+     *
+     * As mmu-type property contains one of string:
+     *  - riscv,sv32
+     *  - riscv,sv39
+     *  - riscv,sv48
+     *  - riscv,sv57
+     * it is needed to add '+P2M_ROOT_EXTRA_BITS' as for G-stage mode GPAs
+     * are extended by P2M_ROOT_EXTRA_BITS.
+     */
+    bd->create_cfg.arch.gstage_addr_bits = bits + P2M_ROOT_EXTRA_BITS;
+
+    return 0;
+}
diff --git a/xen/arch/riscv/include/asm/p2m.h b/xen/arch/riscv/include/asm/p2m.h
index 54ea67990f06..638c60ddc2f7 100644
--- a/xen/arch/riscv/include/asm/p2m.h
+++ b/xen/arch/riscv/include/asm/p2m.h
@@ -13,7 +13,7 @@
 
 #define P2M_ROOT_ORDER  (ilog2(GSTAGE_ROOT_PAGE_TABLE_SIZE) - PAGE_SHIFT)
 #define P2M_ROOT_PAGES  BIT(P2M_ROOT_ORDER, U)
-#define P2M_ROOT_LEVEL(p2m) ((p2m)->mode.paging_levels)
+#define P2M_ROOT_LEVEL(p2m) ((p2m)->mode->paging_levels)
 
 /*
  * According to the RISC-V spec:
@@ -32,10 +32,13 @@
  */
 #define P2M_LEVEL_ORDER(lvl) XEN_PT_LEVEL_ORDER(lvl)
 
-#define P2M_ROOT_EXTRA_BITS(p2m, lvl) (2 * ((lvl) == P2M_ROOT_LEVEL(p2m)))
+#define P2M_ROOT_EXTRA_BITS 2
+
+#define P2M_LEVEL_EXTRA_BITS(p2m, lvl) \
+    (P2M_ROOT_EXTRA_BITS * ((lvl) == P2M_ROOT_LEVEL(p2m)))
 
 #define P2M_PAGETABLE_ENTRIES(p2m, lvl) \
-    (BIT(PAGETABLE_ORDER + P2M_ROOT_EXTRA_BITS(p2m, lvl), UL))
+    (BIT(PAGETABLE_ORDER + P2M_LEVEL_EXTRA_BITS(p2m, lvl), UL))
 
 #define P2M_TABLE_OFFSET(p2m, lvl) (P2M_PAGETABLE_ENTRIES(p2m, lvl) - 1UL)
 
@@ -55,6 +58,8 @@ struct gstage_mode_desc {
     char name[8];
 };
 
+extern const struct gstage_mode_desc *max_gstage_mode;
+
 /* Per-p2m-table state */
 struct p2m_domain {
     /*
@@ -68,7 +73,7 @@ struct p2m_domain {
     /* The root of the p2m tree. May be concatenated */
     struct page_info *root;
 
-    struct gstage_mode_desc mode;
+    const struct gstage_mode_desc *mode;
 
     /* Back pointer to domain */
     struct domain *domain;
@@ -215,9 +220,10 @@ static inline bool arch_acquire_resource_check(struct domain *d)
 }
 
 void guest_mm_init(void);
-unsigned char get_max_supported_mode(void);
 
-int p2m_init(struct domain *d);
+struct xen_domctl_createdomain;
+
+int p2m_init(struct domain *d, const struct xen_domctl_createdomain *config);
 
 static inline void p2m_write_lock(struct p2m_domain *p2m)
 {
diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
index adcf292a7092..13846bee6a4f 100644
--- a/xen/arch/riscv/p2m.c
+++ b/xen/arch/riscv/p2m.c
@@ -45,12 +45,27 @@ struct p2m_pte_ctx {
     unsigned int level;          /* Paging level at which the PTE resides. */
 };
 
-static struct gstage_mode_desc __ro_after_init max_gstage_mode = {
-    .mode = HGATP_MODE_OFF,
-    .paging_levels = 0,
-    .name = "Bare",
+/* Values should be sorted by ->mode in this array */
+static const struct gstage_mode_desc gstage_modes[] = {
+    /*
+     * Based on the RISC-V spec:
+     *   Bare mode is always supported, regardless of SXLEN.
+     *   When SXLEN=32, the only other valid setting for MODE is Sv32.
+     *   When SXLEN=64, three paged virtual-memory schemes are defined:
+     *   Sv39, Sv48, and Sv57.
+     */
+    { HGATP_MODE_OFF,    0, "none" },
+#ifdef CONFIG_RISCV_32
+    { HGATP_MODE_SV32X4, 1, "sv32" },
+#else
+    { HGATP_MODE_SV39X4, 2, "sv39" },
+    { HGATP_MODE_SV48X4, 3, "sv48" },
+    { HGATP_MODE_SV57X4, 4, "sv57" },
+#endif
 };
 
+const struct gstage_mode_desc * __ro_after_init max_gstage_mode = &gstage_modes[0];
+
 static void p2m_free_page(struct p2m_domain *p2m, struct page_info *pg);
 
 static inline void p2m_free_metadata_page(struct p2m_domain *p2m,
@@ -63,11 +78,6 @@ static inline void p2m_free_metadata_page(struct p2m_domain *p2m,
     }
 }
 
-unsigned char get_max_supported_mode(void)
-{
-    return max_gstage_mode.mode;
-}
-
 /*
  * If anything is changed here, it may also require updates to
  * p2m_{get,set}_type().
@@ -148,41 +158,24 @@ static pte_t *p2m_get_root_pointer(struct p2m_domain *p2m, gfn_t gfn)
 
 static void __init gstage_mode_detect(void)
 {
-    static const struct gstage_mode_desc modes[] __initconst = {
-        /*
-         * Based on the RISC-V spec:
-         *   Bare mode is always supported, regardless of SXLEN.
-         *   When SXLEN=32, the only other valid setting for MODE is Sv32.
-         *   When SXLEN=64, three paged virtual-memory schemes are defined:
-         *   Sv39, Sv48, and Sv57.
-         */
-#ifdef CONFIG_RISCV_32
-        { HGATP_MODE_SV32X4, 2, "Sv32x4" }
-#else
-        { HGATP_MODE_SV39X4, 3, "Sv39x4" },
-        { HGATP_MODE_SV48X4, 4, "Sv48x4" },
-        { HGATP_MODE_SV57X4, 5, "Sv57x4" },
-#endif
-    };
-
-    for ( unsigned int mode_idx = ARRAY_SIZE(modes); mode_idx-- > 0; )
+    for ( unsigned int mode_idx = ARRAY_SIZE(gstage_modes); mode_idx-- > 0; )
     {
-        unsigned long mode = modes[mode_idx].mode;
+        unsigned long mode = gstage_modes[mode_idx].mode;
 
         csr_write(CSR_HGATP, MASK_INSR(mode, HGATP_MODE_MASK));
 
         if ( MASK_EXTR(csr_read(CSR_HGATP), HGATP_MODE_MASK) == mode )
         {
-            max_gstage_mode = modes[mode_idx];
+            max_gstage_mode = &gstage_modes[mode_idx];
 
             break;
         }
     }
 
-    if ( max_gstage_mode.mode == HGATP_MODE_OFF )
+    if ( max_gstage_mode->mode == HGATP_MODE_OFF )
         panic("Xen expects that G-stage won't be Bare mode\n");
 
-    printk("Max supported G-stage mode is %s\n", max_gstage_mode.name);
+    printk("Max supported G-stage mode is %sx4\n", max_gstage_mode->name);
 
     csr_write(CSR_HGATP, 0);
 
@@ -283,7 +276,7 @@ static void clear_and_clean_page(struct page_info *page, bool clean_dcache)
 unsigned long construct_hgatp(const struct p2m_domain *p2m, uint16_t vmid)
 {
     return MASK_INSR(mfn_x(page_to_mfn(p2m->root)), HGATP_PPN_MASK) |
-           MASK_INSR(p2m->mode.mode, HGATP_MODE_MASK) |
+           MASK_INSR(p2m->mode->mode, HGATP_MODE_MASK) |
            MASK_INSR(vmid, HGATP_VMID_MASK);
 }
 
@@ -331,8 +324,35 @@ static int p2m_alloc_root_table(struct p2m_domain *p2m)
     return 0;
 }
 
-int p2m_init(struct domain *d)
+static const struct gstage_mode_desc *find_gstage_mode_by_bits(
+    unsigned char gpa_bits)
+{
+    ASSERT(gstage_modes[0].mode == HGATP_MODE_OFF);
+
+    for ( unsigned int i = 1; i < ARRAY_SIZE(gstage_modes); i++ )
+    {
+        unsigned int lvl = gstage_modes[i].paging_levels + 1;
+        unsigned int bits = P2M_GFN_LEVEL_SHIFT(lvl) + P2M_ROOT_EXTRA_BITS;
+
+        if ( gpa_bits == bits )
+        {
+            if ( gstage_modes[i].mode > max_gstage_mode->mode )
+                return NULL;
+            return &gstage_modes[i];
+        }
+    }
+
+    return NULL;
+}
+
+int p2m_init(struct domain *d, const struct xen_domctl_createdomain *config)
 {
+    /*
+     * TODO: This static is a temporary constraint: all guests must use the
+     * same MMU mode because p2m_gpa_bits is not yet per-domain.
+     * Drop this once per-domain p2m_gpa_bits is introduced.
+     */
+    static const struct gstage_mode_desc __ro_after_init *m = &gstage_modes[0];
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
 
     /*
@@ -341,6 +361,33 @@ int p2m_init(struct domain *d)
      */
     p2m->domain = d;
 
+    if ( !config )
+    {
+        dprintk(XENLOG_ERR, "NULL config is passed\n");
+        return -EINVAL;
+    }
+
+    p2m->mode = find_gstage_mode_by_bits(config->arch.gstage_addr_bits);
+
+    if ( !p2m->mode )
+    {
+        dprintk(XENLOG_ERR,
+                "Unsupported or unavailable gstage addr bits: %u\n",
+                config->arch.gstage_addr_bits);
+
+        return -EINVAL;
+    }
+
+    if ( m->mode == HGATP_MODE_OFF )
+        m = p2m->mode;
+
+    if ( m->mode != p2m->mode->mode )
+    {
+        dprintk(XENLOG_ERR,
+                "Mode should be the same for all guests at the moment\n");
+        return -EINVAL;
+    }
+
     paging_domain_init(d);
 
     rwlock_init(&p2m->lock);
@@ -362,11 +409,6 @@ int p2m_init(struct domain *d)
 #   error "Add init of p2m->clean_dcache"
 #endif
 
-    /* TODO: don't hardcode used for a domain g-stage mode. */
-    p2m->mode.mode = HGATP_MODE_SV39X4;
-    p2m->mode.paging_levels = 2;
-    safe_strcpy(p2m->mode.name, "Sv39x4");
-
     return 0;
 }
 
@@ -1304,7 +1346,7 @@ static mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
 {
     unsigned int level = P2M_ROOT_LEVEL(p2m);
     unsigned int gfn_limit_bits =
-        P2M_LEVEL_ORDER(level + 1) + P2M_ROOT_EXTRA_BITS(p2m, level);
+        P2M_LEVEL_ORDER(level + 1) + P2M_LEVEL_EXTRA_BITS(p2m, level);
     pte_t entry, *table;
     int rc;
     mfn_t mfn = INVALID_MFN;
diff --git a/xen/arch/riscv/vmid.c b/xen/arch/riscv/vmid.c
index 8fbcd500f24d..11c7e9d6d6c8 100644
--- a/xen/arch/riscv/vmid.c
+++ b/xen/arch/riscv/vmid.c
@@ -52,7 +52,7 @@ static DEFINE_PER_CPU(struct vmid_data, vmid_data);
 static unsigned int vmidlen_detect(void)
 {
     unsigned int vmid_bits;
-    unsigned char gstage_mode = get_max_supported_mode();
+    unsigned char gstage_mode = max_gstage_mode->mode;
 
     /*
      * According to the RISC-V Privileged Architecture Spec:
diff --git a/xen/include/public/arch-riscv.h b/xen/include/public/arch-riscv.h
index 360d8e6871ba..da4cc0cbfcfb 100644
--- a/xen/include/public/arch-riscv.h
+++ b/xen/include/public/arch-riscv.h
@@ -56,6 +56,11 @@ typedef struct vcpu_guest_context vcpu_guest_context_t;
 DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
 
 struct xen_arch_domainconfig {
+    /*
+     * G-stage GPA address width in bits.
+     * Valid values: 34 (sv32x4), 41 (sv39x4), 50 (sv48x4), 59 (sv57x4).
+     */
+    unsigned char gstage_addr_bits;
 };
 
 #endif
-- 
2.53.0

[PATCH v4 09/11] xen/riscv: introduce p2m_gpa_bits

[Oleksii Kurochko]

common/device-tree/domain-build.c uses p2m_gpa_bits to determine the
upper bound of the GPA space when searching for unused regions, so it
must be defined when CONFIG_DOMAIN_BUILD_HELPERS=y.

The variable is initialised to PADDR_BITS and narrowed in p2m_init() to
the GPA width of the selected G-stage mode, allowing an external entity
(e.g. an IOMMU) to restrict it further if needed.

p2m_gpa_bits is a global rather than a per-domain value, which is
acceptable for now because all domains are required to use the same
G-stage MMU mode, as dom0less common code allocates it per all
domains.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v4:
 - Add Acked-by: Jan Beulich <jbeulich@suse.com>.
---
Changes in v3:
 - Update initialization of p2m_gpa_bits and the comment above.
 - Rework how p2m_gpa_bits is limited.
 - Update the commit message.
---
Changes in v2:
 - New patch.
---
---
 xen/arch/riscv/include/asm/p2m.h |  3 +++
 xen/arch/riscv/p2m.c             | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/xen/arch/riscv/include/asm/p2m.h b/xen/arch/riscv/include/asm/p2m.h
index 638c60ddc2f7..0d1dace1a0d8 100644
--- a/xen/arch/riscv/include/asm/p2m.h
+++ b/xen/arch/riscv/include/asm/p2m.h
@@ -47,6 +47,9 @@
 #define P2M_LEVEL_MASK(p2m, lvl) \
     (P2M_TABLE_OFFSET(p2m, lvl) << P2M_GFN_LEVEL_SHIFT(lvl))
 
+/* Holds the bit size of GPAs in p2m tables */
+extern unsigned int p2m_gpa_bits;
+
 #define paddr_bits PADDR_BITS
 
 /* Get host p2m table */
diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
index 13846bee6a4f..b7c36a75f175 100644
--- a/xen/arch/riscv/p2m.c
+++ b/xen/arch/riscv/p2m.c
@@ -11,6 +11,8 @@
 #include <xen/sections.h>
 #include <xen/xvmalloc.h>
 
+#include <public/domctl.h>
+
 #include <asm/cpufeature.h>
 #include <asm/csr.h>
 #include <asm/flushtlb.h>
@@ -66,6 +68,12 @@ static const struct gstage_mode_desc gstage_modes[] = {
 
 const struct gstage_mode_desc * __ro_after_init max_gstage_mode = &gstage_modes[0];
 
+/*
+ * Set to the maximum configured support for GPA bits, so the number of GPA
+ * bits can be restricted by an external entity (e.g. IOMMU).
+ */
+unsigned int __ro_after_init p2m_gpa_bits = PADDR_BITS;
+
 static void p2m_free_page(struct p2m_domain *p2m, struct page_info *pg);
 
 static inline void p2m_free_metadata_page(struct p2m_domain *p2m,
@@ -354,6 +362,7 @@ int p2m_init(struct domain *d, const struct xen_domctl_createdomain *config)
      */
     static const struct gstage_mode_desc __ro_after_init *m = &gstage_modes[0];
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
+    unsigned int gpa_bits;
 
     /*
      * "Trivial" initialisation is now complete.  Set the backpointer so the
@@ -409,6 +418,12 @@ int p2m_init(struct domain *d, const struct xen_domctl_createdomain *config)
 #   error "Add init of p2m->clean_dcache"
 #endif
 
+    gpa_bits = P2M_GFN_LEVEL_SHIFT(p2m->mode->paging_levels + 1) +
+               P2M_ROOT_EXTRA_BITS;
+
+    if ( gpa_bits < p2m_gpa_bits )
+        p2m_gpa_bits = gpa_bits;
+
     return 0;
 }
 
-- 
2.53.0

[PATCH v4 10/11] xen/riscv: add definition of guest RAM banks

[Oleksii Kurochko]

The dom0less solution uses defined RAM banks as compile-time constants,
so introduce macros to describe guest RAM banks.

The reason for 2 banks is that there is typically always a use case for
low memory under 4 GB, but the bank under 4 GB ends up being small because
there are other things under 4 GB it can conflict with (interrupt
controller, PCI BARs, etc.). So a second bank is added above that MMIO
region (starting at 8 GiB) to provide the remaining RAM; the gap between
the two banks also exercises code paths handling discontiguous memory.
For Sv32 guests (34-bit GPA, 16 GiB addressable), bank0 provides 2 GB
(2–4 GB) and the first 8 GB of bank1 (8–16 GB) is accessible.

Extended regions are useful for RISC-V: they could be used to provide a
"space" for Linux to map grant mappings.

Despite the fact that for every guest MMU mode the GPA could be up
to 56 bits wide (except Sv32 whose GPA is 34 bits), the combined size
of both banks is limited to 1018 GB as it is more than enough for most
use cases.

Add inclusion of asm/guest-layout.h to asm/domain.h to make dom0less
common code build happy.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v4:
 - Add Acked-by: Jan Beulich <jbeulich@suse.com>.
---
Changes in v3:
 - Move GUEST_RAM*-related defines to asm/p2m.h instead of public header.
---
Changes in v2:
 - New patch.
---
---
 xen/arch/riscv/include/asm/domain.h       |  1 +
 xen/arch/riscv/include/asm/guest-layout.h | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/guest-layout.h

diff --git a/xen/arch/riscv/include/asm/domain.h b/xen/arch/riscv/include/asm/domain.h
index 6c48bf13111d..6044ce0feee0 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -7,6 +7,7 @@
 #include <xen/xmalloc.h>
 #include <public/hvm/params.h>
 
+#include <asm/guest-layout.h>
 #include <asm/p2m.h>
 #include <asm/vtimer.h>
 
diff --git a/xen/arch/riscv/include/asm/guest-layout.h b/xen/arch/riscv/include/asm/guest-layout.h
new file mode 100644
index 000000000000..68d95a09394c
--- /dev/null
+++ b/xen/arch/riscv/include/asm/guest-layout.h
@@ -0,0 +1,23 @@
+#ifndef ASM_RISCV_GUEST_LAYOUT_H
+#define ASM_RISCV_GUEST_LAYOUT_H
+
+#include <public/xen.h>
+
+#define GUEST_RAM_BANKS   2
+
+/*
+ * The way to find the extended regions (to be exposed to the guest as unused
+ * address space) relies on the fact that the regions reserved for the RAM
+ * below are big enough to also accommodate such regions.
+ */
+#define GUEST_RAM0_BASE   xen_mk_ullong(0x80000000) /* 2GB of low RAM @ 2GB */
+#define GUEST_RAM0_SIZE   xen_mk_ullong(0x80000000)
+
+#define GUEST_RAM1_BASE   xen_mk_ullong(0x0200000000) /* 1016 GB of RAM @ 8GB */
+#define GUEST_RAM1_SIZE   xen_mk_ullong(0xFE00000000)
+
+/* TODO: allocate these all dynamically */
+#define GUEST_RAM_BANK_BASES   { GUEST_RAM0_BASE, GUEST_RAM1_BASE }
+#define GUEST_RAM_BANK_SIZES   { GUEST_RAM0_SIZE, GUEST_RAM1_SIZE }
+
+#endif /* ASM_RISCV_GUEST_LAYOUT_H */
-- 
2.53.0

[PATCH v4 11/11] xen/riscv: enable DOMAIN_BUILD_HELPERS

[Oleksii Kurochko]

Everything is ready to enable DOMAIN_BUILD_HELPER which are necessary
for dom0less common code. So enable it.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v4:
 - Nothing changed. Only rebase.
---
Changes in v3:
 - Add Acked-by: Jan Beulich <jbeulich@suse.com>.
---
Changes in v2:
 - Move introduction of p2m_ipa_bits to separate patch.
 - Move to separate patch introduction of guest banks constansts.
---
---
 xen/arch/riscv/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/arch/riscv/Kconfig b/xen/arch/riscv/Kconfig
index a5e87c1757f7..41426c205292 100644
--- a/xen/arch/riscv/Kconfig
+++ b/xen/arch/riscv/Kconfig
@@ -1,5 +1,6 @@
 config RISCV
 	def_bool y
+	select DOMAIN_BUILD_HELPERS
 	select FUNCTION_ALIGNMENT_16B
 	select GENERIC_BUG_FRAME
 	select GENERIC_UART_INIT
-- 
2.53.0

Re: [PATCH v4 01/11] xen: arm: fix len type for guest copy functions

[Luca Fancellu]

Hi Oleksii,

> xen/arch/arm/guestcopy.c                | 6 +++---
> xen/arch/arm/include/asm/guest_access.h | 2 +-
> xen/include/xen/fdt-domain-build.h      | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
> index fdb06422b8e9..86f1c9d0e318 100644
> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -53,7 +53,7 @@ static struct page_info *translate_get_page(copy_info_t info, uint64_t addr,
>     return page;
> }
> 
> -static unsigned long copy_guest(void *buf, uint64_t addr, unsigned int len,
> +static unsigned long copy_guest(void *buf, uint64_t addr, unsigned long len,
>                                 copy_info_t info, unsigned int flags)
> {
>     /* XXX needs to handle faults */
> @@ -65,7 +65,7 @@ static unsigned long copy_guest(void *buf, uint64_t addr, unsigned int len,
>     while ( len )
>     {
>         void *p;
> -        unsigned int size = min(len, (unsigned int)PAGE_SIZE - offset);
> +        unsigned long size = min(len, PAGE_SIZE + 0UL - offset);
>         struct page_info *page;
> 
>         page = translate_get_page(info, addr, flags & COPY_linear,
> @@ -136,7 +136,7 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from,
> unsigned long copy_to_guest_phys_flush_dcache(struct domain *d,
>                                               paddr_t gpa,
>                                               void *buf,
> -                                              unsigned int len)
> +                                              unsigned long len)
> {

Now that we do this, potentially we could have truncation in the places where we store its return value
inside an int:

https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/kernel.c;h=7544fd50a20f08b5ba31cad7b94925112fdee956;hb=refs/heads/staging#l131

https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/common/device-tree/domain-build.c;h=c51520ebadf9950311f6c071e7e07042c7076a27;hb=refs/heads/staging#l442

Could you check and let me know if I’m correct or not?

Cheers,
Luca

Re: [PATCH v4 02/11] xen/dom0less: rename kernel_zimage_probe() to kernel_image_probe()

[Luca Fancellu]

Hi Oleksii,

> 
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 9395b5af8745..a5554714cd7b 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -101,8 +101,8 @@ static paddr_t __init kernel_zimage_place(struct kernel_info *info)
>     paddr_t load_addr;
> 
> #ifdef CONFIG_HAS_DOMAIN_TYPE
> -    if ( (info->type == DOMAIN_64BIT) && (info->zimage.start == 0) )
> -        return mem->bank[0].start + info->zimage.text_offset;
> +    if ( (info->type == DOMAIN_64BIT) && (info->image.start == 0) )
> +        return mem->bank[0].start + info->image.text_offset;
> #endif
> 
>     /*
> @@ -111,19 +111,19 @@ static paddr_t __init kernel_zimage_place(struct kernel_info *info)
>      * and above 32MiB. Load it as high as possible within these
>      * constraints, while also avoiding the DTB.
>      */
> -    if ( info->zimage.start == 0 )
> +    if ( info->image.start == 0 )
>     {
>         paddr_t load_end;
> 
>         load_end = mem->bank[0].start + mem->bank[0].size;
>         load_end = MIN(mem->bank[0].start + MB(128), load_end);
> 
> -        load_addr = load_end - info->zimage.len;
> +        load_addr = load_end - info->image.len;
>         /* Align to 2MB */
>         load_addr &= ~((2 << 20) - 1);
>     }
>     else
> -        load_addr = info->zimage.start;
> +        load_addr = info->image.start;
> 
>     return load_addr;
> }
> @@ -131,8 +131,8 @@ static paddr_t __init kernel_zimage_place(struct kernel_info *info)
> static void __init kernel_zimage_load(struct kernel_info *info)
> {
>     paddr_t load_addr = kernel_zimage_place(info);
> -    paddr_t paddr = info->zimage.kernel_addr;
> -    paddr_t len = info->zimage.len;
> +    paddr_t paddr = info->image.kernel_addr;
> +    paddr_t len = info->image.len;
>     void *kernel;
>     int rc;
> 
> @@ -215,7 +215,7 @@ int __init kernel_uimage_probe(struct kernel_info *info,
>         return -EOPNOTSUPP;
>     }
> 
> -    info->zimage.start = be32_to_cpu(uimage.load);
> +    info->image.start = be32_to_cpu(uimage.load);
>     info->entry = be32_to_cpu(uimage.ep);
> 
>     /*
> @@ -224,20 +224,20 @@ int __init kernel_uimage_probe(struct kernel_info *info,
>      * independent image. That means Xen is free to load such an image at
>      * any valid address.
>      */
> -    if ( info->zimage.start == 0 )
> +    if ( info->image.start == 0 )
>         printk(XENLOG_INFO
>                "No load address provided. Xen will decide where to load it.\n");
>     else
>         printk(XENLOG_INFO
>                "Provided load address: %"PRIpaddr" and entry address: %"PRIpaddr"\n",
> -               info->zimage.start, info->entry);
> +               info->image.start, info->entry);
> 
>     /*
>      * If the image supports position independent execution, then user cannot
>      * provide an entry point as Xen will load such an image at any appropriate
>      * memory address. Thus, we need to return error.
>      */
> -    if ( (info->zimage.start == 0) && (info->entry != 0) )
> +    if ( (info->image.start == 0) && (info->entry != 0) )
>     {
>         printk(XENLOG_ERR
>                "Entry point cannot be non zero for PIE image.\n");
> @@ -257,13 +257,13 @@ int __init kernel_uimage_probe(struct kernel_info *info,
>         if ( rc )
>             return rc;
> 
> -        info->zimage.kernel_addr = mod->start;
> -        info->zimage.len = mod->size;
> +        info->image.kernel_addr = mod->start;
> +        info->image.len = mod->size;
>     }
>     else
>     {
> -        info->zimage.kernel_addr = addr + sizeof(uimage);
> -        info->zimage.len = len;
> +        info->image.kernel_addr = addr + sizeof(uimage);
> +        info->image.len = len;
>     }
> 
>     info->load = kernel_zimage_load;
> @@ -289,7 +289,7 @@ int __init kernel_uimage_probe(struct kernel_info *info,
>      * Thus, Xen uses uimage.load attribute to determine the load address and
>      * zimage.text_offset is ignored.

Should we update the comment as well?

Also in here:
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/include/xen/fdt-kernel.h;h=4d0467bb396a9cf317954fd511469e7f56d67844;hb=refs/heads/staging#l111

Cheers,
Luca

Re: [PATCH v4 03/11] xen: move declaration of fw_unreserved_regions() to common header

[Luca Fancellu]

Hi Oleksii,

> On 28 Apr 2026, at 15:33, Oleksii Kurochko <oleksii.kurochko@gmail.com> wrote:
> 
> Since the implementation of fw_unreserved_regions() is in common code, move
> its declaration to xen/bootinfo.h.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in v2-v4:
> - Nothing changed. Only rebase.
> ---
> ---

looks ok to me.

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

Cheers,
Luca

Re: [PATCH v4 04/11] xen: introduce domain-layout.h with common domain_use_host_layout()

[Luca Fancellu]

Hi Oleksii,

> On 28 Apr 2026, at 15:33, Oleksii Kurochko <oleksii.kurochko@gmail.com> wrote:
> 
> domain_use_host_layout() is not architecture-specific and may be needed
> on x86 [1]. Replace the ARM-specific macro in asm/domain.h with a common
> static inline in a new dedicated header, xen/domain-layout.h.
> 
> xen/domain.h would be the natural home, but placing it there would
> require including xen/paging.h (for paging_mode_translate()) and
> xen/sched.h (for is_hardware_domain()), which would introduce circular
> dependencies. A separate header that callers opt into avoids this.
> 
> Adjust the implementation to take paging_mode_translate() into account
> so it works correctly for all architectures, including x86. Some extra
> details about implementation [2] and [3].
> 
> [1] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602161038120.359097@ubuntu-linux-20-04-desktop/
> [2] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602271742400.3148344@ubuntu-linux-20-04-desktop/
> [3] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602271750190.3148344@ubuntu-linux-20-04-desktop/
> 
> Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in v4:
> - Update the comment above domain_use_host_layout().
> ---
> Changes in v3:
> - Make argument of domain_use_host_layout() const.
> - Create a separate header to avoid circular heder dependecy and making
>   domain_use_host_layour() as static inline.
> - Rework domain_use_host_layout() to be protected by paging_mode_translate().
> - Update the commit message.
> ---
> Changes in v2:
> - Drop ifdef around defintion of domain_use_host_layout() as it
>   was suggested generic version. It could be returned back when
>   the real use case for it will appear.
> - Add Suggested-by: and update the commit message.
> - Make domain_use_host_layout() function instead of macros to
>   avoid ciclular header dependecies. Look at more details in
>   the commit message.
> ---
> ---
> xen/arch/arm/domain_build.c           |  1 +
> xen/arch/arm/include/asm/domain.h     | 14 --------------
> xen/arch/arm/vgic-v3.c                |  1 +
> xen/common/device-tree/domain-build.c |  1 +
> xen/include/xen/domain-layout.h       | 27 +++++++++++++++++++++++++++
> 5 files changed, 30 insertions(+), 14 deletions(-)
> create mode 100644 xen/include/xen/domain-layout.h
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index ad665cd3c045..1efddc60ef0a 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2,6 +2,7 @@
> #include <xen/init.h>
> #include <xen/bootinfo.h>
> #include <xen/compile.h>
> +#include <xen/domain-layout.h>
> #include <xen/dom0less-build.h>
> #include <xen/fdt-domain-build.h>
> #include <xen/fdt-kernel.h>
> diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
> index b24f02d269be..46a5cdc0c800 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -18,20 +18,6 @@ struct hvm_domain
>     uint64_t              params[HVM_NR_PARAMS];
> };
> 
> -/*
> - * Is the domain using the host memory layout?
> - *
> - * Direct-mapped domain will always have the RAM mapped with GFN == MFN.
> - * To avoid any trouble finding space, it is easier to force using the
> - * host memory layout.
> - *
> - * The hardware domain will use the host layout regardless of
> - * direct-mapped because some OS may rely on a specific address ranges
> - * for the devices.
> - */
> -#define domain_use_host_layout(d) (is_domain_direct_mapped(d) || \
> -                                   is_hardware_domain(d))
> -
> struct vtimer {
>     struct vcpu *v;
>     int irq;
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 77aab5c3c293..77517c303061 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -10,6 +10,7 @@
>  */
> 
> #include <xen/bitops.h>
> +#include <xen/domain-layout.h>
> #include <xen/init.h>
> #include <xen/irq.h>
> #include <xen/lib.h>
> diff --git a/xen/common/device-tree/domain-build.c b/xen/common/device-tree/domain-build.c
> index c51520ebadf9..6949203dacdc 100644
> --- a/xen/common/device-tree/domain-build.c
> +++ b/xen/common/device-tree/domain-build.c
> @@ -1,6 +1,7 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> 
> #include <xen/bootinfo.h>
> +#include <xen/domain-layout.h>
> #include <xen/fdt-domain-build.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> diff --git a/xen/include/xen/domain-layout.h b/xen/include/xen/domain-layout.h
> new file mode 100644
> index 000000000000..0532a27b44ce
> --- /dev/null
> +++ b/xen/include/xen/domain-layout.h

New files should have SPDX tag

> @@ -0,0 +1,27 @@
> +#ifndef __XEN_DOMAIN_LAYOUT_H__
> +#define __XEN_DOMAIN_LAYOUT_H__

I think this include guard doesn’t satisfy the coding style:
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE;h=aae5a47ac20345978b3c465b9d85f1d5f6774731;hb=refs/heads/staging#l167

Apart from this, the rest looks ok to me, after fixing the above I will leave my R-by

Cheers,
Luca

Re: [PATCH v4 05/11] xen: rename p2m_ipa_bits to p2m_gpa_bits

[Luca Fancellu]

Hi Oleksii,

I would rephrase “xen: rename p2m_ipa_bits to p2m_gpa_bits” into something like
“xen/device-tree: use p2m_gpa_bits in common code”, because we are not really
renaming p2m_ipa_bits, we are only defining an alias.

> On 28 Apr 2026, at 15:33, Oleksii Kurochko <oleksii.kurochko@gmail.com> wrote:
> 
> The IPA terminology is Arm-specific, so rename p2m_ipa_bits to
> p2m_gpa_bits to use architecture-neutral naming in
> xen/common/device-tree/ code.
> 
> No functional changes.
> 
> Reported-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in v4:
> - Nothing changed only rebase.
> ---
> Changes in v3:
> - Introduce #define p2m_gpa_bits p2m_ipa_bits for Arm instead of
>   renaming of p2m_ipa_bits to p2m_gpa_bits to keep Arm part of
>   changes clearer and keep using Arm-specific terminolgy inside
>   Arm code.
> ---
> Changes in v2:
> - New patch.
> ---
> ---

If the maintainer agrees and the title is fixed:

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

Cheers,
Luca

Re: [PATCH v4 01/11] xen: arm: fix len type for guest copy functions

[Jan Beulich]

On 29.04.2026 12:08, Luca Fancellu wrote:
>> @@ -136,7 +136,7 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from,
>> unsigned long copy_to_guest_phys_flush_dcache(struct domain *d,
>>                                               paddr_t gpa,
>>                                               void *buf,
>> -                                              unsigned int len)
>> +                                              unsigned long len)
>> {
> 
> Now that we do this, potentially we could have truncation in the places where we store its return value
> inside an int:

Those would suffer from truncation before and after this change, wouldn't they?
Just that where the truncation occurs does move. I.e. if necessary they would
want dealing with separately.

Jan

> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/kernel.c;h=7544fd50a20f08b5ba31cad7b94925112fdee956;hb=refs/heads/staging#l131
> 
> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/common/device-tree/domain-build.c;h=c51520ebadf9950311f6c071e7e07042c7076a27;hb=refs/heads/staging#l442
> 
> Could you check and let me know if I’m correct or not?
> 
> Cheers,
> Luca
> 
> 
> 

Re: [PATCH v4 04/11] xen: introduce domain-layout.h with common domain_use_host_layout()

[Jan Beulich]

On 28.04.2026 16:33, Oleksii Kurochko wrote:
> domain_use_host_layout() is not architecture-specific and may be needed
> on x86 [1]. Replace the ARM-specific macro in asm/domain.h with a common
> static inline in a new dedicated header, xen/domain-layout.h.
> 
> xen/domain.h would be the natural home, but placing it there would
> require including xen/paging.h (for paging_mode_translate()) and
> xen/sched.h (for is_hardware_domain()), which would introduce circular
> dependencies. A separate header that callers opt into avoids this.
> 
> Adjust the implementation to take paging_mode_translate() into account
> so it works correctly for all architectures, including x86. Some extra
> details about implementation [2] and [3].
> 
> [1] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602161038120.359097@ubuntu-linux-20-04-desktop/
> [2] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602271742400.3148344@ubuntu-linux-20-04-desktop/
> [3] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602271750190.3148344@ubuntu-linux-20-04-desktop/
> 
> Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

With the SPDX tag added as indicated by Luca:
Acked-by: Jan Beulich <jbeulich@suse.com>

One further minor remark:

> --- /dev/null
> +++ b/xen/include/xen/domain-layout.h
> @@ -0,0 +1,27 @@
> +#ifndef __XEN_DOMAIN_LAYOUT_H__
> +#define __XEN_DOMAIN_LAYOUT_H__
> +
> +#include <xen/domain.h>

This isn't really needed. It is ...

> +#include <xen/paging.h>
> +#include <xen/sched.h>

... included by this one anyway (pretty much unavoidably right now, I
guess).

Jan

Re: [PATCH v4 07/11] xen/riscv: add Linux kernel loading support

[Jan Beulich]

On 28.04.2026 16:33, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/kernel.c
> @@ -0,0 +1,242 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/bug.h>
> +#include <xen/compiler.h>
> +#include <xen/errno.h>
> +#include <xen/fdt-kernel.h>
> +#include <xen/guest_access.h>
> +#include <xen/init.h>
> +#include <xen/libfdt/libfdt.h>
> +#include <xen/mm.h>
> +#include <xen/types.h>
> +#include <xen/vmap.h>
> +
> +#include <asm/setup.h>
> +
> +#define IMAGE64_MAGIC_V2 0x05435352 /* Magic number 2, le, "RSC\x05" */
> +
> +static void __init place_modules(struct kernel_info *info, paddr_t kernbase,
> +                                 paddr_t kernend)
> +{
> +    const struct boot_module *mod = info->bd.initrd;
> +    const struct membanks *banks = kernel_info_get_mem_const(info);
> +    const paddr_t initrd_len = ROUNDUP(mod ? mod->size : 0,
> +                                       KERNEL_LOAD_ADDR_ALIGNMENT);
> +    const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt),
> +                                    KERNEL_LOAD_ADDR_ALIGNMENT);

Why would modules need to be this strongly aligned?

> +    const paddr_t modsize = initrd_len + dtb_len;
> +    unsigned int bi = banks->nr_banks;
> +
> +    BUG_ON(modsize < initrd_len);

Where's the earlier check that allows this to be BUG_ON()?

> +    /*
> +     * Place modules as high in RAM as possible, scanning banks from
> +     * last to first so that the end of the last bank is preferred.
> +     */
> +    while ( bi-- > 0 )
> +    {
> +        const struct membank *bank = &banks->bank[bi];
> +        const paddr_t bank_end = bank->start + bank->size;
> +        paddr_t modbase;
> +
> +        if ( modsize > bank->size )
> +            continue;
> +
> +        modbase = ROUNDDOWN(bank_end - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);

Same question here.

> +        if ( modbase < bank->start )
> +            continue;
> +
> +        /*
> +         * If modules would overlap the kernel, try placing them below it.
> +         */

With how kernel_image_place() works, and with the heavy alignment applied
above, is this even possible to succeed? Oh, wait, yes - for the not-
position-independent case.

> +        if ( (modbase < ROUNDUP(kernend, KERNEL_LOAD_ADDR_ALIGNMENT)) &&
> +             (modbase + modsize > kernbase) )
> +        {
> +            modbase = ROUNDDOWN(kernbase - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);

What prevents this subtraction from underflowing?

> +static paddr_t __init kernel_image_place(struct kernel_info *info)
> +{
> +    paddr_t load_addr = INVALID_PADDR;
> +    uint64_t image_size = info->image.image_size ?: info->image.len;
> +    const struct membanks *banks = kernel_info_get_mem_const(info);
> +    unsigned int nr_banks = banks->nr_banks;
> +    unsigned int bi;
> +
> +    dprintk(XENLOG_DEBUG, "nr_banks(%u)\n", nr_banks);

Did you mean to drop this before submitting?

> +    /*
> +     * At the moment, RISC-V's Linux kernel should be always position
> +     * independent based on "Per-MMU execution" of boot.rst:
> +     *   https://docs.kernel.org/arch/riscv/boot.html#pre-mmu-execution
> +     *
> +     * But just for the case when RISC-V's Linux kernel isn't position
> +     * independent it is needed to take load address from
> +     * info->image.start.
> +     *
> +     * If `start` is zero, the Image is position independent.
> +     */
> +    if ( likely(!info->image.start) )
> +    {
> +        for ( bi = 0; bi != nr_banks; bi++ )
> +        {
> +            const struct membank *bank = &banks->bank[bi];
> +            paddr_t bank_start = bank->start;
> +            /*
> +             * According to boot.rst kernel load address should be properly
> +             * aligned:
> +             *   https://docs.kernel.org/arch/riscv/boot.html#kernel-location
> +             *
> +             * As Image in this case is PIC we can ignore
> +             * info->image.text_offset.
> +             */
> +            paddr_t aligned_start = ROUNDUP(bank_start, KERNEL_LOAD_ADDR_ALIGNMENT);
> +            paddr_t bank_end = bank_start + bank->size;
> +            paddr_t bank_size;
> +
> +            if ( aligned_start > bank_end )
> +                continue;
> +
> +            bank_size = bank_end - aligned_start;
> +
> +            dprintk(XENLOG_DEBUG, "bank[%u].start=%"PRIpaddr"\n", bi, bank->start);

And this one? (I also find it puzzling that ->start would be of (primary) interest
here, when ...

> +            if ( image_size <= bank_size )

... bank_size is what's relevant.

> +/* Check if the image is a 64-bit Image */
> +static int __init kernel_image64_probe(struct kernel_info *info,
> +                                       paddr_t addr, paddr_t size)
> +{
> +    /* https://www.kernel.org/doc/Documentation/riscv/boot-image-header.rst */
> +    struct {
> +        uint32_t code0;         /* Executable code */
> +        uint32_t code1;         /* Executable code */
> +        uint64_t text_offset;   /* Image load offset, little endian */
> +        uint64_t image_size;    /* Effective Image size, little endian */
> +        uint64_t flags;         /* kernel flags, little endian */
> +        uint32_t version;       /* Version of this header */
> +        uint32_t res1;          /* Reserved */
> +        uint64_t res2;          /* Reserved */
> +        uint64_t magic;         /* Deprecated: Magic number, little endian, "RISCV" */
> +        uint32_t magic2;        /* Magic number 2, little endian, "RSC\x05" */
> +        uint32_t res3;          /* Reserved for PE COFF offset */
> +    } image;
> +    uint64_t effective_size;
> +
> +    if ( size < sizeof(image) )
> +        return -EINVAL;
> +
> +    copy_from_paddr(&image, addr, sizeof(image));
> +
> +    /* Magic v1 is deprecated and may be removed.  Only use v2 */
> +    if ( le32_to_cpu(image.magic2) != IMAGE64_MAGIC_V2 )
> +        return -EINVAL;
> +
> +    effective_size = le64_to_cpu(image.image_size);
> +
> +    if ( effective_size && size > effective_size )
> +        return -EINVAL;

Is the rhs of the && the wrong way round? If effective_size > size,
aren't you in trouble? Question of course is what "effective" really
means. Yet in any event it seems dubious to me that effective_size <
size would really be a problem. IOW this will want commenting upon
if the check is to stay.

Actually ...

> +    info->image.kernel_addr = addr;
> +    /* Actual size in the binary file */
> +    info->image.len = size;
> +    /* Total memory the kernel occupies at runtime */
> +    info->image.image_size = effective_size;

... this looks to suggest something .bss-like.

> --- a/xen/include/xen/fdt-kernel.h
> +++ b/xen/include/xen/fdt-kernel.h
> @@ -59,8 +59,15 @@ struct kernel_info {
>          struct {
>              paddr_t kernel_addr;
>              paddr_t len;
> -#if defined(CONFIG_ARM_64) || defined(CONFIG_RISCV_64)
> -            paddr_t text_offset; /* 64-bit Image only */
> +#if defined(CONFIG_ARM_64) || defined(CONFIG_RISCV)
> +            /*
> +             * ARM: 64-bit Image only.
> +             * RISC-V: both 32-bit and 64-bit Images.
> +             */
> +            paddr_t text_offset;
> +#endif
> +#if defined(CONFIG_RISCV)
> +            uint64_t image_size; /* Effective size of Image */

As this (apparently) is for both RV64 and RV32 - can the latter really have
wider than 32-bit image sizes? If not - use size_t or unsigned long here?

Jan

Re: [PATCH v4 08/11] xen/riscv: rework G-stage mode handling

[Jan Beulich]

On 28.04.2026 16:33, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/dom0less-build.c
> @@ -0,0 +1,70 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/bootfdt.h>
> +#include <xen/device_tree.h>
> +#include <xen/init.h>
> +
> +#include <asm/p2m.h>
> +
> +int __init arch_parse_dom0less_node(struct dt_device_node *node,
> +                                    struct boot_domain *bd)
> +{
> +    const char *mmu_type;
> +    unsigned long bits;
> +    const char *end;
> +
> +    if ( dt_property_read_string(node, "mmu-type", &mmu_type) )
> +    {
> +        dprintk(XENLOG_WARNING, "mmu-type property is missing in guest domain "
> +                "node. %s will be used as fallback\n", max_gstage_mode->name);
> +
> +        bits = P2M_GFN_LEVEL_SHIFT(max_gstage_mode->paging_levels + 1);
> +
> +        goto out;
> +    }
> +
> +    if ( !strcasecmp(mmu_type, "riscv,none") )
> +    {
> +        dprintk(XENLOG_ERR, "Bare mode isn't supported by Xen\n");
> +
> +        return -EOPNOTSUPP;
> +    }
> +
> +    if ( strncasecmp(mmu_type, "riscv,sv", 8) )
> +    {
> +        dprintk(XENLOG_ERR, "mmu-type value \"%s\" is incorrect\n", mmu_type);
> +
> +        return -EINVAL;
> +    }
> +
> +    bits = simple_strtoul(mmu_type + 8, &end, 10);
> +    if ( (*end != '\0') || (end == mmu_type + 8) )
> +    {
> +        dprintk(XENLOG_ERR, "mmu-type value \"%s\" is incorrect\n", mmu_type);
> +
> +        return -EINVAL;
> +    }
> +
> + out:
> +    if ( bits > (UINT8_MAX - P2M_ROOT_EXTRA_BITS) )
> +    {
> +        dprintk(XENLOG_ERR, "gstage addr bits value overflows uint8\n");
> +
> +        return -EINVAL;
> +    }
> +    /*
> +     * The correct value of bits will be checked in p2m_init() by call of
> +     * find_gstage_mode_by_bits().
> +     *
> +     * As mmu-type property contains one of string:
> +     *  - riscv,sv32
> +     *  - riscv,sv39
> +     *  - riscv,sv48
> +     *  - riscv,sv57

Or about any other riscv,sv<N> with N up to somewhere around 250. I see
that ...

> +     * it is needed to add '+P2M_ROOT_EXTRA_BITS' as for G-stage mode GPAs
> +     * are extended by P2M_ROOT_EXTRA_BITS.
> +     */
> +    bd->create_cfg.arch.gstage_addr_bits = bits + P2M_ROOT_EXTRA_BITS;

... the value calculated here is later checked for validity, so it's
really only the comment which may want clarifying a little.

> --- a/xen/arch/riscv/p2m.c
> +++ b/xen/arch/riscv/p2m.c
> @@ -45,12 +45,27 @@ struct p2m_pte_ctx {
>      unsigned int level;          /* Paging level at which the PTE resides. */
>  };
>  
> -static struct gstage_mode_desc __ro_after_init max_gstage_mode = {
> -    .mode = HGATP_MODE_OFF,
> -    .paging_levels = 0,
> -    .name = "Bare",
> +/* Values should be sorted by ->mode in this array */
> +static const struct gstage_mode_desc gstage_modes[] = {
> +    /*
> +     * Based on the RISC-V spec:
> +     *   Bare mode is always supported, regardless of SXLEN.
> +     *   When SXLEN=32, the only other valid setting for MODE is Sv32.
> +     *   When SXLEN=64, three paged virtual-memory schemes are defined:
> +     *   Sv39, Sv48, and Sv57.
> +     */
> +    { HGATP_MODE_OFF,    0, "none" },
> +#ifdef CONFIG_RISCV_32
> +    { HGATP_MODE_SV32X4, 1, "sv32" },
> +#else
> +    { HGATP_MODE_SV39X4, 2, "sv39" },
> +    { HGATP_MODE_SV48X4, 3, "sv48" },
> +    { HGATP_MODE_SV57X4, 4, "sv57" },
> +#endif
>  };
>  
> +const struct gstage_mode_desc * __ro_after_init max_gstage_mode = &gstage_modes[0];

Nit: Overlong line (and, strictly speaking, a stray blank after *).

> @@ -331,8 +324,35 @@ static int p2m_alloc_root_table(struct p2m_domain *p2m)
>      return 0;
>  }
>  
> -int p2m_init(struct domain *d)
> +static const struct gstage_mode_desc *find_gstage_mode_by_bits(

Is "_by_bits" adding much value to the function name? Especially ...

> +    unsigned char gpa_bits)

... seeing that the parameter name is making things pretty clear?

> +int p2m_init(struct domain *d, const struct xen_domctl_createdomain *config)
>  {
> +    /*
> +     * TODO: This static is a temporary constraint: all guests must use the
> +     * same MMU mode because p2m_gpa_bits is not yet per-domain.
> +     * Drop this once per-domain p2m_gpa_bits is introduced.
> +     */
> +    static const struct gstage_mode_desc __ro_after_init *m = &gstage_modes[0];
>      struct p2m_domain *p2m = p2m_get_hostp2m(d);
>  
>      /*
> @@ -341,6 +361,33 @@ int p2m_init(struct domain *d)
>       */
>      p2m->domain = d;
>  
> +    if ( !config )
> +    {
> +        dprintk(XENLOG_ERR, "NULL config is passed\n");
> +        return -EINVAL;
> +    }
> +
> +    p2m->mode = find_gstage_mode_by_bits(config->arch.gstage_addr_bits);
> +
> +    if ( !p2m->mode )
> +    {
> +        dprintk(XENLOG_ERR,
> +                "Unsupported or unavailable gstage addr bits: %u\n",
> +                config->arch.gstage_addr_bits);
> +
> +        return -EINVAL;
> +    }
> +
> +    if ( m->mode == HGATP_MODE_OFF )
> +        m = p2m->mode;
> +
> +    if ( m->mode != p2m->mode->mode )

Since m always points into gstage_modes[], do you really need the extra
indirection to compare the two ->mode fields? You could simply compare
the pointers, couldn't you?

> --- a/xen/include/public/arch-riscv.h
> +++ b/xen/include/public/arch-riscv.h
> @@ -56,6 +56,11 @@ typedef struct vcpu_guest_context vcpu_guest_context_t;
>  DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
>  
>  struct xen_arch_domainconfig {
> +    /*
> +     * G-stage GPA address width in bits.
> +     * Valid values: 34 (sv32x4), 41 (sv39x4), 50 (sv48x4), 59 (sv57x4).
> +     */
> +    unsigned char gstage_addr_bits;

Fixed-width types only in the public interface please.

Also, isn't the field effectively describing the maximum width of a
guest (physical) address? In which case - simply gaddr_bits?

Jan

Re: [PATCH v4 01/11] xen: arm: fix len type for guest copy functions

[Luca Fancellu]

Hi Jan,

> On 4 May 2026, at 06:30, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 29.04.2026 12:08, Luca Fancellu wrote:
>>> @@ -136,7 +136,7 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from,
>>> unsigned long copy_to_guest_phys_flush_dcache(struct domain *d,
>>>                                              paddr_t gpa,
>>>                                              void *buf,
>>> -                                              unsigned int len)
>>> +                                              unsigned long len)
>>> {
>> 
>> Now that we do this, potentially we could have truncation in the places where we store its return value
>> inside an int:
> 
> Those would suffer from truncation before and after this change, wouldn't they?
> Just that where the truncation occurs does move. I.e. if necessary they would
> want dealing with separately.

yes that’s true, truncation was already there in different places, do you want to deal with it separately so that
we have a Fixes tag for it?

Cheers,
Luca

Re: [PATCH v4 01/11] xen: arm: fix len type for guest copy functions

[Jan Beulich]

On 05.05.2026 10:27, Luca Fancellu wrote:
>> On 4 May 2026, at 06:30, Jan Beulich <jbeulich@suse.com> wrote:
>> On 29.04.2026 12:08, Luca Fancellu wrote:
>>>> @@ -136,7 +136,7 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from,
>>>> unsigned long copy_to_guest_phys_flush_dcache(struct domain *d,
>>>>                                              paddr_t gpa,
>>>>                                              void *buf,
>>>> -                                              unsigned int len)
>>>> +                                              unsigned long len)
>>>> {
>>>
>>> Now that we do this, potentially we could have truncation in the places where we store its return value
>>> inside an int:
>>
>> Those would suffer from truncation before and after this change, wouldn't they?
>> Just that where the truncation occurs does move. I.e. if necessary they would
>> want dealing with separately.
> 
> yes that’s true, truncation was already there in different places, do you want to deal with it separately so that
> we have a Fixes tag for it?

I already said I'd like that to be dealt with separately, didn't I?

Jan

Re: [PATCH v4 01/11] xen: arm: fix len type for guest copy functions

[Luca Fancellu]

> On 5 May 2026, at 10:05, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 05.05.2026 10:27, Luca Fancellu wrote:
>>> On 4 May 2026, at 06:30, Jan Beulich <jbeulich@suse.com> wrote:
>>> On 29.04.2026 12:08, Luca Fancellu wrote:
>>>>> @@ -136,7 +136,7 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from,
>>>>> unsigned long copy_to_guest_phys_flush_dcache(struct domain *d,
>>>>>                                             paddr_t gpa,
>>>>>                                             void *buf,
>>>>> -                                              unsigned int len)
>>>>> +                                              unsigned long len)
>>>>> {
>>>> 
>>>> Now that we do this, potentially we could have truncation in the places where we store its return value
>>>> inside an int:
>>> 
>>> Those would suffer from truncation before and after this change, wouldn't they?
>>> Just that where the truncation occurs does move. I.e. if necessary they would
>>> want dealing with separately.
>> 
>> yes that’s true, truncation was already there in different places, do you want to deal with it separately so that
>> we have a Fixes tag for it?
> 
> I already said I'd like that to be dealt with separately, didn't I?

I understood the “separately” part.

What I was asking is whether the reason is that this should be its own fix, with its own Fixes tag, since the truncation predates this patch.

I’m asking to understand your reasoning and calibrate my reviews accordingly. When the reasoning is not stated, it is hard to tell whether
this is a general review rule I should apply in similar cases or just a preference for this patch.

Cheers,
Luca

Re: [PATCH v4 01/11] xen: arm: fix len type for guest copy functions

[Jan Beulich]

On 05.05.2026 12:07, Luca Fancellu wrote:
> 
> 
>> On 5 May 2026, at 10:05, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 05.05.2026 10:27, Luca Fancellu wrote:
>>>> On 4 May 2026, at 06:30, Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 29.04.2026 12:08, Luca Fancellu wrote:
>>>>>> @@ -136,7 +136,7 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from,
>>>>>> unsigned long copy_to_guest_phys_flush_dcache(struct domain *d,
>>>>>>                                             paddr_t gpa,
>>>>>>                                             void *buf,
>>>>>> -                                              unsigned int len)
>>>>>> +                                              unsigned long len)
>>>>>> {
>>>>>
>>>>> Now that we do this, potentially we could have truncation in the places where we store its return value
>>>>> inside an int:
>>>>
>>>> Those would suffer from truncation before and after this change, wouldn't they?
>>>> Just that where the truncation occurs does move. I.e. if necessary they would
>>>> want dealing with separately.
>>>
>>> yes that’s true, truncation was already there in different places, do you want to deal with it separately so that
>>> we have a Fixes tag for it?
>>
>> I already said I'd like that to be dealt with separately, didn't I?
> 
> I understood the “separately” part.
> 
> What I was asking is whether the reason is that this should be its own fix, with its own Fixes tag, since the truncation predates this patch.

The reason isn't so much the Fixes: tag, but independent issues generally
wanting independent fixes. This also helps with backporting (in general;
maybe not so much here).

Jan

Re: [PATCH v4 01/11] xen: arm: fix len type for guest copy functions

[Luca Fancellu]

> On 5 May 2026, at 11:13, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 05.05.2026 12:07, Luca Fancellu wrote:
>> 
>> 
>>> On 5 May 2026, at 10:05, Jan Beulich <jbeulich@suse.com> wrote:
>>> 
>>> On 05.05.2026 10:27, Luca Fancellu wrote:
>>>>> On 4 May 2026, at 06:30, Jan Beulich <jbeulich@suse.com> wrote:
>>>>> On 29.04.2026 12:08, Luca Fancellu wrote:
>>>>>>> @@ -136,7 +136,7 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from,
>>>>>>> unsigned long copy_to_guest_phys_flush_dcache(struct domain *d,
>>>>>>>                                            paddr_t gpa,
>>>>>>>                                            void *buf,
>>>>>>> -                                              unsigned int len)
>>>>>>> +                                              unsigned long len)
>>>>>>> {
>>>>>> 
>>>>>> Now that we do this, potentially we could have truncation in the places where we store its return value
>>>>>> inside an int:
>>>>> 
>>>>> Those would suffer from truncation before and after this change, wouldn't they?
>>>>> Just that where the truncation occurs does move. I.e. if necessary they would
>>>>> want dealing with separately.
>>>> 
>>>> yes that’s true, truncation was already there in different places, do you want to deal with it separately so that
>>>> we have a Fixes tag for it?
>>> 
>>> I already said I'd like that to be dealt with separately, didn't I?
>> 
>> I understood the “separately” part.
>> 
>> What I was asking is whether the reason is that this should be its own fix, with its own Fixes tag, since the truncation predates this patch.
> 
> The reason isn't so much the Fixes: tag, but independent issues generally
> wanting independent fixes. This also helps with backporting (in general;
> maybe not so much here).

Thanks.

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

Cheers,
Luca

Re: [PATCH v4 02/11] xen/dom0less: rename kernel_zimage_probe() to kernel_image_probe()

[Oleksii Kurochko]

On 4/29/26 12:59 PM, Luca Fancellu wrote:
> Hi Oleksii,
> 
>>
>> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
>> index 9395b5af8745..a5554714cd7b 100644
>> --- a/xen/arch/arm/kernel.c
>> +++ b/xen/arch/arm/kernel.c
>> @@ -101,8 +101,8 @@ static paddr_t __init kernel_zimage_place(struct kernel_info *info)
>>      paddr_t load_addr;
>>
>> #ifdef CONFIG_HAS_DOMAIN_TYPE
>> -    if ( (info->type == DOMAIN_64BIT) && (info->zimage.start == 0) )
>> -        return mem->bank[0].start + info->zimage.text_offset;
>> +    if ( (info->type == DOMAIN_64BIT) && (info->image.start == 0) )
>> +        return mem->bank[0].start + info->image.text_offset;
>> #endif
>>
>>      /*
>> @@ -111,19 +111,19 @@ static paddr_t __init kernel_zimage_place(struct kernel_info *info)
>>       * and above 32MiB. Load it as high as possible within these
>>       * constraints, while also avoiding the DTB.
>>       */
>> -    if ( info->zimage.start == 0 )
>> +    if ( info->image.start == 0 )
>>      {
>>          paddr_t load_end;
>>
>>          load_end = mem->bank[0].start + mem->bank[0].size;
>>          load_end = MIN(mem->bank[0].start + MB(128), load_end);
>>
>> -        load_addr = load_end - info->zimage.len;
>> +        load_addr = load_end - info->image.len;
>>          /* Align to 2MB */
>>          load_addr &= ~((2 << 20) - 1);
>>      }
>>      else
>> -        load_addr = info->zimage.start;
>> +        load_addr = info->image.start;
>>
>>      return load_addr;
>> }
>> @@ -131,8 +131,8 @@ static paddr_t __init kernel_zimage_place(struct kernel_info *info)
>> static void __init kernel_zimage_load(struct kernel_info *info)
>> {
>>      paddr_t load_addr = kernel_zimage_place(info);
>> -    paddr_t paddr = info->zimage.kernel_addr;
>> -    paddr_t len = info->zimage.len;
>> +    paddr_t paddr = info->image.kernel_addr;
>> +    paddr_t len = info->image.len;
>>      void *kernel;
>>      int rc;
>>
>> @@ -215,7 +215,7 @@ int __init kernel_uimage_probe(struct kernel_info *info,
>>          return -EOPNOTSUPP;
>>      }
>>
>> -    info->zimage.start = be32_to_cpu(uimage.load);
>> +    info->image.start = be32_to_cpu(uimage.load);
>>      info->entry = be32_to_cpu(uimage.ep);
>>
>>      /*
>> @@ -224,20 +224,20 @@ int __init kernel_uimage_probe(struct kernel_info *info,
>>       * independent image. That means Xen is free to load such an image at
>>       * any valid address.
>>       */
>> -    if ( info->zimage.start == 0 )
>> +    if ( info->image.start == 0 )
>>          printk(XENLOG_INFO
>>                 "No load address provided. Xen will decide where to load it.\n");
>>      else
>>          printk(XENLOG_INFO
>>                 "Provided load address: %"PRIpaddr" and entry address: %"PRIpaddr"\n",
>> -               info->zimage.start, info->entry);
>> +               info->image.start, info->entry);
>>
>>      /*
>>       * If the image supports position independent execution, then user cannot
>>       * provide an entry point as Xen will load such an image at any appropriate
>>       * memory address. Thus, we need to return error.
>>       */
>> -    if ( (info->zimage.start == 0) && (info->entry != 0) )
>> +    if ( (info->image.start == 0) && (info->entry != 0) )
>>      {
>>          printk(XENLOG_ERR
>>                 "Entry point cannot be non zero for PIE image.\n");
>> @@ -257,13 +257,13 @@ int __init kernel_uimage_probe(struct kernel_info *info,
>>          if ( rc )
>>              return rc;
>>
>> -        info->zimage.kernel_addr = mod->start;
>> -        info->zimage.len = mod->size;
>> +        info->image.kernel_addr = mod->start;
>> +        info->image.len = mod->size;
>>      }
>>      else
>>      {
>> -        info->zimage.kernel_addr = addr + sizeof(uimage);
>> -        info->zimage.len = len;
>> +        info->image.kernel_addr = addr + sizeof(uimage);
>> +        info->image.len = len;
>>      }
>>
>>      info->load = kernel_zimage_load;
>> @@ -289,7 +289,7 @@ int __init kernel_uimage_probe(struct kernel_info *info,
>>       * Thus, Xen uses uimage.load attribute to determine the load address and
>>       * zimage.text_offset is ignored.
> 
> Should we update the comment as well?
> 
> Also in here:
> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/include/xen/fdt-kernel.h;h=4d0467bb396a9cf317954fd511469e7f56d67844;hb=refs/heads/staging#l111

Agree, it should be updated. I'll do that in the next version.

Thanks.

~ Oleksii

Re: [PATCH v4 04/11] xen: introduce domain-layout.h with common domain_use_host_layout()

[Oleksii Kurochko]

Hello Luca,

On 4/29/26 5:10 PM, Luca Fancellu wrote:
>> diff --git a/xen/common/device-tree/domain-build.c b/xen/common/device-tree/domain-build.c
>> index c51520ebadf9..6949203dacdc 100644
>> --- a/xen/common/device-tree/domain-build.c
>> +++ b/xen/common/device-tree/domain-build.c
>> @@ -1,6 +1,7 @@
>> /* SPDX-License-Identifier: GPL-2.0-only */
>>
>> #include <xen/bootinfo.h>
>> +#include <xen/domain-layout.h>
>> #include <xen/fdt-domain-build.h>
>> #include <xen/init.h>
>> #include <xen/lib.h>
>> diff --git a/xen/include/xen/domain-layout.h b/xen/include/xen/domain-layout.h
>> new file mode 100644
>> index 000000000000..0532a27b44ce
>> --- /dev/null
>> +++ b/xen/include/xen/domain-layout.h
> 
> New files should have SPDX tag

I will add:
   /* SPDX-License-Identifier: GPL-2.0-only */

> 
>> @@ -0,0 +1,27 @@
>> +#ifndef __XEN_DOMAIN_LAYOUT_H__
>> +#define __XEN_DOMAIN_LAYOUT_H__
> 
> I think this include guard doesn’t satisfy the coding style:
> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE;h=aae5a47ac20345978b3c465b9d85f1d5f6774731;hb=refs/heads/staging#l167

Will rename to XEN_DOMAIN_LAYOUT_H.

> 
> Apart from this, the rest looks ok to me, after fixing the above I will leave my R-by

Thanks for review.

~ Oleksii

Re: [PATCH v4 05/11] xen: rename p2m_ipa_bits to p2m_gpa_bits

[Oleksii Kurochko]

Hello Luca,

On 4/29/26 5:15 PM, Luca Fancellu wrote:
> Hi Oleksii,
> 
> I would rephrase “xen: rename p2m_ipa_bits to p2m_gpa_bits” into something like
> “xen/device-tree: use p2m_gpa_bits in common code”, because we are not really
> renaming p2m_ipa_bits, we are only defining an alias.

Make sense. I will rephrase commit message according to your suggestion.

> 
>> On 28 Apr 2026, at 15:33, Oleksii Kurochko <oleksii.kurochko@gmail.com> wrote:
>>
>> The IPA terminology is Arm-specific, so rename p2m_ipa_bits to
>> p2m_gpa_bits to use architecture-neutral naming in
>> xen/common/device-tree/ code.
>>
>> No functional changes.
>>
>> Reported-by: Jan Beulich <jbeulich@suse.com>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> ---
>> Changes in v4:
>> - Nothing changed only rebase.
>> ---
>> Changes in v3:
>> - Introduce #define p2m_gpa_bits p2m_ipa_bits for Arm instead of
>>    renaming of p2m_ipa_bits to p2m_gpa_bits to keep Arm part of
>>    changes clearer and keep using Arm-specific terminolgy inside
>>    Arm code.
>> ---
>> Changes in v2:
>> - New patch.
>> ---
>> ---
> 
> If the maintainer agrees and the title is fixed:
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

Thanks.

~ Oleksii

Re: [PATCH v4 07/11] xen/riscv: add Linux kernel loading support

[Oleksii Kurochko]

On 5/4/26 4:05 PM, Jan Beulich wrote:
> On 28.04.2026 16:33, Oleksii Kurochko wrote:
>> --- /dev/null
>> +++ b/xen/arch/riscv/kernel.c
>> @@ -0,0 +1,242 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#include <xen/bug.h>
>> +#include <xen/compiler.h>
>> +#include <xen/errno.h>
>> +#include <xen/fdt-kernel.h>
>> +#include <xen/guest_access.h>
>> +#include <xen/init.h>
>> +#include <xen/libfdt/libfdt.h>
>> +#include <xen/mm.h>
>> +#include <xen/types.h>
>> +#include <xen/vmap.h>
>> +
>> +#include <asm/setup.h>
>> +
>> +#define IMAGE64_MAGIC_V2 0x05435352 /* Magic number 2, le, "RSC\x05" */
>> +
>> +static void __init place_modules(struct kernel_info *info, paddr_t kernbase,
>> +                                 paddr_t kernend)
>> +{
>> +    const struct boot_module *mod = info->bd.initrd;
>> +    const struct membanks *banks = kernel_info_get_mem_const(info);
>> +    const paddr_t initrd_len = ROUNDUP(mod ? mod->size : 0,
>> +                                       KERNEL_LOAD_ADDR_ALIGNMENT);
>> +    const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt),
>> +                                    KERNEL_LOAD_ADDR_ALIGNMENT);
> 

> Why would modules need to be this strongly aligned?
No specific reason except to be aligned with similar alignment below, it 
could be lesser (PAGE_SIZE or even just unsigned long aligned) or even 
dropped, I think. It was just easier then to calculate aligned 
addresses. But I don't see any big issue to have such alignments except 
maybe that it will waste some memory.

> 
>> +    const paddr_t modsize = initrd_len + dtb_len;
>> +    unsigned int bi = banks->nr_banks;
>> +
>> +    BUG_ON(modsize < initrd_len);
> 
> Where's the earlier check that allows this to be BUG_ON()?

There isn't one. I will replace that with:

if ( modsize < initrd_len )
     panic("Module size overflow: initrd + dtb size wraps paddr_t\n");


> 
>> +    /*
>> +     * Place modules as high in RAM as possible, scanning banks from
>> +     * last to first so that the end of the last bank is preferred.
>> +     */
>> +    while ( bi-- > 0 )
>> +    {
>> +        const struct membank *bank = &banks->bank[bi];
>> +        const paddr_t bank_end = bank->start + bank->size;
>> +        paddr_t modbase;
>> +
>> +        if ( modsize > bank->size )
>> +            continue;
>> +
>> +        modbase = ROUNDDOWN(bank_end - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);
> 
> Same question here.

I used KERNEL_LOAD_ADDR_ALIGNMENT to be sure that big page tables be 
potentially used in page table.

> 
>> +        if ( modbase < bank->start )
>> +            continue;
>> +
>> +        /*
>> +         * If modules would overlap the kernel, try placing them below it.
>> +         */
> 
> With how kernel_image_place() works, and with the heavy alignment applied
> above, is this even possible to succeed? Oh, wait, yes - for the not-
> position-independent case.

I don't understand what is wrong with putting initrd and dtb to 
basically any place except where kernel is expected to be placed (as in 
case of position dependent case I assume it is pretty crucial).

> 
>> +        if ( (modbase < ROUNDUP(kernend, KERNEL_LOAD_ADDR_ALIGNMENT)) &&
>> +             (modbase + modsize > kernbase) )
>> +        {
>> +            modbase = ROUNDDOWN(kernbase - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);
> 
> What prevents this subtraction from underflowing?

I will put the following check at the start of the place_modules() function:
if ( kernbase < modsize )
    panic("Underflow could happen between kernbase and modsize\n");


> 
>> +static paddr_t __init kernel_image_place(struct kernel_info *info)
>> +{
>> +    paddr_t load_addr = INVALID_PADDR;
>> +    uint64_t image_size = info->image.image_size ?: info->image.len;
>> +    const struct membanks *banks = kernel_info_get_mem_const(info);
>> +    unsigned int nr_banks = banks->nr_banks;
>> +    unsigned int bi;
>> +
>> +    dprintk(XENLOG_DEBUG, "nr_banks(%u)\n", nr_banks);
> 
> Did you mean to drop this before submitting?

Sure, it should be dropped.

> 
>> +    /*
>> +     * At the moment, RISC-V's Linux kernel should be always position
>> +     * independent based on "Per-MMU execution" of boot.rst:
>> +     *   https://docs.kernel.org/arch/riscv/boot.html#pre-mmu-execution
>> +     *
>> +     * But just for the case when RISC-V's Linux kernel isn't position
>> +     * independent it is needed to take load address from
>> +     * info->image.start.
>> +     *
>> +     * If `start` is zero, the Image is position independent.
>> +     */
>> +    if ( likely(!info->image.start) )
>> +    {
>> +        for ( bi = 0; bi != nr_banks; bi++ )
>> +        {
>> +            const struct membank *bank = &banks->bank[bi];
>> +            paddr_t bank_start = bank->start;
>> +            /*
>> +             * According to boot.rst kernel load address should be properly
>> +             * aligned:
>> +             *   https://docs.kernel.org/arch/riscv/boot.html#kernel-location
>> +             *
>> +             * As Image in this case is PIC we can ignore
>> +             * info->image.text_offset.
>> +             */
>> +            paddr_t aligned_start = ROUNDUP(bank_start, KERNEL_LOAD_ADDR_ALIGNMENT);
>> +            paddr_t bank_end = bank_start + bank->size;
>> +            paddr_t bank_size;
>> +
>> +            if ( aligned_start > bank_end )
>> +                continue;
>> +
>> +            bank_size = bank_end - aligned_start;
>> +
>> +            dprintk(XENLOG_DEBUG, "bank[%u].start=%"PRIpaddr"\n", bi, bank->start);
> 
> And this one? (I also find it puzzling that ->start would be of (primary) interest
> here, when ...
> 
>> +            if ( image_size <= bank_size )
> 
> ... bank_size is what's relevant.

This one should be dropped too.

> 
>> +/* Check if the image is a 64-bit Image */
>> +static int __init kernel_image64_probe(struct kernel_info *info,
>> +                                       paddr_t addr, paddr_t size)
>> +{
>> +    /* https://www.kernel.org/doc/Documentation/riscv/boot-image-header.rst */
>> +    struct {
>> +        uint32_t code0;         /* Executable code */
>> +        uint32_t code1;         /* Executable code */
>> +        uint64_t text_offset;   /* Image load offset, little endian */
>> +        uint64_t image_size;    /* Effective Image size, little endian */
>> +        uint64_t flags;         /* kernel flags, little endian */
>> +        uint32_t version;       /* Version of this header */
>> +        uint32_t res1;          /* Reserved */
>> +        uint64_t res2;          /* Reserved */
>> +        uint64_t magic;         /* Deprecated: Magic number, little endian, "RISCV" */
>> +        uint32_t magic2;        /* Magic number 2, little endian, "RSC\x05" */
>> +        uint32_t res3;          /* Reserved for PE COFF offset */
>> +    } image;
>> +    uint64_t effective_size;
>> +
>> +    if ( size < sizeof(image) )
>> +        return -EINVAL;
>> +
>> +    copy_from_paddr(&image, addr, sizeof(image));
>> +
>> +    /* Magic v1 is deprecated and may be removed.  Only use v2 */
>> +    if ( le32_to_cpu(image.magic2) != IMAGE64_MAGIC_V2 )
>> +        return -EINVAL;
>> +
>> +    effective_size = le64_to_cpu(image.image_size);
>> +
>> +    if ( effective_size && size > effective_size )
>> +        return -EINVAL;
> 
> Is the rhs of the && the wrong way round? If effective_size > size,
> aren't you in trouble? Question of course is what "effective" really
> means. Yet in any event it seems dubious to me that effective_size <
> size would really be a problem. IOW this will want commenting upon
> if the check is to stay.
> 
> Actually ...
> 
>> +    info->image.kernel_addr = addr;
>> +    /* Actual size in the binary file */
>> +    info->image.len = size;
>> +    /* Total memory the kernel occupies at runtime */
>> +    info->image.image_size = effective_size;
> 
> ... this looks to suggest something .bss-like.

Yes, effective_size it is size which included .bss.

size it of LK after decompression of Image.gz and it doesn't include 
.bss so it should be lesser then effective_size.

I don't think that I am in trouble that effective_size is bigger then 
size if we allocate enough space in memory effective_size is fine to be 
bigger.

It is a good question if effective_size < size is a problem. I think it
isn't but could it be really happen?
I think that I am okay to drop that part of if().

> 
>> --- a/xen/include/xen/fdt-kernel.h
>> +++ b/xen/include/xen/fdt-kernel.h
>> @@ -59,8 +59,15 @@ struct kernel_info {
>>           struct {
>>               paddr_t kernel_addr;
>>               paddr_t len;
>> -#if defined(CONFIG_ARM_64) || defined(CONFIG_RISCV_64)
>> -            paddr_t text_offset; /* 64-bit Image only */
>> +#if defined(CONFIG_ARM_64) || defined(CONFIG_RISCV)
>> +            /*
>> +             * ARM: 64-bit Image only.
>> +             * RISC-V: both 32-bit and 64-bit Images.
>> +             */
>> +            paddr_t text_offset;
>> +#endif
>> +#if defined(CONFIG_RISCV)
>> +            uint64_t image_size; /* Effective size of Image */
> 
> As this (apparently) is for both RV64 and RV32 - can the latter really have
> wider than 32-bit image sizes? If not - use size_t or unsigned long here?

Agree, unsigned long should be enough.

Thanks.

~ Oleksii

Re: [PATCH v4 07/11] xen/riscv: add Linux kernel loading support

[Jan Beulich]

On 06.05.2026 13:57, Oleksii Kurochko wrote:
> On 5/4/26 4:05 PM, Jan Beulich wrote:
>> On 28.04.2026 16:33, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/kernel.c
>>> @@ -0,0 +1,242 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +
>>> +#include <xen/bug.h>
>>> +#include <xen/compiler.h>
>>> +#include <xen/errno.h>
>>> +#include <xen/fdt-kernel.h>
>>> +#include <xen/guest_access.h>
>>> +#include <xen/init.h>
>>> +#include <xen/libfdt/libfdt.h>
>>> +#include <xen/mm.h>
>>> +#include <xen/types.h>
>>> +#include <xen/vmap.h>
>>> +
>>> +#include <asm/setup.h>
>>> +
>>> +#define IMAGE64_MAGIC_V2 0x05435352 /* Magic number 2, le, "RSC\x05" */
>>> +
>>> +static void __init place_modules(struct kernel_info *info, paddr_t kernbase,
>>> +                                 paddr_t kernend)
>>> +{
>>> +    const struct boot_module *mod = info->bd.initrd;
>>> +    const struct membanks *banks = kernel_info_get_mem_const(info);
>>> +    const paddr_t initrd_len = ROUNDUP(mod ? mod->size : 0,
>>> +                                       KERNEL_LOAD_ADDR_ALIGNMENT);
>>> +    const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt),
>>> +                                    KERNEL_LOAD_ADDR_ALIGNMENT);
>>
> 
>> Why would modules need to be this strongly aligned?
> No specific reason except to be aligned with similar alignment below, it 
> could be lesser (PAGE_SIZE or even just unsigned long aligned) or even 
> dropped, I think. It was just easier then to calculate aligned 
> addresses. But I don't see any big issue to have such alignments except 
> maybe that it will waste some memory.

Or result in there not being enough memory to hold everything.

>>> +    /*
>>> +     * Place modules as high in RAM as possible, scanning banks from
>>> +     * last to first so that the end of the last bank is preferred.
>>> +     */
>>> +    while ( bi-- > 0 )
>>> +    {
>>> +        const struct membank *bank = &banks->bank[bi];
>>> +        const paddr_t bank_end = bank->start + bank->size;
>>> +        paddr_t modbase;
>>> +
>>> +        if ( modsize > bank->size )
>>> +            continue;
>>> +
>>> +        modbase = ROUNDDOWN(bank_end - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);
>>
>> Same question here.
> 
> I used KERNEL_LOAD_ADDR_ALIGNMENT to be sure that big page tables be 
> potentially used in page table.

I fear I'm lost. All the modules are temporary entities, aren't they?

>>> +        if ( (modbase < ROUNDUP(kernend, KERNEL_LOAD_ADDR_ALIGNMENT)) &&
>>> +             (modbase + modsize > kernbase) )
>>> +        {
>>> +            modbase = ROUNDDOWN(kernbase - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);
>>
>> What prevents this subtraction from underflowing?
> 
> I will put the following check at the start of the place_modules() function:
> if ( kernbase < modsize )
>     panic("Underflow could happen between kernbase and modsize\n");

Wait - why would this be a legitimate condition to panic?

>>> +/* Check if the image is a 64-bit Image */
>>> +static int __init kernel_image64_probe(struct kernel_info *info,
>>> +                                       paddr_t addr, paddr_t size)
>>> +{
>>> +    /* https://www.kernel.org/doc/Documentation/riscv/boot-image-header.rst */
>>> +    struct {
>>> +        uint32_t code0;         /* Executable code */
>>> +        uint32_t code1;         /* Executable code */
>>> +        uint64_t text_offset;   /* Image load offset, little endian */
>>> +        uint64_t image_size;    /* Effective Image size, little endian */
>>> +        uint64_t flags;         /* kernel flags, little endian */
>>> +        uint32_t version;       /* Version of this header */
>>> +        uint32_t res1;          /* Reserved */
>>> +        uint64_t res2;          /* Reserved */
>>> +        uint64_t magic;         /* Deprecated: Magic number, little endian, "RISCV" */
>>> +        uint32_t magic2;        /* Magic number 2, little endian, "RSC\x05" */
>>> +        uint32_t res3;          /* Reserved for PE COFF offset */
>>> +    } image;
>>> +    uint64_t effective_size;
>>> +
>>> +    if ( size < sizeof(image) )
>>> +        return -EINVAL;
>>> +
>>> +    copy_from_paddr(&image, addr, sizeof(image));
>>> +
>>> +    /* Magic v1 is deprecated and may be removed.  Only use v2 */
>>> +    if ( le32_to_cpu(image.magic2) != IMAGE64_MAGIC_V2 )
>>> +        return -EINVAL;
>>> +
>>> +    effective_size = le64_to_cpu(image.image_size);
>>> +
>>> +    if ( effective_size && size > effective_size )
>>> +        return -EINVAL;
>>
>> Is the rhs of the && the wrong way round? If effective_size > size,
>> aren't you in trouble? Question of course is what "effective" really
>> means. Yet in any event it seems dubious to me that effective_size <
>> size would really be a problem. IOW this will want commenting upon
>> if the check is to stay.
>>
>> Actually ...
>>
>>> +    info->image.kernel_addr = addr;
>>> +    /* Actual size in the binary file */
>>> +    info->image.len = size;
>>> +    /* Total memory the kernel occupies at runtime */
>>> +    info->image.image_size = effective_size;
>>
>> ... this looks to suggest something .bss-like.
> 
> Yes, effective_size it is size which included .bss.
> 
> size it of LK after decompression of Image.gz and it doesn't include 
> .bss so it should be lesser then effective_size.
> 
> I don't think that I am in trouble that effective_size is bigger then 
> size if we allocate enough space in memory effective_size is fine to be 
> bigger.
> 
> It is a good question if effective_size < size is a problem. I think it
> isn't but could it be really happen?

A kernel image (file) could have data appended to it, e.g. a certificate.
With only small .bss that certificate could end up larger than the .bss
size, and hence effective_size < size.

> I think that I am okay to drop that part of if().

Please do, unless there is a(nother) reason to have it.

Jan

Re: [PATCH v4 07/11] xen/riscv: add Linux kernel loading support

[Oleksii Kurochko]

On 5/6/26 2:45 PM, Jan Beulich wrote:
> On 06.05.2026 13:57, Oleksii Kurochko wrote:
>> On 5/4/26 4:05 PM, Jan Beulich wrote:
>>> On 28.04.2026 16:33, Oleksii Kurochko wrote:
>>>> --- /dev/null
>>>> +++ b/xen/arch/riscv/kernel.c
>>>> @@ -0,0 +1,242 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>> +
>>>> +#include <xen/bug.h>
>>>> +#include <xen/compiler.h>
>>>> +#include <xen/errno.h>
>>>> +#include <xen/fdt-kernel.h>
>>>> +#include <xen/guest_access.h>
>>>> +#include <xen/init.h>
>>>> +#include <xen/libfdt/libfdt.h>
>>>> +#include <xen/mm.h>
>>>> +#include <xen/types.h>
>>>> +#include <xen/vmap.h>
>>>> +
>>>> +#include <asm/setup.h>
>>>> +
>>>> +#define IMAGE64_MAGIC_V2 0x05435352 /* Magic number 2, le, "RSC\x05" */
>>>> +
>>>> +static void __init place_modules(struct kernel_info *info, paddr_t kernbase,
>>>> +                                 paddr_t kernend)
>>>> +{
>>>> +    const struct boot_module *mod = info->bd.initrd;
>>>> +    const struct membanks *banks = kernel_info_get_mem_const(info);
>>>> +    const paddr_t initrd_len = ROUNDUP(mod ? mod->size : 0,
>>>> +                                       KERNEL_LOAD_ADDR_ALIGNMENT);
>>>> +    const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt),
>>>> +                                    KERNEL_LOAD_ADDR_ALIGNMENT);
>>>
>>
>>> Why would modules need to be this strongly aligned?
>> No specific reason except to be aligned with similar alignment below, it
>> could be lesser (PAGE_SIZE or even just unsigned long aligned) or even
>> dropped, I think. It was just easier then to calculate aligned
>> addresses. But I don't see any big issue to have such alignments except
>> maybe that it will waste some memory.
> 
> Or result in there not being enough memory to hold everything.

Do you prefer than not to have alignment at all?

> 
>>>> +    /*
>>>> +     * Place modules as high in RAM as possible, scanning banks from
>>>> +     * last to first so that the end of the last bank is preferred.
>>>> +     */
>>>> +    while ( bi-- > 0 )
>>>> +    {
>>>> +        const struct membank *bank = &banks->bank[bi];
>>>> +        const paddr_t bank_end = bank->start + bank->size;
>>>> +        paddr_t modbase;
>>>> +
>>>> +        if ( modsize > bank->size )
>>>> +            continue;
>>>> +
>>>> +        modbase = ROUNDDOWN(bank_end - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);
>>>
>>> Same question here.
>>
>> I used KERNEL_LOAD_ADDR_ALIGNMENT to be sure that big page tables be
>> potentially used in page table.
> 
> I fear I'm lost. All the modules are temporary entities, aren't they?

They are temporary entities but they should be copied to guest memory, 
right?

So ioremap() should be called for paddr where module is located and so 
at least less cycles will be needed to add entries to Xen page tables.

I don't know if it makes sense to have such type of optimizations. If 
not then probably we don't need alignment here too. I don't see at the 
moment any alignment requirements for initrd and dtb.

The only theoretical reason why at least for dtb we need requirement is 
that what Arm mentioned in its booting.rst:

The device tree blob (dtb) must be placed on an 8-byte boundary and must
not exceed 2 megabytes in size. Since the dtb will be mapped cacheable
using blocks of up to 2 megabytes in size, it must not be placed within
any 2M region which must be mapped with any specific attributes.

It likely should be true for RISC-V as I assume this part was copied 
from Arm port. On other side it doesn't mentioned explicitly in boot.rst 
of RISC-V in LK.

> 
>>>> +        if ( (modbase < ROUNDUP(kernend, KERNEL_LOAD_ADDR_ALIGNMENT)) &&
>>>> +             (modbase + modsize > kernbase) )
>>>> +        {
>>>> +            modbase = ROUNDDOWN(kernbase - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);
>>>
>>> What prevents this subtraction from underflowing?
>>
>> I will put the following check at the start of the place_modules() function:
>> if ( kernbase < modsize )
>>      panic("Underflow could happen between kernbase and modsize\n");
> 
> Wait - why would this be a legitimate condition to panic?

It is legitimate to panic() as common API which leads to place_module() 
has void in its return type (what should be changed in future, I have 
this in TODO) and so if something is going wrong in place_module() there 
is not better option except panic() for now.

But generally i think it was too much to panic and it would be just 
better to put:
   if ( kernbase < modsize )
       continue;
above modbase = ROUNDDOWN(...) so it will just put modules in different 
bank.

Thanks.

~ Oleksii

Re: [PATCH v4 07/11] xen/riscv: add Linux kernel loading support

[Jan Beulich]

On 06.05.2026 15:43, Oleksii Kurochko wrote:
> On 5/6/26 2:45 PM, Jan Beulich wrote:
>> On 06.05.2026 13:57, Oleksii Kurochko wrote:
>>> On 5/4/26 4:05 PM, Jan Beulich wrote:
>>>> On 28.04.2026 16:33, Oleksii Kurochko wrote:
>>>>> --- /dev/null
>>>>> +++ b/xen/arch/riscv/kernel.c
>>>>> @@ -0,0 +1,242 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>>> +
>>>>> +#include <xen/bug.h>
>>>>> +#include <xen/compiler.h>
>>>>> +#include <xen/errno.h>
>>>>> +#include <xen/fdt-kernel.h>
>>>>> +#include <xen/guest_access.h>
>>>>> +#include <xen/init.h>
>>>>> +#include <xen/libfdt/libfdt.h>
>>>>> +#include <xen/mm.h>
>>>>> +#include <xen/types.h>
>>>>> +#include <xen/vmap.h>
>>>>> +
>>>>> +#include <asm/setup.h>
>>>>> +
>>>>> +#define IMAGE64_MAGIC_V2 0x05435352 /* Magic number 2, le, "RSC\x05" */
>>>>> +
>>>>> +static void __init place_modules(struct kernel_info *info, paddr_t kernbase,
>>>>> +                                 paddr_t kernend)
>>>>> +{
>>>>> +    const struct boot_module *mod = info->bd.initrd;
>>>>> +    const struct membanks *banks = kernel_info_get_mem_const(info);
>>>>> +    const paddr_t initrd_len = ROUNDUP(mod ? mod->size : 0,
>>>>> +                                       KERNEL_LOAD_ADDR_ALIGNMENT);
>>>>> +    const paddr_t dtb_len = ROUNDUP(fdt_totalsize(info->fdt),
>>>>> +                                    KERNEL_LOAD_ADDR_ALIGNMENT);
>>>>
>>>
>>>> Why would modules need to be this strongly aligned?
>>> No specific reason except to be aligned with similar alignment below, it
>>> could be lesser (PAGE_SIZE or even just unsigned long aligned) or even
>>> dropped, I think. It was just easier then to calculate aligned
>>> addresses. But I don't see any big issue to have such alignments except
>>> maybe that it will waste some memory.
>>
>> Or result in there not being enough memory to hold everything.
> 
> Do you prefer than not to have alignment at all?

Some minimal alignment (like to machine word size) may be helpful, for
copying to be more efficient. There may also be reasons to use page
alignment (e.g. if the pages were to be assigned directly to the
domains, without any copying, as we do for PV Dom0 under certain
conditions on x86). That in turn may even justify super-page
alignment.

>>>>> +    /*
>>>>> +     * Place modules as high in RAM as possible, scanning banks from
>>>>> +     * last to first so that the end of the last bank is preferred.
>>>>> +     */
>>>>> +    while ( bi-- > 0 )
>>>>> +    {
>>>>> +        const struct membank *bank = &banks->bank[bi];
>>>>> +        const paddr_t bank_end = bank->start + bank->size;
>>>>> +        paddr_t modbase;
>>>>> +
>>>>> +        if ( modsize > bank->size )
>>>>> +            continue;
>>>>> +
>>>>> +        modbase = ROUNDDOWN(bank_end - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);
>>>>
>>>> Same question here.
>>>
>>> I used KERNEL_LOAD_ADDR_ALIGNMENT to be sure that big page tables be
>>> potentially used in page table.
>>
>> I fear I'm lost. All the modules are temporary entities, aren't they?
> 
> They are temporary entities but they should be copied to guest memory, 
> right?
> 
> So ioremap() should be called for paddr where module is located and so 
> at least less cycles will be needed to add entries to Xen page tables.

Okay, that would be a small win in time for perhaps a boot failure when
the modules would fit in memory if they weren't this heavily aligned.
You're judgement, but please whatever higher-than-expected alignment
you decide to use, please comment upon this.

>>>>> +        if ( (modbase < ROUNDUP(kernend, KERNEL_LOAD_ADDR_ALIGNMENT)) &&
>>>>> +             (modbase + modsize > kernbase) )
>>>>> +        {
>>>>> +            modbase = ROUNDDOWN(kernbase - modsize, KERNEL_LOAD_ADDR_ALIGNMENT);
>>>>
>>>> What prevents this subtraction from underflowing?
>>>
>>> I will put the following check at the start of the place_modules() function:
>>> if ( kernbase < modsize )
>>>      panic("Underflow could happen between kernbase and modsize\n");
>>
>> Wait - why would this be a legitimate condition to panic?
> 
> It is legitimate to panic() as common API which leads to place_module() 
> has void in its return type (what should be changed in future, I have 
> this in TODO) and so if something is going wrong in place_module() there 
> is not better option except panic() for now.

Feels like you're mixing two things here. I didn't ask whether panic()
was appropriate to use here, but whether the condition is one upon
which panic()ing is the only option (right now). And voila, ...

> But generally i think it was too much to panic and it would be just 
> better to put:
>    if ( kernbase < modsize )
>        continue;
> above modbase = ROUNDDOWN(...) so it will just put modules in different 
> bank.

... looks like the situation can be handled without panic().

Jan

Re: [PATCH v4 08/11] xen/riscv: rework G-stage mode handling

[Oleksii Kurochko]

On 5/4/26 4:23 PM, Jan Beulich wrote:
> On 28.04.2026 16:33, Oleksii Kurochko wrote:
>> --- /dev/null
>> +++ b/xen/arch/riscv/dom0less-build.c
>> @@ -0,0 +1,70 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#include <xen/bootfdt.h>
>> +#include <xen/device_tree.h>
>> +#include <xen/init.h>
>> +
>> +#include <asm/p2m.h>
>> +
>> +int __init arch_parse_dom0less_node(struct dt_device_node *node,
>> +                                    struct boot_domain *bd)
>> +{
>> +    const char *mmu_type;
>> +    unsigned long bits;
>> +    const char *end;
>> +
>> +    if ( dt_property_read_string(node, "mmu-type", &mmu_type) )
>> +    {
>> +        dprintk(XENLOG_WARNING, "mmu-type property is missing in guest domain "
>> +                "node. %s will be used as fallback\n", max_gstage_mode->name);
>> +
>> +        bits = P2M_GFN_LEVEL_SHIFT(max_gstage_mode->paging_levels + 1);
>> +
>> +        goto out;
>> +    }
>> +
>> +    if ( !strcasecmp(mmu_type, "riscv,none") )
>> +    {
>> +        dprintk(XENLOG_ERR, "Bare mode isn't supported by Xen\n");
>> +
>> +        return -EOPNOTSUPP;
>> +    }
>> +
>> +    if ( strncasecmp(mmu_type, "riscv,sv", 8) )
>> +    {
>> +        dprintk(XENLOG_ERR, "mmu-type value \"%s\" is incorrect\n", mmu_type);
>> +
>> +        return -EINVAL;
>> +    }
>> +
>> +    bits = simple_strtoul(mmu_type + 8, &end, 10);
>> +    if ( (*end != '\0') || (end == mmu_type + 8) )
>> +    {
>> +        dprintk(XENLOG_ERR, "mmu-type value \"%s\" is incorrect\n", mmu_type);
>> +
>> +        return -EINVAL;
>> +    }
>> +
>> + out:
>> +    if ( bits > (UINT8_MAX - P2M_ROOT_EXTRA_BITS) )
>> +    {
>> +        dprintk(XENLOG_ERR, "gstage addr bits value overflows uint8\n");
>> +
>> +        return -EINVAL;
>> +    }
>> +    /*
>> +     * The correct value of bits will be checked in p2m_init() by call of
>> +     * find_gstage_mode_by_bits().
>> +     *
>> +     * As mmu-type property contains one of string:
>> +     *  - riscv,sv32
>> +     *  - riscv,sv39
>> +     *  - riscv,sv48
>> +     *  - riscv,sv57
> 
> Or about any other riscv,sv<N> with N up to somewhere around 250. I see
> that ...
> 
>> +     * it is needed to add '+P2M_ROOT_EXTRA_BITS' as for G-stage mode GPAs
>> +     * are extended by P2M_ROOT_EXTRA_BITS.
>> +     */
>> +    bd->create_cfg.arch.gstage_addr_bits = bits + P2M_ROOT_EXTRA_BITS;
> 
> ... the value calculated here is later checked for validity, so it's
> really only the comment which may want clarifying a little.

I will update the comment to:

/*
  * The mmu-type property may specify any riscv,sv<N> string, but only the
  * following are currently supported:
  *  - riscv,sv32
  *  - riscv,sv39
  *  - riscv,sv48
  *  - riscv,sv57
  * Any other value will be rejected by find_gstage_mode_by_bits().
  *
  * P2M_ROOT_EXTRA_BITS is added because for G-stage mode, GPAs are
  * extended by that many bits.
  */

> 
>> --- a/xen/arch/riscv/p2m.c
>> +++ b/xen/arch/riscv/p2m.c
>> @@ -45,12 +45,27 @@ struct p2m_pte_ctx {
>>       unsigned int level;          /* Paging level at which the PTE resides. */
>>   };
>>   
>> -static struct gstage_mode_desc __ro_after_init max_gstage_mode = {
>> -    .mode = HGATP_MODE_OFF,
>> -    .paging_levels = 0,
>> -    .name = "Bare",
>> +/* Values should be sorted by ->mode in this array */
>> +static const struct gstage_mode_desc gstage_modes[] = {
>> +    /*
>> +     * Based on the RISC-V spec:
>> +     *   Bare mode is always supported, regardless of SXLEN.
>> +     *   When SXLEN=32, the only other valid setting for MODE is Sv32.
>> +     *   When SXLEN=64, three paged virtual-memory schemes are defined:
>> +     *   Sv39, Sv48, and Sv57.
>> +     */
>> +    { HGATP_MODE_OFF,    0, "none" },
>> +#ifdef CONFIG_RISCV_32
>> +    { HGATP_MODE_SV32X4, 1, "sv32" },
>> +#else
>> +    { HGATP_MODE_SV39X4, 2, "sv39" },
>> +    { HGATP_MODE_SV48X4, 3, "sv48" },
>> +    { HGATP_MODE_SV57X4, 4, "sv57" },
>> +#endif
>>   };
>>   
>> +const struct gstage_mode_desc * __ro_after_init max_gstage_mode = &gstage_modes[0];
> 
> Nit: Overlong line (and, strictly speaking, a stray blank after *).

I will put "&gstage_modes[0];" on next line.

> 
>> @@ -331,8 +324,35 @@ static int p2m_alloc_root_table(struct p2m_domain *p2m)
>>       return 0;
>>   }
>>   
>> -int p2m_init(struct domain *d)
>> +static const struct gstage_mode_desc *find_gstage_mode_by_bits(
> 
> Is "_by_bits" adding much value to the function name? Especially ...
> 
>> +    unsigned char gpa_bits)
> 
> ... seeing that the parameter name is making things pretty clear?

Maybe not too much, I will drop that.

> 
>> +int p2m_init(struct domain *d, const struct xen_domctl_createdomain *config)
>>   {
>> +    /*
>> +     * TODO: This static is a temporary constraint: all guests must use the
>> +     * same MMU mode because p2m_gpa_bits is not yet per-domain.
>> +     * Drop this once per-domain p2m_gpa_bits is introduced.
>> +     */
>> +    static const struct gstage_mode_desc __ro_after_init *m = &gstage_modes[0];
>>       struct p2m_domain *p2m = p2m_get_hostp2m(d);
>>   
>>       /*
>> @@ -341,6 +361,33 @@ int p2m_init(struct domain *d)
>>        */
>>       p2m->domain = d;
>>   
>> +    if ( !config )
>> +    {
>> +        dprintk(XENLOG_ERR, "NULL config is passed\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    p2m->mode = find_gstage_mode_by_bits(config->arch.gstage_addr_bits);
>> +
>> +    if ( !p2m->mode )
>> +    {
>> +        dprintk(XENLOG_ERR,
>> +                "Unsupported or unavailable gstage addr bits: %u\n",
>> +                config->arch.gstage_addr_bits);
>> +
>> +        return -EINVAL;
>> +    }
>> +
>> +    if ( m->mode == HGATP_MODE_OFF )
>> +        m = p2m->mode;
>> +
>> +    if ( m->mode != p2m->mode->mode )
> 
> Since m always points into gstage_modes[], do you really need the extra
> indirection to compare the two ->mode fields? You could simply compare
> the pointers, couldn't you?

Agree, just "if ( m != p2m->mode )" will work.

> 
>> --- a/xen/include/public/arch-riscv.h
>> +++ b/xen/include/public/arch-riscv.h
>> @@ -56,6 +56,11 @@ typedef struct vcpu_guest_context vcpu_guest_context_t;
>>   DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
>>   
>>   struct xen_arch_domainconfig {
>> +    /*
>> +     * G-stage GPA address width in bits.
>> +     * Valid values: 34 (sv32x4), 41 (sv39x4), 50 (sv48x4), 59 (sv57x4).
>> +     */
>> +    unsigned char gstage_addr_bits;
> 
> Fixed-width types only in the public interface please.

I will use uint8_t then.

> 
> Also, isn't the field effectively describing the maximum width of a
> guest (physical) address? In which case - simply gaddr_bits?

gaddr_bits would be okay in this case.

Thanks.

~ Oleksii

Re: [PATCH v4 07/11] xen/riscv: add Linux kernel loading support

[Oleksii Kurochko]

On 5/4/26 4:05 PM, Jan Beulich wrote:
>> --- a/xen/include/xen/fdt-kernel.h
>> +++ b/xen/include/xen/fdt-kernel.h
>> @@ -59,8 +59,15 @@ struct kernel_info {
>>           struct {
>>               paddr_t kernel_addr;
>>               paddr_t len;
>> -#if defined(CONFIG_ARM_64) || defined(CONFIG_RISCV_64)
>> -            paddr_t text_offset; /* 64-bit Image only */
>> +#if defined(CONFIG_ARM_64) || defined(CONFIG_RISCV)
>> +            /*
>> +             * ARM: 64-bit Image only.
>> +             * RISC-V: both 32-bit and 64-bit Images.
>> +             */
>> +            paddr_t text_offset;
>> +#endif
>> +#if defined(CONFIG_RISCV)
>> +            uint64_t image_size; /* Effective size of Image */
> As this (apparently) is for both RV64 and RV32 - can the latter really have
> wider than 32-bit image sizes? If not - use size_t or unsigned long here?

It seems like we want to have uint64_t as it is explicitly mentioned in 
image header:

https://elixir.bootlin.com/linux/v7.0.1/source/arch/riscv/include/asm/image.h#L57

and also it is used .dword here:

https://elixir.bootlin.com/linux/v7.0.1/source/arch/riscv/kernel/head.S#L55

~ Oleksii

Re: [PATCH v4 07/11] xen/riscv: add Linux kernel loading support

[Jan Beulich]

On 06.05.2026 16:53, Oleksii Kurochko wrote:
> 
> 
> On 5/4/26 4:05 PM, Jan Beulich wrote:
>>> --- a/xen/include/xen/fdt-kernel.h
>>> +++ b/xen/include/xen/fdt-kernel.h
>>> @@ -59,8 +59,15 @@ struct kernel_info {
>>>           struct {
>>>               paddr_t kernel_addr;
>>>               paddr_t len;
>>> -#if defined(CONFIG_ARM_64) || defined(CONFIG_RISCV_64)
>>> -            paddr_t text_offset; /* 64-bit Image only */
>>> +#if defined(CONFIG_ARM_64) || defined(CONFIG_RISCV)
>>> +            /*
>>> +             * ARM: 64-bit Image only.
>>> +             * RISC-V: both 32-bit and 64-bit Images.
>>> +             */
>>> +            paddr_t text_offset;
>>> +#endif
>>> +#if defined(CONFIG_RISCV)
>>> +            uint64_t image_size; /* Effective size of Image */
>> As this (apparently) is for both RV64 and RV32 - can the latter really have
>> wider than 32-bit image sizes? If not - use size_t or unsigned long here?
> 
> It seems like we want to have uint64_t as it is explicitly mentioned in 
> image header:
> 
> https://elixir.bootlin.com/linux/v7.0.1/source/arch/riscv/include/asm/image.h#L57
> 
> and also it is used .dword here:
> 
> https://elixir.bootlin.com/linux/v7.0.1/source/arch/riscv/kernel/head.S#L55

Well. The image header having a 64-bit wide field still raises the
question how the value being wider than 32 bits would work on RV32. IOW
for Xen internal purposes I expect unsigned long or size_t would be more
appropriate. When that value is taken from the image header and copied
into the internal struct, it would need checking for absence of
truncation.

Jan

Re: [PATCH v4 04/11] xen: introduce domain-layout.h with common domain_use_host_layout()

[Oleksii Kurochko]

On 5/4/26 2:59 PM, Jan Beulich wrote:
> On 28.04.2026 16:33, Oleksii Kurochko wrote:
>> domain_use_host_layout() is not architecture-specific and may be needed
>> on x86 [1]. Replace the ARM-specific macro in asm/domain.h with a common
>> static inline in a new dedicated header, xen/domain-layout.h.
>>
>> xen/domain.h would be the natural home, but placing it there would
>> require including xen/paging.h (for paging_mode_translate()) and
>> xen/sched.h (for is_hardware_domain()), which would introduce circular
>> dependencies. A separate header that callers opt into avoids this.
>>
>> Adjust the implementation to take paging_mode_translate() into account
>> so it works correctly for all architectures, including x86. Some extra
>> details about implementation [2] and [3].
>>
>> [1] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602161038120.359097@ubuntu-linux-20-04-desktop/
>> [2] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602271742400.3148344@ubuntu-linux-20-04-desktop/
>> [3] https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2602271750190.3148344@ubuntu-linux-20-04-desktop/
>>
>> Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> With the SPDX tag added as indicated by Luca:
> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks.

> 
> One further minor remark:
> 
>> --- /dev/null
>> +++ b/xen/include/xen/domain-layout.h
>> @@ -0,0 +1,27 @@
>> +#ifndef __XEN_DOMAIN_LAYOUT_H__
>> +#define __XEN_DOMAIN_LAYOUT_H__
>> +
>> +#include <xen/domain.h>
> 
> This isn't really needed. It is ...
> 
>> +#include <xen/paging.h>
>> +#include <xen/sched.h>
> 
> ... included by this one anyway (pretty much unavoidably right now, I
> guess).

Then it will be needed to re-order them.

Do you want to put the comment above xen/sched.h:

/*
  * Ensure xen/sched.h is included before xen/paging.h, since paging.h 
depends
  * on xen/domain.h, which is pulled in via sched.h.
  */
#include <xen/sched.h>
#include <xen/paging.h>

or just add this to commit message instead?

~ Oleksii

> 
> Jan

Re: [PATCH v4 04/11] xen: introduce domain-layout.h with common domain_use_host_layout()

[Jan Beulich]

On 06.05.2026 17:47, Oleksii Kurochko wrote:
> On 5/4/26 2:59 PM, Jan Beulich wrote:
>> On 28.04.2026 16:33, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/include/xen/domain-layout.h
>>> @@ -0,0 +1,27 @@
>>> +#ifndef __XEN_DOMAIN_LAYOUT_H__
>>> +#define __XEN_DOMAIN_LAYOUT_H__
>>> +
>>> +#include <xen/domain.h>
>>
>> This isn't really needed. It is ...
>>
>>> +#include <xen/paging.h>
>>> +#include <xen/sched.h>
>>
>> ... included by this one anyway (pretty much unavoidably right now, I
>> guess).
> 
> Then it will be needed to re-order them.
> 
> Do you want to put the comment above xen/sched.h:
> 
> /*
>   * Ensure xen/sched.h is included before xen/paging.h, since paging.h 
> depends
>   * on xen/domain.h, which is pulled in via sched.h.
>   */
> #include <xen/sched.h>
> #include <xen/paging.h>
> 
> or just add this to commit message instead?

No. xen/paging.h only includes two asm/*.h, so doesn't itself require anything.
If there's anything missing for that header to be included first, I would assume
it's then RISC-V's asm/paging.h or asm/p2m.h which lack a necessary #include?
Yet without you indicating what exactly the missing piece is, this is somewhat
guesswork on my part.

Jan

Re: [PATCH v4 04/11] xen: introduce domain-layout.h with common domain_use_host_layout()

[Oleksii Kurochko]

On 5/7/26 9:25 AM, Jan Beulich wrote:
> On 06.05.2026 17:47, Oleksii Kurochko wrote:
>> On 5/4/26 2:59 PM, Jan Beulich wrote:
>>> On 28.04.2026 16:33, Oleksii Kurochko wrote:
>>>> --- /dev/null
>>>> +++ b/xen/include/xen/domain-layout.h
>>>> @@ -0,0 +1,27 @@
>>>> +#ifndef __XEN_DOMAIN_LAYOUT_H__
>>>> +#define __XEN_DOMAIN_LAYOUT_H__
>>>> +
>>>> +#include <xen/domain.h>
>>>
>>> This isn't really needed. It is ...
>>>
>>>> +#include <xen/paging.h>
>>>> +#include <xen/sched.h>
>>>
>>> ... included by this one anyway (pretty much unavoidably right now, I
>>> guess).
>>
>> Then it will be needed to re-order them.
>>
>> Do you want to put the comment above xen/sched.h:
>>
>> /*
>>    * Ensure xen/sched.h is included before xen/paging.h, since paging.h
>> depends
>>    * on xen/domain.h, which is pulled in via sched.h.
>>    */
>> #include <xen/sched.h>
>> #include <xen/paging.h>
>>
>> or just add this to commit message instead?
> 
> No. xen/paging.h only includes two asm/*.h, so doesn't itself require anything.
> If there's anything missing for that header to be included first, I would assume
> it's then RISC-V's asm/paging.h or asm/p2m.h which lack a necessary #include?
> Yet without you indicating what exactly the missing piece is, this is somewhat
> guesswork on my part.

The first time I read compiler error inattentively.

The following compilation error occurs after xen/domain.h is dropped 
from xen/domain-layout.h:

In file included from ./include/xen/paging.h:4,
                  from ./include/xen/domain-layout.h:6,
                  from common/device-tree/domain-build.c:4:
./arch/riscv/include/asm/paging.h:17:48: error: 'struct page_info' 
declared inside parameter list will not be visible outside of this 
definition or declaration [-Werror]
    17 | void paging_free_page(struct domain *d, struct page_info *pg);

So the correct fix is to add forward declaration of struct page_info to 
RISC-V's asm/paging.h.

I will add the following to commit message:
"
To avoid the following compilation issue:

In file included from ./include/xen/paging.h:4,
                  from ./include/xen/domain-layout.h:6,
                  from common/device-tree/domain-build.c:4:
./arch/riscv/include/asm/paging.h:17:48: error: 'struct page_info' 
declared inside parameter list will not be visible outside of this 
definition or declaration [-Werror]
    17 | void paging_free_page(struct domain *d, struct page_info *pg);

add the forward declaration of struct page_info to RISC-V's asm/paging.h.
"

~ Oleksii