Various improvements on runstate and vcpu_time areas + xen_features
Subsystems (3)
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.
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]
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
@@ -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
@@ -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
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).
* Markxen_setup_featuresas__init, as it's only called in__initpaths.
* Return bool instead of int inxen_feature()Suggested-by Andrew Cooper <[email protected]>Signed-off-by Teddy Astie <[email protected]>
Juergen
PATCH v2 3/8 xen: Add gpa conversion helpers
In reply to a message not in the archiveIntroduce 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
@@ -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
@@ -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
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.Signed-off-by Teddy Astie <[email protected]>
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
PATCH v2 4/8 x86/xen/time: Correct xen_save_time_memory_area() failure message
In reply to a message not in the archiveWe 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
@@ -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
Re: [PATCH v2 4/8] x86/xen/time: Correct xen_save_time_memory_area() failure message
Quoted 5 lines · Teddy Astie
We actually didn't fail to "save secondary vcpu time info" but just to unregister the mapping of it.
Reported-by Andrew Cooper <[email protected]>Signed-off-by Teddy Astie <[email protected]>
Juergen
PATCH v2 5/8 x86/xen/time: Clear xen_clock mapping later
In reply to a message not in the archiveThe 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.
---
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
@@ -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
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()andxen_restore_time_memory_area()fails. In this case, the mapping is never cleared and kept with previous pvclock values (invalidating the comment regardingVDSO_CLOCKMODE_PVCLOCKas the data may not have been zeroed).Also convert the
clear_pagewith 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")Signed-off-by Teddy Astie <[email protected]>
Juergen
PATCH v2 6/8 x86/xen/time: Mark xen_setup_vsyscall_time_info() __init
In reply to a message not in the archiveThis 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
@@ -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
Re: [PATCH v2 6/8] x86/xen/time: Mark xen_setup_vsyscall_time_info() __init
Quoted 3 lines · Teddy Astie
This function is only called by
xen_time_init()being itself__init.Signed-off-by Teddy Astie <[email protected]>
Juergen
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 archiveUpdating 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.
---
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
@@ -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
@@ -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
@@ -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
@@ -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
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.
Juergen
RFC PATCH v2 8/8 xen/sysfs: Honor cached xen_features instead
In reply to a message not in the archiveCurrently, /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
@@ -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
Re: [RFC PATCH v2 8/8] xen/sysfs: Honor cached xen_features instead
Quoted 17 lines · Teddy Astie
Currently,
/sys/hypervisor/properties/featuresqueries 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_featuresinstead, that also avoids making hypercalls to query this information.Signed-off-by Teddy Astie <[email protected]>---
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/featuresis expected to show between what Linux considers (proposal) and what the hypervisor tells (current behavior).
I think it should be the former, so:
Juergen
PATCH v2 1/8 x86/xen/pv: Don't setup runstate area too early
In reply to Teddy AstieThis 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
@@ -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
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_cpuareas are not initialized, which prevents usingVCPUOP_register_runstate_phys_areaasper_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: dropxen_sched_clockin favour of using plain wallclock time")
---
Missing SoB.
~Andrew
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_cpuareas are not initialized, which prevents usingVCPUOP_register_runstate_phys_areaasper_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 tellsper_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()inxen_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: dropxen_sched_clockin 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
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_cpuareas are not initialized, which prevents usingVCPUOP_register_runstate_phys_areaasper_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 tellsper_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()inxen_setup_runstate_info(), but this call is too early. It turns out to be no longer required since ..."Quoted 2 lines
[1]
8a22b9996b00("xen: dropxen_sched_clockin 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
Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early
In reply to Andrew CooperLe 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_cpuareas are not initialized, which prevents usingVCPUOP_register_runstate_phys_areaasper_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 tellsper_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()inxen_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: dropxen_sched_clockin favour of using plain wallclock time")
---Missing SoB.
~Andrew
Teddy
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_cpuareas are not initialized, which prevents usingVCPUOP_register_runstate_phys_areaasper_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 tellsper_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()inxen_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_cpuwas incorrect at this point. But that's not exactly the issue here.Will adjust for the v3.
With the suggested wording above:
Juergen