Re: [PATCH v4 2/5] x86: extend mod_l*_entry() to optionally return the old PTE value

[chunjie.zhu@xxxxxxxxxx]

On Fri, Oct 16, 2026 at 05:48:12 PM, Kevin Lampis <kevin.lampis@xxxxxxxxxx> 
wrote:

> Calling mod_l*_entry() with a valid ol*e_out pointer will do atomic
> CMPXCHG to set the new pte value and return the old pte value through
> the ol*e_out pointer. If the ol*e_out pointer is NULL then old behavior
> is preserved.
>
> Signed-off-by: Kevin Lampis <kevin.lampis@xxxxxxxxxx>
> ---
> Changes in v4:
> - Refactor how UPDATE_ENTRY is used
> - Update commit message to specify CMPXCHG instead of xchg
>
> Changes in v3:
> - Add swap to mod_l{2,3,4}_entry()
>
> Changes in v2:
> - UPDATE_ENTRY now returns the old pte value instead of using an out
>   pointer
> ---
>  xen/arch/x86/mm.c | 106 ++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 84 insertions(+), 22 deletions(-)
>
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index e42cd33c0d09..609c8595a487 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -2193,7 +2193,8 @@ static void l3t_unlock(struct page_info *page)
>  /* Update the L1 entry at pl1e to new value nl1e. */
>  static int mod_l1_entry(l1_pgentry_t *pl1e, l1_pgentry_t nl1e,
>                          mfn_t gl1mfn, unsigned int update_flags,
> -                        struct vcpu *pt_vcpu, struct domain *pg_dom)
> +                        struct vcpu *pt_vcpu, struct domain *pg_dom,
> +                        l1_pgentry_t *ol1e_out)
>  {
>      l1_pgentry_t ol1e = l1e_read(pl1e);
>      struct domain *pt_dom = pt_vcpu->domain;
> @@ -2254,7 +2255,11 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, 
> l1_pgentry_t nl1e,
>          /* Fast path for sufficiently-similar mappings. */
>          if ( !l1e_has_changed(ol1e, nl1e, ~FASTPATH_FLAG_WHITELIST) )
>          {
> -            UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu, 
> update_flags);
> +            intpte_t old = UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, 
> pt_vcpu,
> +                                        update_flags);
> +            if ( ol1e_out )
> +                *ol1e_out = l1e_from_intpte(old);
> +

Same "intpte_t old = UPDATE_ENTRY(); if ( olNe_out ) ..." code block is now
repeated three times in each of mod_l{1,2,3,4}_entry().

Alternatively, a single local variable per function, assigned at each call
site and copied out once at the end. Does this avoid the duplication?