[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 2/2] xen/mm: Introduce per-arch pte_attr_t type for PTE flags
- To: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- From: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
- Date: Fri, 25 Apr 2025 13:11:56 +0200
- Cc: tpearson@xxxxxxxxxxxxxxxxxxxxx, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Alistair Francis <alistair.francis@xxxxxxx>, Bob Eshleman <bobbyeshleman@xxxxxxxxx>, Connor Davis <connojdavis@xxxxxxxxx>, "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>, Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Fri, 25 Apr 2025 11:12:09 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 3/10/25 10:22 AM, Oleksii Kurochko
wrote:
On 3/6/25 7:25 PM, Shawn Anastasio
wrote:
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@xxxxxxxxxx>
Signed-off-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
---
Changes in v4:
- Change definitions of map_pages_to_xen, modify_xen_mappings in all arches
to match new prototype.
- Use new flag types in modify_xen_mappings_lite as well (previously missed)
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/arm/mmu/pt.c | 4 ++--
xen/arch/ppc/include/asm/Makefile | 1 -
xen/arch/ppc/include/asm/mm-types.h | 7 +++++++
xen/arch/ppc/mm-radix.c | 2 +-
xen/arch/riscv/pt.c | 2 +-
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
Also as the part of this patch the following changes should be considered (I can send that as
a separate patch):
$ git diff
diff --git a/xen/arch/riscv/include/asm/fixmap.h b/xen/arch/riscv/include/asm/fixmap.h
index e399a15f53..5990c964aa 100644
--- a/xen/arch/riscv/include/asm/fixmap.h
+++ b/xen/arch/riscv/include/asm/fixmap.h
@@ -33,7 +33,7 @@
extern pte_t xen_fixmap[];
/* Map a page in a fixmap entry */
-void set_fixmap(unsigned int map, mfn_t mfn, unsigned int flags);
+void set_fixmap(unsigned int map, mfn_t mfn, pte_attr_t flags);
/* Remove a mapping from a fixmap entry */
void clear_fixmap(unsigned int map);
diff --git a/xen/arch/riscv/include/asm/page.h b/xen/arch/riscv/include/asm/page.h
index 3904d42a71..e23d69214e 100644
--- a/xen/arch/riscv/include/asm/page.h
+++ b/xen/arch/riscv/include/asm/page.h
@@ -217,7 +217,7 @@ static inline pte_t read_pte(const pte_t *p)
return read_atomic(p);
}
-static inline pte_t pte_from_mfn(mfn_t mfn, unsigned int flags)
+static inline pte_t pte_from_mfn(mfn_t mfn, pte_attr_t flags)
{
unsigned long pte = (mfn_x(mfn) << PTE_PPN_SHIFT) | flags;
return (pte_t){ .pte = pte };
diff --git a/xen/arch/riscv/pt.c b/xen/arch/riscv/pt.c
index ef84b6b078..d6cc68b7db 100644
--- a/xen/arch/riscv/pt.c
+++ b/xen/arch/riscv/pt.c
@@ -25,7 +25,7 @@ static inline mfn_t get_root_page(void)
* See the comment about the possible combination of (mfn, flags) in
* the comment above pt_update().
*/
-static bool pt_check_entry(pte_t entry, mfn_t mfn, unsigned int flags)
+static bool pt_check_entry(pte_t entry, mfn_t mfn, pte_attr_t flags)
{
/* Sanity check when modifying an entry. */
if ( (flags & PTE_VALID) && mfn_eq(mfn, INVALID_MFN) )
@@ -260,7 +260,7 @@ pte_t pt_walk(vaddr_t va, unsigned int *pte_level)
*/
static int pt_update_entry(mfn_t root, vaddr_t virt,
mfn_t mfn, unsigned int *target,
- unsigned int flags)
+ pte_attr_t flags)
{
int rc;
/*
@@ -355,7 +355,7 @@ static int pt_update_entry(mfn_t root, vaddr_t virt,
/* Return the level where mapping should be done */
static int pt_mapping_level(unsigned long vfn, mfn_t mfn, unsigned long nr,
- unsigned int flags)
+ pte_attr_t flags)
{
unsigned int level = 0;
unsigned long mask;
@@ -409,7 +409,7 @@ static DEFINE_SPINLOCK(pt_lock);
* inserting will be done.
*/
static int pt_update(vaddr_t virt, mfn_t mfn,
- unsigned long nr_mfns, unsigned int flags)
+ unsigned long nr_mfns, pte_attr_t flags)
{
int rc = 0;
unsigned long vfn = PFN_DOWN(virt);
@@ -537,7 +537,7 @@ int __init populate_pt_range(unsigned long virt, unsigned long nr_mfns)
}
/* Map a 4k page in a fixmap entry */
-void set_fixmap(unsigned int map, mfn_t mfn, unsigned int flags)
+void set_fixmap(unsigned int map, mfn_t mfn, pte_attr_t flags)
{
if ( map_pages_to_xen(FIXMAP_ADDR(map), mfn, 1, flags | PTE_SMALL) != 0 )
BUG();
~ Oleksii
~ Oleksii
xen/arch/x86/mm.c | 6 +++---
xen/common/efi/boot.c | 5 +++--
xen/common/vmap.c | 2 +-
xen/include/asm-generic/mm-types.h | 2 ++
xen/include/xen/mm.h | 7 ++++---
xen/include/xen/vmap.h | 4 +++-
11 files changed, 27 insertions(+), 15 deletions(-)
create mode 100644 xen/arch/ppc/include/asm/mm-types.h
diff --git a/xen/arch/arm/mmu/pt.c b/xen/arch/arm/mmu/pt.c
index da28d669e7..9dc99db352 100644
--- a/xen/arch/arm/mmu/pt.c
+++ b/xen/arch/arm/mmu/pt.c
@@ -701,7 +701,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);
}
@@ -719,7 +719,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/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/arch/riscv/pt.c b/xen/arch/riscv/pt.c
index 857619d48d..918b1b91ab 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 bfdc8fb019..53c17c6f88 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5472,7 +5472,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;
@@ -5890,7 +5890,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;
@@ -6186,7 +6186,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 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..e79f1728c3 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,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 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
|