Skip to content

Commit 2251e9f

Browse files
committed
KVM: arm64: Make vCPU feature flags consistent VM-wide
To date KVM has allowed userspace to construct asymmetric VMs where particular features may only be supported on a subset of vCPUs. This wasn't really the intened usage pattern, and it is a total pain in the ass to keep working in the kernel. What's more, this is at odds with CPU features in host userspace, where asymmetric features are largely hidden or disabled. It's time to put an end to the whole game. Require all vCPUs in the VM to have the same feature set, rejecting deviants in the KVM_ARM_VCPU_INIT ioctl. Preserve some of the vestiges of per-vCPU feature flags in case we need to reinstate the old behavior for some limited configurations. Yes, this is a sign of cowardice around a user-visibile change. Hoist all of the 32-bit limitations into kvm_vcpu_init_check_features() to avoid nested attempts to acquire the config_lock, which won't end well. Link: https://lore.kernel.org/r/20230609190054.1542113-4-oliver.upton@linux.dev Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
1 parent e3c1c0c commit 2251e9f

4 files changed

Lines changed: 40 additions & 78 deletions

File tree

arch/arm64/include/asm/kvm_emulate.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,7 @@ static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
6262
#else
6363
static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
6464
{
65-
struct kvm *kvm = vcpu->kvm;
66-
67-
WARN_ON_ONCE(!test_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED,
68-
&kvm->arch.flags));
69-
70-
return test_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags);
65+
return test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features);
7166
}
7267
#endif
7368

arch/arm64/include/asm/kvm_host.h

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -215,25 +215,21 @@ struct kvm_arch {
215215
#define KVM_ARCH_FLAG_MTE_ENABLED 1
216216
/* At least one vCPU has ran in the VM */
217217
#define KVM_ARCH_FLAG_HAS_RAN_ONCE 2
218-
/*
219-
* The following two bits are used to indicate the guest's EL1
220-
* register width configuration. A value of KVM_ARCH_FLAG_EL1_32BIT
221-
* bit is valid only when KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED is set.
222-
* Otherwise, the guest's EL1 register width has not yet been
223-
* determined yet.
224-
*/
225-
#define KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED 3
226-
#define KVM_ARCH_FLAG_EL1_32BIT 4
218+
/* The vCPU feature set for the VM is configured */
219+
#define KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED 3
227220
/* PSCI SYSTEM_SUSPEND enabled for the guest */
228-
#define KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED 5
221+
#define KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED 4
229222
/* VM counter offset */
230-
#define KVM_ARCH_FLAG_VM_COUNTER_OFFSET 6
223+
#define KVM_ARCH_FLAG_VM_COUNTER_OFFSET 5
231224
/* Timer PPIs made immutable */
232-
#define KVM_ARCH_FLAG_TIMER_PPIS_IMMUTABLE 7
225+
#define KVM_ARCH_FLAG_TIMER_PPIS_IMMUTABLE 6
233226
/* SMCCC filter initialized for the VM */
234-
#define KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED 8
227+
#define KVM_ARCH_FLAG_SMCCC_FILTER_CONFIGURED 7
235228
unsigned long flags;
236229

230+
/* VM-wide vCPU feature set */
231+
DECLARE_BITMAP(vcpu_features, KVM_VCPU_MAX_FEATURES);
232+
237233
/*
238234
* VM-wide PMU filter, implemented as a bitmap and big enough for
239235
* up to 2^10 events (ARMv8.0) or 2^16 events (ARMv8.1+).

arch/arm64/kvm/arm.c

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
170170
*/
171171
kvm->arch.dfr0_pmuver.imp = kvm_arm_pmu_get_pmuver_limit();
172172

173+
bitmap_zero(kvm->arch.vcpu_features, KVM_VCPU_MAX_FEATURES);
174+
173175
return 0;
174176

175177
err_free_cpumask:
@@ -1181,6 +1183,20 @@ static int kvm_vcpu_init_check_features(struct kvm_vcpu *vcpu,
11811183
return -ENOENT;
11821184
}
11831185

1186+
if (!test_bit(KVM_ARM_VCPU_EL1_32BIT, &features))
1187+
return 0;
1188+
1189+
if (!cpus_have_const_cap(ARM64_HAS_32BIT_EL1))
1190+
return -EINVAL;
1191+
1192+
/* MTE is incompatible with AArch32 */
1193+
if (kvm_has_mte(vcpu->kvm))
1194+
return -EINVAL;
1195+
1196+
/* NV is incompatible with AArch32 */
1197+
if (test_bit(KVM_ARM_VCPU_HAS_EL2, &features))
1198+
return -EINVAL;
1199+
11841200
return 0;
11851201
}
11861202

@@ -1197,7 +1213,14 @@ static int __kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
11971213
const struct kvm_vcpu_init *init)
11981214
{
11991215
unsigned long features = init->features[0];
1200-
int ret;
1216+
struct kvm *kvm = vcpu->kvm;
1217+
int ret = -EINVAL;
1218+
1219+
mutex_lock(&kvm->arch.config_lock);
1220+
1221+
if (test_bit(KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED, &kvm->arch.flags) &&
1222+
!bitmap_equal(kvm->arch.vcpu_features, &features, KVM_VCPU_MAX_FEATURES))
1223+
goto out_unlock;
12011224

12021225
vcpu->arch.target = init->target;
12031226
bitmap_copy(vcpu->arch.features, &features, KVM_VCPU_MAX_FEATURES);
@@ -1207,8 +1230,14 @@ static int __kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
12071230
if (ret) {
12081231
vcpu->arch.target = -1;
12091232
bitmap_zero(vcpu->arch.features, KVM_VCPU_MAX_FEATURES);
1233+
goto out_unlock;
12101234
}
12111235

1236+
bitmap_copy(kvm->arch.vcpu_features, &features, KVM_VCPU_MAX_FEATURES);
1237+
set_bit(KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED, &kvm->arch.flags);
1238+
1239+
out_unlock:
1240+
mutex_unlock(&kvm->arch.config_lock);
12121241
return ret;
12131242
}
12141243

arch/arm64/kvm/reset.c

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -186,57 +186,6 @@ static int kvm_vcpu_enable_ptrauth(struct kvm_vcpu *vcpu)
186186
return 0;
187187
}
188188

189-
/**
190-
* kvm_set_vm_width() - set the register width for the guest
191-
* @vcpu: Pointer to the vcpu being configured
192-
*
193-
* Set both KVM_ARCH_FLAG_EL1_32BIT and KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED
194-
* in the VM flags based on the vcpu's requested register width, the HW
195-
* capabilities and other options (such as MTE).
196-
* When REG_WIDTH_CONFIGURED is already set, the vcpu settings must be
197-
* consistent with the value of the FLAG_EL1_32BIT bit in the flags.
198-
*
199-
* Return: 0 on success, negative error code on failure.
200-
*/
201-
static int kvm_set_vm_width(struct kvm_vcpu *vcpu)
202-
{
203-
struct kvm *kvm = vcpu->kvm;
204-
bool is32bit;
205-
206-
is32bit = vcpu_has_feature(vcpu, KVM_ARM_VCPU_EL1_32BIT);
207-
208-
lockdep_assert_held(&kvm->arch.config_lock);
209-
210-
if (test_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED, &kvm->arch.flags)) {
211-
/*
212-
* The guest's register width is already configured.
213-
* Make sure that the vcpu is consistent with it.
214-
*/
215-
if (is32bit == test_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags))
216-
return 0;
217-
218-
return -EINVAL;
219-
}
220-
221-
if (!cpus_have_const_cap(ARM64_HAS_32BIT_EL1) && is32bit)
222-
return -EINVAL;
223-
224-
/* MTE is incompatible with AArch32 */
225-
if (kvm_has_mte(kvm) && is32bit)
226-
return -EINVAL;
227-
228-
/* NV is incompatible with AArch32 */
229-
if (vcpu_has_nv(vcpu) && is32bit)
230-
return -EINVAL;
231-
232-
if (is32bit)
233-
set_bit(KVM_ARCH_FLAG_EL1_32BIT, &kvm->arch.flags);
234-
235-
set_bit(KVM_ARCH_FLAG_REG_WIDTH_CONFIGURED, &kvm->arch.flags);
236-
237-
return 0;
238-
}
239-
240189
/**
241190
* kvm_reset_vcpu - sets core registers and sys_regs to reset value
242191
* @vcpu: The VCPU pointer
@@ -262,13 +211,6 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
262211
bool loaded;
263212
u32 pstate;
264213

265-
mutex_lock(&vcpu->kvm->arch.config_lock);
266-
ret = kvm_set_vm_width(vcpu);
267-
mutex_unlock(&vcpu->kvm->arch.config_lock);
268-
269-
if (ret)
270-
return ret;
271-
272214
spin_lock(&vcpu->arch.mp_state_lock);
273215
reset_state = vcpu->arch.reset_state;
274216
vcpu->arch.reset_state.reset = false;

0 commit comments

Comments
 (0)