On 22.07.2024 16:36, Oleksii wrote: > On Mon, 2024-07-22 at 14:42 +0200, Jan Beulich wrote: >> On 12.07.2024 18:22, Oleksii Kurochko wrote: >>> --- a/xen/arch/riscv/include/asm/config.h >>> +++ b/xen/arch/riscv/include/asm/config.h >>> @@ -74,11 +74,20 @@ >>> #error "unsupported RV_STAGE1_MODE" >>> #endif >>> >>> +#define XEN_SIZE MB(2) >>> +#define XEN_VIRT_END (XEN_VIRT_START + XEN_SIZE) >>> + >>> +#define BOOT_FDT_VIRT_START XEN_VIRT_END >>> +#define BOOT_FDT_VIRT_SIZE MB(4) >>> + >>> #define DIRECTMAP_SLOT_END 509 >>> #define DIRECTMAP_SLOT_START 200 >>> #define DIRECTMAP_VIRT_START SLOTN(DIRECTMAP_SLOT_START) >>> #define DIRECTMAP_SIZE (SLOTN(DIRECTMAP_SLOT_END) - >>> SLOTN(DIRECTMAP_SLOT_START)) >>> >>> +#define FIXMAP_BASE (BOOT_FDT_VIRT_START + >>> BOOT_FDT_VIRT_SIZE) >>> +#define FIXMAP_ADDR(n) (FIXMAP_BASE + (n) * PAGE_SIZE) >> >> Why exactly do you insert this here, and not adjacent to >> BOOT_FDT_VIRT_*, >> which it actually is adjacent with? > I tried to follow alphabetical order.
Oh, X before B (just making fun) ... Anyway, my take here is that sorting by address is going to be more helpful. >>> --- a/xen/arch/riscv/mm.c >>> +++ b/xen/arch/riscv/mm.c >>> @@ -49,6 +49,9 @@ stage1_pgtbl_root[PAGETABLE_ENTRIES]; >>> pte_t __section(".bss.page_aligned") __aligned(PAGE_SIZE) >>> stage1_pgtbl_nonroot[PGTBL_INITIAL_COUNT * PAGETABLE_ENTRIES]; >>> >>> +pte_t __section(".bss.page_aligned") __aligned(PAGE_SIZE) >>> +xen_fixmap[PAGETABLE_ENTRIES]; >> >> Any reason this cannot be static? > It will be used by pmap: > static inline void arch_pmap_map(unsigned int slot, mfn_t mfn) > { > pte_t *entry = &xen_fixmap[slot]; > pte_t pte; > > ASSERT(!pte_is_valid(*entry)); > > pte = mfn_to_xen_entry(mfn, PAGE_HYPERVISOR_RW); > pte.pte |= PTE_LEAF_DEFAULT; > write_pte(entry, pte); > } > > static inline void arch_pmap_unmap(unsigned int slot) > { > pte_t pte = {}; > > write_pte(&xen_fixmap[slot], pte); > } Yet as asked there - shouldn't that be set_fixmap() and clear_fixmap()? Jan