Re: [PATCH v2 1/7] x86: make UPDATE_ENTRY() allow for multiple operation flags

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 11/09/2026 10:45 am, Teddy Astie wrote:
> Le 10/09/2026 à 22:31, Kevin Lampis a écrit : 
>> @@ -2435,7 +2434,7 @@ static int mod_l4_entry(l4_pgentry_t *pl4e,
>>       else if ( pv_l1tf_check_l4e(d, nl4e) )
>>           return -ERESTART;
>>       else if ( unlikely(!UPDATE_ENTRY(l4, pl4e, ol4e, nl4e, mfn, vcpu,
>> -                                     preserve_ad)) )
>> +                                     update_flags)) )
>>       {
>>           return -EFAULT;
>>       }
>> @@ -4135,18 +4134,23 @@ long do_mmu_update(
>>                 if ( page_lock(page) )
>>               {
>> +                unsigned int update_flags = (cmd ==
>> MMU_PT_UPDATE_PRESERVE_AD)
>> +                                            ? PTE_UPDATE_PRESERVE_AD
>> +                                            : (cmd ==
>> MMU_PT_UPDATE_NO_TRANSLATE)
>> +                                              ?
>> PTE_UPDATE_NO_TRANSLATE : 0;
>> +
>
> I'm not fully convinced by the double-ternary operator here. While
> it's a bit more compact, I don't find it very readable, I would prefer
> something like
>
> unsigned int update_flags = 0;
> if ( cmd == MMU_PT_UPDATE_PRESERVE_AD )
>     update_flags = PTE_UPDATE_PRESERVE_AD;
> if ( cmd == MMU_PT_UPDATE_NO_TRANSLATE )
>     update_flags = PTE_UPDATE_NO_TRANSLATE;
>
> Which is one line longer, but looks to me more clear on what it does.

The second wants to be an else if.  I too would prefer to see this
written not squashed onto the RHS.

Given that it doesn't grow later in the series, then this if/else chain
is ok.  If it does get longer, then it probably wants splitting out into
a cmd_to_pte_update_flags() helper.

~Andrew