Re: [PATCH v7 1/5] xen/riscv: add VM space layout

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Tue, 2023-05-16 at 17:42 +0200, Jan Beulich wrote:
> On 11.05.2023 19:09, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/include/asm/config.h
> > +++ b/xen/arch/riscv/include/asm/config.h
> > @@ -4,6 +4,42 @@
> >  #include <xen/const.h>
> >  #include <xen/page-size.h>
> >  
> > +/*
> > + * RISC-V64 Layout:
> > + *
> > + * #ifdef SV39
> 
> I did point you at x86'es similar #ifdef. Unlike here, there we use a
> symbol which actually has a meaning, allowing to spot this comment in
> e.g. grep output when looking for uses of that symbol. Hence here
> e.g.
> 
> #ifdef RV_STAGE1_MODE == SATP_MODE_SV39
> 
> ? (I would also recommend to use the same style as x86 does, such
> that
> the #ifdef and #endif look like normal directives [e.g. again in grep
> output], leaving aside that they're inside a comment.)
It would be better. Thanks.
> 
> > + * From the riscv-privileged doc:
> > + *   When mapping between narrower and wider addresses,
> > + *   RISC-V zero-extends a narrower physical address to a wider
> > size.
> > + *   The mapping between 64-bit virtual addresses and the 39-bit
> > usable
> > + *   address space of Sv39 is not based on zero-extension but
> > instead
> > + *   follows an entrenched convention that allows an OS to use one
> > or
> > + *   a few of the most-significant bits of a full-size (64-bit)
> > virtual
> > + *   address to quickly distinguish user and supervisor address
> > regions.
> > + *
> > + * It means that:
> > + *   top VA bits are simply ignored for the purpose of translating
> > to PA.
> > + *
> > + *
> > ===================================================================
> > =========
> > + *    Start addr    |   End addr        |  Size  | Slot      
> > |area description
> > + *
> > ===================================================================
> > =========
> > + * FFFFFFFFC0800000 |  FFFFFFFFFFFFFFFF |1016 MB | L2 511     |
> > Unused
> > + * FFFFFFFFC0600000 |  FFFFFFFFC0800000 |  2 MB  | L2 511     |
> > Fixmap
> > + * FFFFFFFFC0200000 |  FFFFFFFFC0600000 |  4 MB  | L2 511     |
> > FDT
> > + * FFFFFFFFC0000000 |  FFFFFFFFC0200000 |  2 MB  | L2 511     |
> > Xen
> > + *                 ...                  |  1 GB  | L2 510     |
> > Unused
> > + * 0000003200000000 |  0000007f40000000 | 309 GB | L2 200-509 |
> > Direct map
> 
> The upper bound here is 0000007f80000000 afaict, 
It should be 0000007f80000000. 0000007f40000000 is start address of 509
slot.

> which then also makes
> the earlier gap 1Gb in size.
do you mean that it is better to write start and end address (
0000007f80000000 - 7FC0000000 ) of L2 510 slot explicitly?

~ Oleksii