Skip to contentSkip to conversation

Various improvements on runstate and vcpu_time areas + xen_features

PATCH In review Teddy Astie 3 participants 21 messages Oct 16 – Nov 11 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover Various improvements on runstate and vcpu_time areas + xen_features

Originally worked on for AMD SEV support, but happens to be useful on its own.

From the hypervisor, updating runstate info (and pv timer area) from virtual address is expensive. Moreoever, confidential computing (SEV) can't rely on guest virtual addresses.

Xen 4.18 introduced VCPUOP_register_{runstate,vcpu_time}_phys_area variants that use physical addresses instead of virtual one, which can be persistenly mapped on the hypervisor instead of translated each time.

Also refactor xen_features so that it uses a bitmap instead of one u8 per feature bit.

CC Juergen Gross <[email protected]>
CC Boris Ostrovsky <[email protected]>
CC Stefano Stabellini <[email protected]>
CC Oleksandr Tyshchenko <[email protected]>
CC David Woodhouse <[email protected]>
CC George Dunlap <[email protected]>

Teddy Astie (8):
x86/xen/pv: Don't setup runstate area too early
xen: Refactor xen/features{.c,.h}
xen: Add gpa conversion helpers
x86/xen/time: Correct xen_save_time_memory_area() failure message x86/xen/time: Clear xen_clock mapping later
x86/xen/time: Mark xen_setup_vsyscall_time_info() __init x86/xen/time: Use VCPUOP_register_{runstate,vcpu_time}_phys_area when available xen/sysfs: Honor cached xen_features instead

 arch/x86/include/asm/xen/page.h  | 22 ++++++++++++++
 arch/x86/xen/enlighten_pv.c      |  3 --
 arch/x86/xen/time.c              | 52 +++++++++++++++++++-------------
 drivers/xen/features.c           | 11 ++++---
 drivers/xen/sys-hypervisor.c     | 12 +++-----
 drivers/xen/time.c               | 24 +++++++++++----
 include/xen/arm/page.h           | 21 +++++++++++++
 include/xen/features.h           | 10 +++---
 include/xen/interface/features.h |  9 ++++++
 include/xen/interface/vcpu.h     | 17 +++++++++++
 10 files changed, 134 insertions(+), 47 deletions(-)
Signature · 10 lines
2.56.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw

PATCH v2 2/8 xen: Refactor xen/features{.c,.h}

Improve various aspects of xen/features{.c,.h}
* Use a bitmap instead of a array of u8 (effectively bool).
* Mark xen_setup_features as __init, as it's only called in __init paths.
* Return bool instead of int in xen_feature()

---
CC: Juergen Gross <[email protected]>
CC: Boris Ostrovsky <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Oleksandr Tyshchenko <[email protected]>
CC: [email protected]
CC: [email protected]

 drivers/xen/features.c | 11 ++++++-----
 include/xen/features.h | 10 ++++++----
 2 files changed, 12 insertions(+), 9 deletions(-)
drivers/xen/features.c +6 -5
index 87f1828d40d5..c4a6da6419c5 100644
@@ -7,6 +7,7 @@
  * Copyright (c) 2006, Ian Campbell, XenSource Inc.
  */
 #include <linux/types.h>
+#include <linux/bitmap.h>
 #include <linux/cache.h>
 #include <linux/export.h>
 #include <linux/printk.h>
@@ -29,20 +30,20 @@
 			panic("Xen: feature %s not available!\n", #f);	\
 	}
 
-u8 xen_features[XENFEAT_NR_SUBMAPS * 32] __read_mostly;
+__ro_after_init DECLARE_BITMAP(xen_features, XENFEAT_NR_SUBMAPS * 32);
 EXPORT_SYMBOL_GPL(xen_features);
 
-void xen_setup_features(void)
+void __init xen_setup_features(void)
 {
 	struct xen_feature_info fi;
-	int i, j;
+	int i;
 
 	for (i = 0; i < XENFEAT_NR_SUBMAPS; i++) {
 		fi.submap_idx = i;
 		if (HYPERVISOR_xen_version(XENVER_get_features, &fi) < 0)
 			break;
-		for (j = 0; j < 32; j++)
-			xen_features[i * 32 + j] = !!(fi.submap & 1U << j);
+
+		bitmap_write(xen_features, fi.submap, i * 32, 32);
 	}
 
 	if (xen_pv_domain()) {
include/xen/features.h +6 -4
index e4cb464386a9..8cca0741d77d 100644
@@ -10,15 +10,17 @@
 #ifndef __XEN_FEATURES_H__
 #define __XEN_FEATURES_H__
 
+#include <linux/cache.h>
+#include <linux/bitops.h>
 #include <xen/interface/features.h>
 
-void xen_setup_features(void);
+void __init xen_setup_features(void);
 
-extern u8 xen_features[XENFEAT_NR_SUBMAPS * 32];
+extern __ro_after_init DECLARE_BITMAP(xen_features, XENFEAT_NR_SUBMAPS * 32);
 
-static inline int xen_feature(int flag)
+static inline bool xen_feature(unsigned int flag)
 {
-	return xen_features[flag];
+	return test_bit(flag, xen_features);
 }
 
 #endif /* __ASM_XEN_FEATURES_H__ */
Signature · 10 lines
2.56.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
maintainer Reviewed-by on v2 2/8
#3 lore raw

Re: [PATCH v2 2/8] xen: Refactor xen/features{.c,.h}

Quoted 8 lines · Teddy Astie

Improve various aspects of xen/features{.c,.h}
* Use a bitmap instead of a array of u8 (effectively bool).
* Mark xen_setup_features as __init, as it's only called in __init paths.
* Return bool instead of int in xen_feature()

Juergen

#4 lore raw

PATCH v2 3/8 xen: Add gpa conversion helpers

In reply to a message not in the archive

Introduce various helpers to convert between GPA and virt/paddr.

---
CC: Juergen Gross <[email protected]>
CC: Boris Ostrovsky <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Oleksandr Tyshchenko <[email protected]>
CC: [email protected]
CC: [email protected]

 arch/x86/include/asm/xen/page.h | 22 ++++++++++++++++++++++
 include/xen/arm/page.h          | 21 +++++++++++++++++++++
 2 files changed, 43 insertions(+)
arch/x86/include/asm/xen/page.h +22 -0
index 59f642a94b9d..d8fc2dc24809 100644
@@ -26,6 +26,11 @@ typedef struct xpaddr {
 	phys_addr_t paddr;
 } xpaddr_t;
 
+/* Xen guest physical address */
+typedef struct xgaddr {
+	phys_addr_t gaddr;
+} xgaddr_t;
+
 #ifdef CONFIG_X86_64
 #define XEN_PHYSICAL_MASK	__sme_clr((1UL << 52) - 1)
 #else
@@ -37,6 +42,7 @@ typedef struct xpaddr {
 
 #define XMADDR(x)	((xmaddr_t) { .maddr = (x) })
 #define XPADDR(x)	((xpaddr_t) { .paddr = (x) })
+#define XGADDR(x) ((xgaddr_t) { .gaddr = (x) })
 
 /**** MACHINE <-> PHYSICAL CONVERSION MACROS ****/
 #define INVALID_P2M_ENTRY	(~0UL)
@@ -256,6 +262,18 @@ static inline unsigned long gfn_to_pfn(unsigned long gfn)
 		return mfn_to_pfn(gfn);
 }
 
+static inline xgaddr_t phys_to_guest(xpaddr_t phys)
+{
+	unsigned int offset = phys.paddr & ~PAGE_MASK;
+	return XGADDR(PFN_PHYS(pfn_to_gfn(PFN_DOWN(phys.paddr))) | offset);
+}
+
+static inline xpaddr_t guest_to_phys(xgaddr_t guest)
+{
+	unsigned int offset = guest.gaddr & ~PAGE_MASK;
+	return XPADDR(PFN_PHYS(gfn_to_pfn(PFN_DOWN(guest.gaddr))) | offset);
+}
+
 /* Pseudo-physical <-> Bus conversion */
 #define pfn_to_bfn(pfn)		pfn_to_gfn(pfn)
 #define bfn_to_pfn(bfn)		gfn_to_pfn(bfn)
@@ -305,6 +323,10 @@ static inline unsigned long virt_to_pfn(const void *v)
 /* VIRT <-> GUEST conversion */
 #define virt_to_gfn(v)		(pfn_to_gfn(virt_to_pfn(v)))
 #define gfn_to_virt(g)		(__va(gfn_to_pfn(g) << PAGE_SHIFT))
+#define virt_to_guest(v)  (phys_to_guest(XPADDR(__pa(v))))
+
+#define percpu_to_gaddr(v) \
+	(phys_to_guest(XPADDR(per_cpu_ptr_to_phys(v))))
 
 static inline unsigned long pte_mfn(pte_t pte)
 {
include/xen/arm/page.h +21 -0
index e5c84ff28c8b..1119bde2994c 100644
@@ -24,8 +24,14 @@ typedef struct xpaddr {
 	phys_addr_t paddr;
 } xpaddr_t;
 
+/* Xen guest physical address */
+typedef struct xgaddr {
+	phys_addr_t gaddr;
+} xgaddr_t;
+
 #define XMADDR(x)	((xmaddr_t) { .maddr = (x) })
 #define XPADDR(x)	((xpaddr_t) { .paddr = (x) })
+#define XGADDR(x)	((xgaddr_t) { .gaddr = (x) })
 
 #define INVALID_P2M_ENTRY      (~0UL)
 
@@ -54,6 +60,18 @@ static inline unsigned long gfn_to_pfn(unsigned long gfn)
 	return gfn;
 }
 
+static inline xgaddr_t phys_to_guest(xpaddr_t phys)
+{
+	unsigned int offset = phys.paddr & ~PAGE_MASK;
+	return XGADDR(PFN_PHYS(pfn_to_gfn(PFN_DOWN(phys.paddr))) | offset);
+}
+
+static inline xpaddr_t guest_to_phys(xgaddr_t guest)
+{
+	unsigned int offset = guest.gaddr & ~PAGE_MASK;
+	return XPADDR(PFN_PHYS(gfn_to_pfn(PFN_DOWN(guest.gaddr))) | offset);
+}
+
 /* Pseudo-physical <-> BUS conversion */
 static inline unsigned long pfn_to_bfn(unsigned long pfn)
 {
@@ -82,9 +100,12 @@ static inline unsigned long bfn_to_pfn(unsigned long bfn)
 		pfn_to_gfn(virt_to_phys(v) >> XEN_PAGE_SHIFT);                 \
 	})
 #define gfn_to_virt(m)		(__va(gfn_to_pfn(m) << XEN_PAGE_SHIFT))
+#define virt_to_guest(v)  (phys_to_guest(XPADDR(__pa(v))))
 
 #define percpu_to_gfn(v)	\
 	(pfn_to_gfn(per_cpu_ptr_to_phys(v) >> XEN_PAGE_SHIFT))
+#define percpu_to_gaddr(v) \
+	(phys_to_guest(XPADDR(per_cpu_ptr_to_phys(v))))
 
 /* Only used in PV code. But ARM guests are always HVM. */
 static inline xmaddr_t arbitrary_virt_to_machine(void *vaddr)
Signature · 10 lines
2.56.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
maintainer Reviewed-by on v2 3/8
#5 lore raw

Re: [PATCH v2 3/8] xen: Add gpa conversion helpers

Quoted 3 lines · Teddy Astie

Introduce various helpers to convert between GPA and virt/paddr.

If you are up to it, there is some potential for cleaning up the Xen related page.h include files.

You are adding the same definitions to the x86 and arm specific page.h files, which are located at different positions regarding the arch trees.

I think this should be cleaned up by moving the common parts into include/xen/page.h. The arch specific include files should both be under include/xen/<arch>/page.h, I think (using arch/<arch>/include/asm/xen/page.h won't work, as the arm file is used by arm and arm64).

In case you don't want to do that, I'm fine to do that later, e.g. when the riscv Xen port is arriving in the kernel.

Juergen

#6 lore raw

PATCH v2 4/8 x86/xen/time: Correct xen_save_time_memory_area() failure message

In reply to a message not in the archive

We actually didn't fail to "save secondary vcpu time info" but just to unregister the mapping of it.

---
CC: Juergen Gross <[email protected]>
CC: Boris Ostrovsky <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Oleksandr Tyshchenko <[email protected]>
CC: [email protected]
CC: [email protected]

 arch/x86/xen/time.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
arch/x86/xen/time.c +2 -3
index 5c7822254a01..2656b3a08553 100644
@@ -407,8 +407,7 @@ void xen_save_time_memory_area(void)
 
 	ret = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_time_memory_area, 0, &t);
 	if (ret != 0)
-		pr_notice("Cannot save secondary vcpu_time_info (err %d)",
-			  ret);
+		pr_notice("Cannot unregister vcpu_time_info (err %d)\n", ret);
 	else
 		clear_page(xen_clock);
 }
@@ -435,7 +434,7 @@ void xen_restore_time_memory_area(void)
 	 * call for a reliable timestamp.
 	 */
 	if (ret != 0)
-		pr_notice("Cannot restore secondary vcpu_time_info (err %d)",
+		pr_notice("Cannot restore secondary vcpu_time_info (err %d)\n",
 			  ret);
 
 out:
Signature · 10 lines
2.56.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
#8 lore raw

PATCH v2 5/8 x86/xen/time: Clear xen_clock mapping later

In reply to a message not in the archive

The current logic has a corner case when both xen_save_time_memory_area() and xen_restore_time_memory_area() fails. In this case, the mapping is never cleared and kept with previous pvclock values (invalidating the comment regarding VDSO_CLOCKMODE_PVCLOCK as the data may not have been zeroed).

Also convert the clear_page with a memset, there's no need to clear the entire page when we only care about 64 bytes of it.

Fixes 2229f70b5bbb ("x86/xen/time: setup vcpu 0 time info page")

---
CC: Juergen Gross <[email protected]>
CC: Boris Ostrovsky <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Oleksandr Tyshchenko <[email protected]>
CC: David Woodhouse <[email protected]>
CC: [email protected]
CC: [email protected]

 arch/x86/xen/time.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
arch/x86/xen/time.c +2 -3
index 2656b3a08553..87b890691fcb 100644
@@ -407,9 +407,7 @@ void xen_save_time_memory_area(void)
 
 	ret = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_time_memory_area, 0, &t);
 	if (ret != 0)
-		pr_notice("Cannot unregister vcpu_time_info (err %d)\n", ret);
-	else
-		clear_page(xen_clock);
+		pr_notice("Cannot unregister vcpu_time_info (err %d)", ret);
 }
 
 void xen_restore_time_memory_area(void)
@@ -420,6 +418,7 @@ void xen_restore_time_memory_area(void)
 	if (!xen_clock)
 		goto out;
 
+	memset(xen_clock, 0, sizeof(*xen_clock));
 	t.addr.v = &xen_clock->pvti;
 
 	ret = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_time_memory_area, 0, &t);
Signature · 10 lines
2.56.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
maintainer Reviewed-by on v2 5/8
#9 lore raw

Re: [PATCH v2 5/8] x86/xen/time: Clear xen_clock mapping later

Quoted 10 lines · Teddy Astie

The current logic has a corner case when both xen_save_time_memory_area() and xen_restore_time_memory_area() fails. In this case, the mapping is never cleared and kept with previous pvclock values (invalidating the comment regarding VDSO_CLOCKMODE_PVCLOCK as the data may not have been zeroed).

Also convert the clear_page with a memset, there's no need to clear the entire page when we only care about 64 bytes of it.

Fixes 2229f70b5bbb ("x86/xen/time: setup vcpu 0 time info page")

Juergen

#10 lore raw

PATCH v2 6/8 x86/xen/time: Mark xen_setup_vsyscall_time_info() __init

In reply to a message not in the archive

This function is only called by xen_time_init() being itself __init.

---
CC: Juergen Gross <[email protected]>
CC: Boris Ostrovsky <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Oleksandr Tyshchenko <[email protected]>
CC: [email protected]
CC: [email protected]

 arch/x86/xen/time.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/xen/time.c +1 -1
index 87b890691fcb..0d8643cc8159 100644
@@ -442,7 +442,7 @@ void xen_restore_time_memory_area(void)
 	xen_sched_clock_offset = xen_clocksource_read() - xen_clock_value_saved;
 }
 
-static void xen_setup_vsyscall_time_info(void)
+static void __init xen_setup_vsyscall_time_info(void)
 {
 	struct vcpu_register_time_memory_area t;
 	struct pvclock_vsyscall_time_info *ti;
Signature · 10 lines
2.56.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
#12 lore raw

PATCH v2 7/8 x86/xen/time: Use VCPUOP_register_{runstate,vcpu_time}_phys_area when available

In reply to a message not in the archive

Updating these area can be expensive when done through virtual addresses (especially for runstate one), in particular for HVM/PVH guests. Xen 4.18 introduced physical address alternatives to address those short-commings, but they aren't used currently.

Fix it by using these alternatives when available (when reported in XENFEAT_{runstate,vcpu_time}_phys_area.

Assisted-by LLM
[guidance and testing]

---
CC: Juergen Gross <[email protected]>
CC: Boris Ostrovsky <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Oleksandr Tyshchenko <[email protected]>
CC: David Woodhouse <[email protected]>
CC: George Dunlap <[email protected]>
CC: [email protected]
CC: [email protected]

For PV, this actually depends on
"x86/xen/pv: Don't setup runstate area too early"

When using phys_area ABI, the struct is not allowed to cross a page boundary anymore, hence we need to be careful with per-cpu struct alignment.

v2:
- Add PV domain support.
- Clarify xen_register_time_area()
- fold out &per_cpu(xen_runstate, cpu) in a separate variable

---
 arch/x86/xen/time.c              | 42 ++++++++++++++++++++------------
 drivers/xen/time.c               | 24 +++++++++++++-----
 include/xen/interface/features.h |  9 +++++++
 include/xen/interface/vcpu.h     | 17 +++++++++++++
 4 files changed, 71 insertions(+), 21 deletions(-)
arch/x86/xen/time.c +27 -15
index 0d8643cc8159..5fa22925c0d5 100644
@@ -21,6 +21,7 @@
 #include <asm/cpuid/api.h>
 #include <asm/pvclock.h>
 #include <asm/timer.h>
+#include <asm/xen/page.h>
 #include <asm/xen/hypervisor.h>
 #include <asm/xen/hypercall.h>
 #include <asm/xen/cpuid.h>
@@ -37,6 +38,28 @@
 
 static u64 xen_sched_clock_offset __read_mostly;
 
+/*
+ * Register the vcpu time info for vCPU 0 to pvti.
+ * When NULL is passed, unregister the time area instead.
+ *
+ * Register using physical address when available (reported by
+ * XENFEAT_vcpu_time_phys_area), and use virtual address otherwise.
+ */
+static int xen_register_time_area(struct pvclock_vcpu_time_info *pvti)
+{
+	struct vcpu_register_time_memory_area t;
+	bool phys = xen_feature(XENFEAT_vcpu_time_phys_area);
+	int op = phys ? VCPUOP_register_vcpu_time_phys_area
+		      : VCPUOP_register_vcpu_time_memory_area;
+
+	if (phys)
+		t.addr.p = pvti ? virt_to_guest(pvti).gaddr : ~0LLU;
+	else
+		t.addr.v = pvti;
+
+	return HYPERVISOR_vcpu_op(op, 0, &t);
+}
+
 /* Get the TSC speed from Xen */
 static unsigned long xen_tsc_khz(void)
 {
@@ -395,7 +418,6 @@ static u64 xen_clock_value_saved;
 
 void xen_save_time_memory_area(void)
 {
-	struct vcpu_register_time_memory_area t;
 	int ret;
 
 	xen_clock_value_saved = xen_clocksource_read() - xen_sched_clock_offset;
@@ -403,25 +425,20 @@ void xen_save_time_memory_area(void)
 	if (!xen_clock)
 		return;
 
-	t.addr.v = NULL;
-
-	ret = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_time_memory_area, 0, &t);
+	ret = xen_register_time_area(NULL);
 	if (ret != 0)
 		pr_notice("Cannot unregister vcpu_time_info (err %d)", ret);
 }
 
 void xen_restore_time_memory_area(void)
 {
-	struct vcpu_register_time_memory_area t;
 	int ret;
 
 	if (!xen_clock)
 		goto out;
 
 	memset(xen_clock, 0, sizeof(*xen_clock));
-	t.addr.v = &xen_clock->pvti;
-
-	ret = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_time_memory_area, 0, &t);
+	ret = xen_register_time_area(&xen_clock->pvti);
 
 	/*
 	 * We don't disable VDSO_CLOCKMODE_PVCLOCK entirely if it fails to
@@ -444,7 +461,6 @@ void xen_restore_time_memory_area(void)
 
 static void __init xen_setup_vsyscall_time_info(void)
 {
-	struct vcpu_register_time_memory_area t;
 	struct pvclock_vsyscall_time_info *ti;
 	int ret;
 
@@ -452,9 +468,7 @@ static void __init xen_setup_vsyscall_time_info(void)
 	if (!ti)
 		return;
 
-	t.addr.v = &ti->pvti;
-
-	ret = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_time_memory_area, 0, &t);
+	ret = xen_register_time_area(&ti->pvti);
 	if (ret) {
 		pr_notice("xen: VDSO_CLOCKMODE_PVCLOCK not supported (err %d)\n", ret);
 		free_page((unsigned long)ti);
@@ -467,9 +481,7 @@ static void __init xen_setup_vsyscall_time_info(void)
 	 * still check it in case hypervisor is buggy.
 	 */
 	if (!(ti->pvti.flags & PVCLOCK_TSC_STABLE_BIT)) {
-		t.addr.v = NULL;
-		ret = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_time_memory_area,
-					 0, &t);
+		ret = xen_register_time_area(NULL);
 		if (!ret)
 			free_page((unsigned long)ti);
 
drivers/xen/time.c +18 -6
index a2be0a4d45b0..49e61ad905d9 100644
@@ -10,6 +10,7 @@
 #include <linux/static_call.h>
 #include <linux/sched/cputime.h>
 
+#include <asm/xen/page.h>
 #include <asm/xen/hypervisor.h>
 #include <asm/xen/hypercall.h>
 
@@ -19,8 +20,12 @@
 #include <xen/interface/vcpu.h>
 #include <xen/xen-ops.h>
 
-/* runstate info updated by Xen */
-static DEFINE_PER_CPU(struct vcpu_runstate_info, xen_runstate);
+/*
+ * Runstate info updated by Xen. The phys-area ABI doesn't allow it to cross
+ * a page boundary; the struct is 48 bytes, so 64 is the smallest alignment
+ * that guarantees this.
+ */
+static DEFINE_PER_CPU(struct vcpu_runstate_info, xen_runstate) __aligned(64);
 
 static DEFINE_PER_CPU(u64[4], old_runstate_time);
 
@@ -152,11 +157,18 @@ u64 xen_steal_clock(int cpu)
 void xen_setup_runstate_info(int cpu)
 {
 	struct vcpu_register_runstate_memory_area area;
+	unsigned int op;
+	struct vcpu_runstate_info *cpu_runstate = &per_cpu(xen_runstate, cpu);
+
+	if (xen_feature(XENFEAT_runstate_phys_area)) {
+		op = VCPUOP_register_runstate_phys_area;
+		area.addr.p = percpu_to_gaddr(cpu_runstate).gaddr;
+	} else {
+		op = VCPUOP_register_runstate_memory_area;
+		area.addr.v = cpu_runstate;
+	}
 
-	area.addr.v = &per_cpu(xen_runstate, cpu);
-
-	if (HYPERVISOR_vcpu_op(VCPUOP_register_runstate_memory_area,
-			       xen_vcpu_nr(cpu), &area))
+	if (HYPERVISOR_vcpu_op(op, xen_vcpu_nr(cpu), &area))
 		BUG();
 }
 
include/xen/interface/features.h +9 -0
index 53f760378e39..f00144a7c790 100644
@@ -97,6 +97,15 @@
 #define XENFEAT_not_direct_mapped         16
 #define XENFEAT_direct_mapped             17
 
+/*
+ * Signal whether the domain is able to use the following hypercalls:
+ *
+ * VCPUOP_register_runstate_phys_area
+ * VCPUOP_register_vcpu_time_phys_area
+ */
+#define XENFEAT_runstate_phys_area        18
+#define XENFEAT_vcpu_time_phys_area       19
+
 #define XENFEAT_NR_SUBMAPS 1
 
 #endif /* __XEN_PUBLIC_FEATURES_H__ */
include/xen/interface/vcpu.h +17 -0
index c7cc28ad8d63..d3858233c32f 100644
@@ -203,4 +203,21 @@ struct vcpu_register_time_memory_area {
 };
 DEFINE_GUEST_HANDLE_STRUCT(vcpu_register_time_memory_area);
 
+/*
+ * Like the respective VCPUOP_register_*_memory_area, just using the "addr.p"
+ * field of the supplied struct as a guest physical address (i.e. in GFN space).
+ * The respective area may not cross a page boundary.  Pass ~0 to unregister an
+ * area.  Note that as long as an area is registered by physical address, the
+ * linear address based area will not be serviced (updated) by the hypervisor.
+ *
+ * Note that the area registered via VCPUOP_register_runstate_phys_area will
+ * be updated in the same manner as the one registered via virtual address PLUS
+ * VMASST_TYPE_runstate_update_flag engaged by the domain.
+ *
+ * XENFEAT_{runstate,vcpu_time}_phys_area feature bits signal the availability
+ * of these ops.
+ */
+#define VCPUOP_register_runstate_phys_area      14
+#define VCPUOP_register_vcpu_time_phys_area     15
+
 #endif /* __XEN_PUBLIC_VCPU_H__ */
Signature · 10 lines
2.56.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
maintainer Reviewed-by on v2 7/8
#13 lore raw

Re: [PATCH v2 7/8] x86/xen/time: Use VCPUOP_register_{runstate,vcpu_time}_phys_area when available

Quoted 11 lines · Teddy Astie

Updating these area can be expensive when done through virtual addresses (especially for runstate one), in particular for HVM/PVH guests. Xen 4.18 introduced physical address alternatives to address those short-commings, but they aren't used currently.

Fix it by using these alternatives when available (when reported in XENFEAT_{runstate,vcpu_time}_phys_area.

Assisted-by LLM
[guidance and testing]

Juergen

#14 lore raw

RFC PATCH v2 8/8 xen/sysfs: Honor cached xen_features instead

In reply to a message not in the archive

Currently, /sys/hypervisor/properties/features queries the xen features from the hypervisor even if it doesn't match what Linux uses itself (i.e what were the features at the time of the boot).

For consistency when migrations are involved, report cached xen_features instead, that also avoids making hypercalls to query this information.

---
CC: Juergen Gross <[email protected]>
CC: Boris Ostrovsky <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Oleksandr Tyshchenko <[email protected]>
CC: [email protected]
CC: [email protected]

I'm not sure what /sys/hypervisor/features is expected to show between what Linux considers (proposal) and what the hypervisor tells (current behavior).

 drivers/xen/sys-hypervisor.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/xen/sys-hypervisor.c +4 -8
index 91923242a5ae..7786bb40b966 100644
@@ -4,6 +4,7 @@
  *  Authored by: Mike D. Day <[email protected]>
  */
 
+#include <linux/bitmap.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
@@ -15,6 +16,7 @@
 
 #include <xen/xen.h>
 #include <xen/xenbus.h>
+#include <xen/features.h>
 #include <xen/interface/xen.h>
 #include <xen/interface/version.h>
 #ifdef CONFIG_XEN_HAVE_VPMU
@@ -321,15 +323,9 @@ HYPERVISOR_ATTR_RO(pagesize);
 
 static ssize_t xen_feature_show(int index, char *buffer)
 {
-	ssize_t ret;
-	struct xen_feature_info info;
-
-	info.submap_idx = index;
-	ret = HYPERVISOR_xen_version(XENVER_get_features, &info);
-	if (!ret)
-		ret = sprintf(buffer, "%08x", info.submap);
+	unsigned long submap = bitmap_read(xen_features, index * 32, 32);
 
-	return ret;
+	return sprintf(buffer, "%08x", (unsigned int)submap);
 }
 
 static ssize_t features_show(struct hyp_sysfs_attr *attr, char *buffer)
Signature · 10 lines
2.56.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
maintainer Reviewed-by on v2 8/8
#15 lore raw

Re: [RFC PATCH v2 8/8] xen/sysfs: Honor cached xen_features instead

Quoted 17 lines · Teddy Astie

Currently, /sys/hypervisor/properties/features queries the xen features from the hypervisor even if it doesn't match what Linux uses itself (i.e what were the features at the time of the boot).

For consistency when migrations are involved, report cached xen_features instead, that also avoids making hypercalls to query this information.

---
CC: Juergen Gross <[email protected]>
CC: Boris Ostrovsky <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Oleksandr Tyshchenko <[email protected]>
CC: [email protected]
CC: [email protected]

Quoted 3 lines

I'm not sure what /sys/hypervisor/features is expected to show between what Linux considers (proposal) and what the hypervisor tells (current behavior).

I think it should be the former, so:

Juergen

#16 lore raw

PATCH v2 1/8 x86/xen/pv: Don't setup runstate area too early

In reply to Teddy Astie

This is no longer required since [1].

At this moment of boot, per_cpu areas are not initialized, which prevents using VCPUOP_register_runstate_phys_area as per_cpu_ptr_to_phys() would misbehave in this case.

[1] 8a22b9996b00 ("xen: drop xen_sched_clock in favour of using plain wallclock time")
---
CC: Juergen Gross <[email protected]>
CC: Boris Ostrovsky <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Oleksandr Tyshchenko <[email protected]>
CC: David Woodhouse <[email protected]>
CC: [email protected]
CC: [email protected]

 arch/x86/xen/enlighten_pv.c | 3 ---
 1 file changed, 3 deletions(-)
arch/x86/xen/enlighten_pv.c +0 -3
index 2c64b388f616..ab3374b2f012 100644
@@ -1551,9 +1551,6 @@ asmlinkage __visible void __init xen_start_kernel(struct start_info *si)
 #endif
 	xen_raw_console_write("about to get started...\n");
 
-	/* We need this for printk timestamps */
-	xen_setup_runstate_info(0);
-
 	xen_efi_init(&boot_params);
 
 	/* Start the world */
Signature · 10 lines
2.56.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
#17 lore raw

Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early

Quoted 5 lines · Teddy Astie

This is no longer required since [1].

At this moment of boot, per_cpu areas are not initialized, which prevents using VCPUOP_register_runstate_phys_area as per_cpu_ptr_to_phys() would misbehave in this case.

This isn't really correct.

The per-cpu area for the BSP is within the main image, which is why the logic functions today.  What's missing is the call to pcpu_setup_first_chunk() which tells per_cpu_ptr_to_phys() about where the BSP data is.

I think you want to say:

"A future change wants to use per_cpu_ptr_to_phys() in xen_setup_runstate_info(), but this call is too early.  It turns out to be no longer required since ..."

Quoted 3 lines

[1] 8a22b9996b00 ("xen: drop xen_sched_clock in favour of using plain wallclock time")
---

Missing SoB.

~Andrew

#18 lore raw

Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early

Quoted 19 lines · Andrew Cooper
Quoted 5 lines · Teddy Astie

This is no longer required since [1].

At this moment of boot, per_cpu areas are not initialized, which prevents using VCPUOP_register_runstate_phys_area as per_cpu_ptr_to_phys() would misbehave in this case.

This isn't really correct.

The per-cpu area for the BSP is within the main image, which is why the logic functions today.  What's missing is the call to pcpu_setup_first_chunk() which tells per_cpu_ptr_to_phys() about where the BSP data is.

I think you want to say:

"A future change wants to use per_cpu_ptr_to_phys() in xen_setup_runstate_info(), but this call is too early.  It turns out to be no longer required since ..."

Quoted 5 lines
Quoted 2 lines

[1] 8a22b9996b00 ("xen: drop xen_sched_clock in favour of using plain wallclock time")
---

Missing SoB.

~Andrew

But this does pose another question...   Linux only ever registers time referencing vCPU 0.  Why are the targets of the registration in the per-cpu area?  That's presumably wasted space on all APs.

~Andrew

#19 lore raw

Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early

Le 16/10/2026 à 20:04, Andrew Cooper a écrit :

Quoted 22 lines · Andrew Cooper
Quoted 21 lines · Teddy Astie
Quoted 5 lines

This is no longer required since [1].

At this moment of boot, per_cpu areas are not initialized, which prevents using VCPUOP_register_runstate_phys_area as per_cpu_ptr_to_phys() would misbehave in this case.

This isn't really correct.

The per-cpu area for the BSP is within the main image, which is why the logic functions today.  What's missing is the call to pcpu_setup_first_chunk() which tells per_cpu_ptr_to_phys() about where the BSP data is.

I think you want to say:

"A future change wants to use per_cpu_ptr_to_phys() in xen_setup_runstate_info(), but this call is too early.  It turns out to be no longer required since ..."

Quoted 2 lines

[1] 8a22b9996b00 ("xen: drop xen_sched_clock in favour of using plain wallclock time")
---

Quoted 8 lines
Quoted 3 lines

Missing SoB.

~Andrew

But this does pose another question...   Linux only ever registers time referencing vCPU 0.  Why are the targets of the registration in the per-cpu area?  That's presumably wasted space on all APs.

Yes but only for this early PV case, there are other places where xen_setup_runstate_info() is called for other APs.

But looking at it, there may be redundant calls of xen_setup_runstate_info().

Quoted 1 line

~Andrew

Teddy

#20 lore raw

Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early

In reply to Andrew Cooper

Le 16/10/2026 à 19:34, Andrew Cooper a écrit :

Quoted 14 lines · Teddy Astie
Quoted 5 lines

This is no longer required since [1].

At this moment of boot, per_cpu areas are not initialized, which prevents using VCPUOP_register_runstate_phys_area as per_cpu_ptr_to_phys() would misbehave in this case.

This isn't really correct.

The per-cpu area for the BSP is within the main image, which is why the logic functions today.  What's missing is the call to pcpu_setup_first_chunk() which tells per_cpu_ptr_to_phys() about where the BSP data is.

Quoted 6 lines

I think you want to say:

"A future change wants to use per_cpu_ptr_to_phys() in xen_setup_runstate_info(), but this call is too early.  It turns out to be no longer required since ..."

Right, I was thinking that the use of the per_cpu was incorrect at this point. But that's not exactly the issue here.

Will adjust for the v3.

Quoted 7 lines
Quoted 3 lines

[1] 8a22b9996b00 ("xen: drop xen_sched_clock in favour of using plain wallclock time")
---

Missing SoB.

~Andrew

Teddy

maintainer Reviewed-by
#21 lore raw

Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early

Quoted 17 lines · Teddy Astie

Le 16/10/2026 à 19:34, Andrew Cooper a écrit :

Quoted 16 lines · Teddy Astie
Quoted 5 lines

This is no longer required since [1].

At this moment of boot, per_cpu areas are not initialized, which prevents using VCPUOP_register_runstate_phys_area as per_cpu_ptr_to_phys() would misbehave in this case.

This isn't really correct.

The per-cpu area for the BSP is within the main image, which is why the logic functions today.  What's missing is the call to pcpu_setup_first_chunk() which tells per_cpu_ptr_to_phys() about where the BSP data is.

I think you want to say:

Quoted 8 lines
Quoted 4 lines

"A future change wants to use per_cpu_ptr_to_phys() in xen_setup_runstate_info(), but this call is too early.  It turns out to be no longer required since ..."

Right, I was thinking that the use of the per_cpu was incorrect at this point. But that's not exactly the issue here.

Will adjust for the v3.

With the suggested wording above:

Juergen