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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.04.2023 12:22, Julien Grall wrote:
> Hi,
> 
> On 24/04/2023 10:33, 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. 
> 
> I can't exactly remember. But I would like to rework the struct 
> page_info on Arm64 because...
> 
> But then why
>> artificially increase the struct from 48 to 56 bytes on Arm64? And hence
>> why have the same oddity on RV64?
> 
> 
> ... with 56 bytes, some struct page_info may cross a cache boundary.

I guess that's going to be challenging, unless you mean to go further up
to 64 bytes?

> For 
> RISC-V, I would recommend to make sure the struct page_info will never 
> cross a cache boundary.

Since going up to 64 bytes is wasteful, and going down to 32 bytes likely
isn't going to be easy, sticking to 48 bytes for now would seem reasonable
to me.

Jan