Re: [PATCH v2 30/39] xen/riscv: define an address of frame table

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.12.2023 11:36, Oleksii wrote:
> On Thu, 2023-12-14 at 16:48 +0100, Jan Beulich wrote:
>> On 24.11.2023 11:30, Oleksii Kurochko wrote:
>>> +#define SLOTN_ENTRY_SIZE        SLOTN(1)
>>> +
>>>  #define XEN_VIRT_START 0xFFFFFFFFC0000000 /* (_AC(-1, UL) + 1 -
>>> GB(1)) */
>>> +
>>> +#define FRAMETABLE_VIRT_START   SLOTN(196)
>>> +#define FRAMETABLE_SIZE         GB(3)
>>> +#define FRAMETABLE_NR           (FRAMETABLE_SIZE /
>>> sizeof(*frame_table))
>>> +#define FRAMETABLE_VIRT_END     (FRAMETABLE_VIRT_START +
>>> FRAMETABLE_SIZE - 1)
>>> +
>>> +#define VMAP_VIRT_START         SLOTN(194)
>>> +#define VMAP_VIRT_SIZE          GB(1)
>>
>> May I suggest that you keep these blocks sorted by slot number? Or
>> wait,
>> the layout comment further up is also in decreasing order, so that's
>> fine here, but then can all of this please be moved next to the
>> comment
>> actually providing the necessary context (thus eliminating the need
>> for
>> new comments)?
> Sure, I'll put this part close to layout comment.
> 
>>  You'll then also notice that the generalization here
>> (keeping basically the same layout for e.g. SATP_MODE_SV48, just
>> shifted
>> by 9 bits) isn't in line with the comment there.
> Does it make sense to add another one table with updated addresses for
> SATP_MODE_SV48?

Well, especially if you mean to support that mode, its layout surely
wants writing down. I was hoping though that maybe you/we could get away
without multiple tables, but e.g. use one having multiple columns.

Jan

> Microchip has h/w which requires SATP_MODE_SV48 ( at least ), so I have
> a patch which introduces SATP_MODE_SV48 and I planned to update the
> layout table in this patch.