Re: [PATCH v5 1/4] xen/riscv: add VM space layout

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Thu, 2023-04-20 at 14:58 +0200, Jan Beulich wrote:
> On 19.04.2023 17:42, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/include/asm/config.h
> > +++ b/xen/arch/riscv/include/asm/config.h
> > @@ -4,6 +4,37 @@
> >  #include <xen/const.h>
> >  #include <xen/page-size.h>
> >  
> > +/*
> > + * RISC-V64 Layout:
> > + *
> > + * 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.
> > + *
> > + * The similar is true for other Sv{32, 39, 48, 57}.
> 
> Personally I find it odd that Sv32 is mentioned here (there's no
> truncation /
> aliasing there aiui), and that Sv39 is mentioned here again (when
> that's what
> the earlier paragraph talks about).
It looks that I wrote bad explanation.

I meant that all the mentioned above ( that usable address space isn't
based on zero-extension ) works for Sv{39, 48, 57} except size of
usable address space ( 39-bit, 48-bit and 57-bit usable address space
of 64-bit virtual address space ).

And I agree that this is not for Sv32 as it covers full 32-bit virtual
address space

> 
> > + *
> > ===================================================================
> > =========
> > + *    Start addr    |   End addr        |  Size  | VM area
> > description
> > + *
> > ===================================================================
> > =========
> > + * FFFFFFFFC0000000 |  FFFFFFFFC0200000 |  2 MB  | Xen
> > + * FFFFFFFFC0200000 |  FFFFFFFFC0600000 |  4 MB  | FDT
> > + * FFFFFFFFC0600000 |  FFFFFFFFC0800000 |  2 MB  | Fixmap
> 
> These are all L2 slot 511 aiui, which may be worth mentioning
> especially since
> the top bits don't match the top bits further down in the table
> (because of the
> aliasing).

Than I'll add one more column where I'll put slot number

> 
> > + *     .................. unused ..................
> 
> This is covering slot 510, which again may be worth mentioning.
> 
> > + * 0000003200000000 |  0000007f40000000 | 331 GB | Direct map(L2
> > slot: 200-509)
> > + * 0000003100000000 |  0000003140000000 |  1 GB  | Frametable(L2
> > slot: 196-197)
> 
> 1Gb is, if I'm not mistaken, a single L2 slot.
Yeah, it can be misunderstood. I meant [196, 197), so 197 isn't
included. I'll update the table.

> 
> Also assuming a 32-byte struct page_info (I don't think you'll get
> away with
> less than that, when even Arm32 requires this much), there's a
> mismatch
> between direct map and frame table size: With 4k pages, the scaling
> factor
> would be 128 if I'm not mistaken. So perhaps you really mean 3Gb here
> to
> cover for (slightly more than) the 331Gb of memory you mean to be
> able to
> map?
For RV64 page_info size will be 56-byte and 32-byte for RV32 but you
are right it should 3 Gb in that case what will be enough ( taking into
account both available sizes of page_info structure ).
Thanks for the catch.
> 
> > + * 0000003080000000 |  00000030c0000000 |  1 GB  | VMAP (L2 slot:
> > 194-195)
> > + *     .................. unused ..................
> 
> There are further unused regions between these three entries, while
> imo will
> want making explicit (for the avoidance of doubt, if nothing else).
Yeah, there are some. I'll add them explicitly.


~ Oleksii