Re: [PATCH v5 2/6] x86: rearrange how mod_l*_entry() functions call UPDATE_ENTRY

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.11.2026 14:56, Andrew Cooper wrote:
> On 12/11/2026 10:00 am, Kevin Lampis wrote:
>> bloat-o-meter:
>> Function                                     old     new   delta
>> mod_l1_entry                                1186    1113     -73
>> do_mmu_update                               5546    5392    -154
> 
> This was only for the sample I pasted inline.  For this patch it's
> 
> add/remove: 0/0 grow/shrink: 1/5 up/down: 10/-641 (-631)
> Function                                     old     new   delta
> mod_l1_entry.cold                             56      66     +10
> mod_l1_entry                                1891    1885      -6
> mod_l4_entry.cold                             39      31      -8
> do_mmu_update.cold                           122     112     -10
> mod_l4_entry                                 917     760    -157
> do_mmu_update                               5745    5285    -460
> 
> although it's not particularly relevant to live in the commit message. 
> (I use it as a litmus test that the changes are along the expected lines.)
> 
> For the commit message, you probably want "Fold two UPDATE_ENTRY() paths
> together, in preparation for an upcoming change." or so.
> 
>>
>> Signed-off-by: Kevin Lampis <kevin.lampis@xxxxxxxxxx>
>>
>> ---
>> Changes in v5:
>> - New patch
>> ---
>>  xen/arch/x86/mm.c | 45 +++++++++++++++++++--------------------------
>>  1 file changed, 19 insertions(+), 26 deletions(-)
>>
>> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
>> index e42cd33c0d09..dc0e0d451c02 100644
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -2197,12 +2197,12 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, 
>> l1_pgentry_t nl1e,
>>  {
>>      l1_pgentry_t ol1e = l1e_read(pl1e);
>>      struct domain *pt_dom = pt_vcpu->domain;
>> -    int rc = 0;
>>  
>>      ASSERT(!paging_mode_refcounts(pt_dom));
>>  
>>      if ( l1e_get_flags(nl1e) & _PAGE_PRESENT )
>>      {
>> +        int rc;
>>          struct page_info *page = NULL;
>>          uint32_t disallow = l1_disallow_mask(pt_dom);
>>  
>> @@ -2276,16 +2276,14 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, 
>> l1_pgentry_t nl1e,
>>          }
>>          if ( page )
>>              put_page(page);
>> -
>> -        UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu, update_flags);
>>      }
>>      else if ( pv_l1tf_check_l1e(pt_dom, nl1e) )
>>          return -ERESTART;
>> -    else
>> -        UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu, update_flags);
>> +
>> +    UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu, update_flags);
>>  
>>      put_page_from_l1e(ol1e, pt_dom);
>> -    return rc;
>> +    return 0;
>>  }
>>  
>>  
>> @@ -2300,7 +2298,6 @@ static int mod_l2_entry(l2_pgentry_t *pl2e,
>>      struct domain *d = vcpu->domain;
>>      struct page_info *l2pg = mfn_to_page(mfn);
>>      unsigned long type = l2pg->u.inuse.type_info;
>> -    int rc = 0;
>>  
>>      if ( unlikely(!is_guest_l2_slot(d, type, pgentry_ptr_to_slot(pl2e))) )
>>      {
>> @@ -2313,6 +2310,8 @@ static int mod_l2_entry(l2_pgentry_t *pl2e,
>>  
>>      if ( l2e_get_flags(nl2e) & _PAGE_PRESENT )
>>      {
>> +        int rc;
>> +
>>          if ( unlikely(l2e_get_flags(nl2e) & L2_DISALLOW_MASK) )
>>          {
>>              gdprintk(XENLOG_WARNING, "Bad L2 flags %x\n",
>> @@ -2331,17 +2330,15 @@ static int mod_l2_entry(l2_pgentry_t *pl2e,
>>  
>>          if ( unlikely((rc = get_page_from_l2e(nl2e, mfn, d, 0)) < 0) )
>>              return rc;
> 
> Looking further through the call tree, I'm now confident that
> get_page_from_l?e() can't return a positive number.

They can return L{2,4}E_RECURSIVE (i.e. 1).

Jan