[PATCH v3 1/2] asm-generic: Introduce mm-types.h header

[PATCH v3 1/2] asm-generic: Introduce mm-types.h header

[Shawn Anastasio]

Introduce a new header, mm-types.h, which will be used to define
architecture-specific types pertinent to memory management. This will be
used by a future commit to enable >32 bit PTE flags.

Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Suggested-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v3:
  - Introduced this file per Jan's suggestion

 xen/arch/arm/include/asm/Makefile   | 1 +
 xen/arch/ppc/include/asm/Makefile   | 1 +
 xen/arch/riscv/include/asm/Makefile | 1 +
 xen/arch/x86/include/asm/Makefile   | 1 +
 xen/include/asm-generic/mm-types.h  | 5 +++++
 5 files changed, 9 insertions(+)
 create mode 100644 xen/include/asm-generic/mm-types.h

diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index 4a4036c951..f8249b2439 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -3,6 +3,7 @@ generic-y += altp2m.h
 generic-y += device.h
 generic-y += hardirq.h
 generic-y += iocap.h
+generic-y += mm-types.h
 generic-y += paging.h
 generic-y += percpu.h
 generic-y += random.h
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index c989a7f89b..c0dbc68ac6 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -5,6 +5,7 @@ generic-y += div64.h
 generic-y += hardirq.h
 generic-y += hypercall.h
 generic-y += iocap.h
+generic-y += mm-types.h
 generic-y += paging.h
 generic-y += percpu.h
 generic-y += perfc_defn.h
diff --git a/xen/arch/riscv/include/asm/Makefile b/xen/arch/riscv/include/asm/Makefile
index c989a7f89b..c0dbc68ac6 100644
--- a/xen/arch/riscv/include/asm/Makefile
+++ b/xen/arch/riscv/include/asm/Makefile
@@ -5,6 +5,7 @@ generic-y += div64.h
 generic-y += hardirq.h
 generic-y += hypercall.h
 generic-y += iocap.h
+generic-y += mm-types.h
 generic-y += paging.h
 generic-y += percpu.h
 generic-y += perfc_defn.h
diff --git a/xen/arch/x86/include/asm/Makefile b/xen/arch/x86/include/asm/Makefile
index 2c27787d31..26650707e6 100644
--- a/xen/arch/x86/include/asm/Makefile
+++ b/xen/arch/x86/include/asm/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 generic-y += div64.h
+generic-y += mm-types.h
diff --git a/xen/include/asm-generic/mm-types.h b/xen/include/asm-generic/mm-types.h
new file mode 100644
index 0000000000..26490e48db
--- /dev/null
+++ b/xen/include/asm-generic/mm-types.h
@@ -0,0 +1,5 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_MM_TYPES_H__
+#define __ASM_GENERIC_MM_TYPES_H__
+
+#endif /* __ASM_GENERIC_MM_TYPES_H__ */
--
2.30.2

[PATCH v3 2/2] xen/mm: Introduce per-arch pte_attr_t type for PTE flags

[Shawn Anastasio]

Xen's memory management APIs map_pages_to_xen, modify_xen_mappings,
set_fixmap, ioremap_attr, and __vmap all use an unsigned int to
represent architecture-dependent page table entry flags. This assumption
is not well-suited for PPC/radix where some flags go past 32-bits, so
introduce the pte_attr_t type to allow architectures to opt in to larger
types to store PTE flags.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
---
Changes in v3:
  - Use new asm/mm-types.h to pull in pte_attr_t definition when
  necessary.
  - Drop define+ifdef since pte_attr_t is now always defined.

Changes in v2:
  - Drop Kconfig option and use `#define pte_attr_t pte_attr_t` for arches to
  opt-in to defining the type.
  - Move default pte_attr_definition to xen/types.h
  - Update commit message to reflect that this change isn't strictly
  necessary for arches w/ >32bit pte flags

 xen/arch/ppc/include/asm/Makefile   | 1 -
 xen/arch/ppc/include/asm/mm-types.h | 7 +++++++
 xen/arch/ppc/mm-radix.c             | 2 +-
 xen/common/efi/boot.c               | 5 +++--
 xen/common/vmap.c                   | 2 +-
 xen/include/asm-generic/mm-types.h  | 2 ++
 xen/include/xen/mm.h                | 5 +++--
 xen/include/xen/vmap.h              | 4 +++-
 8 files changed, 20 insertions(+), 8 deletions(-)
 create mode 100644 xen/arch/ppc/include/asm/mm-types.h

diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index c0dbc68ac6..c989a7f89b 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -5,7 +5,6 @@ generic-y += div64.h
 generic-y += hardirq.h
 generic-y += hypercall.h
 generic-y += iocap.h
-generic-y += mm-types.h
 generic-y += paging.h
 generic-y += percpu.h
 generic-y += perfc_defn.h
diff --git a/xen/arch/ppc/include/asm/mm-types.h b/xen/arch/ppc/include/asm/mm-types.h
new file mode 100644
index 0000000000..0cb850f4f6
--- /dev/null
+++ b/xen/arch/ppc/include/asm/mm-types.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_PPC_MM_TYPES_H__
+#define __ASM_PPC_MM_TYPES_H__
+
+typedef unsigned long pte_attr_t;
+
+#endif /* __ASM_PPC_MM_TYPES_H__ */
diff --git a/xen/arch/ppc/mm-radix.c b/xen/arch/ppc/mm-radix.c
index 24232f3907..e02dffa7c5 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/common/efi/boot.c b/xen/common/efi/boot.c
index efbec00af9..999dbce4dc 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1,4 +1,5 @@
 #include "efi.h"
+#include <asm/mm-types.h>
 #include <efi/efiprot.h>
 #include <efi/efipciio.h>
 #include <public/xen.h>
@@ -1656,7 +1657,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();
@@ -1671,7 +1672,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 47225fecc0..d6991421f3 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/asm-generic/mm-types.h b/xen/include/asm-generic/mm-types.h
index 26490e48db..9eb3cba698 100644
--- a/xen/include/asm-generic/mm-types.h
+++ b/xen/include/asm-generic/mm-types.h
@@ -2,4 +2,6 @@
 #ifndef __ASM_GENERIC_MM_TYPES_H__
 #define __ASM_GENERIC_MM_TYPES_H__

+typedef unsigned int pte_attr_t;
+
 #endif /* __ASM_GENERIC_MM_TYPES_H__ */
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 16f733281a..bdb71a99ca 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -69,6 +69,7 @@
 #include <xen/spinlock.h>
 #include <xen/perfc.h>
 #include <public/memory.h>
+#include <asm/mm-types.h>

 struct page_info;

@@ -113,9 +114,9 @@ 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);
 int destroy_xen_mappings(unsigned long s, unsigned long e);
diff --git a/xen/include/xen/vmap.h b/xen/include/xen/vmap.h
index 26c831757a..e1155ed14a 100644
--- a/xen/include/xen/vmap.h
+++ b/xen/include/xen/vmap.h
@@ -8,8 +8,10 @@
 #ifndef __XEN_VMAP_H__
 #define __XEN_VMAP_H__

+#include <xen/mm.h>
 #include <xen/mm-frame.h>
 #include <xen/page-size.h>
+#include <asm/mm-types.h>

 /* Identifiers for the linear ranges tracked by vmap */
 enum vmap_region {
@@ -57,7 +59,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
--
2.30.2

Re: [PATCH v3 1/2] asm-generic: Introduce mm-types.h header

[Jan Beulich]

On 21.02.2025 21:10, Shawn Anastasio wrote:
> Introduce a new header, mm-types.h, which will be used to define
> architecture-specific types pertinent to memory management. This will be
> used by a future commit to enable >32 bit PTE flags.
> 
> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
> Suggested-by: Jan Beulich <jbeulich@suse.com>

Perhaps implicit from the Suggested-by:, but nevertheless:
Acked-by: Jan Beulich <jbeulich@suse.com>

Just one nit: Tags generally want to be in chronological order.

Jan

Re: [PATCH v3 2/2] xen/mm: Introduce per-arch pte_attr_t type for PTE flags

[Jan Beulich]

On 21.02.2025 21:10, Shawn Anastasio wrote:
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -69,6 +69,7 @@
>  #include <xen/spinlock.h>
>  #include <xen/perfc.h>
>  #include <public/memory.h>
> +#include <asm/mm-types.h>
> 
>  struct page_info;
> 
> @@ -113,9 +114,9 @@ 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);

These declaration adjustments need to be carried through to all definitions,
not just PPC's. Without doing so there'll be new Misra violations (requiring
that declaration and definition agree not just in effective types, but also
in spelling), just like ...

> --- a/xen/include/xen/vmap.h
> +++ b/xen/include/xen/vmap.h
> @@ -8,8 +8,10 @@
>  #ifndef __XEN_VMAP_H__
>  #define __XEN_VMAP_H__
> 
> +#include <xen/mm.h>
>  #include <xen/mm-frame.h>
>  #include <xen/page-size.h>
> +#include <asm/mm-types.h>
> 
>  /* Identifiers for the linear ranges tracked by vmap */
>  enum vmap_region {
> @@ -57,7 +59,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);

... you already do for __vmap().

Jan

Re: [PATCH v3 2/2] xen/mm: Introduce per-arch pte_attr_t type for PTE flags

[Shawn Anastasio]

On 2/26/25 7:26 AM, Jan Beulich wrote:
> On 21.02.2025 21:10, Shawn Anastasio wrote:
>> --- a/xen/include/xen/mm.h
>> +++ b/xen/include/xen/mm.h
>> @@ -69,6 +69,7 @@
>>  #include <xen/spinlock.h>
>>  #include <xen/perfc.h>
>>  #include <public/memory.h>
>> +#include <asm/mm-types.h>
>>
>>  struct page_info;
>>
>> @@ -113,9 +114,9 @@ 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);
> 
> These declaration adjustments need to be carried through to all definitions,
> not just PPC's. Without doing so there'll be new Misra violations (requiring
> that declaration and definition agree not just in effective types, but also
> in spelling),

Understood -- I figured that if pte_attr_t was ever defined as non-int
on the other arches the non-matching declarations would raise a compiler
warning and thus it'd be fine to leave them for now, but didn't consider
MISRA. Will update.

Thanks,
Shawn