On 06.11.25 13:35, Jan Beulich wrote:
On 31.10.2025 17:17, Grygorii Strashko wrote:
--- 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/
With this, what is the point of the additions to
viridian_load_{domain,vcpu}_ctxt()?
You're adding dead code there, aren't you?
Hgrr. And we end up back to v3 regarding this part.
Check in viridian_load_{domain,vcpu}_ctxt() may/may not work depending on
toolstack
behavior which is not strictly defined (loading HVM_PARAM_VIRIDIAN before/after
viridian_load_{domain,vcpu}_ctxt()).
So what's the conclusion here - drop this check?
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4231,8 +4231,9 @@ static int hvm_set_param(struct domain *d, uint32_t
index, uint64_t value)
rc = -EINVAL;
break;
case HVM_PARAM_VIRIDIAN:
- if ( (value & ~HVMPV_feature_mask) ||
- !(value & HVMPV_base_freq) )
+ if ( !IS_ENABLED(CONFIG_VIRIDIAN) && value )
+ rc = -ENODEV;
+ else if ( (value & ~HVMPV_feature_mask) || !(value & HVMPV_base_freq) )
rc = -EINVAL;
I find the check for value to be (non-)zero a little dubious here: If any caller
passed in 0, it would get back -EINVAL anyway. Imo -ENODEV would be more
suitable
in that case as well. Things would be different if 0 was a valid value to pass
in.
The idea was to distinguish between "Feature enabled, Invalid parameter" and
"Feature disabled".
--
Best regards,
-grygorii
