On 23.01.2024 17:50, Oleksii wrote: > On Tue, 2024-01-23 at 12:32 +0100, Jan Beulich wrote: >> On 22.12.2023 16:13, Oleksii Kurochko wrote: >>> @@ -22,25 +30,56 @@ >>> * >>> * It means that: >>> * top VA bits are simply ignored for the purpose of translating >>> to PA. >>> +#endif >>> * >>> - * >>> =================================================================== >>> ========= >>> - * Start addr | End addr | Size | Slot >>> |area description >>> - * >>> =================================================================== >>> ========= >>> - * FFFFFFFFC0800000 | FFFFFFFFFFFFFFFF |1016 MB | L2 511 | >>> Unused >>> - * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | L2 511 | >>> Fixmap >>> - * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | L2 511 | >>> FDT >>> - * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | L2 511 | >>> Xen >>> - * ... | 1 GB | L2 510 | >>> Unused >>> - * 0000003200000000 | 0000007F80000000 | 309 GB | L2 200-509 | >>> Direct map >>> - * ... | 1 GB | L2 199 | >>> Unused >>> - * 0000003100000000 | 00000031C0000000 | 3 GB | L2 196-198 | >>> Frametable >>> - * ... | 1 GB | L2 195 | >>> Unused >>> - * 0000003080000000 | 00000030C0000000 | 1 GB | L2 194 | >>> VMAP >>> - * ... | 194 GB | L2 0 - 193 | >>> Unused >>> - * >>> =================================================================== >>> ========= >>> + * 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 >>> * >>> -#endif >>> + * >>> =================================================================== >>> ============ >>> + * Start addr | End addr | Size | Root PT slot | >>> Area description >>> + * >>> =================================================================== >>> ============ >>> + * BA0 + 0x800000 | FFFFFFFFFFFFFFFF |1016 MB | 511 | >>> Unused >>> + * BA0 + 0x400000 | BA0 + 0x800000 | 2 MB | 511 | >>> Fixmap >>> + * BA0 + 0x200000 | BA0 + 0x400000 | 4 MB | 511 | >>> FDT >>> + * BA0 | BA0 + 0x200000 | 2 MB | 511 | >>> Xen >>> + * ... | 1 GB | 510 | >>> Unused >>> + * BA1 + 0x000000 | BA1 + 0x4D80000000 | 309 GB | 200-509 | >>> Direct map >> >> This definitely can't be right for SV32. Others may be problematic, >> too, like ... >> >>> + * ... | 1 GB | 199 | >>> Unused >>> + * BA2 + 0x000000 | BA2 + 0xC0000000 | 3 GB | 196-198 | >>> Frametable >> >> ... this one. Otoh I'd expect both to potentially be much larger in >> SV48 and SV57 modes. > Regarding Sv32, it looks to me the only BA0 and End addr at the first > line isn't correct as address size is 32. > > Regarding other modes, yes, it should be changed Size column. Also, the > size of frame table should be recalculated. > > Do we really need size column? > > Wouldn't it be enough only have PT slot number?
Perhaps. > Would it be better to have separate table for each mode? Don't know. >>> +#define VPN_BITS (9) >> >> This need to move ... >> >>> +#define HYP_PT_ROOT_LEVEL (CONFIG_PAGING_LEVELS - 1) >>> + >>> +#ifdef CONFIG_RISCV_64 >> >> ... here, I think, for not being applicable to SV32? > You are right, it is not applicable for Sv32. In case of Sv32, it > should be 10. > But I am not sure that it is correct only to move this definition as > RISCV-64 can also use Sv32. So it looks like VPN_BITS should be "#ifdef > RV_STAGE1_MODE == Sv32". Can it? The spec talks of SXLEN=32 implying SV32, while SXLEN=64 permits SV39, SV48, and SV57. No mention of SV32 there. >>> +#define SLOTN_ENTRY_BITS (HYP_PT_ROOT_LEVEL * VPN_BITS + >>> PAGE_SHIFT) >>> +#define SLOTN(slot) (_AT(vaddr_t, slot) << >>> SLOTN_ENTRY_BITS) >>> +#define SLOTN_ENTRY_SIZE SLOTN(1) >> >> Do you have any example of how/where this going to be used? > Yes, it will be used to define DIRECTMAP_SIZE: > #define DIRECTMAP_SIZE (SLOTN_ENTRY_SIZE * (509-200)) How about #define DIRECTMAP_SIZE (SLOTN(509) - SLOTN(200)) instead? >>> +#define XEN_VIRT_START 0xFFFFFFFFC0000000 /* (_AC(-1, UL) + 1 - >>> GB(1)) */ >> >> Won't /* -GB(1) */ do, thus allowing the line to also be padded such >> that >> it matches neighboring ones in layout? > Could you please clarify what do you mean by padded here? The intention > was to show that 1 GB is used for Xen, FDT and fixmap. I'm talking of blank padding in the source file. Note how preceding and following #define-s blank-pad expansions so they all align. Just this one in the middle does not. Jan >>> +#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) >>> [...] >> >
