Skip to content

Commit ccb2280

Browse files
calmisisean-jc
authored andcommitted
x86/kvm: Use separate percpu variable to track the enabling of asyncpf
Refer to commit fd10cde ("KVM paravirt: Add async PF initialization to PV guest") and commit 344d958 ("KVM: Add PV MSR to enable asynchronous page faults delivery"). It turns out that at the time when asyncpf was introduced, the purpose was defining the shared PV data 'struct kvm_vcpu_pv_apf_data' with the size of 64 bytes. However, it made a mistake and defined the size to 68 bytes, which failed to make fit in a cache line and made the code inconsistent with the documentation. Below justification quoted from Sean[*] KVM (the host side) has *never* read kvm_vcpu_pv_apf_data.enabled, and the documentation clearly states that enabling is based solely on the bit in the synthetic MSR. So rather than update the documentation, fix the goof by removing the enabled filed and use the separate percpu variable instread. KVM-as-a-host obviously doesn't enforce anything or consume the size, and changing the header will only affect guests that are rebuilt against the new header, so there's no chance of ABI breakage between KVM and its guests. The only possible breakage is if some other hypervisor is emulating KVM's async #PF (LOL) and relies on the guest to set kvm_vcpu_pv_apf_data.enabled. But (a) I highly doubt such a hypervisor exists, (b) that would arguably be a violation of KVM's "spec", and (c) the worst case scenario is that the guest would simply lose async #PF functionality. [*] https://lore.kernel.org/all/[email protected]/T/#u Suggested-by: Sean Christopherson <[email protected]> Signed-off-by: Xiaoyao Li <[email protected]> Link: https://lore.kernel.org/r/[email protected] [sean: use true/false instead of 1/0 for booleans] Signed-off-by: Sean Christopherson <[email protected]>
1 parent 41bccc9 commit ccb2280

File tree

3 files changed

+6
-7
lines changed

3 files changed

+6
-7
lines changed

Documentation/virt/kvm/x86/msr.rst

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,6 @@ data:
204204
__u32 token;
205205

206206
__u8 pad[56];
207-
__u32 enabled;
208207
};
209208

210209
Bits 5-4 of the MSR are reserved and should be zero. Bit 0 is set to 1

arch/x86/include/uapi/asm/kvm_para.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,6 @@ struct kvm_vcpu_pv_apf_data {
142142
__u32 token;
143143

144144
__u8 pad[56];
145-
__u32 enabled;
146145
};
147146

148147
#define KVM_PV_EOI_BIT 0

arch/x86/kernel/kvm.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ static int __init parse_no_stealacc(char *arg)
6565

6666
early_param("no-steal-acc", parse_no_stealacc);
6767

68+
static DEFINE_PER_CPU_READ_MOSTLY(bool, async_pf_enabled);
6869
static DEFINE_PER_CPU_DECRYPTED(struct kvm_vcpu_pv_apf_data, apf_reason) __aligned(64);
6970
DEFINE_PER_CPU_DECRYPTED(struct kvm_steal_time, steal_time) __aligned(64) __visible;
7071
static int has_steal_clock = 0;
@@ -244,7 +245,7 @@ noinstr u32 kvm_read_and_reset_apf_flags(void)
244245
{
245246
u32 flags = 0;
246247

247-
if (__this_cpu_read(apf_reason.enabled)) {
248+
if (__this_cpu_read(async_pf_enabled)) {
248249
flags = __this_cpu_read(apf_reason.flags);
249250
__this_cpu_write(apf_reason.flags, 0);
250251
}
@@ -295,7 +296,7 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_kvm_asyncpf_interrupt)
295296

296297
inc_irq_stat(irq_hv_callback_count);
297298

298-
if (__this_cpu_read(apf_reason.enabled)) {
299+
if (__this_cpu_read(async_pf_enabled)) {
299300
token = __this_cpu_read(apf_reason.token);
300301
kvm_async_pf_task_wake(token);
301302
__this_cpu_write(apf_reason.token, 0);
@@ -362,7 +363,7 @@ static void kvm_guest_cpu_init(void)
362363
wrmsrl(MSR_KVM_ASYNC_PF_INT, HYPERVISOR_CALLBACK_VECTOR);
363364

364365
wrmsrl(MSR_KVM_ASYNC_PF_EN, pa);
365-
__this_cpu_write(apf_reason.enabled, 1);
366+
__this_cpu_write(async_pf_enabled, true);
366367
pr_debug("setup async PF for cpu %d\n", smp_processor_id());
367368
}
368369

@@ -383,11 +384,11 @@ static void kvm_guest_cpu_init(void)
383384

384385
static void kvm_pv_disable_apf(void)
385386
{
386-
if (!__this_cpu_read(apf_reason.enabled))
387+
if (!__this_cpu_read(async_pf_enabled))
387388
return;
388389

389390
wrmsrl(MSR_KVM_ASYNC_PF_EN, 0);
390-
__this_cpu_write(apf_reason.enabled, 0);
391+
__this_cpu_write(async_pf_enabled, false);
391392

392393
pr_debug("disable async PF for cpu %d\n", smp_processor_id());
393394
}

0 commit comments

Comments
 (0)