* [PATCH 0/4] unmap_page_range optimisation
@ 2026-07-27 15:06 Kevin Lampis
2026-07-27 15:06 ` [PATCH 1/4] x86: extend update_intpte() to support atomic get-and-update Kevin Lampis
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Kevin Lampis @ 2026-07-27 15:06 UTC (permalink / raw)
To: xen-devel; +Cc: jbeulich, andrew.cooper3, roger.pau, Kevin Lampis
Previous discussion:
RFC: unmap_page_range optimisation (avoiding emulation faults during VM migration)
https://lore.kernel.org/xen-devel/16133EFF-88FF-467F-B78F-E96EB148C3A5@citrix.com/
This series adds a new pte_get_and_clear hypercall which Linux can use
to significantly improve the performance of unmap_page_range.
The new hypercall reuses existing code from do_mmu_update, mod_l1_entry
and update_intpte but is careful not to impact performance of the
non-pte-get-and-clear code paths.
A microbenchmark[1] which allocates a large number of pages and then clears
them shows a performance increase from 1100ms to 700ms using the
new pte_get_and_clear hypercall.
Further profiling the microbenchmark with bpftrace[2] shows that Linux calls
unmap_page_range() 23 times with an average execution time of 50ms
dropping to 30ms when using the new hypercall.
[1] microbenchmark
#include <err.h>
#include <sys/mman.h>
#include <time.h>
#include <stdint.h>
#include <stdio.h>
#include <unistd.h>
static uint64_t nsec(void)
{
struct timespec ts;
clock_gettime(CLOCK_MONOTONIC, &ts);
return (uint64_t)ts.tv_sec * 1e9 + ts.tv_nsec;
}
int main(int argc, char **argv)
{
const size_t len = 1024UL * 1024 * 1024 * 4;
const long pagesz = sysconf(_SC_PAGESIZE);
char *p = mmap(NULL, len,
PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS,
-1, 0);
if ( p == MAP_FAILED )
err(1, "mmap");
/* Fault every page in */
for (size_t i = 0; i < len; i += pagesz)
p[i] = 1;
uint64_t start = nsec();
if ( madvise(p, len, MADV_DONTNEED) )
err(1, "madvise");
uint64_t end = nsec();
printf("MADV_DONTNEED on %zu MB took %.3f ms\n",
len / 1024 / 1024,
(end - start) / 1e6);
munmap(p, len);
return 0;
}
[2] bpftrace
bpftrace -e '
kprobe:unmap_page_range
/comm == "a.out"/
{
@start[tid] = nsecs;
}
kretprobe:unmap_page_range
/@start[tid] && comm == "a.out"/
{
$delta = nsecs - @start[tid];
@count = count();
@total = sum($delta);
delete(@start[tid]);
}
interval:s:10
{
printf("avg call time = %d ns (%d calls)\n",
(@total / @count), (uint64)@count);
exit();
}'
Kevin Lampis (4):
x86: extend update_intpte() to support atomic get-and-update
x86: extend mod_l1_entry() to optionally return the old PTE value
x86: extend do_mmu_update() to support returning the old PTE value
x86: add new pte_get_and_clear hypercall
xen/arch/x86/mm.c | 106 +++++++++++++++++++++++++++++------
xen/arch/x86/pv/mm.h | 29 ++++++----
xen/include/hypercall-defs.c | 2 +
xen/include/public/xen.h | 1 +
4 files changed, 109 insertions(+), 29 deletions(-)
--
2.52.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] x86: extend update_intpte() to support atomic get-and-update
2026-07-27 15:06 [PATCH 0/4] unmap_page_range optimisation Kevin Lampis
@ 2026-07-27 15:06 ` Kevin Lampis
2026-08-13 12:01 ` Jan Beulich
2026-07-27 15:06 ` [PATCH 2/4] x86: extend mod_l1_entry() to optionally return the old PTE value Kevin Lampis
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Kevin Lampis @ 2026-07-27 15:06 UTC (permalink / raw)
To: xen-devel; +Cc: jbeulich, andrew.cooper3, roger.pau, Kevin Lampis
The update_intpte() function now accepts a new use_cmpxchg flag and if set
returns the old pte value through the new *old pointer.
No functional change for existing callers.
Signed-off-by: Kevin Lampis <kevin.lampis@citrix.com>
---
xen/arch/x86/pv/mm.h | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/xen/arch/x86/pv/mm.h b/xen/arch/x86/pv/mm.h
index 4564cab9fc0f..8c42cf3b3f4d 100644
--- 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)
{
bool rv = true;
#ifndef PTE_UPDATE_WITH_CMPXCHG
- if ( !preserve_ad )
+ if ( !preserve_ad && !use_cmpxchg )
paging_write_guest_entry(v, p, new, mfn);
else
#endif
@@ -82,30 +83,36 @@ static inline bool update_intpte(intpte_t *p, intpte_t old, intpte_t new,
intpte_t _new = new, t;
if ( preserve_ad )
- _new |= old & (_PAGE_ACCESSED | _PAGE_DIRTY);
+ _new |= *old & (_PAGE_ACCESSED | _PAGE_DIRTY);
- t = paging_cmpxchg_guest_entry(v, p, old, _new, mfn);
+ t = paging_cmpxchg_guest_entry(v, p, *old, _new, mfn);
- if ( t == old )
+ if ( t == *old )
break;
/* Allowed to change in Accessed/Dirty flags only. */
- BUG_ON((t ^ old) & ~(intpte_t)(_PAGE_ACCESSED|_PAGE_DIRTY));
+ BUG_ON((t ^ *old) & ~(intpte_t)(_PAGE_ACCESSED|_PAGE_DIRTY));
- old = t;
+ *old = t;
}
}
return rv;
}
+static inline bool _update_intpte(intpte_t *p, intpte_t old, intpte_t new,
+ mfn_t mfn, struct vcpu *v, bool preserve_ad)
+{
+ return update_intpte(p, &old, new, mfn, v, preserve_ad, false);
+}
+
/*
* Macro that wraps the appropriate type-changes around update_intpte().
* Arguments are: type, ptr, old, new, mfn, vcpu
*/
#define UPDATE_ENTRY(_t,_p,_o,_n,_m,_v,_ad) \
- update_intpte(&_t ## e_get_intpte(*(_p)), \
- _t ## e_get_intpte(_o), _t ## e_get_intpte(_n), \
- (_m), (_v), (_ad))
+ _update_intpte(&_t ## e_get_intpte(*(_p)), \
+ _t ## e_get_intpte(_o), _t ## e_get_intpte(_n), \
+ (_m), (_v), (_ad))
static always_inline l1_pgentry_t adjust_guest_l1e(l1_pgentry_t l1e,
const struct domain *d)
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] x86: extend mod_l1_entry() to optionally return the old PTE value
2026-07-27 15:06 [PATCH 0/4] unmap_page_range optimisation Kevin Lampis
2026-07-27 15:06 ` [PATCH 1/4] x86: extend update_intpte() to support atomic get-and-update Kevin Lampis
@ 2026-07-27 15:06 ` Kevin Lampis
2026-07-27 15:06 ` [PATCH 3/4] x86: extend do_mmu_update() to support returning " Kevin Lampis
2026-07-27 15:06 ` [PATCH 4/4] x86: add new pte_get_and_clear hypercall Kevin Lampis
3 siblings, 0 replies; 10+ messages in thread
From: Kevin Lampis @ 2026-07-27 15:06 UTC (permalink / raw)
To: xen-devel; +Cc: jbeulich, andrew.cooper3, roger.pau, Kevin Lampis
Calling mod_l1_entry() with a valid ol1e_out pointer will do an atomic xchg to
set the new pte value and return the old pte value through the ol1e_out
pointer. If the ol1e_out pointer is NULL then old behavior is preserved.
Signed-off-by: Kevin Lampis <kevin.lampis@citrix.com>
---
xen/arch/x86/mm.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index bb4ba0afe2d4..500549c8e036 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2151,7 +2151,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 cmd,
- struct vcpu *pt_vcpu, struct domain *pg_dom)
+ struct vcpu *pt_vcpu, struct domain *pg_dom,
+ l1_pgentry_t *ol1e_out)
{
bool preserve_ad = (cmd == MMU_PT_UPDATE_PRESERVE_AD);
l1_pgentry_t ol1e = l1e_read(pl1e);
@@ -2213,8 +2214,8 @@ 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) )
{
- rc = UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
- preserve_ad);
+ rc = update_intpte(&pl1e->l1, &ol1e.l1, nl1e.l1, gl1mfn, pt_vcpu,
+ preserve_ad, !!ol1e_out);
if ( page )
put_page(page);
return rc ? 0 : -EBUSY;
@@ -2237,8 +2238,8 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, l1_pgentry_t nl1e,
if ( page )
put_page(page);
- if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
- preserve_ad)) )
+ if ( unlikely(!update_intpte(&pl1e->l1, &ol1e.l1, nl1e.l1, gl1mfn,
+ pt_vcpu, preserve_ad, !!ol1e_out)) )
{
ol1e = nl1e;
rc = -EBUSY;
@@ -2246,13 +2247,17 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, l1_pgentry_t nl1e,
}
else if ( pv_l1tf_check_l1e(pt_dom, nl1e) )
return -ERESTART;
- else if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
- preserve_ad)) )
+ else if ( unlikely(!update_intpte(&pl1e->l1, &ol1e.l1, nl1e.l1, gl1mfn,
+ pt_vcpu, preserve_ad, !!ol1e_out)) )
{
return -EBUSY;
}
put_page_from_l1e(ol1e, pt_dom);
+
+ if ( !rc && ol1e_out )
+ *ol1e_out = ol1e;
+
return rc;
}
@@ -4140,7 +4145,7 @@ long do_mmu_update(
{
case PGT_l1_page_table:
rc = mod_l1_entry(va, l1e_from_intpte(req.val), mfn,
- cmd, v, pg_owner);
+ cmd, v, pg_owner, NULL);
break;
case PGT_l2_page_table:
@@ -4505,7 +4510,8 @@ static int __do_update_va_mapping(
goto out;
}
- rc = mod_l1_entry(pl1e, val, gl1mfn, MMU_NORMAL_PT_UPDATE, v, pg_owner);
+ rc = mod_l1_entry(pl1e, val, gl1mfn, MMU_NORMAL_PT_UPDATE, v, pg_owner,
+ NULL);
page_unlock(gl1pg);
put_page(gl1pg);
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] x86: extend do_mmu_update() to support returning the old PTE value
2026-07-27 15:06 [PATCH 0/4] unmap_page_range optimisation Kevin Lampis
2026-07-27 15:06 ` [PATCH 1/4] x86: extend update_intpte() to support atomic get-and-update Kevin Lampis
2026-07-27 15:06 ` [PATCH 2/4] x86: extend mod_l1_entry() to optionally return the old PTE value Kevin Lampis
@ 2026-07-27 15:06 ` Kevin Lampis
2026-08-13 12:51 ` Jan Beulich
2026-07-27 15:06 ` [PATCH 4/4] x86: add new pte_get_and_clear hypercall Kevin Lampis
3 siblings, 1 reply; 10+ messages in thread
From: Kevin Lampis @ 2026-07-27 15:06 UTC (permalink / raw)
To: xen-devel; +Cc: jbeulich, andrew.cooper3, roger.pau, Kevin Lampis
A new parameter write_back_old when set will return the old PTE value.
- The new PTE value in req.val must be 0, this is for clearing only.
- The PRESERVE_AD flag is rejected with -EINVAL because preserving
Accessed/Dirty bits into a zero'ed PTE doesn't make sense.
- Only l1 PTEs are supported because they are the most frequent and have the
biggest performance impact.
- The old PTE value is passed back to the guest through the req.val field
If the write_back_old flag is not set then the old behavior is preserved
do_mmu_update -> mod_l1_entry -> UPDATE_ENTRY -> paging_write_guest_entry
The new get_and_clear call chain looks like this
do_mmu_update -> mod_l1_entry -> update_intpte -> paging_cmpxchg_guest_entry
Signed-off-by: Kevin Lampis <kevin.lampis@citrix.com>
---
xen/arch/x86/mm.c | 62 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 58 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 500549c8e036..278b992aca5c 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3988,11 +3988,12 @@ long do_mmuext_op(
return rc;
}
-long do_mmu_update(
+static long __do_mmu_update(
XEN_GUEST_HANDLE_PARAM(mmu_update_t) ureqs,
unsigned int count,
XEN_GUEST_HANDLE_PARAM(uint) pdone,
- unsigned int foreigndom)
+ unsigned int foreigndom,
+ bool write_back_old)
{
struct mmu_update req;
void *va = NULL;
@@ -4144,13 +4145,41 @@ long do_mmu_update(
switch ( page->u.inuse.type_info & PGT_type_mask )
{
case PGT_l1_page_table:
- rc = mod_l1_entry(va, l1e_from_intpte(req.val), mfn,
- cmd, v, pg_owner, NULL);
+ {
+ if ( !write_back_old )
+ rc = mod_l1_entry(va, l1e_from_intpte(req.val), mfn,
+ cmd, v, pg_owner, NULL);
+ else
+ {
+ l1_pgentry_t ol1e;
+ if ( unlikely(req.val != 0 ||
+ cmd == MMU_PT_UPDATE_PRESERVE_AD) )
+ {
+ rc = -EINVAL;
+ break;
+ }
+
+ rc = mod_l1_entry(va, l1e_from_intpte(req.val), mfn,
+ cmd, v, pg_owner, &ol1e);
+
+ if ( !rc )
+ {
+ req.val = ol1e.l1;
+ if ( unlikely(copy_to_guest(ureqs, &req, 1)) )
+ rc = -EFAULT;
+ }
+ }
break;
+ }
case PGT_l2_page_table:
if ( unlikely(pg_owner != pt_owner) )
break;
+ if ( unlikely(write_back_old) )
+ {
+ rc = -EINVAL;
+ break;
+ }
rc = mod_l2_entry(va, l2e_from_intpte(req.val), mfn,
cmd == MMU_PT_UPDATE_PRESERVE_AD, v);
if ( !rc )
@@ -4160,6 +4189,11 @@ long do_mmu_update(
case PGT_l3_page_table:
if ( unlikely(pg_owner != pt_owner) )
break;
+ if ( unlikely(write_back_old) )
+ {
+ rc = -EINVAL;
+ break;
+ }
rc = mod_l3_entry(va, l3e_from_intpte(req.val), mfn,
cmd == MMU_PT_UPDATE_PRESERVE_AD, v);
if ( !rc )
@@ -4169,6 +4203,11 @@ long do_mmu_update(
case PGT_l4_page_table:
if ( unlikely(pg_owner != pt_owner) )
break;
+ if ( unlikely(write_back_old) )
+ {
+ rc = -EINVAL;
+ break;
+ }
rc = mod_l4_entry(va, l4e_from_intpte(req.val), mfn,
cmd == MMU_PT_UPDATE_PRESERVE_AD, v);
if ( !rc )
@@ -4198,6 +4237,11 @@ long do_mmu_update(
break;
case PGT_writable_page:
+ if ( unlikely(write_back_old) )
+ {
+ rc = -EINVAL;
+ break;
+ }
perfc_incr(writable_mmu_updates);
paging_write_guest_entry(v, va, req.val, mfn);
rc = 0;
@@ -4366,6 +4410,16 @@ long do_mmu_update(
return rc;
}
+
+long do_mmu_update(
+ XEN_GUEST_HANDLE_PARAM(mmu_update_t) ureqs,
+ unsigned int count,
+ XEN_GUEST_HANDLE_PARAM(uint) pdone,
+ unsigned int foreigndom)
+{
+ return __do_mmu_update(ureqs, count, pdone, foreigndom, false);
+}
+
#endif /* CONFIG_PV */
/*
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] x86: add new pte_get_and_clear hypercall
2026-07-27 15:06 [PATCH 0/4] unmap_page_range optimisation Kevin Lampis
` (2 preceding siblings ...)
2026-07-27 15:06 ` [PATCH 3/4] x86: extend do_mmu_update() to support returning " Kevin Lampis
@ 2026-07-27 15:06 ` Kevin Lampis
2026-08-04 10:30 ` Teddy Astie
2026-08-13 12:55 ` Jan Beulich
3 siblings, 2 replies; 10+ messages in thread
From: Kevin Lampis @ 2026-07-27 15:06 UTC (permalink / raw)
To: xen-devel; +Cc: jbeulich, andrew.cooper3, roger.pau, Kevin Lampis
This new hypercall uses the same interface as the mmu_update hypercall except
the old PTE value is returned in the mmu_update_t->val field.
The purpose of this new hypercall is to improve performance over the current
trap and emulate behavior. Only l1 PTEs are supported because they have the
biggest performance impact.
Signed-off-by: Kevin Lampis <kevin.lampis@citrix.com>
---
xen/arch/x86/mm.c | 24 +++++++++++++++++-------
xen/include/hypercall-defs.c | 2 ++
xen/include/public/xen.h | 1 +
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 278b992aca5c..e5fdfc66081a 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3993,6 +3993,7 @@ static long __do_mmu_update(
unsigned int count,
XEN_GUEST_HANDLE_PARAM(uint) pdone,
unsigned int foreigndom,
+ unsigned int op,
bool write_back_old)
{
struct mmu_update req;
@@ -4013,8 +4014,7 @@ static long __do_mmu_update(
{
if ( likely(rc == -ERESTART) )
rc = hypercall_create_continuation(
- __HYPERVISOR_mmu_update, "hihi", ureqs, count, pdone,
- foreigndom);
+ op, "hihi", ureqs, count, pdone, foreigndom);
return rc;
}
@@ -4316,8 +4316,8 @@ static long __do_mmu_update(
if ( rc == -ERESTART )
rc = hypercall_create_continuation(
- __HYPERVISOR_mmu_update, "hihi",
- ureqs, (count - i) | MMU_UPDATE_PREEMPTED, pdone, foreigndom);
+ op, "hihi", ureqs, (count - i) | MMU_UPDATE_PREEMPTED,
+ pdone, foreigndom);
else if ( curr->arch.old_guest_table )
{
XEN_GUEST_HANDLE_PARAM(void) null;
@@ -4330,8 +4330,7 @@ static long __do_mmu_update(
* on the fact that this argument isn't needed anymore.
*/
rc = hypercall_create_continuation(
- __HYPERVISOR_mmu_update, "hihi", null,
- MMU_UPDATE_PREEMPTED, null, rc);
+ op, "hihi", null, MMU_UPDATE_PREEMPTED, null, rc);
}
put_pg_owner(pg_owner);
@@ -4417,7 +4416,18 @@ long do_mmu_update(
XEN_GUEST_HANDLE_PARAM(uint) pdone,
unsigned int foreigndom)
{
- return __do_mmu_update(ureqs, count, pdone, foreigndom, false);
+ return __do_mmu_update(ureqs, count, pdone, foreigndom,
+ __HYPERVISOR_mmu_update, false);
+}
+
+long do_pte_get_and_clear(
+ XEN_GUEST_HANDLE_PARAM(mmu_update_t) ureqs,
+ unsigned int count,
+ XEN_GUEST_HANDLE_PARAM(uint) pdone,
+ unsigned int foreigndom)
+{
+ return __do_mmu_update(ureqs, count, pdone, foreigndom,
+ __HYPERVISOR_pte_get_and_clear, true);
}
#endif /* CONFIG_PV */
diff --git a/xen/include/hypercall-defs.c b/xen/include/hypercall-defs.c
index a625d634b694..0552e93ab560 100644
--- a/xen/include/hypercall-defs.c
+++ b/xen/include/hypercall-defs.c
@@ -174,6 +174,7 @@ multicall(multicall_entry_t *call_list, unsigned long nr_calls)
#ifdef CONFIG_PV
mmuext_op(mmuext_op_t *uops, unsigned int count, unsigned int *pdone, unsigned int foreigndom)
mmu_update(mmu_update_t *ureqs, unsigned int count, unsigned int *pdone, unsigned int foreigndom)
+pte_get_and_clear(mmu_update_t *ureqs, unsigned int count, unsigned int *pdone, unsigned int foreigndom)
stack_switch(unsigned long ss, unsigned long esp)
fpu_taskswitch(int set)
set_debugreg(int reg, unsigned long value)
@@ -232,6 +233,7 @@ caller: arm
table: pv32 pv64 hvm32 hvm64 arm
set_trap_table compat do - - -
mmu_update do:1 do:1 - - -
+pte_get_and_clear do:1 do:1 - - -
set_gdt compat do - - -
stack_switch do:2 do:2 - - -
set_callbacks compat do - - -
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 2149b8dd3808..9e2ba0107d3b 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -118,6 +118,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
#define __HYPERVISOR_xenpmu_op 40
#define __HYPERVISOR_dm_op 41
#define __HYPERVISOR_hypfs_op 42
+#define __HYPERVISOR_pte_get_and_clear 43
/* Architecture-specific hypercall definitions. */
#define __HYPERVISOR_arch_0 48
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] x86: add new pte_get_and_clear hypercall
2026-07-27 15:06 ` [PATCH 4/4] x86: add new pte_get_and_clear hypercall Kevin Lampis
@ 2026-08-04 10:30 ` Teddy Astie
2026-08-13 12:55 ` Jan Beulich
1 sibling, 0 replies; 10+ messages in thread
From: Teddy Astie @ 2026-08-04 10:30 UTC (permalink / raw)
To: Kevin Lampis, xen-devel; +Cc: jbeulich, andrew.cooper3, roger.pau
[-- Attachment #1.1.1: Type: text/plain, Size: 3824 bytes --]
Le 27/07/2026 à 17:07, Kevin Lampis a écrit :
> This new hypercall uses the same interface as the mmu_update hypercall except
> the old PTE value is returned in the mmu_update_t->val field.
>
> The purpose of this new hypercall is to improve performance over the current
> trap and emulate behavior. Only l1 PTEs are supported because they have the
> biggest performance impact.
>
> Signed-off-by: Kevin Lampis <kevin.lampis@citrix.com>
> ---
> xen/arch/x86/mm.c | 24 +++++++++++++++++-------
> xen/include/hypercall-defs.c | 2 ++
> xen/include/public/xen.h | 1 +
> 3 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 278b992aca5c..e5fdfc66081a 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -3993,6 +3993,7 @@ static long __do_mmu_update(
> unsigned int count,
...
> #endif /* CONFIG_PV */
> diff --git a/xen/include/hypercall-defs.c b/xen/include/hypercall-defs.c
> index a625d634b694..0552e93ab560 100644
> --- a/xen/include/hypercall-defs.c
> +++ b/xen/include/hypercall-defs.c
> @@ -174,6 +174,7 @@ multicall(multicall_entry_t *call_list, unsigned long nr_calls)
> #ifdef CONFIG_PV
> mmuext_op(mmuext_op_t *uops, unsigned int count, unsigned int *pdone, unsigned int foreigndom)
> mmu_update(mmu_update_t *ureqs, unsigned int count, unsigned int *pdone, unsigned int foreigndom)
> +pte_get_and_clear(mmu_update_t *ureqs, unsigned int count, unsigned int *pdone, unsigned int foreigndom)
> stack_switch(unsigned long ss, unsigned long esp)
> fpu_taskswitch(int set)
> set_debugreg(int reg, unsigned long value)
> @@ -232,6 +233,7 @@ caller: arm
> table: pv32 pv64 hvm32 hvm64 arm
> set_trap_table compat do - - -
> mmu_update do:1 do:1 - - -
> +pte_get_and_clear do:1 do:1 - - -
> set_gdt compat do - - -
> stack_switch do:2 do:2 - - -
> set_callbacks compat do - - -
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 2149b8dd3808..9e2ba0107d3b 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -118,6 +118,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
> #define __HYPERVISOR_xenpmu_op 40
> #define __HYPERVISOR_dm_op 41
> #define __HYPERVISOR_hypfs_op 42
> +#define __HYPERVISOR_pte_get_and_clear 43
>
I'm not sure __HYPERVISOR_pte_get_and_clear is a great name, as I
understand it, it's more that it's doing (more general) CMPXCHG
operation and returning the old value than strictly doing a
get_and_clear one (which is the main intent for Linux).
I also think we can find a way to expand HYPERVISOR_mmu_update instead
of introducing a new hypercall.
HYPERVISOR_mmu_update actually has a undocumented "actually reserved
bit" (at least, for PV32-pae and PV64 guests) which currently must be
zero (otherwise, hypercall fails); but we can repurpose it to expand
available command count to add "cmpxchg semantics" variants. That would
greatly simplify the implementation as we won't have to introduce a new
separate hypercall just for this.
(I will send a patch regarding this in particular)
---
Aside that, new features wants to be enumerated so that the kernel knows
that it's supported before trying to use it. We can expand features.h
with a new flag for this.
> /* Architecture-specific hypercall definitions. */
> #define __HYPERVISOR_arch_0 48
Teddy
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 2489 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] x86: extend update_intpte() to support atomic get-and-update
2026-07-27 15:06 ` [PATCH 1/4] x86: extend update_intpte() to support atomic get-and-update Kevin Lampis
@ 2026-08-13 12:01 ` Jan Beulich
2026-08-13 12:22 ` Andrew Cooper
0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2026-08-13 12:01 UTC (permalink / raw)
To: Kevin Lampis; +Cc: Andrew Cooper, Teddy Astie, Roger Pau Monné, xen-devel
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?) As you switch old to being a pointer, and
as ...
> {
> bool rv = true;
>
> #ifndef PTE_UPDATE_WITH_CMPXCHG
> - if ( !preserve_ad )
> + if ( !preserve_ad && !use_cmpxchg )
> paging_write_guest_entry(v, p, new, mfn);
... *old isn't used here, having callers pass in NULL in that case may be
one option.
(In any event, old becoming a pointer imo needs commenting upon, as otherwise
one might expect this to be only an output.)
Alternatively I have an old patch lying around which looks to apply cleanly,
and which may be useful here; see at the bottom.
> @@ -82,30 +83,36 @@ static inline bool update_intpte(intpte_t *p, intpte_t old, intpte_t new,
> intpte_t _new = new, t;
>
> if ( preserve_ad )
> - _new |= old & (_PAGE_ACCESSED | _PAGE_DIRTY);
> + _new |= *old & (_PAGE_ACCESSED | _PAGE_DIRTY);
>
> - t = paging_cmpxchg_guest_entry(v, p, old, _new, mfn);
> + t = paging_cmpxchg_guest_entry(v, p, *old, _new, mfn);
>
> - if ( t == old )
> + if ( t == *old )
> break;
>
> /* Allowed to change in Accessed/Dirty flags only. */
> - BUG_ON((t ^ old) & ~(intpte_t)(_PAGE_ACCESSED|_PAGE_DIRTY));
> + BUG_ON((t ^ *old) & ~(intpte_t)(_PAGE_ACCESSED|_PAGE_DIRTY));
>
> - old = t;
> + *old = t;
> }
> }
> return rv;
> }
>
> +static inline bool _update_intpte(intpte_t *p, intpte_t old, intpte_t new,
> + mfn_t mfn, struct vcpu *v, bool preserve_ad)
> +{
> + return update_intpte(p, &old, new, mfn, v, preserve_ad, false);
> +}
> +
> /*
> * Macro that wraps the appropriate type-changes around update_intpte().
> * Arguments are: type, ptr, old, new, mfn, vcpu
> */
> #define UPDATE_ENTRY(_t,_p,_o,_n,_m,_v,_ad) \
> - update_intpte(&_t ## e_get_intpte(*(_p)), \
> - _t ## e_get_intpte(_o), _t ## e_get_intpte(_n), \
> - (_m), (_v), (_ad))
> + _update_intpte(&_t ## e_get_intpte(*(_p)), \
> + _t ## e_get_intpte(_o), _t ## e_get_intpte(_n), \
> + (_m), (_v), (_ad))
Like the patch below does - if already this last line needs touching, I
think the excess parentheses then also want dropping.
Tangentially: We will want to consider dropping the function's return
value, since as of 1bc30c076a7f ("x86/mm:
{paging, sh}_{cmpxchg, write}_guest_entry() cannot fault") it only ever
returns true.
Jan
x86: make UPDATE_ENTRY() allow for multiple operation flags
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2149,10 +2149,9 @@ static void l3t_unlock(struct page_info
/* 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 cmd,
+ mfn_t gl1mfn, unsigned int update_flags,
struct vcpu *pt_vcpu, struct domain *pg_dom)
{
- bool preserve_ad = (cmd == MMU_PT_UPDATE_PRESERVE_AD);
l1_pgentry_t ol1e = l1e_read(pl1e);
struct domain *pt_dom = pt_vcpu->domain;
int rc = 0;
@@ -2172,7 +2171,7 @@ static int mod_l1_entry(l1_pgentry_t *pl
}
/* Translate foreign guest address. */
- if ( cmd != MMU_PT_UPDATE_NO_TRANSLATE &&
+ if ( !(update_flags & PTE_UPDATE_NO_TRANSLATE) &&
paging_mode_translate(pg_dom) )
{
p2m_type_t p2mt;
@@ -2213,7 +2212,7 @@ static int mod_l1_entry(l1_pgentry_t *pl
if ( !l1e_has_changed(ol1e, nl1e, ~FASTPATH_FLAG_WHITELIST) )
{
rc = UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
- preserve_ad);
+ update_flags);
if ( page )
put_page(page);
return rc ? 0 : -EBUSY;
@@ -2237,7 +2236,7 @@ static int mod_l1_entry(l1_pgentry_t *pl
put_page(page);
if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
- preserve_ad)) )
+ update_flags)) )
{
ol1e = nl1e;
rc = -EBUSY;
@@ -2246,7 +2245,7 @@ static int mod_l1_entry(l1_pgentry_t *pl
else if ( pv_l1tf_check_l1e(pt_dom, nl1e) )
return -ERESTART;
else if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, pt_vcpu,
- preserve_ad)) )
+ update_flags)) )
{
return -EBUSY;
}
@@ -2260,7 +2259,7 @@ static int mod_l1_entry(l1_pgentry_t *pl
static int mod_l2_entry(l2_pgentry_t *pl2e,
l2_pgentry_t nl2e,
mfn_t mfn,
- int preserve_ad,
+ unsigned int update_flags,
struct vcpu *vcpu)
{
l2_pgentry_t ol2e;
@@ -2292,7 +2291,7 @@ static int mod_l2_entry(l2_pgentry_t *pl
/* Fast path for sufficiently-similar mappings. */
if ( !l2e_has_changed(ol2e, nl2e, ~FASTPATH_PDE_FLAG_WHITELIST) )
{
- if ( UPDATE_ENTRY(l2, pl2e, ol2e, nl2e, mfn, vcpu, preserve_ad) )
+ if ( UPDATE_ENTRY(l2, pl2e, ol2e, nl2e, mfn, vcpu, update_flags) )
return 0;
return -EBUSY;
}
@@ -2301,7 +2300,7 @@ static int mod_l2_entry(l2_pgentry_t *pl
return rc;
if ( unlikely(!UPDATE_ENTRY(l2, pl2e, ol2e, nl2e, mfn, vcpu,
- preserve_ad)) )
+ update_flags)) )
{
ol2e = nl2e;
rc = -EBUSY;
@@ -2310,7 +2309,7 @@ static int mod_l2_entry(l2_pgentry_t *pl
else if ( pv_l1tf_check_l2e(d, nl2e) )
return -ERESTART;
else if ( unlikely(!UPDATE_ENTRY(l2, pl2e, ol2e, nl2e, mfn, vcpu,
- preserve_ad)) )
+ update_flags)) )
{
return -EBUSY;
}
@@ -2324,7 +2323,7 @@ static int mod_l2_entry(l2_pgentry_t *pl
static int mod_l3_entry(l3_pgentry_t *pl3e,
l3_pgentry_t nl3e,
mfn_t mfn,
- int preserve_ad,
+ unsigned int update_flags,
struct vcpu *vcpu)
{
l3_pgentry_t ol3e;
@@ -2354,7 +2353,7 @@ static int mod_l3_entry(l3_pgentry_t *pl
/* Fast path for sufficiently-similar mappings. */
if ( !l3e_has_changed(ol3e, nl3e, ~FASTPATH_PDE_FLAG_WHITELIST) )
{
- rc = UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, mfn, vcpu, preserve_ad);
+ rc = UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, mfn, vcpu, update_flags);
return rc ? 0 : -EFAULT;
}
@@ -2364,7 +2363,7 @@ static int mod_l3_entry(l3_pgentry_t *pl
rc = 0;
if ( unlikely(!UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, mfn, vcpu,
- preserve_ad)) )
+ update_flags)) )
{
ol3e = nl3e;
rc = -EFAULT;
@@ -2373,7 +2372,7 @@ static int mod_l3_entry(l3_pgentry_t *pl
else if ( pv_l1tf_check_l3e(d, nl3e) )
return -ERESTART;
else if ( unlikely(!UPDATE_ENTRY(l3, pl3e, ol3e, nl3e, mfn, vcpu,
- preserve_ad)) )
+ update_flags)) )
{
return -EFAULT;
}
@@ -2386,7 +2385,7 @@ static int mod_l3_entry(l3_pgentry_t *pl
static int mod_l4_entry(l4_pgentry_t *pl4e,
l4_pgentry_t nl4e,
mfn_t mfn,
- int preserve_ad,
+ unsigned int update_flags,
struct vcpu *vcpu)
{
struct domain *d = vcpu->domain;
@@ -2416,7 +2415,7 @@ static int mod_l4_entry(l4_pgentry_t *pl
/* Fast path for sufficiently-similar mappings. */
if ( !l4e_has_changed(ol4e, nl4e, ~FASTPATH_PDE_FLAG_WHITELIST) )
{
- rc = UPDATE_ENTRY(l4, pl4e, ol4e, nl4e, mfn, vcpu, preserve_ad);
+ rc = UPDATE_ENTRY(l4, pl4e, ol4e, nl4e, mfn, vcpu, update_flags);
return rc ? 0 : -EFAULT;
}
@@ -2426,7 +2425,7 @@ static int mod_l4_entry(l4_pgentry_t *pl
rc = 0;
if ( unlikely(!UPDATE_ENTRY(l4, pl4e, ol4e, nl4e, mfn, vcpu,
- preserve_ad)) )
+ update_flags)) )
{
ol4e = nl4e;
rc = -EFAULT;
@@ -2435,7 +2434,7 @@ static int mod_l4_entry(l4_pgentry_t *pl
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;
+
switch ( page->u.inuse.type_info & PGT_type_mask )
{
case PGT_l1_page_table:
rc = mod_l1_entry(va, l1e_from_intpte(req.val), mfn,
- cmd, v, pg_owner);
+ update_flags, v, pg_owner);
break;
case PGT_l2_page_table:
if ( unlikely(pg_owner != pt_owner) )
break;
rc = mod_l2_entry(va, l2e_from_intpte(req.val), mfn,
- cmd == MMU_PT_UPDATE_PRESERVE_AD, v);
+ update_flags, v);
if ( !rc )
flush_linear_pt = true;
break;
@@ -4155,7 +4159,7 @@ long do_mmu_update(
if ( unlikely(pg_owner != pt_owner) )
break;
rc = mod_l3_entry(va, l3e_from_intpte(req.val), mfn,
- cmd == MMU_PT_UPDATE_PRESERVE_AD, v);
+ update_flags, v);
if ( !rc )
flush_linear_pt = true;
break;
@@ -4164,7 +4168,7 @@ long do_mmu_update(
if ( unlikely(pg_owner != pt_owner) )
break;
rc = mod_l4_entry(va, l4e_from_intpte(req.val), mfn,
- cmd == MMU_PT_UPDATE_PRESERVE_AD, v);
+ update_flags, v);
if ( !rc )
flush_linear_pt = true;
if ( !rc && pt_owner->arch.pv.xpti )
--- a/xen/arch/x86/pv/mm.h
+++ b/xen/arch/x86/pv/mm.h
@@ -62,17 +62,20 @@ static inline intpte_t paging_cmpxchg_gu
#undef PTE_UPDATE_WITH_CMPXCHG
#endif
+#define PTE_UPDATE_PRESERVE_AD (1u << 0)
+#define PTE_UPDATE_NO_TRANSLATE (1u << 1)
+
/*
* 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)
+ mfn_t mfn, struct vcpu *v, unsigned int flags)
{
bool rv = true;
#ifndef PTE_UPDATE_WITH_CMPXCHG
- if ( !preserve_ad )
+ if ( !(flags & PTE_UPDATE_PRESERVE_AD) )
paging_write_guest_entry(v, p, new, mfn);
else
#endif
@@ -81,7 +84,7 @@ static inline bool update_intpte(intpte_
{
intpte_t _new = new, t;
- if ( preserve_ad )
+ if ( flags & PTE_UPDATE_PRESERVE_AD )
_new |= old & (_PAGE_ACCESSED | _PAGE_DIRTY);
t = paging_cmpxchg_guest_entry(v, p, old, _new, mfn);
@@ -102,10 +105,10 @@ static inline bool update_intpte(intpte_
* Macro that wraps the appropriate type-changes around update_intpte().
* Arguments are: type, ptr, old, new, mfn, vcpu
*/
-#define UPDATE_ENTRY(_t,_p,_o,_n,_m,_v,_ad) \
+#define UPDATE_ENTRY(_t ,_p ,_o ,_n ,_m ,_v , fl) \
update_intpte(&_t ## e_get_intpte(*(_p)), \
_t ## e_get_intpte(_o), _t ## e_get_intpte(_n), \
- (_m), (_v), (_ad))
+ _m, _v, fl)
static always_inline l1_pgentry_t adjust_guest_l1e(l1_pgentry_t l1e,
const struct domain *d)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] x86: extend update_intpte() to support atomic get-and-update
2026-08-13 12:01 ` Jan Beulich
@ 2026-08-13 12:22 ` Andrew Cooper
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Cooper @ 2026-08-13 12:22 UTC (permalink / raw)
To: Jan Beulich, Kevin Lampis
Cc: Andrew Cooper, Teddy Astie, Roger Pau Monné, xen-devel
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] x86: extend do_mmu_update() to support returning the old PTE value
2026-07-27 15:06 ` [PATCH 3/4] x86: extend do_mmu_update() to support returning " Kevin Lampis
@ 2026-08-13 12:51 ` Jan Beulich
0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2026-08-13 12:51 UTC (permalink / raw)
To: Kevin Lampis; +Cc: Andrew Cooper, Teddy Astie, Roger Pau Monné, xen-devel
On 27.07.2026 17:06, Kevin Lampis wrote:
> A new parameter write_back_old when set will return the old PTE value.
>
> - The new PTE value in req.val must be 0, this is for clearing only.
As per Teddy's comment it needs to be determined whether we really want to
limit this to "clear". Even if we do for now, naming of the new sub-op may
want to be such that relaxing later is an option.
> - The PRESERVE_AD flag is rejected with -EINVAL because preserving
> Accessed/Dirty bits into a zero'ed PTE doesn't make sense.
>
> - Only l1 PTEs are supported because they are the most frequent and have the
> biggest performance impact.
>
> - The old PTE value is passed back to the guest through the req.val field
>
> If the write_back_old flag is not set then the old behavior is preserved
> do_mmu_update -> mod_l1_entry -> UPDATE_ENTRY -> paging_write_guest_entry
>
> The new get_and_clear call chain looks like this
> do_mmu_update -> mod_l1_entry -> update_intpte -> paging_cmpxchg_guest_entry
>
> Signed-off-by: Kevin Lampis <kevin.lampis@citrix.com>
Apart from the above only a couple of cosmetic comments, as based on other
replies to this series things will likely change quite a bit here.
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -3988,11 +3988,12 @@ long do_mmuext_op(
> return rc;
> }
>
> -long do_mmu_update(
> +static long __do_mmu_update(
No need for two leading underscores (making the identifier a reserved one),
when one will do. In fact with this becoming a local helper, I question the
need for a prefix altogether: Just mmu_update() would likely do.
> @@ -4144,13 +4145,41 @@ long do_mmu_update(
> switch ( page->u.inuse.type_info & PGT_type_mask )
> {
> case PGT_l1_page_table:
> - rc = mod_l1_entry(va, l1e_from_intpte(req.val), mfn,
> - cmd, v, pg_owner, NULL);
> + {
Why this curly brace, when there are no declarations?
> + if ( !write_back_old )
> + rc = mod_l1_entry(va, l1e_from_intpte(req.val), mfn,
> + cmd, v, pg_owner, NULL);
Even this little bit of churn could be avoided if you inserted ...
> + else
if ( write_back_old )
... above the existing code, and ...
> + {
> + l1_pgentry_t ol1e;
> + if ( unlikely(req.val != 0 ||
> + cmd == MMU_PT_UPDATE_PRESERVE_AD) )
> + {
> + rc = -EINVAL;
> + break;
> + }
> +
> + rc = mod_l1_entry(va, l1e_from_intpte(req.val), mfn,
> + cmd, v, pg_owner, &ol1e);
> +
> + if ( !rc )
> + {
> + req.val = ol1e.l1;
> + if ( unlikely(copy_to_guest(ureqs, &req, 1)) )
> + rc = -EFAULT;
> + }
... a separate "break" here.
> + }
> break;
> + }
>
> case PGT_l2_page_table:
> if ( unlikely(pg_owner != pt_owner) )
> break;
> + if ( unlikely(write_back_old) )
> + {
> + rc = -EINVAL;
rc already is -EINVAL when make it here, isn't it? That's also leveraged by
the owner check visible in context. (Same for the further cases below,
obviously.)
> @@ -4198,6 +4237,11 @@ long do_mmu_update(
> break;
>
> case PGT_writable_page:
> + if ( unlikely(write_back_old) )
> + {
> + rc = -EINVAL;
> + break;
> + }
> perfc_incr(writable_mmu_updates);
> paging_write_guest_entry(v, va, req.val, mfn);
> rc = 0;
Below here there's a copy of this PGT_writable_page handling, which looks
as if it also wants to reject write_back_old being true.
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] x86: add new pte_get_and_clear hypercall
2026-07-27 15:06 ` [PATCH 4/4] x86: add new pte_get_and_clear hypercall Kevin Lampis
2026-08-04 10:30 ` Teddy Astie
@ 2026-08-13 12:55 ` Jan Beulich
1 sibling, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2026-08-13 12:55 UTC (permalink / raw)
To: Kevin Lampis; +Cc: Andrew Cooper, Teddy Astie, Roger Pau Monné, xen-devel
On 27.07.2026 17:06, Kevin Lampis wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -3993,6 +3993,7 @@ static long __do_mmu_update(
> unsigned int count,
> XEN_GUEST_HANDLE_PARAM(uint) pdone,
> unsigned int foreigndom,
> + unsigned int op,
> bool write_back_old)
> {
Likely irrelevant anyway when this becomes a new sub-op instead of a new
top-level hypercall, but: "op" is redundant with "write_back_old". Such
redundancy wants avoiding.
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-08-13 12:56 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-27 15:06 [PATCH 0/4] unmap_page_range optimisation Kevin Lampis
2026-07-27 15:06 ` [PATCH 1/4] x86: extend update_intpte() to support atomic get-and-update Kevin Lampis
2026-08-13 12:01 ` Jan Beulich
2026-08-13 12:22 ` Andrew Cooper
2026-07-27 15:06 ` [PATCH 2/4] x86: extend mod_l1_entry() to optionally return the old PTE value Kevin Lampis
2026-07-27 15:06 ` [PATCH 3/4] x86: extend do_mmu_update() to support returning " Kevin Lampis
2026-08-13 12:51 ` Jan Beulich
2026-07-27 15:06 ` [PATCH 4/4] x86: add new pte_get_and_clear hypercall Kevin Lampis
2026-08-04 10:30 ` Teddy Astie
2026-08-13 12:55 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.