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
