Re: [PATCH v5 2/3] xen/mm: Switch some APIs over to pte_attr_t

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 4/25/25 1:24 PM, Andrew Cooper wrote:

> From: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
>
> Several APIs take an architecture-dependent set of flags in an unsigned int,
> but this needs to be a wider type to support PPC.
>
> The new type pte_attr_t has been introduced for this purpose, so switch to it
> in map_pages_to_xen(), __vmap() and modify_xen_mappings{,_lite}().
>
> No functional change.
>
> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> CC: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> CC: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  xen/arch/arm/mmu/pt.c   | 4 ++--
>  xen/arch/ppc/mm-radix.c | 2 +-
>  xen/arch/riscv/pt.c     | 2 +-
>     
As I wrote here, probably, as a part of this patch we also wants to do the following changes:
  https://lore.kernel.org/xen-devel/192683d3-8777-45c2-b8a8-546bf171a7aa@xxxxxxxxx/T/#u

I'm okay with sending the mentioned changes in the link as a separate patch when I will introduce
Svpmbt extension which uses 61 and 62 bits. Let me know if you are okay with that?

If you think that these changes should be part of this patch then something similar for Arm should
be done too now.

~ Oleksii
>  xen/arch/x86/mm.c       | 6 +++---
>  xen/common/efi/boot.c   | 4 ++--
>  xen/common/vmap.c       | 2 +-
>  xen/include/xen/mm.h    | 7 ++++---
>  xen/include/xen/vmap.h  | 3 ++-
>  8 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/xen/arch/arm/mmu/pt.c b/xen/arch/arm/mmu/pt.c
> index 11cb1c66dac8..4726e713efd3 100644
> --- a/xen/arch/arm/mmu/pt.c
> +++ b/xen/arch/arm/mmu/pt.c
> @@ -696,7 +696,7 @@ static int xen_pt_update(unsigned long virt,
>  int map_pages_to_xen(unsigned long virt,
>                       mfn_t mfn,
>                       unsigned long nr_mfns,
> -                     unsigned int flags)
> +                     pte_attr_t flags)
>  {
>      return xen_pt_update(virt, mfn, nr_mfns, flags);
>  }
> @@ -714,7 +714,7 @@ int destroy_xen_mappings(unsigned long s, unsigned long e)
>      return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, 0);
>  }
>  
> -int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf)
> +int modify_xen_mappings(unsigned long s, unsigned long e, pte_attr_t nf)
>  {
>      ASSERT(IS_ALIGNED(s, PAGE_SIZE));
>      ASSERT(IS_ALIGNED(e, PAGE_SIZE));
> diff --git a/xen/arch/ppc/mm-radix.c b/xen/arch/ppc/mm-radix.c
> index 9a00ae416af0..d5385ec9dd4b 100644
> --- a/xen/arch/ppc/mm-radix.c
> +++ b/xen/arch/ppc/mm-radix.c
> @@ -265,7 +265,7 @@ int destroy_xen_mappings(unsigned long s, unsigned long e)
>  int map_pages_to_xen(unsigned long virt,
>                       mfn_t mfn,
>                       unsigned long nr_mfns,
> -                     unsigned int flags)
> +                     pte_attr_t flags)
>  {
>      BUG_ON("unimplemented");
>  }
> diff --git a/xen/arch/riscv/pt.c b/xen/arch/riscv/pt.c
> index 857619d48df1..918b1b91abde 100644
> --- a/xen/arch/riscv/pt.c
> +++ b/xen/arch/riscv/pt.c
> @@ -504,7 +504,7 @@ static int pt_update(vaddr_t virt, mfn_t mfn,
>  int map_pages_to_xen(unsigned long virt,
>                       mfn_t mfn,
>                       unsigned long nr_mfns,
> -                     unsigned int flags)
> +                     pte_attr_t flags)
>  {
>      /*
>       * Ensure that flags has PTE_VALID bit as map_pages_to_xen() is supposed
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 1cf236516789..0e6c766be4aa 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5442,7 +5442,7 @@ int map_pages_to_xen(
>      unsigned long virt,
>      mfn_t mfn,
>      unsigned long nr_mfns,
> -    unsigned int flags)
> +    pte_attr_t flags)
>  {
>      bool locking = system_state > SYS_STATE_boot;
>      l3_pgentry_t *pl3e = NULL, ol3e;
> @@ -5860,7 +5860,7 @@ int __init populate_pt_range(unsigned long virt, unsigned long nr_mfns)
>   *
>   * It is an error to call with present flags over an unpopulated range.
>   */
> -int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf)
> +int modify_xen_mappings(unsigned long s, unsigned long e, pte_attr_t nf)
>  {
>      bool locking = system_state > SYS_STATE_boot;
>      l3_pgentry_t *pl3e = NULL;
> @@ -6156,7 +6156,7 @@ int destroy_xen_mappings(unsigned long s, unsigned long e)
>   * the non-inclusive boundary will be updated.
>   */
>  void init_or_livepatch modify_xen_mappings_lite(
> -    unsigned long s, unsigned long e, unsigned int nf)
> +    unsigned long s, unsigned long e, pte_attr_t nf)
>  {
>      unsigned long v = s, fm, flags;
>  
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index 143b5681ba92..e39fbc3529c4 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1676,7 +1676,7 @@ void __init efi_init_memory(void)
>      struct rt_extra {
>          struct rt_extra *next;
>          unsigned long smfn, emfn;
> -        unsigned int prot;
> +        pte_attr_t prot;
>      } *extra, *extra_head = NULL;
>  
>      free_ebmalloc_unused_mem();
> @@ -1691,7 +1691,7 @@ void __init efi_init_memory(void)
>          EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
>          u64 len = desc->NumberOfPages << EFI_PAGE_SHIFT;
>          unsigned long smfn, emfn;
> -        unsigned int prot = PAGE_HYPERVISOR_RWX;
> +        pte_attr_t prot = PAGE_HYPERVISOR_RWX;
>          paddr_t mem_base;
>          unsigned long mem_npages;
>  
> diff --git a/xen/common/vmap.c b/xen/common/vmap.c
> index 47225fecc067..d6991421f3f7 100644
> --- a/xen/common/vmap.c
> +++ b/xen/common/vmap.c
> @@ -222,7 +222,7 @@ static void vm_free(const void *va)
>  }
>  
>  void *__vmap(const mfn_t *mfn, unsigned int granularity,
> -             unsigned int nr, unsigned int align, unsigned int flags,
> +             unsigned int nr, unsigned int align, pte_attr_t flags,
>               enum vmap_region type)
>  {
>      void *va = vm_alloc(nr * granularity, align, type);
> diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
> index ae1c48a61545..e89942b87d1e 100644
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -64,6 +64,7 @@
>  #include <xen/bug.h>
>  #include <xen/compiler.h>
>  #include <xen/mm-frame.h>
> +#include <xen/mm-types.h>
>  #include <xen/types.h>
>  #include <xen/list.h>
>  #include <xen/spinlock.h>
> @@ -113,11 +114,11 @@ int map_pages_to_xen(
>      unsigned long virt,
>      mfn_t mfn,
>      unsigned long nr_mfns,
> -    unsigned int flags);
> +    pte_attr_t flags);
>  /* Alter the permissions of a range of Xen virtual address space. */
> -int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf);
> +int modify_xen_mappings(unsigned long s, unsigned long e, pte_attr_t nf);
>  void modify_xen_mappings_lite(unsigned long s, unsigned long e,
> -                              unsigned int nf);
> +                              pte_attr_t nf);
>  int destroy_xen_mappings(unsigned long s, unsigned long e);
>  /* Retrieve the MFN mapped by VA in Xen virtual address space. */
>  mfn_t xen_map_to_mfn(unsigned long va);
> diff --git a/xen/include/xen/vmap.h b/xen/include/xen/vmap.h
> index 26c831757a11..327a2597826d 100644
> --- a/xen/include/xen/vmap.h
> +++ b/xen/include/xen/vmap.h
> @@ -9,6 +9,7 @@
>  #define __XEN_VMAP_H__
>  
>  #include <xen/mm-frame.h>
> +#include <xen/mm-types.h>
>  #include <xen/page-size.h>
>  
>  /* Identifiers for the linear ranges tracked by vmap */
> @@ -57,7 +58,7 @@ void vm_init_type(enum vmap_region type, void *start, void *end);
>   * @return Pointer to the mapped area on success; NULL otherwise.
>   */
>  void *__vmap(const mfn_t *mfn, unsigned int granularity, unsigned int nr,
> -             unsigned int align, unsigned int flags, enum vmap_region type);
> +             unsigned int align, pte_attr_t flags, enum vmap_region type);
>  
>  /*
>   * Map an array of pages contiguously into the VMAP_DEFAULT vmap region
>