Instead of returning EINVAL on an unknown feature bit, clamp to the
known feature set. This simplifies upgrades since when the hypervisor
and toolstack are updated without an immediate reboot, the toolstack may
set unknown feature bits and the existing behaviour leaves the VM
without any Virdian features enabled.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/hvm/hvm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 192309c2fccc..67afa00a0f13 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4234,8 +4234,10 @@ static int hvm_set_param(struct domain *d, uint32_t index, uint64_t value)
     case HVM_PARAM_VIRIDIAN:
         if ( !IS_ENABLED(CONFIG_VIRIDIAN) )
             rc = -ENODEV;
-        else if ( (value & ~HVMPV_feature_mask) || !(value & HVMPV_base_freq) )
+        else if ( !(value & HVMPV_base_freq) )
             rc = -EINVAL;
+
+        value &= HVMPV_feature_mask;
         break;
     case HVM_PARAM_IDENT_PT:
         /*
-- 
2.55.0

On 24/09/2026 12:03 pm, Ross Lagerwall wrote:
> Instead of returning EINVAL on an unknown feature bit, clamp to the
> known feature set.

Sorry, but no.

This is equivalent to saying "I've got a VM using AVX512" and Xen saying
"ok, I'll ignore that safety check and let you run on non-AVX512 capable
hardware".

Requesting a feature that Xen doesn't know about is a hard error. 
Truncating features out like this will cause a guest using those
features to malfunction.

It is a bug that this was expressed as an HVM Param in the first place. 
It should be part of CPU Policy, and it's on a TODO list.

~Andrew

On 9/24/26 12:20 PM, Andrew Cooper wrote:
> On 24/09/2026 12:03 pm, Ross Lagerwall wrote:
>> Instead of returning EINVAL on an unknown feature bit, clamp to the
>> known feature set.
> 
> Sorry, but no.
> 
> This is equivalent to saying "I've got a VM using AVX512" and Xen saying
> "ok, I'll ignore that safety check and let you run on non-AVX512 capable
> hardware".
> 
> Requesting a feature that Xen doesn't know about is a hard error.
> Truncating features out like this will cause a guest using those
> features to malfunction.
> 
> It is a bug that this was expressed as an HVM Param in the first place.
> It should be part of CPU Policy, and it's on a TODO list.

But isn't this the same thing the CPU Policy code does? In
recalculate_cpuid_policy(), it silently clamps the toolstack's choices to the
max featureset.

Ross

On 24.09.2026 14:56, Ross Lagerwall wrote:
> On 9/24/26 12:20 PM, Andrew Cooper wrote:
>> On 24/09/2026 12:03 pm, Ross Lagerwall wrote:
>>> Instead of returning EINVAL on an unknown feature bit, clamp to the
>>> known feature set.
>>
>> Sorry, but no.
>>
>> This is equivalent to saying "I've got a VM using AVX512" and Xen saying
>> "ok, I'll ignore that safety check and let you run on non-AVX512 capable
>> hardware".
>>
>> Requesting a feature that Xen doesn't know about is a hard error.
>> Truncating features out like this will cause a guest using those
>> features to malfunction.
>>
>> It is a bug that this was expressed as an HVM Param in the first place.
>> It should be part of CPU Policy, and it's on a TODO list.
> 
> But isn't this the same thing the CPU Policy code does? In
> recalculate_cpuid_policy(), it silently clamps the toolstack's choices to the
> max featureset.

Which is behavior that, if I'm not mistaken, is supposed to go away. Such
requests are intended to instead fail, down the road.

Jan

On 9/24/26 2:08 PM, Jan Beulich wrote:
> On 24.09.2026 14:56, Ross Lagerwall wrote:
>> On 9/24/26 12:20 PM, Andrew Cooper wrote:
>>> On 24/09/2026 12:03 pm, Ross Lagerwall wrote:
>>>> Instead of returning EINVAL on an unknown feature bit, clamp to the
>>>> known feature set.
>>>
>>> Sorry, but no.
>>>
>>> This is equivalent to saying "I've got a VM using AVX512" and Xen saying
>>> "ok, I'll ignore that safety check and let you run on non-AVX512 capable
>>> hardware".
>>>
>>> Requesting a feature that Xen doesn't know about is a hard error.
>>> Truncating features out like this will cause a guest using those
>>> features to malfunction.
>>>
>>> It is a bug that this was expressed as an HVM Param in the first place.
>>> It should be part of CPU Policy, and it's on a TODO list.
>>
>> But isn't this the same thing the CPU Policy code does? In
>> recalculate_cpuid_policy(), it silently clamps the toolstack's choices to the
>> max featureset.
> 
> Which is behavior that, if I'm not mistaken, is supposed to go away. Such
> requests are intended to instead fail, down the road.

OK, if that is undesirable behaviour and going to change in future I'll retract
this patch.

Ross