Re: [PATCH v4 1/5] x86: extend update_intpte() to support atomic get-and-update

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/10/2026 5:48 pm, Kevin Lampis wrote:
> The update_intpte() now accepts a new swap flag and if set
> returns the old pte value.
>
> No functional change for existing callers.
>
> Signed-off-by: Kevin Lampis <kevin.lampis@xxxxxxxxxx>
> ---
> Changes in v4:
> - Remove UPDATE_ENTRY_SWAP
> - Don't move the backslashes in UPDATE_ENTRY
>
> Changes in v3:
> - Return 0 in the paging_write_guest_entry() case (update_intpte)
> - Add UPDATE_ENTRY_SWAP to convert return value to correct type
>
> Changes in v2:
> - Add new PTE_UPDATE_SWAP flag instead of a bool argument
> - Return the old pte value instead of turning `old` into an out pointer
> ---
>  xen/arch/x86/pv/mm.h | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/pv/mm.h b/xen/arch/x86/pv/mm.h
> index bfee0feb7b21..381a0e25b986 100644
> --- a/xen/arch/x86/pv/mm.h
> +++ b/xen/arch/x86/pv/mm.h
> @@ -64,16 +64,22 @@ static inline intpte_t paging_cmpxchg_guest_entry(
>  
>  #define PTE_UPDATE_PRESERVE_AD  (1u << 0)
>  #define PTE_UPDATE_NO_TRANSLATE (1u << 1)
> +#define PTE_UPDATE_SWAP         (1u << 2)
>  
>  /*
>   * How to write an entry to the guest pagetables.
> + * If the SWAP flag is set then does an atomic swap and returns the old PTE
> + * value, otherwise returns 0.

This isn't quite true.  It will also return the old PTE for PRESERVE_AD,
and unconditionally in debug builds which set PTE_UPDATE_WITH_CMPXCHG.

I'd be tempted to just finish the sentence at "value."   SWAP is the
only one we care about giving a guarantee of a return, and the other
cases are far easier to see from looking at the function rather than
attempting to describe in the comment.

I can fix this up on commit if you're happy.

~Andrew