On 09/01/2025 9:15 am, Jan Beulich wrote: > On 31.12.2024 19:27, 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 >> does not work on 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.coop...@citrix.com> >> Signed-off-by: Shawn Anastasio <sanasta...@raptorengineering.com> > As iirc Andrew had indicated when suggesting this, what you say for PPC is > true for x86 as well. Yet still we get around with unsigned int. Hence I > think the change needs describing differently.
x86 is currently unsigned int, and with some reasonably expensive packing/unpacking under the hood. x86 ought to become unsigned long, now we don't have the 32bit build to worry about. (back of the envelope calculation says up/down: 400/-3868 (-3468) but I've not checked that this boots). > >> --- a/xen/include/xen/mm.h >> +++ b/xen/include/xen/mm.h >> @@ -70,6 +70,10 @@ >> #include <xen/perfc.h> >> #include <public/memory.h> >> >> +#ifndef CONFIG_HAS_PTE_ATTR_T >> +typedef unsigned int pte_attr_t; >> +#endif > This imo makes the Kconfig control a misnomer: We'll always have pte_attr_t. > I wonder whether this actually needs a Kconfig setting in the first place: > It's hardly the end of the world for all architectures to specify the type > (later: the underlying type, for the real type to become type-safe) > explicitly. All architectures strictly need this. It's not optional, so doesn't warrant a Kconfig option. Either, arch/mm.h is required to provide the typedef, or we could have a common one as so: #ifndef pte_attr_t typedef unsigned int pte_attr_t; #endif which architectures can influence with: typedef unsigned long pte_attr_t; #define pte_attr_t pte_attr_t in the usual way. One thing however does occur to me. ARM and RISC-V have systems with MPU protections rather than MMU, with Xen already starting to support this on ARM. Therefore we might want to reconsider the name pte_attr_t to be something slightly less pagetable specific. Then again, I'm not sure how much overlap there is between the map* functions and MPUs, where mapping is of course not possible. Finally, this wants to be at least 2 patches. One introducing pte_attr_t, and one changing PPC to be unsigned long. ~Andrew
