Re: [PATCH v1 1/3] xen/riscv: introduce setup_initial_pages

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/02/2023 3:06 pm, Oleksii Kurochko wrote:
> diff --git a/xen/arch/riscv/include/asm/page.h 
> b/xen/arch/riscv/include/asm/page.h
> new file mode 100644
> index 0000000000..fabbe1305f
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/page.h
> @@ -0,0 +1,90 @@
> +#ifndef _ASM_RISCV_PAGE_H
> +#define _ASM_RISCV_PAGE_H
> +
> +#include <xen/const.h>
> +#include <xen/types.h>
> +
> +#define PAGE_ENTRIES            512
> +#define VPN_BITS                (9)
> +#define VPN_MASK                ((unsigned long)((1 << VPN_BITS) - 1))
> +
> +#ifdef CONFIG_RISCV_64
> +/* L3 index Bit[47:39] */
> +#define THIRD_SHIFT             (39)
> +#define THIRD_MASK              (VPN_MASK << THIRD_SHIFT)
> +/* L2 index Bit[38:30] */
> +#define SECOND_SHIFT            (30)
> +#define SECOND_MASK             (VPN_MASK << SECOND_SHIFT)
> +/* L1 index Bit[29:21] */
> +#define FIRST_SHIFT             (21)
> +#define FIRST_MASK              (VPN_MASK << FIRST_SHIFT)
> +/* L0 index Bit[20:12] */
> +#define ZEROETH_SHIFT           (12)
> +#define ZEROETH_MASK            (VPN_MASK << ZEROETH_SHIFT)

Don't name these with words.  That's an error ultimately inherited from
an architectural mistake ARM.

These should be named L1 (4k) thru L4 (512T), and don't need separate
separate masks or shifts because it looks like RISC-V designed their
pagetables in a coherent and uniform way.

You'll find the code simplifies substantially if you have
PAGETABLE_ORDER 9 somewhere in here.

The shift is always (PAGE_ORDER + level * PAGETABLE_ORDER), and it's
rare that you need something other than "(addr >> shift) & mask".  About
the only time you need a virtual address masked but unshifted is for
debugging.

~Andrew