xen/arch/x86/hvm/Kconfig | 10 ++++++++++ xen/arch/x86/hvm/Makefile | 2 +- xen/arch/x86/hvm/hvm.c | 27 ++++++++++++++++++--------- xen/arch/x86/hvm/viridian/viridian.c | 14 ++++++++++---- xen/arch/x86/hvm/vlapic.c | 11 +++++++---- xen/arch/x86/include/asm/hvm/domain.h | 2 ++ xen/arch/x86/include/asm/hvm/hvm.h | 3 ++- xen/arch/x86/include/asm/hvm/vcpu.h | 2 ++ 8 files changed, 52 insertions(+), 19 deletions(-)
From: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Add config option VIRIDIAN that covers viridian code within HVM.
Calls to viridian functions guarded by is_viridian_domain() and related macros.
Having this option may be beneficial by reducing code footprint for systems
that are not using Hyper-V.
[grygorii_strashko@epam.com: fixed NULL pointer deref in
viridian_save_domain_ctxt()]
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
changes in v5:
- drop "depends on AMD_SVM || INTEL_VMX"
- return -EILSEQ from viridian_load_x() if !VIRIDIAN
changes in v4:
- s/HVM_VIRIDIAN/VIRIDIAN
- add "depends on AMD_SVM || INTEL_VMX"
- add guard !is_viridian_vcpu() checks in viridian_load_vcpu_ctxt/viridian_load_domain_ctxt
changes in v3:
- fixed NULL pointer deref in viridian_save_domain_ctxt() reported for v2,
which caused v2 revert by commit 1fffcf10cd71 ("Revert "x86: make Viridian
support optional"")
v4: https://patchwork.kernel.org/project/xen-devel/patch/20250919163139.2821531-1-grygorii_strashko@epam.com/
v3: https://patchwork.kernel.org/project/xen-devel/patch/20250916134114.2214104-1-grygorii_strashko@epam.com/
v2: https://patchwork.kernel.org/project/xen-devel/patch/20250321092633.3982645-1-Sergiy_Kibrik@epam.com/
xen/arch/x86/hvm/Kconfig | 10 ++++++++++
xen/arch/x86/hvm/Makefile | 2 +-
xen/arch/x86/hvm/hvm.c | 27 ++++++++++++++++++---------
xen/arch/x86/hvm/viridian/viridian.c | 14 ++++++++++----
xen/arch/x86/hvm/vlapic.c | 11 +++++++----
xen/arch/x86/include/asm/hvm/domain.h | 2 ++
xen/arch/x86/include/asm/hvm/hvm.h | 3 ++-
xen/arch/x86/include/asm/hvm/vcpu.h | 2 ++
8 files changed, 52 insertions(+), 19 deletions(-)
diff --git a/xen/arch/x86/hvm/Kconfig b/xen/arch/x86/hvm/Kconfig
index 5cb9f2904255..928bb5662b89 100644
--- a/xen/arch/x86/hvm/Kconfig
+++ b/xen/arch/x86/hvm/Kconfig
@@ -62,6 +62,16 @@ config ALTP2M
If unsure, stay with defaults.
+config VIRIDIAN
+ bool "Hyper-V enlightenments for guests" if EXPERT
+ default y
+ help
+ Support optimizations for Hyper-V guests such as faster hypercalls,
+ efficient timer and interrupt handling, and enhanced paravirtualized
+ I/O. This is to improve performance and compatibility of Windows VMs.
+
+ If unsure, say Y.
+
config MEM_PAGING
bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
depends on VM_EVENT
diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
index 6ec2c8f2db56..736eb3f966e9 100644
--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -1,6 +1,6 @@
obj-$(CONFIG_AMD_SVM) += svm/
obj-$(CONFIG_INTEL_VMX) += vmx/
-obj-y += viridian/
+obj-$(CONFIG_VIRIDIAN) += viridian/
obj-y += asid.o
obj-y += dm.o
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 23bd7f078a1d..95a80369b9b8 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
if ( hvm_tsc_scaling_supported )
d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
- rc = viridian_domain_init(d);
- if ( rc )
- goto fail2;
+ if ( is_viridian_domain(d) )
+ {
+ rc = viridian_domain_init(d);
+ if ( rc )
+ goto fail2;
+ }
rc = alternative_call(hvm_funcs.domain_initialise, d);
if ( rc != 0 )
@@ -739,7 +742,8 @@ void hvm_domain_relinquish_resources(struct domain *d)
if ( hvm_funcs.nhvm_domain_relinquish_resources )
alternative_vcall(hvm_funcs.nhvm_domain_relinquish_resources, d);
- viridian_domain_deinit(d);
+ if ( is_viridian_domain(d) )
+ viridian_domain_deinit(d);
ioreq_server_destroy_all(d);
@@ -1643,9 +1647,12 @@ int hvm_vcpu_initialise(struct vcpu *v)
&& (rc = nestedhvm_vcpu_initialise(v)) < 0 ) /* teardown: nestedhvm_vcpu_destroy */
goto fail5;
- rc = viridian_vcpu_init(v);
- if ( rc )
- goto fail6;
+ if ( is_viridian_domain(d) )
+ {
+ rc = viridian_vcpu_init(v);
+ if ( rc )
+ goto fail6;
+ }
rc = ioreq_server_add_vcpu_all(d, v);
if ( rc != 0 )
@@ -1675,13 +1682,15 @@ int hvm_vcpu_initialise(struct vcpu *v)
fail2:
hvm_vcpu_cacheattr_destroy(v);
fail1:
- viridian_vcpu_deinit(v);
+ if ( is_viridian_domain(d) )
+ viridian_vcpu_deinit(v);
return rc;
}
void hvm_vcpu_destroy(struct vcpu *v)
{
- viridian_vcpu_deinit(v);
+ if ( is_viridian_domain(v->domain) )
+ viridian_vcpu_deinit(v);
ioreq_server_remove_vcpu_all(v->domain, v);
diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index c0be24bd2210..1212cc418728 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -1116,14 +1116,14 @@ static int cf_check viridian_save_domain_ctxt(
{
const struct domain *d = v->domain;
const struct viridian_domain *vd = d->arch.hvm.viridian;
- struct hvm_viridian_domain_context ctxt = {
- .hypercall_gpa = vd->hypercall_gpa.raw,
- .guest_os_id = vd->guest_os_id.raw,
- };
+ struct hvm_viridian_domain_context ctxt = {};
if ( !is_viridian_domain(d) )
return 0;
+ ctxt.hypercall_gpa = vd->hypercall_gpa.raw;
+ ctxt.guest_os_id = vd->guest_os_id.raw,
+
viridian_time_save_domain_ctxt(d, &ctxt);
viridian_synic_save_domain_ctxt(d, &ctxt);
@@ -1136,6 +1136,9 @@ static int cf_check viridian_load_domain_ctxt(
struct viridian_domain *vd = d->arch.hvm.viridian;
struct hvm_viridian_domain_context ctxt;
+ if ( !is_viridian_domain(d) )
+ return -EILSEQ;
+
if ( hvm_load_entry_zeroextend(VIRIDIAN_DOMAIN, h, &ctxt) != 0 )
return -EINVAL;
@@ -1172,6 +1175,9 @@ static int cf_check viridian_load_vcpu_ctxt(
struct vcpu *v;
struct hvm_viridian_vcpu_context ctxt;
+ if ( !is_viridian_domain(d) )
+ return -EILSEQ;
+
if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
{
dprintk(XENLOG_G_ERR, "HVM restore: dom%d has no vcpu%u\n",
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 993e972cd71e..79697487ba90 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -426,7 +426,8 @@ void vlapic_EOI_set(struct vlapic *vlapic)
* priority vector and then recurse to handle the lower priority
* vector.
*/
- bool missed_eoi = viridian_apic_assist_completed(v);
+ bool missed_eoi = has_viridian_apic_assist(v->domain) &&
+ viridian_apic_assist_completed(v);
int vector;
again:
@@ -442,7 +443,7 @@ void vlapic_EOI_set(struct vlapic *vlapic)
* NOTE: It is harmless to call viridian_apic_assist_clear() on a
* recursion, even though it is not necessary.
*/
- if ( !missed_eoi )
+ if ( has_viridian_apic_assist(v->domain) && !missed_eoi )
viridian_apic_assist_clear(v);
vlapic_clear_vector(vector, &vlapic->regs->data[APIC_ISR]);
@@ -1360,7 +1361,8 @@ int vlapic_has_pending_irq(struct vcpu *v)
* If so, we need to emulate the EOI here before comparing ISR
* with IRR.
*/
- if ( viridian_apic_assist_completed(v) )
+ if ( has_viridian_apic_assist(v->domain) &&
+ viridian_apic_assist_completed(v) )
vlapic_EOI_set(vlapic);
isr = vlapic_find_highest_isr(vlapic);
@@ -1373,7 +1375,8 @@ int vlapic_has_pending_irq(struct vcpu *v)
if ( isr >= 0 &&
(irr & 0xf0) <= (isr & 0xf0) )
{
- viridian_apic_assist_clear(v);
+ if ( has_viridian_apic_assist(v->domain) )
+ viridian_apic_assist_clear(v);
return -1;
}
diff --git a/xen/arch/x86/include/asm/hvm/domain.h b/xen/arch/x86/include/asm/hvm/domain.h
index 333501d5f2ac..95d9336a28f0 100644
--- a/xen/arch/x86/include/asm/hvm/domain.h
+++ b/xen/arch/x86/include/asm/hvm/domain.h
@@ -111,7 +111,9 @@ struct hvm_domain {
/* hypervisor intercepted msix table */
struct list_head msixtbl_list;
+#ifdef CONFIG_VIRIDIAN
struct viridian_domain *viridian;
+#endif
/*
* TSC value that VCPUs use to calculate their tsc_offset value.
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
index f02183691ea6..7312cdd878e1 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -510,7 +510,8 @@ hvm_get_cpl(struct vcpu *v)
(has_hvm_params(d) ? (d)->arch.hvm.params[HVM_PARAM_VIRIDIAN] : 0)
#define is_viridian_domain(d) \
- (is_hvm_domain(d) && (viridian_feature_mask(d) & HVMPV_base_freq))
+ (IS_ENABLED(CONFIG_VIRIDIAN) && \
+ is_hvm_domain(d) && (viridian_feature_mask(d) & HVMPV_base_freq))
#define is_viridian_vcpu(v) \
is_viridian_domain((v)->domain)
diff --git a/xen/arch/x86/include/asm/hvm/vcpu.h b/xen/arch/x86/include/asm/hvm/vcpu.h
index 924af890c5b2..9ed9eaff3bc5 100644
--- a/xen/arch/x86/include/asm/hvm/vcpu.h
+++ b/xen/arch/x86/include/asm/hvm/vcpu.h
@@ -176,7 +176,9 @@ struct hvm_vcpu {
/* Pending hw/sw interrupt (.vector = -1 means nothing pending). */
struct x86_event inject_event;
+#ifdef CONFIG_VIRIDIAN
struct viridian_vcpu *viridian;
+#endif
};
#endif /* __ASM_X86_HVM_VCPU_H__ */
--
2.34.1On Tue, Sep 30, 2025 at 12:52:16PM +0000, Grygorii Strashko wrote:
> From: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>
> Add config option VIRIDIAN that covers viridian code within HVM.
> Calls to viridian functions guarded by is_viridian_domain() and related macros.
> Having this option may be beneficial by reducing code footprint for systems
> that are not using Hyper-V.
>
> [grygorii_strashko@epam.com: fixed NULL pointer deref in
> viridian_save_domain_ctxt()]
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
> ---
> changes in v5:
> - drop "depends on AMD_SVM || INTEL_VMX"
> - return -EILSEQ from viridian_load_x() if !VIRIDIAN
>
> changes in v4:
> - s/HVM_VIRIDIAN/VIRIDIAN
> - add "depends on AMD_SVM || INTEL_VMX"
> - add guard !is_viridian_vcpu() checks in viridian_load_vcpu_ctxt/viridian_load_domain_ctxt
>
> changes in v3:
> - fixed NULL pointer deref in viridian_save_domain_ctxt() reported for v2,
> which caused v2 revert by commit 1fffcf10cd71 ("Revert "x86: make Viridian
> support optional"")
>
> v4: https://patchwork.kernel.org/project/xen-devel/patch/20250919163139.2821531-1-grygorii_strashko@epam.com/
> v3: https://patchwork.kernel.org/project/xen-devel/patch/20250916134114.2214104-1-grygorii_strashko@epam.com/
> v2: https://patchwork.kernel.org/project/xen-devel/patch/20250321092633.3982645-1-Sergiy_Kibrik@epam.com/
>
> xen/arch/x86/hvm/Kconfig | 10 ++++++++++
> xen/arch/x86/hvm/Makefile | 2 +-
> xen/arch/x86/hvm/hvm.c | 27 ++++++++++++++++++---------
> xen/arch/x86/hvm/viridian/viridian.c | 14 ++++++++++----
> xen/arch/x86/hvm/vlapic.c | 11 +++++++----
> xen/arch/x86/include/asm/hvm/domain.h | 2 ++
> xen/arch/x86/include/asm/hvm/hvm.h | 3 ++-
> xen/arch/x86/include/asm/hvm/vcpu.h | 2 ++
> 8 files changed, 52 insertions(+), 19 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/Kconfig b/xen/arch/x86/hvm/Kconfig
> index 5cb9f2904255..928bb5662b89 100644
> --- a/xen/arch/x86/hvm/Kconfig
> +++ b/xen/arch/x86/hvm/Kconfig
> @@ -62,6 +62,16 @@ config ALTP2M
>
> If unsure, stay with defaults.
>
> +config VIRIDIAN
> + bool "Hyper-V enlightenments for guests" if EXPERT
> + default y
> + help
> + Support optimizations for Hyper-V guests such as faster hypercalls,
> + efficient timer and interrupt handling, and enhanced paravirtualized
> + I/O. This is to improve performance and compatibility of Windows VMs.
I would leave "paravirtualized I/O" out of the text, as the hypervisor
Viridian extensions don't provide anything related to I/O. AFAICT
that would be the vmbus stuff, which I'm not sure is supported when
running as a Xen guest, and would require QEMU to emulate such
interfaces? IOW: the paravirtualized I/O part is out-of-scope for an
hypervisor-only related config option:
Support optimizations for Hyper-V guests such as hypercalls,
efficient timers and interrupt handling. This is to improve
performance and compatibility of Windows VMs.
Nit: I would also drop the "faster" prefix for hypercalls. Without
this option enabled there are no Hyper-V hypercalls available,
neither fast nor slow.
> +
> + If unsure, say Y.
> +
> config MEM_PAGING
> bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
> depends on VM_EVENT
> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> index 6ec2c8f2db56..736eb3f966e9 100644
> --- a/xen/arch/x86/hvm/Makefile
> +++ b/xen/arch/x86/hvm/Makefile
> @@ -1,6 +1,6 @@
> obj-$(CONFIG_AMD_SVM) += svm/
> obj-$(CONFIG_INTEL_VMX) += vmx/
> -obj-y += viridian/
> +obj-$(CONFIG_VIRIDIAN) += viridian/
>
> obj-y += asid.o
> obj-y += dm.o
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 23bd7f078a1d..95a80369b9b8 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
> if ( hvm_tsc_scaling_supported )
> d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
>
> - rc = viridian_domain_init(d);
> - if ( rc )
> - goto fail2;
> + if ( is_viridian_domain(d) )
> + {
> + rc = viridian_domain_init(d);
> + if ( rc )
> + goto fail2;
> + }
Are you sure this works as expected?
The viridian_feature_mask() check is implemented using an HVM param,
and hence can only be possibly set after the domain object is created.
AFAICT is_viridian_domain(d) will unconditionally return false when
called from domain_create() context, because the HVM params cannot
possibly be set ahead of the domain being created.
If you want to do anything like this you will possibly need to
introduce a new flag to XEN_DOMCTL_createdomain to signal whether the
domain has Viridian extensions are enabled or not, so that it's know
in the context where domain_create() gets called.
Given that HyperV is available on arm64 also it should be a global
flag, as opposed to a per-arch one in xen_arch_domainconfig IMO.
>
> rc = alternative_call(hvm_funcs.domain_initialise, d);
> if ( rc != 0 )
> @@ -739,7 +742,8 @@ void hvm_domain_relinquish_resources(struct domain *d)
> if ( hvm_funcs.nhvm_domain_relinquish_resources )
> alternative_vcall(hvm_funcs.nhvm_domain_relinquish_resources, d);
>
> - viridian_domain_deinit(d);
> + if ( is_viridian_domain(d) )
> + viridian_domain_deinit(d);
>
> ioreq_server_destroy_all(d);
>
> @@ -1643,9 +1647,12 @@ int hvm_vcpu_initialise(struct vcpu *v)
> && (rc = nestedhvm_vcpu_initialise(v)) < 0 ) /* teardown: nestedhvm_vcpu_destroy */
> goto fail5;
>
> - rc = viridian_vcpu_init(v);
> - if ( rc )
> - goto fail6;
> + if ( is_viridian_domain(d) )
> + {
> + rc = viridian_vcpu_init(v);
> + if ( rc )
> + goto fail6;
> + }
>
> rc = ioreq_server_add_vcpu_all(d, v);
> if ( rc != 0 )
> @@ -1675,13 +1682,15 @@ int hvm_vcpu_initialise(struct vcpu *v)
> fail2:
> hvm_vcpu_cacheattr_destroy(v);
> fail1:
> - viridian_vcpu_deinit(v);
> + if ( is_viridian_domain(d) )
> + viridian_vcpu_deinit(v);
> return rc;
> }
>
> void hvm_vcpu_destroy(struct vcpu *v)
> {
> - viridian_vcpu_deinit(v);
> + if ( is_viridian_domain(v->domain) )
> + viridian_vcpu_deinit(v);
>
> ioreq_server_remove_vcpu_all(v->domain, v);
>
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
> index c0be24bd2210..1212cc418728 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -1116,14 +1116,14 @@ static int cf_check viridian_save_domain_ctxt(
> {
> const struct domain *d = v->domain;
> const struct viridian_domain *vd = d->arch.hvm.viridian;
> - struct hvm_viridian_domain_context ctxt = {
> - .hypercall_gpa = vd->hypercall_gpa.raw,
> - .guest_os_id = vd->guest_os_id.raw,
> - };
> + struct hvm_viridian_domain_context ctxt = {};
>
> if ( !is_viridian_domain(d) )
> return 0;
>
> + ctxt.hypercall_gpa = vd->hypercall_gpa.raw;
> + ctxt.guest_os_id = vd->guest_os_id.raw,
> +
> viridian_time_save_domain_ctxt(d, &ctxt);
> viridian_synic_save_domain_ctxt(d, &ctxt);
>
> @@ -1136,6 +1136,9 @@ static int cf_check viridian_load_domain_ctxt(
> struct viridian_domain *vd = d->arch.hvm.viridian;
> struct hvm_viridian_domain_context ctxt;
>
> + if ( !is_viridian_domain(d) )
> + return -EILSEQ;
> +
> if ( hvm_load_entry_zeroextend(VIRIDIAN_DOMAIN, h, &ctxt) != 0 )
> return -EINVAL;
>
> @@ -1172,6 +1175,9 @@ static int cf_check viridian_load_vcpu_ctxt(
> struct vcpu *v;
> struct hvm_viridian_vcpu_context ctxt;
>
> + if ( !is_viridian_domain(d) )
> + return -EILSEQ;
Nit: we usually use EILSEQ for unreachable conditions, but here it's a
toolstack controlled input. Maybe we could instead use ENODEV here?
As it's not really an illegal restore stream, but the selected guest
configuration doesn't match what's then loaded.
Thanks, Roger.
Hi Roger, All,
Thank you for your comments.
On 13.10.25 15:17, Roger Pau Monné wrote:
> On Tue, Sep 30, 2025 at 12:52:16PM +0000, Grygorii Strashko wrote:
>> From: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>>
>> Add config option VIRIDIAN that covers viridian code within HVM.
>> Calls to viridian functions guarded by is_viridian_domain() and related macros.
>> Having this option may be beneficial by reducing code footprint for systems
>> that are not using Hyper-V.
>>
>> [grygorii_strashko@epam.com: fixed NULL pointer deref in
>> viridian_save_domain_ctxt()]
>> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>> ---
>> changes in v5:
>> - drop "depends on AMD_SVM || INTEL_VMX"
>> - return -EILSEQ from viridian_load_x() if !VIRIDIAN
>>
>> changes in v4:
>> - s/HVM_VIRIDIAN/VIRIDIAN
>> - add "depends on AMD_SVM || INTEL_VMX"
>> - add guard !is_viridian_vcpu() checks in viridian_load_vcpu_ctxt/viridian_load_domain_ctxt
>>
>> changes in v3:
>> - fixed NULL pointer deref in viridian_save_domain_ctxt() reported for v2,
>> which caused v2 revert by commit 1fffcf10cd71 ("Revert "x86: make Viridian
>> support optional"")
>>
>> v4: https://patchwork.kernel.org/project/xen-devel/patch/20250919163139.2821531-1-grygorii_strashko@epam.com/
>> v3: https://patchwork.kernel.org/project/xen-devel/patch/20250916134114.2214104-1-grygorii_strashko@epam.com/
>> v2: https://patchwork.kernel.org/project/xen-devel/patch/20250321092633.3982645-1-Sergiy_Kibrik@epam.com/
>>
>> xen/arch/x86/hvm/Kconfig | 10 ++++++++++
>> xen/arch/x86/hvm/Makefile | 2 +-
>> xen/arch/x86/hvm/hvm.c | 27 ++++++++++++++++++---------
>> xen/arch/x86/hvm/viridian/viridian.c | 14 ++++++++++----
>> xen/arch/x86/hvm/vlapic.c | 11 +++++++----
>> xen/arch/x86/include/asm/hvm/domain.h | 2 ++
>> xen/arch/x86/include/asm/hvm/hvm.h | 3 ++-
>> xen/arch/x86/include/asm/hvm/vcpu.h | 2 ++
>> 8 files changed, 52 insertions(+), 19 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/Kconfig b/xen/arch/x86/hvm/Kconfig
>> index 5cb9f2904255..928bb5662b89 100644
>> --- a/xen/arch/x86/hvm/Kconfig
>> +++ b/xen/arch/x86/hvm/Kconfig
>> @@ -62,6 +62,16 @@ config ALTP2M
>>
>> If unsure, stay with defaults.
>>
>> +config VIRIDIAN
>> + bool "Hyper-V enlightenments for guests" if EXPERT
>> + default y
>> + help
>> + Support optimizations for Hyper-V guests such as faster hypercalls,
>> + efficient timer and interrupt handling, and enhanced paravirtualized
>> + I/O. This is to improve performance and compatibility of Windows VMs.
>
> I would leave "paravirtualized I/O" out of the text, as the hypervisor
> Viridian extensions don't provide anything related to I/O. AFAICT
> that would be the vmbus stuff, which I'm not sure is supported when
> running as a Xen guest, and would require QEMU to emulate such
> interfaces? IOW: the paravirtualized I/O part is out-of-scope for an
> hypervisor-only related config option:
>
> Support optimizations for Hyper-V guests such as hypercalls,
> efficient timers and interrupt handling. This is to improve
> performance and compatibility of Windows VMs.
>
> Nit: I would also drop the "faster" prefix for hypercalls. Without
> this option enabled there are no Hyper-V hypercalls available,
> neither fast nor slow.
If no objections I'll change it as you proposed (and thank you for this proposal):
"
Support optimizations for Hyper-V guests such as hypercalls,
efficient timers and interrupt handling. This is to improve
performance and compatibility of Windows VMs.
"
>
>
>> +
>> + If unsure, say Y.
>> +
>> config MEM_PAGING
>> bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
>> depends on VM_EVENT
>> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
>> index 6ec2c8f2db56..736eb3f966e9 100644
>> --- a/xen/arch/x86/hvm/Makefile
>> +++ b/xen/arch/x86/hvm/Makefile
>> @@ -1,6 +1,6 @@
>> obj-$(CONFIG_AMD_SVM) += svm/
>> obj-$(CONFIG_INTEL_VMX) += vmx/
>> -obj-y += viridian/
>> +obj-$(CONFIG_VIRIDIAN) += viridian/
>>
>> obj-y += asid.o
>> obj-y += dm.o
>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> index 23bd7f078a1d..95a80369b9b8 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
>> if ( hvm_tsc_scaling_supported )
>> d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
>>
>> - rc = viridian_domain_init(d);
>> - if ( rc )
>> - goto fail2;
>> + if ( is_viridian_domain(d) )
>> + {
>> + rc = viridian_domain_init(d);
>> + if ( rc )
>> + goto fail2;
>> + }
>
> Are you sure this works as expected?
>
> The viridian_feature_mask() check is implemented using an HVM param,
> and hence can only be possibly set after the domain object is created.
> AFAICT is_viridian_domain(d) will unconditionally return false when
> called from domain_create() context, because the HVM params cannot
> possibly be set ahead of the domain being created.
You are right. Thanks for the this catch.
Taking above into account above, it seems Jan's proposal to convert below
viridian APIs into wrappers for VIRIDIAN=n case is right way to move forward:
int viridian_vcpu_init(struct vcpu *v);
int viridian_domain_init(struct domain *d);
void viridian_vcpu_deinit(struct vcpu *v);
void viridian_domain_deinit(struct domain *d);
Right?
[1] https://patchwork.kernel.org/comment/26595213/
>
> If you want to do anything like this you will possibly need to
> introduce a new flag to XEN_DOMCTL_createdomain to signal whether the
> domain has Viridian extensions are enabled or not, so that it's know
> in the context where domain_create() gets called.
In my opinion, it might be good not to go so far within this submission.
- It's not intended to change existing behavior of neither Xen nor toolstack
for VIRIDIAN=y (default)
- just optout Viridian support when not needed.
>
> Given that HyperV is available on arm64 also it should be a global
> flag, as opposed to a per-arch one in xen_arch_domainconfig IMO.
>>
>> rc = alternative_call(hvm_funcs.domain_initialise, d);
>> if ( rc != 0 )
>> @@ -739,7 +742,8 @@ void hvm_domain_relinquish_resources(struct domain *d)
>> if ( hvm_funcs.nhvm_domain_relinquish_resources )
>> alternative_vcall(hvm_funcs.nhvm_domain_relinquish_resources, d);
>>
>> - viridian_domain_deinit(d);
>> + if ( is_viridian_domain(d) )
>> + viridian_domain_deinit(d);
>>
>> ioreq_server_destroy_all(d);
>>
>> @@ -1643,9 +1647,12 @@ int hvm_vcpu_initialise(struct vcpu *v)
>> && (rc = nestedhvm_vcpu_initialise(v)) < 0 ) /* teardown: nestedhvm_vcpu_destroy */
>> goto fail5;
>>
>> - rc = viridian_vcpu_init(v);
>> - if ( rc )
>> - goto fail6;
>> + if ( is_viridian_domain(d) )
>> + {
>> + rc = viridian_vcpu_init(v);
>> + if ( rc )
>> + goto fail6;
>> + }
>>
>> rc = ioreq_server_add_vcpu_all(d, v);
>> if ( rc != 0 )
>> @@ -1675,13 +1682,15 @@ int hvm_vcpu_initialise(struct vcpu *v)
>> fail2:
>> hvm_vcpu_cacheattr_destroy(v);
>> fail1:
>> - viridian_vcpu_deinit(v);
>> + if ( is_viridian_domain(d) )
>> + viridian_vcpu_deinit(v);
>> return rc;
>> }
>>
>> void hvm_vcpu_destroy(struct vcpu *v)
>> {
>> - viridian_vcpu_deinit(v);
>> + if ( is_viridian_domain(v->domain) )
>> + viridian_vcpu_deinit(v);
>>
>> ioreq_server_remove_vcpu_all(v->domain, v);
>>
>> diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
>> index c0be24bd2210..1212cc418728 100644
>> --- a/xen/arch/x86/hvm/viridian/viridian.c
>> +++ b/xen/arch/x86/hvm/viridian/viridian.c
>> @@ -1116,14 +1116,14 @@ static int cf_check viridian_save_domain_ctxt(
>> {
>> const struct domain *d = v->domain;
>> const struct viridian_domain *vd = d->arch.hvm.viridian;
>> - struct hvm_viridian_domain_context ctxt = {
>> - .hypercall_gpa = vd->hypercall_gpa.raw,
>> - .guest_os_id = vd->guest_os_id.raw,
>> - };
>> + struct hvm_viridian_domain_context ctxt = {};
>>
>> if ( !is_viridian_domain(d) )
>> return 0;
>>
>> + ctxt.hypercall_gpa = vd->hypercall_gpa.raw;
>> + ctxt.guest_os_id = vd->guest_os_id.raw,
>> +
>> viridian_time_save_domain_ctxt(d, &ctxt);
>> viridian_synic_save_domain_ctxt(d, &ctxt);
>>
>> @@ -1136,6 +1136,9 @@ static int cf_check viridian_load_domain_ctxt(
>> struct viridian_domain *vd = d->arch.hvm.viridian;
>> struct hvm_viridian_domain_context ctxt;
>>
>> + if ( !is_viridian_domain(d) )
>> + return -EILSEQ;
>> +
>> if ( hvm_load_entry_zeroextend(VIRIDIAN_DOMAIN, h, &ctxt) != 0 )
>> return -EINVAL;
>>
>> @@ -1172,6 +1175,9 @@ static int cf_check viridian_load_vcpu_ctxt(
>> struct vcpu *v;
>> struct hvm_viridian_vcpu_context ctxt;
>>
>> + if ( !is_viridian_domain(d) )
>> + return -EILSEQ;
>
> Nit: we usually use EILSEQ for unreachable conditions, but here it's a
> toolstack controlled input. Maybe we could instead use ENODEV here?
>
> As it's not really an illegal restore stream, but the selected guest
> configuration doesn't match what's then loaded.
I'm a "working bee" here and can change it once again her to -ENODEV here.
But It will be really cool if it will be agreed on Maintainers level somehow.
EILSEQ was used as per [2]
[2] https://patchwork.kernel.org/project/xen-devel/patch/20250919163139.2821531-1-grygorii_strashko@epam.com/#26579990
--
Best regards,
-grygorii
On Tue, Oct 14, 2025 at 04:24:53PM +0300, Grygorii Strashko wrote:
> On 13.10.25 15:17, Roger Pau Monné wrote:
> > On Tue, Sep 30, 2025 at 12:52:16PM +0000, Grygorii Strashko wrote:
> > > From: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> > > +
> > > + If unsure, say Y.
> > > +
> > > config MEM_PAGING
> > > bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
> > > depends on VM_EVENT
> > > diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> > > index 6ec2c8f2db56..736eb3f966e9 100644
> > > --- a/xen/arch/x86/hvm/Makefile
> > > +++ b/xen/arch/x86/hvm/Makefile
> > > @@ -1,6 +1,6 @@
> > > obj-$(CONFIG_AMD_SVM) += svm/
> > > obj-$(CONFIG_INTEL_VMX) += vmx/
> > > -obj-y += viridian/
> > > +obj-$(CONFIG_VIRIDIAN) += viridian/
> > > obj-y += asid.o
> > > obj-y += dm.o
> > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > > index 23bd7f078a1d..95a80369b9b8 100644
> > > --- a/xen/arch/x86/hvm/hvm.c
> > > +++ b/xen/arch/x86/hvm/hvm.c
> > > @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
> > > if ( hvm_tsc_scaling_supported )
> > > d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
> > > - rc = viridian_domain_init(d);
> > > - if ( rc )
> > > - goto fail2;
> > > + if ( is_viridian_domain(d) )
> > > + {
> > > + rc = viridian_domain_init(d);
> > > + if ( rc )
> > > + goto fail2;
> > > + }
> >
> > Are you sure this works as expected?
> >
> > The viridian_feature_mask() check is implemented using an HVM param,
> > and hence can only be possibly set after the domain object is created.
> > AFAICT is_viridian_domain(d) will unconditionally return false when
> > called from domain_create() context, because the HVM params cannot
> > possibly be set ahead of the domain being created.
>
> You are right. Thanks for the this catch.
>
> Taking above into account above, it seems Jan's proposal to convert below
> viridian APIs into wrappers for VIRIDIAN=n case is right way to move forward:
>
> int viridian_vcpu_init(struct vcpu *v);
> int viridian_domain_init(struct domain *d);
> void viridian_vcpu_deinit(struct vcpu *v);
> void viridian_domain_deinit(struct domain *d);
>
> Right?
Possibly. If you don't want to introduce a XEN_DOMCTL_createdomain
flag you need to exclusively use the Kconfig option to decide whether
the Viridian related structs must be allocated. IOW: you could also
solve it by using IS_ENABLED(CONFIG_VIRIDIAN) instead of
is_viridian_domain() for most of the calls here.
The wrapper option might be better IMO, rather than adding
IS_ENABLED(CONFIG_VIRIDIAN) around.
> [1] https://patchwork.kernel.org/comment/26595213/
>
> >
> > If you want to do anything like this you will possibly need to
> > introduce a new flag to XEN_DOMCTL_createdomain to signal whether the
> > domain has Viridian extensions are enabled or not, so that it's know
> > in the context where domain_create() gets called.
>
> In my opinion, it might be good not to go so far within this submission.
> - It's not intended to change existing behavior of neither Xen nor toolstack
> for VIRIDIAN=y (default)
> - just optout Viridian support when not needed.
OK, that's fine.
On further request though: if Viridian is build-time disabled in
Kconfig, setting or fetching HVM_PARAM_VIRIDIAN should return -ENODEV
or similar error. I don't think this is done as part of this patch.
> >
> > Given that HyperV is available on arm64 also it should be a global
> > flag, as opposed to a per-arch one in xen_arch_domainconfig IMO.
> > > rc = alternative_call(hvm_funcs.domain_initialise, d);
> > > if ( rc != 0 )
> > > @@ -739,7 +742,8 @@ void hvm_domain_relinquish_resources(struct domain *d)
> > > if ( hvm_funcs.nhvm_domain_relinquish_resources )
> > > alternative_vcall(hvm_funcs.nhvm_domain_relinquish_resources, d);
> > > - viridian_domain_deinit(d);
> > > + if ( is_viridian_domain(d) )
> > > + viridian_domain_deinit(d);
> > > ioreq_server_destroy_all(d);
> > > @@ -1643,9 +1647,12 @@ int hvm_vcpu_initialise(struct vcpu *v)
> > > && (rc = nestedhvm_vcpu_initialise(v)) < 0 ) /* teardown: nestedhvm_vcpu_destroy */
> > > goto fail5;
> > > - rc = viridian_vcpu_init(v);
> > > - if ( rc )
> > > - goto fail6;
> > > + if ( is_viridian_domain(d) )
> > > + {
> > > + rc = viridian_vcpu_init(v);
> > > + if ( rc )
> > > + goto fail6;
> > > + }
> > > rc = ioreq_server_add_vcpu_all(d, v);
> > > if ( rc != 0 )
> > > @@ -1675,13 +1682,15 @@ int hvm_vcpu_initialise(struct vcpu *v)
> > > fail2:
> > > hvm_vcpu_cacheattr_destroy(v);
> > > fail1:
> > > - viridian_vcpu_deinit(v);
> > > + if ( is_viridian_domain(d) )
> > > + viridian_vcpu_deinit(v);
> > > return rc;
> > > }
> > > void hvm_vcpu_destroy(struct vcpu *v)
> > > {
> > > - viridian_vcpu_deinit(v);
> > > + if ( is_viridian_domain(v->domain) )
> > > + viridian_vcpu_deinit(v);
> > > ioreq_server_remove_vcpu_all(v->domain, v);
> > > diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
> > > index c0be24bd2210..1212cc418728 100644
> > > --- a/xen/arch/x86/hvm/viridian/viridian.c
> > > +++ b/xen/arch/x86/hvm/viridian/viridian.c
> > > @@ -1116,14 +1116,14 @@ static int cf_check viridian_save_domain_ctxt(
> > > {
> > > const struct domain *d = v->domain;
> > > const struct viridian_domain *vd = d->arch.hvm.viridian;
> > > - struct hvm_viridian_domain_context ctxt = {
> > > - .hypercall_gpa = vd->hypercall_gpa.raw,
> > > - .guest_os_id = vd->guest_os_id.raw,
> > > - };
> > > + struct hvm_viridian_domain_context ctxt = {};
> > > if ( !is_viridian_domain(d) )
> > > return 0;
> > > + ctxt.hypercall_gpa = vd->hypercall_gpa.raw;
> > > + ctxt.guest_os_id = vd->guest_os_id.raw,
> > > +
> > > viridian_time_save_domain_ctxt(d, &ctxt);
> > > viridian_synic_save_domain_ctxt(d, &ctxt);
> > > @@ -1136,6 +1136,9 @@ static int cf_check viridian_load_domain_ctxt(
> > > struct viridian_domain *vd = d->arch.hvm.viridian;
> > > struct hvm_viridian_domain_context ctxt;
> > > + if ( !is_viridian_domain(d) )
> > > + return -EILSEQ;
> > > +
> > > if ( hvm_load_entry_zeroextend(VIRIDIAN_DOMAIN, h, &ctxt) != 0 )
> > > return -EINVAL;
> > > @@ -1172,6 +1175,9 @@ static int cf_check viridian_load_vcpu_ctxt(
> > > struct vcpu *v;
> > > struct hvm_viridian_vcpu_context ctxt;
> > > + if ( !is_viridian_domain(d) )
> > > + return -EILSEQ;
> >
> > Nit: we usually use EILSEQ for unreachable conditions, but here it's a
> > toolstack controlled input. Maybe we could instead use ENODEV here?
> >
> > As it's not really an illegal restore stream, but the selected guest
> > configuration doesn't match what's then loaded.
>
> I'm a "working bee" here and can change it once again her to -ENODEV here.
> But It will be really cool if it will be agreed on Maintainers level somehow.
>
> EILSEQ was used as per [2]
Didn't know it was explicitly requested, then leave it like that and
ignore this comment.
Thanks, Roger.
On 14.10.25 17:38, Roger Pau Monné wrote:
> On Tue, Oct 14, 2025 at 04:24:53PM +0300, Grygorii Strashko wrote:
>> On 13.10.25 15:17, Roger Pau Monné wrote:
>>> On Tue, Sep 30, 2025 at 12:52:16PM +0000, Grygorii Strashko wrote:
>>>> From: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>>>> +
>>>> + If unsure, say Y.
>>>> +
>>>> config MEM_PAGING
>>>> bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
>>>> depends on VM_EVENT
>>>> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
>>>> index 6ec2c8f2db56..736eb3f966e9 100644
>>>> --- a/xen/arch/x86/hvm/Makefile
>>>> +++ b/xen/arch/x86/hvm/Makefile
>>>> @@ -1,6 +1,6 @@
>>>> obj-$(CONFIG_AMD_SVM) += svm/
>>>> obj-$(CONFIG_INTEL_VMX) += vmx/
>>>> -obj-y += viridian/
>>>> +obj-$(CONFIG_VIRIDIAN) += viridian/
>>>> obj-y += asid.o
>>>> obj-y += dm.o
>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>>> index 23bd7f078a1d..95a80369b9b8 100644
>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>> @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
>>>> if ( hvm_tsc_scaling_supported )
>>>> d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
>>>> - rc = viridian_domain_init(d);
>>>> - if ( rc )
>>>> - goto fail2;
>>>> + if ( is_viridian_domain(d) )
>>>> + {
>>>> + rc = viridian_domain_init(d);
>>>> + if ( rc )
>>>> + goto fail2;
>>>> + }
>>>
>>> Are you sure this works as expected?
>>>
>>> The viridian_feature_mask() check is implemented using an HVM param,
>>> and hence can only be possibly set after the domain object is created.
>>> AFAICT is_viridian_domain(d) will unconditionally return false when
>>> called from domain_create() context, because the HVM params cannot
>>> possibly be set ahead of the domain being created.
>>
>> You are right. Thanks for the this catch.
>>
>> Taking above into account above, it seems Jan's proposal to convert below
>> viridian APIs into wrappers for VIRIDIAN=n case is right way to move forward:
>>
>> int viridian_vcpu_init(struct vcpu *v);
>> int viridian_domain_init(struct domain *d);
>> void viridian_vcpu_deinit(struct vcpu *v);
>> void viridian_domain_deinit(struct domain *d);
>>
>> Right?
>
> Possibly. If you don't want to introduce a XEN_DOMCTL_createdomain
> flag you need to exclusively use the Kconfig option to decide whether
> the Viridian related structs must be allocated. IOW: you could also
> solve it by using IS_ENABLED(CONFIG_VIRIDIAN) instead of
> is_viridian_domain() for most of the calls here.
>
> The wrapper option might be better IMO, rather than adding
> IS_ENABLED(CONFIG_VIRIDIAN) around.
I'll do wrappers - less if(s) in common HVM code.
>
>> [1] https://patchwork.kernel.org/comment/26595213/
>>
>>>
>>> If you want to do anything like this you will possibly need to
>>> introduce a new flag to XEN_DOMCTL_createdomain to signal whether the
>>> domain has Viridian extensions are enabled or not, so that it's know
>>> in the context where domain_create() gets called.
>>
>> In my opinion, it might be good not to go so far within this submission.
>> - It's not intended to change existing behavior of neither Xen nor toolstack
>> for VIRIDIAN=y (default)
>> - just optout Viridian support when not needed.
>
> OK, that's fine.
>
> On further request though: if Viridian is build-time disabled in
> Kconfig, setting or fetching HVM_PARAM_VIRIDIAN should return -ENODEV
> or similar error. I don't think this is done as part of this patch.
Sure. Just have to ask for clarification what to return:
-EOPNOTSUPP (my choise) vs -EINVAL.
?
>
>>>
>>> Given that HyperV is available on arm64 also it should be a global
>>> flag, as opposed to a per-arch one in xen_arch_domainconfig IMO.
>>>> rc = alternative_call(hvm_funcs.domain_initialise, d);
>>>> if ( rc != 0 )
>>>> @@ -739,7 +742,8 @@ void hvm_domain_relinquish_resources(struct domain *d)
>>>> if ( hvm_funcs.nhvm_domain_relinquish_resources )
>>>> alternative_vcall(hvm_funcs.nhvm_domain_relinquish_resources, d);
>>>> - viridian_domain_deinit(d);
>>>> + if ( is_viridian_domain(d) )
>>>> + viridian_domain_deinit(d);
>>>> ioreq_server_destroy_all(d);
>>>> @@ -1643,9 +1647,12 @@ int hvm_vcpu_initialise(struct vcpu *v)
>>>> && (rc = nestedhvm_vcpu_initialise(v)) < 0 ) /* teardown: nestedhvm_vcpu_destroy */
>>>> goto fail5;
>>>> - rc = viridian_vcpu_init(v);
>>>> - if ( rc )
>>>> - goto fail6;
>>>> + if ( is_viridian_domain(d) )
>>>> + {
>>>> + rc = viridian_vcpu_init(v);
>>>> + if ( rc )
>>>> + goto fail6;
>>>> + }
>>>> rc = ioreq_server_add_vcpu_all(d, v);
>>>> if ( rc != 0 )
>>>> @@ -1675,13 +1682,15 @@ int hvm_vcpu_initialise(struct vcpu *v)
>>>> fail2:
>>>> hvm_vcpu_cacheattr_destroy(v);
>>>> fail1:
>>>> - viridian_vcpu_deinit(v);
>>>> + if ( is_viridian_domain(d) )
>>>> + viridian_vcpu_deinit(v);
>>>> return rc;
>>>> }
>>>> void hvm_vcpu_destroy(struct vcpu *v)
>>>> {
>>>> - viridian_vcpu_deinit(v);
>>>> + if ( is_viridian_domain(v->domain) )
>>>> + viridian_vcpu_deinit(v);
>>>> ioreq_server_remove_vcpu_all(v->domain, v);
>>>> diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
>>>> index c0be24bd2210..1212cc418728 100644
>>>> --- a/xen/arch/x86/hvm/viridian/viridian.c
>>>> +++ b/xen/arch/x86/hvm/viridian/viridian.c
>>>> @@ -1116,14 +1116,14 @@ static int cf_check viridian_save_domain_ctxt(
>>>> {
>>>> const struct domain *d = v->domain;
>>>> const struct viridian_domain *vd = d->arch.hvm.viridian;
>>>> - struct hvm_viridian_domain_context ctxt = {
>>>> - .hypercall_gpa = vd->hypercall_gpa.raw,
>>>> - .guest_os_id = vd->guest_os_id.raw,
>>>> - };
>>>> + struct hvm_viridian_domain_context ctxt = {};
>>>> if ( !is_viridian_domain(d) )
>>>> return 0;
>>>> + ctxt.hypercall_gpa = vd->hypercall_gpa.raw;
>>>> + ctxt.guest_os_id = vd->guest_os_id.raw,
>>>> +
>>>> viridian_time_save_domain_ctxt(d, &ctxt);
>>>> viridian_synic_save_domain_ctxt(d, &ctxt);
>>>> @@ -1136,6 +1136,9 @@ static int cf_check viridian_load_domain_ctxt(
>>>> struct viridian_domain *vd = d->arch.hvm.viridian;
>>>> struct hvm_viridian_domain_context ctxt;
>>>> + if ( !is_viridian_domain(d) )
>>>> + return -EILSEQ;
>>>> +
>>>> if ( hvm_load_entry_zeroextend(VIRIDIAN_DOMAIN, h, &ctxt) != 0 )
>>>> return -EINVAL;
>>>> @@ -1172,6 +1175,9 @@ static int cf_check viridian_load_vcpu_ctxt(
>>>> struct vcpu *v;
>>>> struct hvm_viridian_vcpu_context ctxt;
>>>> + if ( !is_viridian_domain(d) )
>>>> + return -EILSEQ;
>>>
>>> Nit: we usually use EILSEQ for unreachable conditions, but here it's a
>>> toolstack controlled input. Maybe we could instead use ENODEV here?
>>>
>>> As it's not really an illegal restore stream, but the selected guest
>>> configuration doesn't match what's then loaded.
>>
>> I'm a "working bee" here and can change it once again her to -ENODEV here.
>> But It will be really cool if it will be agreed on Maintainers level somehow.
>>
>> EILSEQ was used as per [2]
>
> Didn't know it was explicitly requested, then leave it like that and
> ignore this comment.
>
> Thanks, Roger.
--
Best regards,
-grygorii
On Tue, Oct 14, 2025 at 06:48:23PM +0300, Grygorii Strashko wrote:
>
>
> On 14.10.25 17:38, Roger Pau Monné wrote:
> > On Tue, Oct 14, 2025 at 04:24:53PM +0300, Grygorii Strashko wrote:
> > > On 13.10.25 15:17, Roger Pau Monné wrote:
> > > > On Tue, Sep 30, 2025 at 12:52:16PM +0000, Grygorii Strashko wrote:
> > > > > From: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> > > > > +
> > > > > + If unsure, say Y.
> > > > > +
> > > > > config MEM_PAGING
> > > > > bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
> > > > > depends on VM_EVENT
> > > > > diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> > > > > index 6ec2c8f2db56..736eb3f966e9 100644
> > > > > --- a/xen/arch/x86/hvm/Makefile
> > > > > +++ b/xen/arch/x86/hvm/Makefile
> > > > > @@ -1,6 +1,6 @@
> > > > > obj-$(CONFIG_AMD_SVM) += svm/
> > > > > obj-$(CONFIG_INTEL_VMX) += vmx/
> > > > > -obj-y += viridian/
> > > > > +obj-$(CONFIG_VIRIDIAN) += viridian/
> > > > > obj-y += asid.o
> > > > > obj-y += dm.o
> > > > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > > > > index 23bd7f078a1d..95a80369b9b8 100644
> > > > > --- a/xen/arch/x86/hvm/hvm.c
> > > > > +++ b/xen/arch/x86/hvm/hvm.c
> > > > > @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
> > > > > if ( hvm_tsc_scaling_supported )
> > > > > d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
> > > > > - rc = viridian_domain_init(d);
> > > > > - if ( rc )
> > > > > - goto fail2;
> > > > > + if ( is_viridian_domain(d) )
> > > > > + {
> > > > > + rc = viridian_domain_init(d);
> > > > > + if ( rc )
> > > > > + goto fail2;
> > > > > + }
> > > >
> > > > Are you sure this works as expected?
> > > >
> > > > The viridian_feature_mask() check is implemented using an HVM param,
> > > > and hence can only be possibly set after the domain object is created.
> > > > AFAICT is_viridian_domain(d) will unconditionally return false when
> > > > called from domain_create() context, because the HVM params cannot
> > > > possibly be set ahead of the domain being created.
> > >
> > > You are right. Thanks for the this catch.
> > >
> > > Taking above into account above, it seems Jan's proposal to convert below
> > > viridian APIs into wrappers for VIRIDIAN=n case is right way to move forward:
> > >
> > > int viridian_vcpu_init(struct vcpu *v);
> > > int viridian_domain_init(struct domain *d);
> > > void viridian_vcpu_deinit(struct vcpu *v);
> > > void viridian_domain_deinit(struct domain *d);
> > >
> > > Right?
> >
> > Possibly. If you don't want to introduce a XEN_DOMCTL_createdomain
> > flag you need to exclusively use the Kconfig option to decide whether
> > the Viridian related structs must be allocated. IOW: you could also
> > solve it by using IS_ENABLED(CONFIG_VIRIDIAN) instead of
> > is_viridian_domain() for most of the calls here.
> >
> > The wrapper option might be better IMO, rather than adding
> > IS_ENABLED(CONFIG_VIRIDIAN) around.
>
> I'll do wrappers - less if(s) in common HVM code.
>
> >
> > > [1] https://patchwork.kernel.org/comment/26595213/
> > >
> > > >
> > > > If you want to do anything like this you will possibly need to
> > > > introduce a new flag to XEN_DOMCTL_createdomain to signal whether the
> > > > domain has Viridian extensions are enabled or not, so that it's know
> > > > in the context where domain_create() gets called.
> > >
> > > In my opinion, it might be good not to go so far within this submission.
> > > - It's not intended to change existing behavior of neither Xen nor toolstack
> > > for VIRIDIAN=y (default)
> > > - just optout Viridian support when not needed.
> >
> > OK, that's fine.
> >
> > On further request though: if Viridian is build-time disabled in
> > Kconfig, setting or fetching HVM_PARAM_VIRIDIAN should return -ENODEV
> > or similar error. I don't think this is done as part of this patch.
Another bit I've noticed, you will need to adjust write_hvm_params()
so it can tolerate xc_hvm_param_get() returning an error when
HVM_PARAM_VIRIDIAN is not implemented by the hypervisor.
Implementing the Viridian features using an HVM parameter was a bad
approach probably.
> Sure. Just have to ask for clarification what to return:
> -EOPNOTSUPP (my choise) vs -EINVAL.
Let me add Jan also to the To: field so we get consensus in one round.
I won't use EINVAL, because that's returned for deprecated parameters
also, and when the passed Viridian feature mask is invalid.
EOPNOTSUPP is also returned for non-implemented hypercalls, so I'm not
sure whether it could cause confusion here, as the hypercall is
implemented, it's just the param that's not supported if
build-disabled. Maybe ENODEV or ENXIO?
Thanks, Roger.
On 15.10.25 11:00, Roger Pau Monné wrote:
> On Tue, Oct 14, 2025 at 06:48:23PM +0300, Grygorii Strashko wrote:
>>
>>
>> On 14.10.25 17:38, Roger Pau Monné wrote:
>>> On Tue, Oct 14, 2025 at 04:24:53PM +0300, Grygorii Strashko wrote:
>>>> On 13.10.25 15:17, Roger Pau Monné wrote:
>>>>> On Tue, Sep 30, 2025 at 12:52:16PM +0000, Grygorii Strashko wrote:
>>>>>> From: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>>>>>> +
>>>>>> + If unsure, say Y.
>>>>>> +
>>>>>> config MEM_PAGING
>>>>>> bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
>>>>>> depends on VM_EVENT
>>>>>> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
>>>>>> index 6ec2c8f2db56..736eb3f966e9 100644
>>>>>> --- a/xen/arch/x86/hvm/Makefile
>>>>>> +++ b/xen/arch/x86/hvm/Makefile
>>>>>> @@ -1,6 +1,6 @@
>>>>>> obj-$(CONFIG_AMD_SVM) += svm/
>>>>>> obj-$(CONFIG_INTEL_VMX) += vmx/
>>>>>> -obj-y += viridian/
>>>>>> +obj-$(CONFIG_VIRIDIAN) += viridian/
>>>>>> obj-y += asid.o
>>>>>> obj-y += dm.o
>>>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>>>>> index 23bd7f078a1d..95a80369b9b8 100644
>>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>>> @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
>>>>>> if ( hvm_tsc_scaling_supported )
>>>>>> d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
>>>>>> - rc = viridian_domain_init(d);
>>>>>> - if ( rc )
>>>>>> - goto fail2;
>>>>>> + if ( is_viridian_domain(d) )
>>>>>> + {
>>>>>> + rc = viridian_domain_init(d);
>>>>>> + if ( rc )
>>>>>> + goto fail2;
>>>>>> + }
>>>>>
>>>>> Are you sure this works as expected?
>>>>>
>>>>> The viridian_feature_mask() check is implemented using an HVM param,
>>>>> and hence can only be possibly set after the domain object is created.
>>>>> AFAICT is_viridian_domain(d) will unconditionally return false when
>>>>> called from domain_create() context, because the HVM params cannot
>>>>> possibly be set ahead of the domain being created.
>>>>
>>>> You are right. Thanks for the this catch.
>>>>
>>>> Taking above into account above, it seems Jan's proposal to convert below
>>>> viridian APIs into wrappers for VIRIDIAN=n case is right way to move forward:
>>>>
>>>> int viridian_vcpu_init(struct vcpu *v);
>>>> int viridian_domain_init(struct domain *d);
>>>> void viridian_vcpu_deinit(struct vcpu *v);
>>>> void viridian_domain_deinit(struct domain *d);
>>>>
>>>> Right?
>>>
>>> Possibly. If you don't want to introduce a XEN_DOMCTL_createdomain
>>> flag you need to exclusively use the Kconfig option to decide whether
>>> the Viridian related structs must be allocated. IOW: you could also
>>> solve it by using IS_ENABLED(CONFIG_VIRIDIAN) instead of
>>> is_viridian_domain() for most of the calls here.
>>>
>>> The wrapper option might be better IMO, rather than adding
>>> IS_ENABLED(CONFIG_VIRIDIAN) around.
>>
>> I'll do wrappers - less if(s) in common HVM code.
>>
>>>
>>>> [1] https://patchwork.kernel.org/comment/26595213/
>>>>
>>>>>
>>>>> If you want to do anything like this you will possibly need to
>>>>> introduce a new flag to XEN_DOMCTL_createdomain to signal whether the
>>>>> domain has Viridian extensions are enabled or not, so that it's know
>>>>> in the context where domain_create() gets called.
>>>>
>>>> In my opinion, it might be good not to go so far within this submission.
>>>> - It's not intended to change existing behavior of neither Xen nor toolstack
>>>> for VIRIDIAN=y (default)
[1]
>>>> - just optout Viridian support when not needed.
>>>
>>> OK, that's fine.
>>>
>>> On further request though: if Viridian is build-time disabled in
>>> Kconfig, setting or fetching HVM_PARAM_VIRIDIAN should return -ENODEV
>>> or similar error. I don't think this is done as part of this patch.
>
> Another bit I've noticed, you will need to adjust write_hvm_params()
> so it can tolerate xc_hvm_param_get() returning an error when
> HVM_PARAM_VIRIDIAN is not implemented by the hypervisor.
>
> Implementing the Viridian features using an HVM parameter was a bad
> approach probably.
I've just realized how toolstack need to be modified and all consequences...
Have to try to push back a little bit:
VIRIDIAN=n: Now HVM_PARAM_VIRIDIAN will be R/W with functionality NOP.
I'd prefer avoid modifying toolstack if possible.
How about sanitizing HVM_PARAM_VIRIDIAN to be RAZ/WI for VIRIDIAN=n?
Or may be produce Xen XENLOG_WARNING"Viridian is disabled" if value!=0?
This an EXPERT option and end-user can get Xen with VIRIDIAN=n only by
manually re-configuring and re-compiling Xen. In other words, the user
is an expert and knows what he is doing.
Another point, assume change like this is to be done for HVM_PARAM_VIRIDIAN
- there are another HVM_PARAM_x which depend on build-time disabled features, like:
HVM_PARAM_VM86_TSS_SIZED
HVM_PARAM_PAGING_RING_PFN,
HVM_PARAM_MONITOR_RING_PFN,
HVM_PARAM_SHARING_RING_PFN,
HVM_PARAM_IDENT_PT
...
if corresponding features are build-time disabled, above HVM_PARAM_x
become R/W with functionality NOP now.
>
>> Sure. Just have to ask for clarification what to return:
>> -EOPNOTSUPP (my choise) vs -EINVAL.
>
> Let me add Jan also to the To: field so we get consensus in one round.
>
> I won't use EINVAL, because that's returned for deprecated parameters
> also, and when the passed Viridian feature mask is invalid.
>
> EOPNOTSUPP is also returned for non-implemented hypercalls, so I'm not
> sure whether it could cause confusion here, as the hypercall is
> implemented, it's just the param that's not supported if
> build-disabled. Maybe ENODEV or ENXIO?
--
Best regards,
-grygorii
On Fri, Oct 17, 2025 at 12:40:33AM +0300, Grygorii Strashko wrote:
>
>
> On 15.10.25 11:00, Roger Pau Monné wrote:
> > On Tue, Oct 14, 2025 at 06:48:23PM +0300, Grygorii Strashko wrote:
> > >
> > >
> > > On 14.10.25 17:38, Roger Pau Monné wrote:
> > > > On Tue, Oct 14, 2025 at 04:24:53PM +0300, Grygorii Strashko wrote:
> > > > > On 13.10.25 15:17, Roger Pau Monné wrote:
> > > > > > On Tue, Sep 30, 2025 at 12:52:16PM +0000, Grygorii Strashko wrote:
> > > > > > > From: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> > > > > > > +
> > > > > > > + If unsure, say Y.
> > > > > > > +
> > > > > > > config MEM_PAGING
> > > > > > > bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
> > > > > > > depends on VM_EVENT
> > > > > > > diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> > > > > > > index 6ec2c8f2db56..736eb3f966e9 100644
> > > > > > > --- a/xen/arch/x86/hvm/Makefile
> > > > > > > +++ b/xen/arch/x86/hvm/Makefile
> > > > > > > @@ -1,6 +1,6 @@
> > > > > > > obj-$(CONFIG_AMD_SVM) += svm/
> > > > > > > obj-$(CONFIG_INTEL_VMX) += vmx/
> > > > > > > -obj-y += viridian/
> > > > > > > +obj-$(CONFIG_VIRIDIAN) += viridian/
> > > > > > > obj-y += asid.o
> > > > > > > obj-y += dm.o
> > > > > > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > > > > > > index 23bd7f078a1d..95a80369b9b8 100644
> > > > > > > --- a/xen/arch/x86/hvm/hvm.c
> > > > > > > +++ b/xen/arch/x86/hvm/hvm.c
> > > > > > > @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
> > > > > > > if ( hvm_tsc_scaling_supported )
> > > > > > > d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
> > > > > > > - rc = viridian_domain_init(d);
> > > > > > > - if ( rc )
> > > > > > > - goto fail2;
> > > > > > > + if ( is_viridian_domain(d) )
> > > > > > > + {
> > > > > > > + rc = viridian_domain_init(d);
> > > > > > > + if ( rc )
> > > > > > > + goto fail2;
> > > > > > > + }
> > > > > >
> > > > > > Are you sure this works as expected?
> > > > > >
> > > > > > The viridian_feature_mask() check is implemented using an HVM param,
> > > > > > and hence can only be possibly set after the domain object is created.
> > > > > > AFAICT is_viridian_domain(d) will unconditionally return false when
> > > > > > called from domain_create() context, because the HVM params cannot
> > > > > > possibly be set ahead of the domain being created.
> > > > >
> > > > > You are right. Thanks for the this catch.
> > > > >
> > > > > Taking above into account above, it seems Jan's proposal to convert below
> > > > > viridian APIs into wrappers for VIRIDIAN=n case is right way to move forward:
> > > > >
> > > > > int viridian_vcpu_init(struct vcpu *v);
> > > > > int viridian_domain_init(struct domain *d);
> > > > > void viridian_vcpu_deinit(struct vcpu *v);
> > > > > void viridian_domain_deinit(struct domain *d);
> > > > >
> > > > > Right?
> > > >
> > > > Possibly. If you don't want to introduce a XEN_DOMCTL_createdomain
> > > > flag you need to exclusively use the Kconfig option to decide whether
> > > > the Viridian related structs must be allocated. IOW: you could also
> > > > solve it by using IS_ENABLED(CONFIG_VIRIDIAN) instead of
> > > > is_viridian_domain() for most of the calls here.
> > > >
> > > > The wrapper option might be better IMO, rather than adding
> > > > IS_ENABLED(CONFIG_VIRIDIAN) around.
> > >
> > > I'll do wrappers - less if(s) in common HVM code.
> > >
> > > >
> > > > > [1] https://patchwork.kernel.org/comment/26595213/
> > > > >
> > > > > >
> > > > > > If you want to do anything like this you will possibly need to
> > > > > > introduce a new flag to XEN_DOMCTL_createdomain to signal whether the
> > > > > > domain has Viridian extensions are enabled or not, so that it's know
> > > > > > in the context where domain_create() gets called.
> > > > >
> > > > > In my opinion, it might be good not to go so far within this submission.
> > > > > - It's not intended to change existing behavior of neither Xen nor toolstack
> > > > > for VIRIDIAN=y (default)
>
> [1]
>
> > > > > - just optout Viridian support when not needed.
> > > >
> > > > OK, that's fine.
> > > >
> > > > On further request though: if Viridian is build-time disabled in
> > > > Kconfig, setting or fetching HVM_PARAM_VIRIDIAN should return -ENODEV
> > > > or similar error. I don't think this is done as part of this patch.
> >
> > Another bit I've noticed, you will need to adjust write_hvm_params()
> > so it can tolerate xc_hvm_param_get() returning an error when
> > HVM_PARAM_VIRIDIAN is not implemented by the hypervisor.
> >
> > Implementing the Viridian features using an HVM parameter was a bad
> > approach probably.
>
> I've just realized how toolstack need to be modified and all consequences...
> Have to try to push back a little bit:
>
> VIRIDIAN=n: Now HVM_PARAM_VIRIDIAN will be R/W with functionality NOP.
>
> I'd prefer avoid modifying toolstack if possible.
>
> How about sanitizing HVM_PARAM_VIRIDIAN to be RAZ/WI for VIRIDIAN=n?
I've been thinking more into it, and we must still allow writes/reads
to it, otherwise migration would fail when restoring a stream that
contains a Viridian record, even if Viridian is not enabled for the
domain. Right now the HVM param is unconditionally part of the
migration stream, even when Viridian is not enabled.
I think Xen could return an error when VIRIDIAN = n and a non-zero
value is passed to HVM_PARAM_VIRIDIAN?
That shouldn't require any changes to the toolstack, as when Viridian
is not enabled the toolstack will just set HVM_PARAM_VIRIDIAN = 0. It
would however still fail if toolstack attempts HVM_PARAM_VIRIDIAN != 0
and Viridian has been build time disabled.
Thanks, Roger.
On 17.10.25 10:38, Roger Pau Monné wrote:
> On Fri, Oct 17, 2025 at 12:40:33AM +0300, Grygorii Strashko wrote:
>>
>>
>> On 15.10.25 11:00, Roger Pau Monné wrote:
>>> On Tue, Oct 14, 2025 at 06:48:23PM +0300, Grygorii Strashko wrote:
>>>>
>>>>
>>>> On 14.10.25 17:38, Roger Pau Monné wrote:
>>>>> On Tue, Oct 14, 2025 at 04:24:53PM +0300, Grygorii Strashko wrote:
>>>>>> On 13.10.25 15:17, Roger Pau Monné wrote:
>>>>>>> On Tue, Sep 30, 2025 at 12:52:16PM +0000, Grygorii Strashko wrote:
>>>>>>>> From: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>>>>>>>> +
>>>>>>>> + If unsure, say Y.
>>>>>>>> +
>>>>>>>> config MEM_PAGING
>>>>>>>> bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
>>>>>>>> depends on VM_EVENT
>>>>>>>> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
>>>>>>>> index 6ec2c8f2db56..736eb3f966e9 100644
>>>>>>>> --- a/xen/arch/x86/hvm/Makefile
>>>>>>>> +++ b/xen/arch/x86/hvm/Makefile
>>>>>>>> @@ -1,6 +1,6 @@
>>>>>>>> obj-$(CONFIG_AMD_SVM) += svm/
>>>>>>>> obj-$(CONFIG_INTEL_VMX) += vmx/
>>>>>>>> -obj-y += viridian/
>>>>>>>> +obj-$(CONFIG_VIRIDIAN) += viridian/
>>>>>>>> obj-y += asid.o
>>>>>>>> obj-y += dm.o
>>>>>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>>>>>>> index 23bd7f078a1d..95a80369b9b8 100644
>>>>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>>>>> @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
>>>>>>>> if ( hvm_tsc_scaling_supported )
>>>>>>>> d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
>>>>>>>> - rc = viridian_domain_init(d);
>>>>>>>> - if ( rc )
>>>>>>>> - goto fail2;
>>>>>>>> + if ( is_viridian_domain(d) )
>>>>>>>> + {
>>>>>>>> + rc = viridian_domain_init(d);
>>>>>>>> + if ( rc )
>>>>>>>> + goto fail2;
>>>>>>>> + }
>>>>>>>
>>>>>>> Are you sure this works as expected?
>>>>>>>
>>>>>>> The viridian_feature_mask() check is implemented using an HVM param,
>>>>>>> and hence can only be possibly set after the domain object is created.
>>>>>>> AFAICT is_viridian_domain(d) will unconditionally return false when
>>>>>>> called from domain_create() context, because the HVM params cannot
>>>>>>> possibly be set ahead of the domain being created.
>>>>>>
>>>>>> You are right. Thanks for the this catch.
>>>>>>
>>>>>> Taking above into account above, it seems Jan's proposal to convert below
>>>>>> viridian APIs into wrappers for VIRIDIAN=n case is right way to move forward:
>>>>>>
>>>>>> int viridian_vcpu_init(struct vcpu *v);
>>>>>> int viridian_domain_init(struct domain *d);
>>>>>> void viridian_vcpu_deinit(struct vcpu *v);
>>>>>> void viridian_domain_deinit(struct domain *d);
>>>>>>
>>>>>> Right?
>>>>>
>>>>> Possibly. If you don't want to introduce a XEN_DOMCTL_createdomain
>>>>> flag you need to exclusively use the Kconfig option to decide whether
>>>>> the Viridian related structs must be allocated. IOW: you could also
>>>>> solve it by using IS_ENABLED(CONFIG_VIRIDIAN) instead of
>>>>> is_viridian_domain() for most of the calls here.
>>>>>
>>>>> The wrapper option might be better IMO, rather than adding
>>>>> IS_ENABLED(CONFIG_VIRIDIAN) around.
>>>>
>>>> I'll do wrappers - less if(s) in common HVM code.
>>>>
>>>>>
>>>>>> [1] https://patchwork.kernel.org/comment/26595213/
>>>>>>
>>>>>>>
>>>>>>> If you want to do anything like this you will possibly need to
>>>>>>> introduce a new flag to XEN_DOMCTL_createdomain to signal whether the
>>>>>>> domain has Viridian extensions are enabled or not, so that it's know
>>>>>>> in the context where domain_create() gets called.
>>>>>>
>>>>>> In my opinion, it might be good not to go so far within this submission.
>>>>>> - It's not intended to change existing behavior of neither Xen nor toolstack
>>>>>> for VIRIDIAN=y (default)
>>
>> [1]
>>
>>>>>> - just optout Viridian support when not needed.
>>>>>
>>>>> OK, that's fine.
>>>>>
>>>>> On further request though: if Viridian is build-time disabled in
>>>>> Kconfig, setting or fetching HVM_PARAM_VIRIDIAN should return -ENODEV
>>>>> or similar error. I don't think this is done as part of this patch.
>>>
>>> Another bit I've noticed, you will need to adjust write_hvm_params()
>>> so it can tolerate xc_hvm_param_get() returning an error when
>>> HVM_PARAM_VIRIDIAN is not implemented by the hypervisor.
>>>
>>> Implementing the Viridian features using an HVM parameter was a bad
>>> approach probably.
>>
>> I've just realized how toolstack need to be modified and all consequences...
>> Have to try to push back a little bit:
>>
>> VIRIDIAN=n: Now HVM_PARAM_VIRIDIAN will be R/W with functionality NOP.
>>
>> I'd prefer avoid modifying toolstack if possible.
>>
>> How about sanitizing HVM_PARAM_VIRIDIAN to be RAZ/WI for VIRIDIAN=n?
>
> I've been thinking more into it, and we must still allow writes/reads
> to it, otherwise migration would fail when restoring a stream that
> contains a Viridian record, even if Viridian is not enabled for the
> domain. Right now the HVM param is unconditionally part of the
> migration stream, even when Viridian is not enabled.
>
> I think Xen could return an error when VIRIDIAN = n and a non-zero
> value is passed to HVM_PARAM_VIRIDIAN?
Right...
>
> That shouldn't require any changes to the toolstack, as when Viridian
> is not enabled the toolstack will just set HVM_PARAM_VIRIDIAN = 0. It
> would however still fail if toolstack attempts HVM_PARAM_VIRIDIAN != 0
> and Viridian has been build time disabled.
I was thinking about the same, in general:
Get HVM_PARAM_VIRIDIAN (VIRIDIAN = n):
- no fail, expected to return 0
[toolstack] write_hvm_params()
...
for ( i = 0; i < ARRAY_SIZE(params); i++ )
{
...
rc = xc_hvm_param_get(xch, ctx->domid, index, &value);
if ( rc )
{
PERROR("Failed to get HVMPARAM at index %u", index);
return rc;
}
if ( value != 0 )
{
entries[hdr.count].index = index;
entries[hdr.count].value = value;
hdr.count++;
}
[gs] will skip HVM_PARAM_VIRIDIAN entry as it's 0 for VIRIDIAN = n
Set HVM_PARAM_VIRIDIAN (VIRIDIAN = n):
- fail if value != 0 with -ENODEV
[toolstack] hvm_set_viridian_features() will fail and reject domain if
Viridian is configured for domain, but VIRIDIAN = n
[toolstack] xg_sr_restore_x86_hvm.c : handle_hvm_params() will fail on attempt to load
domain with Viridian enabled (value != 0) under Xen with VIRIDIAN = n.
The value == 0 will pass
Will send new version.
--
Best regards,
-grygorii
On Fri, Oct 17, 2025 at 12:40:33AM +0300, Grygorii Strashko wrote:
>
>
> On 15.10.25 11:00, Roger Pau Monné wrote:
> > On Tue, Oct 14, 2025 at 06:48:23PM +0300, Grygorii Strashko wrote:
> > >
> > >
> > > On 14.10.25 17:38, Roger Pau Monné wrote:
> > > > On Tue, Oct 14, 2025 at 04:24:53PM +0300, Grygorii Strashko wrote:
> > > > > On 13.10.25 15:17, Roger Pau Monné wrote:
> > > > > > On Tue, Sep 30, 2025 at 12:52:16PM +0000, Grygorii Strashko wrote:
> > > > > > > From: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> > > > > > > +
> > > > > > > + If unsure, say Y.
> > > > > > > +
> > > > > > > config MEM_PAGING
> > > > > > > bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
> > > > > > > depends on VM_EVENT
> > > > > > > diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> > > > > > > index 6ec2c8f2db56..736eb3f966e9 100644
> > > > > > > --- a/xen/arch/x86/hvm/Makefile
> > > > > > > +++ b/xen/arch/x86/hvm/Makefile
> > > > > > > @@ -1,6 +1,6 @@
> > > > > > > obj-$(CONFIG_AMD_SVM) += svm/
> > > > > > > obj-$(CONFIG_INTEL_VMX) += vmx/
> > > > > > > -obj-y += viridian/
> > > > > > > +obj-$(CONFIG_VIRIDIAN) += viridian/
> > > > > > > obj-y += asid.o
> > > > > > > obj-y += dm.o
> > > > > > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > > > > > > index 23bd7f078a1d..95a80369b9b8 100644
> > > > > > > --- a/xen/arch/x86/hvm/hvm.c
> > > > > > > +++ b/xen/arch/x86/hvm/hvm.c
> > > > > > > @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
> > > > > > > if ( hvm_tsc_scaling_supported )
> > > > > > > d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
> > > > > > > - rc = viridian_domain_init(d);
> > > > > > > - if ( rc )
> > > > > > > - goto fail2;
> > > > > > > + if ( is_viridian_domain(d) )
> > > > > > > + {
> > > > > > > + rc = viridian_domain_init(d);
> > > > > > > + if ( rc )
> > > > > > > + goto fail2;
> > > > > > > + }
> > > > > >
> > > > > > Are you sure this works as expected?
> > > > > >
> > > > > > The viridian_feature_mask() check is implemented using an HVM param,
> > > > > > and hence can only be possibly set after the domain object is created.
> > > > > > AFAICT is_viridian_domain(d) will unconditionally return false when
> > > > > > called from domain_create() context, because the HVM params cannot
> > > > > > possibly be set ahead of the domain being created.
> > > > >
> > > > > You are right. Thanks for the this catch.
> > > > >
> > > > > Taking above into account above, it seems Jan's proposal to convert below
> > > > > viridian APIs into wrappers for VIRIDIAN=n case is right way to move forward:
> > > > >
> > > > > int viridian_vcpu_init(struct vcpu *v);
> > > > > int viridian_domain_init(struct domain *d);
> > > > > void viridian_vcpu_deinit(struct vcpu *v);
> > > > > void viridian_domain_deinit(struct domain *d);
> > > > >
> > > > > Right?
> > > >
> > > > Possibly. If you don't want to introduce a XEN_DOMCTL_createdomain
> > > > flag you need to exclusively use the Kconfig option to decide whether
> > > > the Viridian related structs must be allocated. IOW: you could also
> > > > solve it by using IS_ENABLED(CONFIG_VIRIDIAN) instead of
> > > > is_viridian_domain() for most of the calls here.
> > > >
> > > > The wrapper option might be better IMO, rather than adding
> > > > IS_ENABLED(CONFIG_VIRIDIAN) around.
> > >
> > > I'll do wrappers - less if(s) in common HVM code.
> > >
> > > >
> > > > > [1] https://patchwork.kernel.org/comment/26595213/
> > > > >
> > > > > >
> > > > > > If you want to do anything like this you will possibly need to
> > > > > > introduce a new flag to XEN_DOMCTL_createdomain to signal whether the
> > > > > > domain has Viridian extensions are enabled or not, so that it's know
> > > > > > in the context where domain_create() gets called.
> > > > >
> > > > > In my opinion, it might be good not to go so far within this submission.
> > > > > - It's not intended to change existing behavior of neither Xen nor toolstack
> > > > > for VIRIDIAN=y (default)
>
> [1]
>
> > > > > - just optout Viridian support when not needed.
> > > >
> > > > OK, that's fine.
> > > >
> > > > On further request though: if Viridian is build-time disabled in
> > > > Kconfig, setting or fetching HVM_PARAM_VIRIDIAN should return -ENODEV
> > > > or similar error. I don't think this is done as part of this patch.
> >
> > Another bit I've noticed, you will need to adjust write_hvm_params()
> > so it can tolerate xc_hvm_param_get() returning an error when
> > HVM_PARAM_VIRIDIAN is not implemented by the hypervisor.
> >
> > Implementing the Viridian features using an HVM parameter was a bad
> > approach probably.
>
> I've just realized how toolstack need to be modified and all consequences...
> Have to try to push back a little bit:
>
> VIRIDIAN=n: Now HVM_PARAM_VIRIDIAN will be R/W with functionality NOP.
>
> I'd prefer avoid modifying toolstack if possible.
IMHO trying to start a domain with Viridian enabled, while it's compiled
out of the hypervisor, should fail. Not silently be ignored.
> How about sanitizing HVM_PARAM_VIRIDIAN to be RAZ/WI for VIRIDIAN=n?
> Or may be produce Xen XENLOG_WARNING"Viridian is disabled" if value!=0?
>
> This an EXPERT option and end-user can get Xen with VIRIDIAN=n only by
> manually re-configuring and re-compiling Xen. In other words, the user
> is an expert and knows what he is doing.
>
> Another point, assume change like this is to be done for HVM_PARAM_VIRIDIAN
> - there are another HVM_PARAM_x which depend on build-time disabled features, like:
> HVM_PARAM_VM86_TSS_SIZED
> HVM_PARAM_PAGING_RING_PFN,
> HVM_PARAM_MONITOR_RING_PFN,
> HVM_PARAM_SHARING_RING_PFN,
> HVM_PARAM_IDENT_PT
> ...
>
> if corresponding features are build-time disabled, above HVM_PARAM_x
> become R/W with functionality NOP now.
Are you sure? For me it looks like setting build-time disabled feature
returns -ENOSYS. Or do you mean some other interface than
xc_hvm_param_set().
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
On 17.10.2025 00:38, Marek Marczykowski-Górecki wrote: > On Fri, Oct 17, 2025 at 12:40:33AM +0300, Grygorii Strashko wrote: >> Another point, assume change like this is to be done for HVM_PARAM_VIRIDIAN >> - there are another HVM_PARAM_x which depend on build-time disabled features, like: >> HVM_PARAM_VM86_TSS_SIZED >> HVM_PARAM_PAGING_RING_PFN, >> HVM_PARAM_MONITOR_RING_PFN, >> HVM_PARAM_SHARING_RING_PFN, >> HVM_PARAM_IDENT_PT >> ... >> >> if corresponding features are build-time disabled, above HVM_PARAM_x >> become R/W with functionality NOP now. > > Are you sure? For me it looks like setting build-time disabled feature > returns -ENOSYS. Or do you mean some other interface than > xc_hvm_param_set(). Where do you see that ENOSYS coming from? In fact, for example, I don't see any of the *_RING_PFN even mentioned at all in hvm.c's parameter handling. Are you perhaps thinking of only the HVM=n case, whereas I expect Grygorii talks about the more fine-grained controls? That said, whether to uniformly fail requests for params solely related to build-time disabled features isn't quite clear. Arguably for e.g. paging and sharing, setting the ring PFN can as well be silently ignored (no events ever appearing), while failure would then be reported from other compiled-out logic. Jan
On Fri, Oct 17, 2025 at 08:01:13AM +0200, Jan Beulich wrote: > On 17.10.2025 00:38, Marek Marczykowski-Górecki wrote: > > On Fri, Oct 17, 2025 at 12:40:33AM +0300, Grygorii Strashko wrote: > >> Another point, assume change like this is to be done for HVM_PARAM_VIRIDIAN > >> - there are another HVM_PARAM_x which depend on build-time disabled features, like: > >> HVM_PARAM_VM86_TSS_SIZED > >> HVM_PARAM_PAGING_RING_PFN, > >> HVM_PARAM_MONITOR_RING_PFN, > >> HVM_PARAM_SHARING_RING_PFN, > >> HVM_PARAM_IDENT_PT > >> ... > >> > >> if corresponding features are build-time disabled, above HVM_PARAM_x > >> become R/W with functionality NOP now. > > > > Are you sure? For me it looks like setting build-time disabled feature > > returns -ENOSYS. Or do you mean some other interface than > > xc_hvm_param_set(). > > Where do you see that ENOSYS coming from? In fact, for example, I don't see any > of the *_RING_PFN even mentioned at all in hvm.c's parameter handling. Are you > perhaps thinking of only the HVM=n case, whereas I expect Grygorii talks about > the more fine-grained controls? Oh, sorry, I looked at XEN_DOMCTL_vm_event_op -> vm_event_domctl()... > That said, whether to uniformly fail requests for params solely related to > build-time disabled features isn't quite clear. Arguably for e.g. paging and > sharing, setting the ring PFN can as well be silently ignored (no events ever > appearing), while failure would then be reported from other compiled-out logic. > > Jan -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab
On 15.10.2025 10:00, Roger Pau Monné wrote:
> On Tue, Oct 14, 2025 at 06:48:23PM +0300, Grygorii Strashko wrote:
>>
>>
>> On 14.10.25 17:38, Roger Pau Monné wrote:
>>> On Tue, Oct 14, 2025 at 04:24:53PM +0300, Grygorii Strashko wrote:
>>>> On 13.10.25 15:17, Roger Pau Monné wrote:
>>>>> On Tue, Sep 30, 2025 at 12:52:16PM +0000, Grygorii Strashko wrote:
>>>>>> From: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>>>>>> +
>>>>>> + If unsure, say Y.
>>>>>> +
>>>>>> config MEM_PAGING
>>>>>> bool "Xen memory paging support (UNSUPPORTED)" if UNSUPPORTED
>>>>>> depends on VM_EVENT
>>>>>> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
>>>>>> index 6ec2c8f2db56..736eb3f966e9 100644
>>>>>> --- a/xen/arch/x86/hvm/Makefile
>>>>>> +++ b/xen/arch/x86/hvm/Makefile
>>>>>> @@ -1,6 +1,6 @@
>>>>>> obj-$(CONFIG_AMD_SVM) += svm/
>>>>>> obj-$(CONFIG_INTEL_VMX) += vmx/
>>>>>> -obj-y += viridian/
>>>>>> +obj-$(CONFIG_VIRIDIAN) += viridian/
>>>>>> obj-y += asid.o
>>>>>> obj-y += dm.o
>>>>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>>>>> index 23bd7f078a1d..95a80369b9b8 100644
>>>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>>>> @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
>>>>>> if ( hvm_tsc_scaling_supported )
>>>>>> d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
>>>>>> - rc = viridian_domain_init(d);
>>>>>> - if ( rc )
>>>>>> - goto fail2;
>>>>>> + if ( is_viridian_domain(d) )
>>>>>> + {
>>>>>> + rc = viridian_domain_init(d);
>>>>>> + if ( rc )
>>>>>> + goto fail2;
>>>>>> + }
>>>>>
>>>>> Are you sure this works as expected?
>>>>>
>>>>> The viridian_feature_mask() check is implemented using an HVM param,
>>>>> and hence can only be possibly set after the domain object is created.
>>>>> AFAICT is_viridian_domain(d) will unconditionally return false when
>>>>> called from domain_create() context, because the HVM params cannot
>>>>> possibly be set ahead of the domain being created.
>>>>
>>>> You are right. Thanks for the this catch.
>>>>
>>>> Taking above into account above, it seems Jan's proposal to convert below
>>>> viridian APIs into wrappers for VIRIDIAN=n case is right way to move forward:
>>>>
>>>> int viridian_vcpu_init(struct vcpu *v);
>>>> int viridian_domain_init(struct domain *d);
>>>> void viridian_vcpu_deinit(struct vcpu *v);
>>>> void viridian_domain_deinit(struct domain *d);
>>>>
>>>> Right?
>>>
>>> Possibly. If you don't want to introduce a XEN_DOMCTL_createdomain
>>> flag you need to exclusively use the Kconfig option to decide whether
>>> the Viridian related structs must be allocated. IOW: you could also
>>> solve it by using IS_ENABLED(CONFIG_VIRIDIAN) instead of
>>> is_viridian_domain() for most of the calls here.
>>>
>>> The wrapper option might be better IMO, rather than adding
>>> IS_ENABLED(CONFIG_VIRIDIAN) around.
>>
>> I'll do wrappers - less if(s) in common HVM code.
>>
>>>
>>>> [1] https://patchwork.kernel.org/comment/26595213/
>>>>
>>>>>
>>>>> If you want to do anything like this you will possibly need to
>>>>> introduce a new flag to XEN_DOMCTL_createdomain to signal whether the
>>>>> domain has Viridian extensions are enabled or not, so that it's know
>>>>> in the context where domain_create() gets called.
>>>>
>>>> In my opinion, it might be good not to go so far within this submission.
>>>> - It's not intended to change existing behavior of neither Xen nor toolstack
>>>> for VIRIDIAN=y (default)
>>>> - just optout Viridian support when not needed.
>>>
>>> OK, that's fine.
>>>
>>> On further request though: if Viridian is build-time disabled in
>>> Kconfig, setting or fetching HVM_PARAM_VIRIDIAN should return -ENODEV
>>> or similar error. I don't think this is done as part of this patch.
ENODEV was suggested here; it's not clear to me why ...
> Another bit I've noticed, you will need to adjust write_hvm_params()
> so it can tolerate xc_hvm_param_get() returning an error when
> HVM_PARAM_VIRIDIAN is not implemented by the hypervisor.
>
> Implementing the Viridian features using an HVM parameter was a bad
> approach probably.
>
>> Sure. Just have to ask for clarification what to return:
>> -EOPNOTSUPP (my choise) vs -EINVAL.
... other values were suggested here.
> Let me add Jan also to the To: field so we get consensus in one round.
>
> I won't use EINVAL, because that's returned for deprecated parameters
> also, and when the passed Viridian feature mask is invalid.
>
> EOPNOTSUPP is also returned for non-implemented hypercalls, so I'm not
> sure whether it could cause confusion here, as the hypercall is
> implemented, it's just the param that's not supported if
> build-disabled. Maybe ENODEV or ENXIO?
I'd be okay with either of these two, with a slight preference to ENODEV.
Jan
On 30.09.2025 14:52, Grygorii Strashko wrote:
> --- a/xen/arch/x86/hvm/Kconfig
> +++ b/xen/arch/x86/hvm/Kconfig
> @@ -62,6 +62,16 @@ config ALTP2M
>
> If unsure, stay with defaults.
>
> +config VIRIDIAN
> + bool "Hyper-V enlightenments for guests" if EXPERT
> + default y
> + help
> + Support optimizations for Hyper-V guests such as faster hypercalls,
> + efficient timer and interrupt handling, and enhanced paravirtualized
> + I/O. This is to improve performance and compatibility of Windows VMs.
What is "paravirtualized I/O" about in this context?
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
> if ( hvm_tsc_scaling_supported )
> d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
>
> - rc = viridian_domain_init(d);
> - if ( rc )
> - goto fail2;
> + if ( is_viridian_domain(d) )
> + {
> + rc = viridian_domain_init(d);
> + if ( rc )
> + goto fail2;
> + }
While this looks okay to me, ...
> @@ -739,7 +742,8 @@ void hvm_domain_relinquish_resources(struct domain *d)
> if ( hvm_funcs.nhvm_domain_relinquish_resources )
> alternative_vcall(hvm_funcs.nhvm_domain_relinquish_resources, d);
>
> - viridian_domain_deinit(d);
> + if ( is_viridian_domain(d) )
> + viridian_domain_deinit(d);
... I wonder if viridian_{domain,vcpu}_deinit() better wouldn't be tolerant
to be called anyway, thus avoiding the need for conditionals here and below
(and perhaps being a little more robust overall). Thoughts?
Jan
On Wed Oct 8, 2025 at 6:04 PM CEST, Jan Beulich wrote: > On 30.09.2025 14:52, Grygorii Strashko wrote: >> --- a/xen/arch/x86/hvm/Kconfig >> +++ b/xen/arch/x86/hvm/Kconfig >> @@ -62,6 +62,16 @@ config ALTP2M >> >> If unsure, stay with defaults. >> >> +config VIRIDIAN >> + bool "Hyper-V enlightenments for guests" if EXPERT >> + default y >> + help >> + Support optimizations for Hyper-V guests such as faster hypercalls, >> + efficient timer and interrupt handling, and enhanced paravirtualized >> + I/O. This is to improve performance and compatibility of Windows VMs. > > What is "paravirtualized I/O" about in this context? Hypervisor-assisted IPIs, TLB flushes, etc. Or so I understood back when I said that looked ok. I see there could be confusion with Xen PV device protocols, but as far as the user of the help message is concerned it makes no difference. One could even remove the examples and leave it as "... for Hyper-V guests. This is to...". They are truly inconsequential. All that matters is that (modern) Windows won't run without it, and that it provides some indeterminate hypervisor-provided assists to try to reduce some virtualization overheads. Cheers, Alejandro
On 10/13/25 06:01, Alejandro Vallejo wrote: > On Wed Oct 8, 2025 at 6:04 PM CEST, Jan Beulich wrote: >> On 30.09.2025 14:52, Grygorii Strashko wrote: >>> --- a/xen/arch/x86/hvm/Kconfig >>> +++ b/xen/arch/x86/hvm/Kconfig >>> @@ -62,6 +62,16 @@ config ALTP2M >>> >>> If unsure, stay with defaults. >>> >>> +config VIRIDIAN >>> + bool "Hyper-V enlightenments for guests" if EXPERT >>> + default y >>> + help >>> + Support optimizations for Hyper-V guests such as faster hypercalls, >>> + efficient timer and interrupt handling, and enhanced paravirtualized >>> + I/O. This is to improve performance and compatibility of Windows VMs. >> >> What is "paravirtualized I/O" about in this context? > > Hypervisor-assisted IPIs, TLB flushes, etc. Or so I understood back when I said > that looked ok. I see there could be confusion with Xen PV device protocols, > but as far as the user of the help message is concerned it makes no difference. > > One could even remove the examples and leave it as "... for Hyper-V guests. This > is to...". They are truly inconsequential. > > All that matters is that (modern) Windows won't run without it, and that it > provides some indeterminate hypervisor-provided assists to try to reduce some > virtualization overheads. Qubes OS doesn't expose Viridian at all, which is why it wasn't vulnerable to XSA-472. It still runs Windows guests just fine. -- Sincerely, Demi Marie Obenour (she/her/hers)
On Wed Oct 15, 2025 at 7:58 AM CEST, Demi Marie Obenour wrote: > On 10/13/25 06:01, Alejandro Vallejo wrote: >> On Wed Oct 8, 2025 at 6:04 PM CEST, Jan Beulich wrote: >>> On 30.09.2025 14:52, Grygorii Strashko wrote: >>>> --- a/xen/arch/x86/hvm/Kconfig >>>> +++ b/xen/arch/x86/hvm/Kconfig >>>> @@ -62,6 +62,16 @@ config ALTP2M >>>> >>>> If unsure, stay with defaults. >>>> >>>> +config VIRIDIAN >>>> + bool "Hyper-V enlightenments for guests" if EXPERT >>>> + default y >>>> + help >>>> + Support optimizations for Hyper-V guests such as faster hypercalls, >>>> + efficient timer and interrupt handling, and enhanced paravirtualized >>>> + I/O. This is to improve performance and compatibility of Windows VMs. >>> >>> What is "paravirtualized I/O" about in this context? >> >> Hypervisor-assisted IPIs, TLB flushes, etc. Or so I understood back when I said >> that looked ok. I see there could be confusion with Xen PV device protocols, >> but as far as the user of the help message is concerned it makes no difference. >> >> One could even remove the examples and leave it as "... for Hyper-V guests. This >> is to...". They are truly inconsequential. >> >> All that matters is that (modern) Windows won't run without it, and that it >> provides some indeterminate hypervisor-provided assists to try to reduce some >> virtualization overheads. > > Qubes OS doesn't expose Viridian at all, which is why it wasn't > vulnerable to XSA-472. It still runs Windows guests just fine. Can you run Windows 11? I don't remember which, but I do know some versions of Windows refuse to boot if they determine they are virtualised and don't see the mandatory parts of the TLFS. If 11 works, maybe Windows Server? Cheers, Alejandro
On 10/17/25 11:52, Alejandro Vallejo wrote: > On Wed Oct 15, 2025 at 7:58 AM CEST, Demi Marie Obenour wrote: >> On 10/13/25 06:01, Alejandro Vallejo wrote: >>> On Wed Oct 8, 2025 at 6:04 PM CEST, Jan Beulich wrote: >>>> On 30.09.2025 14:52, Grygorii Strashko wrote: >>>>> --- a/xen/arch/x86/hvm/Kconfig >>>>> +++ b/xen/arch/x86/hvm/Kconfig >>>>> @@ -62,6 +62,16 @@ config ALTP2M >>>>> >>>>> If unsure, stay with defaults. >>>>> >>>>> +config VIRIDIAN >>>>> + bool "Hyper-V enlightenments for guests" if EXPERT >>>>> + default y >>>>> + help >>>>> + Support optimizations for Hyper-V guests such as faster hypercalls, >>>>> + efficient timer and interrupt handling, and enhanced paravirtualized >>>>> + I/O. This is to improve performance and compatibility of Windows VMs. >>>> >>>> What is "paravirtualized I/O" about in this context? >>> >>> Hypervisor-assisted IPIs, TLB flushes, etc. Or so I understood back when I said >>> that looked ok. I see there could be confusion with Xen PV device protocols, >>> but as far as the user of the help message is concerned it makes no difference. >>> >>> One could even remove the examples and leave it as "... for Hyper-V guests. This >>> is to...". They are truly inconsequential. >>> >>> All that matters is that (modern) Windows won't run without it, and that it >>> provides some indeterminate hypervisor-provided assists to try to reduce some >>> virtualization overheads. >> >> Qubes OS doesn't expose Viridian at all, which is why it wasn't >> vulnerable to XSA-472. It still runs Windows guests just fine. > > Can you run Windows 11? I haven't tried it, but it is documented as working. > I don't remember which, but I do know some versions of Windows refuse to boot > if they determine they are virtualised and don't see the mandatory parts of > the TLFS. > > If 11 works, maybe Windows Server? Windows Server is more likely. -- Sincerely, Demi Marie Obenour (she/her/hers)
On Sat, Oct 18, 2025 at 09:21:37PM -0400, Demi Marie Obenour wrote: > On 10/17/25 11:52, Alejandro Vallejo wrote: > > On Wed Oct 15, 2025 at 7:58 AM CEST, Demi Marie Obenour wrote: > >> On 10/13/25 06:01, Alejandro Vallejo wrote: > >>> On Wed Oct 8, 2025 at 6:04 PM CEST, Jan Beulich wrote: > >>>> On 30.09.2025 14:52, Grygorii Strashko wrote: > >>>>> --- a/xen/arch/x86/hvm/Kconfig > >>>>> +++ b/xen/arch/x86/hvm/Kconfig > >>>>> @@ -62,6 +62,16 @@ config ALTP2M > >>>>> > >>>>> If unsure, stay with defaults. > >>>>> > >>>>> +config VIRIDIAN > >>>>> + bool "Hyper-V enlightenments for guests" if EXPERT > >>>>> + default y > >>>>> + help > >>>>> + Support optimizations for Hyper-V guests such as faster hypercalls, > >>>>> + efficient timer and interrupt handling, and enhanced paravirtualized > >>>>> + I/O. This is to improve performance and compatibility of Windows VMs. > >>>> > >>>> What is "paravirtualized I/O" about in this context? > >>> > >>> Hypervisor-assisted IPIs, TLB flushes, etc. Or so I understood back when I said > >>> that looked ok. I see there could be confusion with Xen PV device protocols, > >>> but as far as the user of the help message is concerned it makes no difference. > >>> > >>> One could even remove the examples and leave it as "... for Hyper-V guests. This > >>> is to...". They are truly inconsequential. > >>> > >>> All that matters is that (modern) Windows won't run without it, and that it > >>> provides some indeterminate hypervisor-provided assists to try to reduce some > >>> virtualization overheads. > >> > >> Qubes OS doesn't expose Viridian at all, which is why it wasn't > >> vulnerable to XSA-472. It still runs Windows guests just fine. > > > > Can you run Windows 11? > > I haven't tried it, but it is documented as working. > > > I don't remember which, but I do know some versions of Windows refuse to boot > > if they determine they are virtualised and don't see the mandatory parts of > > the TLFS. > > > > If 11 works, maybe Windows Server? > > Windows Server is more likely. FTR, for unrelated reasons I've tested Windows Server 2025 without Viridian, and it does work, albeit painfully slow. Regards, Roger.
On Mon, Oct 20, 2025 at 09:44:29AM +0100, Roger Pau Monné wrote: > On Sat, Oct 18, 2025 at 09:21:37PM -0400, Demi Marie Obenour wrote: > > On 10/17/25 11:52, Alejandro Vallejo wrote: > > > On Wed Oct 15, 2025 at 7:58 AM CEST, Demi Marie Obenour wrote: > > >> On 10/13/25 06:01, Alejandro Vallejo wrote: > > >>> On Wed Oct 8, 2025 at 6:04 PM CEST, Jan Beulich wrote: > > >>>> On 30.09.2025 14:52, Grygorii Strashko wrote: > > >>>>> --- a/xen/arch/x86/hvm/Kconfig > > >>>>> +++ b/xen/arch/x86/hvm/Kconfig > > >>>>> @@ -62,6 +62,16 @@ config ALTP2M > > >>>>> > > >>>>> If unsure, stay with defaults. > > >>>>> > > >>>>> +config VIRIDIAN > > >>>>> + bool "Hyper-V enlightenments for guests" if EXPERT > > >>>>> + default y > > >>>>> + help > > >>>>> + Support optimizations for Hyper-V guests such as faster hypercalls, > > >>>>> + efficient timer and interrupt handling, and enhanced paravirtualized > > >>>>> + I/O. This is to improve performance and compatibility of Windows VMs. > > >>>> > > >>>> What is "paravirtualized I/O" about in this context? > > >>> > > >>> Hypervisor-assisted IPIs, TLB flushes, etc. Or so I understood back when I said > > >>> that looked ok. I see there could be confusion with Xen PV device protocols, > > >>> but as far as the user of the help message is concerned it makes no difference. > > >>> > > >>> One could even remove the examples and leave it as "... for Hyper-V guests. This > > >>> is to...". They are truly inconsequential. > > >>> > > >>> All that matters is that (modern) Windows won't run without it, and that it > > >>> provides some indeterminate hypervisor-provided assists to try to reduce some > > >>> virtualization overheads. > > >> > > >> Qubes OS doesn't expose Viridian at all, which is why it wasn't > > >> vulnerable to XSA-472. It still runs Windows guests just fine. > > > > > > Can you run Windows 11? > > > > I haven't tried it, but it is documented as working. > > > > > I don't remember which, but I do know some versions of Windows refuse to boot > > > if they determine they are virtualised and don't see the mandatory parts of > > > the TLFS. > > > > > > If 11 works, maybe Windows Server? > > > > Windows Server is more likely. > > FTR, for unrelated reasons I've tested Windows Server 2025 without > Viridian, and it does work, albeit painfully slow. Windows 11 works too, but also "painfully slow". Interestingly, Windows 10 works much better in the same environment (but still far from native performance). -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab
On 13.10.2025 12:01, Alejandro Vallejo wrote: > On Wed Oct 8, 2025 at 6:04 PM CEST, Jan Beulich wrote: >> On 30.09.2025 14:52, Grygorii Strashko wrote: >>> --- a/xen/arch/x86/hvm/Kconfig >>> +++ b/xen/arch/x86/hvm/Kconfig >>> @@ -62,6 +62,16 @@ config ALTP2M >>> >>> If unsure, stay with defaults. >>> >>> +config VIRIDIAN >>> + bool "Hyper-V enlightenments for guests" if EXPERT >>> + default y >>> + help >>> + Support optimizations for Hyper-V guests such as faster hypercalls, >>> + efficient timer and interrupt handling, and enhanced paravirtualized >>> + I/O. This is to improve performance and compatibility of Windows VMs. >> >> What is "paravirtualized I/O" about in this context? > > Hypervisor-assisted IPIs, TLB flushes, etc. Or so I understood back when I said > that looked ok. Just to clarify my take: IPIs fall under "interrupt handling", and TLB flushes to me fall under memory management, not I/O. > I see there could be confusion with Xen PV device protocols, > but as far as the user of the help message is concerned it makes no difference. Does it not? To me it does. > One could even remove the examples and leave it as "... for Hyper-V guests. This > is to...". They are truly inconsequential. > > All that matters is that (modern) Windows won't run without it, and that it > provides some indeterminate hypervisor-provided assists to try to reduce some > virtualization overheads. I think I'm happy for the other examples to stay, as they provide some extra context for less aware users. Jan
On 08.10.25 19:04, Jan Beulich wrote:
> On 30.09.2025 14:52, Grygorii Strashko wrote:
>> --- a/xen/arch/x86/hvm/Kconfig
>> +++ b/xen/arch/x86/hvm/Kconfig
>> @@ -62,6 +62,16 @@ config ALTP2M
>>
>> If unsure, stay with defaults.
>>
>> +config VIRIDIAN
>> + bool "Hyper-V enlightenments for guests" if EXPERT
>> + default y
>> + help
>> + Support optimizations for Hyper-V guests such as faster hypercalls,
>> + efficient timer and interrupt handling, and enhanced paravirtualized
>> + I/O. This is to improve performance and compatibility of Windows VMs.
>
> What is "paravirtualized I/O" about in this context?
It was phrased this way and agreed upon from the very beginning [1]
[1] https://patchwork.kernel.org/project/xen-devel/patch/20250317071919.3766976-1-Sergiy_Kibrik@epam.com/#26305207
>
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
>> if ( hvm_tsc_scaling_supported )
>> d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
>>
>> - rc = viridian_domain_init(d);
>> - if ( rc )
>> - goto fail2;
>> + if ( is_viridian_domain(d) )
>> + {
>> + rc = viridian_domain_init(d);
>> + if ( rc )
>> + goto fail2;
>> + }
>
> While this looks okay to me, ...
>
>> @@ -739,7 +742,8 @@ void hvm_domain_relinquish_resources(struct domain *d)
>> if ( hvm_funcs.nhvm_domain_relinquish_resources )
>> alternative_vcall(hvm_funcs.nhvm_domain_relinquish_resources, d);
>>
>> - viridian_domain_deinit(d);
>> + if ( is_viridian_domain(d) )
>> + viridian_domain_deinit(d);
>
> ... I wonder if viridian_{domain,vcpu}_deinit() better wouldn't be tolerant
> to be called anyway, thus avoiding the need for conditionals here and below
> (and perhaps being a little more robust overall). Thoughts?
I think There are no limits for perfection. But at some point - need to stop.
Hence Viridian code will be removed from build when !VIRIDIAN - changing above code will
require to add stubs for viridian_{domain,vcpu}_deinit().
FYI: Xen pipeline for this patch
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2090362213
--
Best regards,
-grygorii
On 09.10.2025 18:33, Grygorii Strashko wrote:
> On 08.10.25 19:04, Jan Beulich wrote:
>> On 30.09.2025 14:52, Grygorii Strashko wrote:
>>> --- a/xen/arch/x86/hvm/Kconfig
>>> +++ b/xen/arch/x86/hvm/Kconfig
>>> @@ -62,6 +62,16 @@ config ALTP2M
>>> If unsure, stay with defaults.
>>> +config VIRIDIAN
>>> + bool "Hyper-V enlightenments for guests" if EXPERT
>>> + default y
>>> + help
>>> + Support optimizations for Hyper-V guests such as faster hypercalls,
>>> + efficient timer and interrupt handling, and enhanced paravirtualized
>>> + I/O. This is to improve performance and compatibility of Windows VMs.
>>
>> What is "paravirtualized I/O" about in this context?
>
> It was phrased this way and agreed upon from the very beginning [1]
>
> [1] https://patchwork.kernel.org/project/xen-devel/patch/20250317071919.3766976-1-Sergiy_Kibrik@epam.com/#26305207
Hmm, I'm not sure I would call this "agreed upon". Plus this doesn't answer
my question.
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
>>> if ( hvm_tsc_scaling_supported )
>>> d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
>>> - rc = viridian_domain_init(d);
>>> - if ( rc )
>>> - goto fail2;
>>> + if ( is_viridian_domain(d) )
>>> + {
>>> + rc = viridian_domain_init(d);
>>> + if ( rc )
>>> + goto fail2;
>>> + }
>>
>> While this looks okay to me, ...
>>
>>> @@ -739,7 +742,8 @@ void hvm_domain_relinquish_resources(struct domain *d)
>>> if ( hvm_funcs.nhvm_domain_relinquish_resources )
>>> alternative_vcall(hvm_funcs.nhvm_domain_relinquish_resources, d);
>>> - viridian_domain_deinit(d);
>>> + if ( is_viridian_domain(d) )
>>> + viridian_domain_deinit(d);
>>
>> ... I wonder if viridian_{domain,vcpu}_deinit() better wouldn't be tolerant
>> to be called anyway, thus avoiding the need for conditionals here and below
>> (and perhaps being a little more robust overall). Thoughts?
>
> I think There are no limits for perfection. But at some point - need to stop.
>
> Hence Viridian code will be removed from build when !VIRIDIAN - changing above code will
> require to add stubs for viridian_{domain,vcpu}_deinit().
Fair point.
Jan
© 2016 - 2026 Red Hat, Inc.