[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 27/27] xen/riscv: add initial dom0less infrastructure support





On 4/7/26 4:11 PM, Jan Beulich wrote:
On 10.03.2026 18:09, Oleksii Kurochko wrote:
Enable dom0less support for RISC-V by selecting HAS_DOM0LESS and
providing the minimal architecture hooks required by the common
dom0less infrastructure.

Add stub implementations for architecture-specific helpers used when
building domains from the device tree. These currently perform no
additional work but allow the generic dom0less code to build and run
on RISC-V.

Introduce max_init_domid as a runtime variable rather than a constant
so that it can be updated during dom0less domain creation.

Provide missing helpers and definitions required by the domain
construction code,

I'm wondering about the splitting among patches: There's half a dozen
(effectively stub) functions which are added here, and then there is
the single init_vuart() which was split out into the earlier patch.
What's the pattern behind this, i.e. why isn't init_vuart() also
being added here?

If it'll be more convenient I am okay to merge prev. patch with the current one.

init_vuart() is in a separate patch as it has some useful functionality inside and thereby it will be more convenient to review.


including domain bitness helpers and the
p2m_set_allocation() prototype.

Additionally define the guest magic memory region in the public
RISC-V interface.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Open questions:
  - Shouldn't declaration/defintion of max_init_domid move to common code
    instead of having it for each architecture separately? If yes, then what
    would be the best place.

What would you use to decide whether the declaration or #define is
needed? (Plausible headers to put it can surely be found: console.h,
domain.h, and perhaps more.)

I thought about to wrap that with CONFIG_DOM0LESS_BOOT as the declaration is needed only for this case, for all other cases it is just #define.

Or as an option we could always use declaration all the time. It won't increase size of Xen too much or decrease performance because of variable access.


--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -20,6 +20,14 @@ struct hvm_domain
      uint64_t              params[HVM_NR_PARAMS];
  };
+#ifdef CONFIG_RISCV_64
+#define is_32bit_domain(d) (0)
+#define is_64bit_domain(d) (1)
+#else
+#define is_32bit_domain(d) (1)
+#define is_64bit_domain(d) (0)
+#endif

First, please use true/false. Then, while I agree with the RV32 part, 32-bit
guests surely will need to be an option on a 64-bit hypervisor. Imo you'd
better introduced a field in struct arch_domain to carry that information
(or to derive it from) right away. That wouldn't be set to non-zero for the
time being, i.e. that same constant-true/false would still result.

Otherwise I don't see why you use #ifdef; you could then have things
simpler as

#define is_32bit_domain(d) IS_ENABLED(CONFIG_RISCV_32)
#define is_64bit_domain(d) IS_ENABLED(CONFIG_RISCV_64)

(but I specifically don't recommend going this route).

I will introduce a type in struct arch_domain then.


--- a/xen/include/public/arch-riscv.h
+++ b/xen/include/public/arch-riscv.h
@@ -58,6 +58,9 @@ typedef uint64_t xen_ulong_t;
  #define GUEST_RAM_BANK_BASES   { GUEST_RAM0_BASE }
  #define GUEST_RAM_BANK_SIZES   { GUEST_RAM0_SIZE }
+#define GUEST_MAGIC_BASE xen_mk_ullong(0x39000000)
+#define GUEST_MAGIC_SIZE  xen_mk_ullong(0x01000000)

What is this, and why does it need putting in the public interface?

In other patch series such related things will live in asm/guest-layout.h. It will be moved there after rebase on top of that patch series.

It is needed now only for common dom0less code compilation as at the moment we don't use xenstore pages for dom0less.

But generally it is region which is used to allocate "magic pages" -special pages that facilitate communication between the guest and the Xen hypervisor such as Console, XenStore pages etc.

I have in my TODO to understand how to remove requirement to have this fixed region from dom0less common code.

Plus
how come the numbers are exactly the same as what Arm uses?

It could be different. But this region is free for RISC-V too so it is fine to re-use.

~ Oleksii



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.