Re: [PATCH v1 6/6] xen/riscv: enable DOMAIN_BUILD_HELPERS

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 2/12/26 5:39 PM, Jan Beulich wrote:
> On 12.02.2026 17:21, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/include/asm/p2m.h
> > +++ b/xen/arch/riscv/include/asm/p2m.h
> > @@ -44,6 +44,9 @@
> >   #define P2M_LEVEL_MASK(p2m, lvl) \
> >       (P2M_TABLE_OFFSET(p2m, lvl) << P2M_GFN_LEVEL_SHIFT(lvl))
> >   
> > +/* Holds the bit size of IPAs in p2m tables */
> > +extern unsigned int p2m_ipa_bits;
> Hmm, I can spot a declaration and ...
>
> > --- a/xen/arch/riscv/p2m.c
> > +++ b/xen/arch/riscv/p2m.c
> > @@ -51,6 +51,12 @@ static struct gstage_mode_desc __ro_after_init 
> > max_gstage_mode = {
> >       .name = "Bare",
> >   };
> >   
> > +/*
> > + * Set to the maximum configured support for IPA bits, so the number of IPA 
> > bits can be
> > + * restricted by external entity (e.g. IOMMU).
> > + */
> > +unsigned int __read_mostly p2m_ipa_bits = PADDR_BITS;
> ... a definition, but neither a use nor a place where the variable would
> be set. Hmm, yes, I see common/device-tree/domain-build.c uses it. Then
> the following questions arise:
> - What does "ipa" stand for? Is this a term sensible in RISC-V context at
>    all?
IPA stands for GPA. (maybe it would better to rename the in common code to gpa 
too).
It was used as common code uses p2m_ipa_bits.

Yes, I miss to set p2m_ipa_bits properly in p2m_init() where G-stage MMU mode 
is set.

> Judging from the comment at the decl, isn't it PPN width (plus
>    PAGE_SHIFT) that it describes?
It is PPN width + PAGE_SHIFT what is equal to PADDR_BITS (44bit PPN + 12 bit 
PAGE_SHIFT).

> - With there not being anyone writing to the variable, why is it not
>    const (or even a #define), or at the very least __ro_after_init?
> And no, "Arm has it like this" doesn't count as an answer. Considering
> all the review comments you've got so far you should know by now that you
> shouldn't copy things blindly.
It was added because of the usage in common/device-tree/domain-build.c.

It was done in the same way as it is also possible that an IOMMU shares the P2M 
page
tables with the CPU's G-stage(stage-2) translation, so GPA size must not exceed 
what
the IOMMU can handle (or G-stage address limitation if it is smaller then 
IOMMU's).

(a) It could be that MMU uses Sv57, IOMMU uses Sv39, so in this case if IOMMU 
and MMU
shares G-stae page tables it is necessary to respect guest address limitation.

But considering that according to RISC-V IOMMU spec ... :
  The IOMMU must support all the virtual memory extensions that are supported by
  any of the harts in the system.
... (a) isn't real issue as we could always program IOMMU to use the same as 
MMU mode
and then p2m_ipa_bits __ro_after_init should work well. It can't be const as I 
mentioned
above I missed to initialize it properly in p2m_init() code. (It is also a case 
for RISC-V
that IOMMU could use x4 mode, so MMU uses Sv57 and IOMMU uses Sv57x4.)

> > --- a/xen/include/public/arch-riscv.h
> > +++ b/xen/include/public/arch-riscv.h
> > @@ -50,6 +50,14 @@ typedef uint64_t xen_ulong_t;
> >   
> >   #if defined(__XEN__) || defined(__XEN_TOOLS__)
> >   
> > +#define GUEST_RAM_BANKS   1
> > +
> > +#define GUEST_RAM0_BASE   xen_mk_ullong(0x80000000) /* 2GB of low RAM @ 2GB */
> > +#define GUEST_RAM0_SIZE   xen_mk_ullong(0x80000000)
> > +
> > +#define GUEST_RAM_BANK_BASES   { GUEST_RAM0_BASE }
> > +#define GUEST_RAM_BANK_SIZES   { GUEST_RAM0_SIZE }
> Hmm, does RISC-V really want to go with compile-time constants here?
It is needed for allocate_memory() for guest domains, so it is expected
to be compile-time constant with the current code of common dom0less
approach.

It represents the start of RAM address for DomU and the maximum RAM size
(the actual size will be calculated based on what is mentioned in DomU node
in dts) and then will be used to generate memory node for DomU (GUEST_RAM0_BASE
as RAM start address and min(GUEST_RAM0_SIZE, dts->domU->memory->size) as a
RAM size).

>   And
> if so, why would guests be limited to just 2 Gb?
It is enough for guest domain I am using in dom0less mode.

> That may more efficiently
> be RV32 guests then, with perhaps just an RV32 hypervisor.
I  didn't get this point. Could you please explain differently what do you
mean?

~ Oleksii