On 16.12.2022 12:48, Julien Grall wrote:
> PMAP will be used in a follow-up patch to bootstap map domain
> page infrastructure -- we need some way to map pages to setup the
> mapcache without a direct map.
But this isn't going to be needed overly early then, seeing that ...
> --- a/xen/arch/x86/include/asm/fixmap.h
> +++ b/xen/arch/x86/include/asm/fixmap.h
> @@ -21,6 +21,8 @@
>
> #include <xen/acpi.h>
> #include <xen/pfn.h>
> +#include <xen/pmap.h>
> +
> #include <asm/apicdef.h>
> #include <asm/msi.h>
> #include <acpi/apei.h>
> @@ -54,6 +56,8 @@ enum fixed_addresses {
> FIX_XEN_SHARED_INFO,
> #endif /* CONFIG_XEN_GUEST */
> /* Everything else should go further down. */
> + FIX_PMAP_BEGIN,
> + FIX_PMAP_END = FIX_PMAP_BEGIN + NUM_FIX_PMAP,
... you've inserted the new entries after the respective comment? Is
there a reason you don't insert farther towards the end of this
enumeration?
> --- /dev/null
> +++ b/xen/arch/x86/include/asm/pmap.h
> @@ -0,0 +1,25 @@
> +#ifndef __ASM_PMAP_H__
> +#define __ASM_PMAP_H__
> +
> +#include <asm/fixmap.h>
> +
> +static inline void arch_pmap_map(unsigned int slot, mfn_t mfn)
> +{
> + unsigned long linear = (unsigned long)fix_to_virt(slot);
> + l1_pgentry_t *pl1e = &l1_fixmap[l1_table_offset(linear)];
> +
> + ASSERT(!(l1e_get_flags(*pl1e) & _PAGE_PRESENT));
> +
> + l1e_write_atomic(pl1e, l1e_from_mfn(mfn, PAGE_HYPERVISOR));
> +}
> +
> +static inline void arch_pmap_unmap(unsigned int slot)
> +{
> + unsigned long linear = (unsigned long)fix_to_virt(slot);
> + l1_pgentry_t *pl1e = &l1_fixmap[l1_table_offset(linear)];
> +
> + l1e_write_atomic(pl1e, l1e_empty());
> + flush_tlb_one_local(linear);
> +}
You're effectively open-coding {set,clear}_fixmap(), just without
the L1 table allocation (should such be necessary). If you depend
on using the build-time L1 table, then you need to move your
entries ahead of said comment. But independent of that you want
to either use the existing macros / functions, or explain why you
can't.
Jan
