On Fri, Jan 12, 2024 at 01:34:48AM -0800, Xin Li wrote:
>
>+#define VMX_BASIC_DUAL_MONITOR_TREATMENT BIT_ULL(49)
>+#define VMX_BASIC_TRUE_CTLS BIT_ULL(55)
>+
>+#define VMX_BASIC_FEATURES_MASK \
>+ (VMX_BASIC_DUAL_MONITOR_TREATMENT | \
>+ VMX_BASIC_INOUT | \
>+ VMX_BASIC_TRUE_CTLS)
>+
>+#define VMX_BASIC_RESERVED_BITS \
>+ (GENMASK_ULL(63, 56) | GENMASK_ULL(47, 45) | BIT_ULL(31))
When we add a new feature (e.g., in CET series, bit 56 is added), the above
two macros need to be modified.
Would it be better to use a macro for bits exempt from the bitwise check below
e.g.,
#define VMX_BASIC_MULTI_BITS_FEATURES_MASK
(GENMASK_ULL(53, 50) | GENMASK_ULL(44, 32) | GENMASK_ULL(30, 0))
and do
if (!is_bitwise_subset(vmx_basic, data,
~VMX_BASIC_MULTI_BITS_FEATURES_MASK)
then we don't need to change the macro when adding new features.
>+
> static int vmx_restore_vmx_basic(struct vcpu_vmx *vmx, u64 data)
> {
>- const u64 feature_and_reserved =
>- /* feature (except bit 48; see below) */
>- BIT_ULL(49) | BIT_ULL(54) | BIT_ULL(55) |
>- /* reserved */
>- BIT_ULL(31) | GENMASK_ULL(47, 45) | GENMASK_ULL(63, 56);
> u64 vmx_basic = vmcs_config.nested.basic;
>
>- if (!is_bitwise_subset(vmx_basic, data, feature_and_reserved))
>+ static_assert(!(VMX_BASIC_FEATURES_MASK & VMX_BASIC_RESERVED_BITS));
>+
>+ if (!is_bitwise_subset(vmx_basic, data,
>+ VMX_BASIC_FEATURES_MASK | VMX_BASIC_RESERVED_BITS))
> return -EINVAL;
>
> /*
> * KVM does not emulate a version of VMX that constrains physical
> * addresses of VMX structures (e.g. VMCS) to 32-bits.
> */
>- if (data & BIT_ULL(48))
>+ if (data & VMX_BASIC_32BIT_PHYS_ADDR_ONLY)
> return -EINVAL;
Side topic:
Actually, there is no need to handle bit 48 as a special case. If we add bit 48
to VMX_BASIC_FEATURES_MASK, the bitwise check will fail if bit 48 of @data is 1.
