On 21.12.2023 20:59, Oleksii wrote:
> On Mon, 2023-12-18 at 12:22 +0100, Jan Beulich wrote:
>> 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.
> I came up with the following but I am not sure that it is really
> convient:
> /*
> * RISC-V64 Layout:
> *
> #if RV_STAGE1_MODE == SATP_MODE_SV39
> *
> * 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.
> #endif
> *
> * SATP_MODE_SV32 SATP_MODE_SV39 SATP_MODE_SV48
> SATP_MODE_SV57
> * ----------------------------------------------------------------
> -----------
> * BA0 | FFFFFFFFFFE00000 | FFFFFFFFC0000000 | FFFFFF8000000000 |
> FFFF000000000000
> * BA1 | 0000000019000000 | 0000003200000000 | 0000640000000000 |
> 00C8000000000000
> * BA2 | 0000000018800000 | 0000003100000000 | 0000620000000000 |
> 00C4000000000000
> * BA3 | 0000000018400000 | 0000003080000000 | 0000610000000000 |
> 00C2000000000000
> *
> *
> =======================================================================
> =====
> * Start addr | End addr | Size | Slot |area
> description
> *
> =======================================================================
> =====
> * BA0 + 0x800000 | FFFFFFFFFFFFFFFF |1016 MB |
> L${HYP_PT_ROOT_LEVEL} 511 | Unused
> * BA0 + 0x400000 | BA0 + 0x800000 | 2 MB |
> L${HYP_PT_ROOT_LEVEL} 511 | Fixmap
> * BA0 + 0x200000 | BA0 + 0x400000 | 4 MB |
> L${HYP_PT_ROOT_LEVEL} 511 | FDT
> * BA0 | BA0 + 0x200000 | 2 MB |
> L${HYP_PT_ROOT_LEVEL} 511 | Xen
> * ... | 1 GB |
> L${HYP_PT_ROOT_LEVEL} 510 | Unused
> * BA1 + 0x000000 | BA1 + 0x4D80000000 | 309 GB |
> L${HYP_PT_ROOT_LEVEL} 200-509 | Direct map
> * ... | 1 GB |
> L${HYP_PT_ROOT_LEVEL} 199 | Unused
> * BA2 + 0x000000 | BA2 + 0xC0000000 | 3 GB |
> L${HYP_PT_ROOT_LEVEL} 196-198 | Frametable
> * ... | 1 GB |
> L${HYP_PT_ROOT_LEVEL} 195 | Unused
> * BA3 + 0x000000 | BA3 + 0x40000000 | 1 GB |
> L${HYP_PT_ROOT_LEVEL} 194 | VMAP
> * ... | 194 GB |
> L${HYP_PT_ROOT_LEVEL} 0 - 193 | Unused
> *
> =======================================================================
> =====
> */
>
> Do you have better ideas?
It doesn't look too bad imo, at the first glance, albeit the line
wrapping damage of course makes it a little hard to look at. In the
last table with all lines saying L${HYP_PT_ROOT_LEVEL}, perhaps that
could be put in the table heading (instead of "Slot" say e.g. "Root
PT slot")?
Jan
