[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [XEN][PATCH v7] x86: make Viridian support optional
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
- Date: Tue, 11 Nov 2025 20:25:21 +0200
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=MujHwEvH9fQGdWuKIksiRjYbWNiTAHP3lIb0CxOPtb4=; b=QFcI3ExIKZd6e+etoJoStdrfvuCm0boJLdO3OtkZ/+xqCXcxuOzx3v4ULi5pwx1TTlr/wvvPKutwO7VjsA1AyOtt15rUdQ/G3bfQNuf+DZV2DDIcVa6KrnCQC6NKhEUXpdvyg+P3z86l8OhayxmgcXe/E7w5W0rWS/XCLmxOaxQbgtSrn34zUegYgEniewuq8x57MKgc2e1/ObN1W4SvGrbNxYQCHgM7+fhrLrdvaWcVnfTzU+WZI9av9Y3UUJmEiVeka+dnOzQ6QM7E6rmv31OyzZLszvWZHJW4xcrJVzlmsYb+PFrxK4NyZXe70L8VKfyHBqRLMOBYpEcDJpeB1A==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=UB+Tu9YSHCFM9Qmzpsrc3SFnOrMeZYJaDcQtz0OJg8oD5zajFw6ckjcUpUGYQjgQHGfu5YQcM8sjbDJ1jZo2XVQEx7mW/rJGcp8pStWLnIjZQrCcpkxAQUIcDhvz2F/WOG/7l+P7pgFplbZmDeHIa9NFZGDDjbPopBzwHO6z23D+ql6UBId7N4qCZx8MHlYpkoXJ8XBEfASQWBkXB5UaEZcFttE1JDsOhcrDtqrGAN/yAfx4gb1EO5Ob3ssZ3qu6AhC5oNNAD1vgoPYV0uakyORU5SfXSVngM18ulfzfD2rqjJ2Us4Zhj/Hc1xnCXi/ebGi6JzWbOwgzRJKwDGvQ6A==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=epam.com;
- Cc: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Jason Andryuk <jason.andryuk@xxxxxxx>
- Delivery-date: Tue, 11 Nov 2025 18:25:31 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 06.11.25 15:47, Jan Beulich wrote:
On 06.11.2025 14:42, Grygorii Strashko wrote:
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?
Well, Misra wants us to not have any dead code. Hence why would we add new
instances of such?
--- 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".
"
But you don't, or else the addition would need to live after the -EINVAL checks.
I also question the utility of knowing "parameter was invalid" when the feature
isn't available anyway.
My understanding here - I need to drop non-zero "value" check.
will be:
case HVM_PARAM_VIRIDIAN:
if ( !IS_ENABLED(CONFIG_VIRIDIAN) )
rc = -ENODEV;
else if ( (value & ~HVMPV_feature_mask) || !(value & HVMPV_base_freq) )
rc = -EINVAL;
break;
--
Best regards,
-grygorii
|
