Re: [PATCH 13/22] xen/x86: Add support for the PMAP

[Jan Beulich <jbeulich@xxxxxxxx>]

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