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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2023-04-24 at 11:33 +0200, Jan Beulich wrote:
> On 21.04.2023 16:41, Oleksii wrote:
> > On Thu, 2023-04-20 at 14:58 +0200, Jan Beulich wrote:
> > > On 19.04.2023 17:42, Oleksii Kurochko wrote:
> > > > + *
> > > > ===============================================================
> > > > ====
> > > > =========
> > > > + *    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 ).
> 
> As to the plan to it being 56 bytes (i.e. like on Arm): Arm forever
> has
> had a 64-bit padding field at the end. My best guess is that the
> field
> was introduced to have a 32-byte struct on Arm32. But then why
> artificially increase the struct from 48 to 56 bytes on Arm64? And
> hence
> why have the same oddity on RV64?
I didn't think about that. I just re-used page_info structure from ARM.
I'll think about your comment. Thanks.

~ Oleksii