Re: [PATCH v5 3/6] x86: extend mod_l*_entry() to optionally return the old PTE value

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.11.2026 11:00, Kevin Lampis wrote:
> --- 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);
> +
>              if ( page )
>                  put_page(page);
>              return 0;
> @@ -2280,7 +2285,12 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, 
> l1_pgentry_t nl1e,
>      else if ( pv_l1tf_check_l1e(pt_dom, nl1e) )
>          return -ERESTART;
>  
> -    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);
> +    }
>  
>      put_page_from_l1e(ol1e, pt_dom);
>      return 0;

This (recurring) pattern still adds quite a bit of redundancy. Was it ruled out
in an earlier version to pass ol1e_out into UPDATE_ENTRY() (and perhaps through
to update_intpte()), to have the updating done there? If so, why?

Otherwise, nit: Blank lines please between declaration(s) and statement(s)
(which, if this form is to be retained, could likely be adjusted while
committing).

Jan