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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.02.2025 00:05, Shawn Anastasio wrote:
> On 2/6/25 6:29 AM, Jan Beulich wrote:
>> On 05.02.2025 22:02, 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 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
>>
>> I can't seem to be able to associate this with anything in the commit
>> message. The comment here to me reads as if this was optional (but then
>> for arches with <=32-bit PTE flags), yet in the description I can't spot
>> anything to the same effect. Recall that it was said before that on x86
>> we also have flags extending beyond bit 31, just that we pass them
>> around in a compacted manner (which, as Andrew has been suggesting, may
>> be undue extra overhead).
>>
> 
> Admittedly the change was subtle, but I changed the wording in the
> commit message as follows:
> 
> - This assumption does not work on PPC/radix where some flags go past
>   32-bits, so [...]
> + This assumption is not well-suited for PPC/radix where some flags go
>   past 32-bits, so [...]
> 
> 
> The softening of "does not work" to "is not well-suited" was meant to
> address your earlier comment and clarify that the change is not strictly
> necessary. Though as you and Andrew pointed out x86_64 is able to make
> do with the 32 bits, I would still argue that the hardcoded `unsigned
> int` flags type is still not well-suited to that application.

Oh, okay, fair enough then.

Jan