Re: [PATCH 1/4] x86: extend update_intpte() to support atomic get-and-update
Andrew Cooper <[email protected]>
| Newsgroups | org.xenproject.lists.xen-devel |
|---|---|
| Message-ID | <[email protected]> |
On 13/08/2026 1:01 pm, Jan Beulich wrote: > On 27.07.2026 17:06, Kevin Lampis wrote: >> --- a/xen/arch/x86/pv/mm.h >> +++ b/xen/arch/x86/pv/mm.h >> @@ -66,13 +66,14 @@ static inline intpte_t paging_cmpxchg_guest_entry( >> * How to write an entry to the guest pagetables. >> * Returns false for failure (pointer not valid), true for success. >> */ >> -static inline bool update_intpte(intpte_t *p, intpte_t old, intpte_t new, >> - mfn_t mfn, struct vcpu *v, bool preserve_ad) >> +static inline bool update_intpte(intpte_t *p, intpte_t *old, intpte_t new, >> + mfn_t mfn, struct vcpu *v, bool preserve_ad, >> + bool use_cmpxchg) > No 2nd boolean parameter, please. (use_cmpxchg also doesn't look to be an > overly good name; "swap" maybe?) This is half of a patch that's been in the XenServer queue for decades for other purposes. (TLB-flush avoidance based on A/D being clear, for which you must use some form of atomic, but it relies on dom0 being trusted not to clear the A/D bits in isolation.) I agree that we don't want more booleans. Your flags proposal looks like the right way to go. XenServer's pre-existing usecase could get away with XCHG. I think it was wired into CMPXCHG simply because that already existed. This new usecase probably wants to be XCHG too. I don't think "please preserve AD while swapping X for Y and also tell the the old value you found" makes much sense at the hypercall level at least. Furthermore, now that the return value is unused, we could return the actual old value to anyone who cares, which avoids turning the input "old" value into a pointer. ~Andrew P.S. looking at the preserve_ad logic, I think it ought to be tightened to only permit A/D becoming set, because that's the only direction that hardware will move the bits. A/D becoming clear is a race against something which is not the pagewalker.