Re: [PATCH v3 27/34] xen/riscv: define an address of frame table

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.12.2023 16:13, Oleksii Kurochko wrote:
> Also, the patch adds some helpful macros that assist in avoiding
> the redefinition of memory layout for each MMU mode.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
> Changes in V3:
>  - drop OFFSET_BITS, and use PAGE_SHIFT instead.
>  - code style fixes.
>  - add comment how macros are useful.
>  - move all memory related layout definitions close to comment with memory 
> layout description.
>  - make memory layout description generic for any MMU mode.
> ---
> Changes in V2:
>  - Nothing changed. Only rebase.
> ---
>  xen/arch/riscv/include/asm/config.h | 85 +++++++++++++++++++----------
>  1 file changed, 55 insertions(+), 30 deletions(-)
> 
> diff --git a/xen/arch/riscv/include/asm/config.h 
> b/xen/arch/riscv/include/asm/config.h
> index f0544c6a20..fb9fc9daaa 100644
> --- a/xen/arch/riscv/include/asm/config.h
> +++ b/xen/arch/riscv/include/asm/config.h
> @@ -6,6 +6,14 @@
>  #include <xen/const.h>
>  #include <xen/page-size.h>
>  
> +#ifdef CONFIG_RISCV_64
> +#define CONFIG_PAGING_LEVELS 3
> +#define RV_STAGE1_MODE SATP_MODE_SV39
> +#else
> +#define CONFIG_PAGING_LEVELS 2
> +#define RV_STAGE1_MODE SATP_MODE_SV32
> +#endif
> +
>  /*
>   * RISC-V64 Layout:
>   *
> @@ -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.

> + *                 ...                  |  1 GB  |     195      | Unused
> + * BA3 + 0x000000 |  BA3 + 0x40000000   |  1 GB  |     194      | VMAP
> + *                 ...                  | 194 GB |   0 - 193    | Unused
> + * 
> ===============================================================================
>   */
> +#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?

> +#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?

> +#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?

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)
>[...]