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?
> --- 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.
Jan
