Alex,
On 8/19/2019 4:49 AM, Alexander Graf wrote:
>
>
> On 15.08.19 18:25, Suthikulpanit, Suravee wrote:
>> Currently, after a VM boots with APICv enabled, it could go into
>> the following states:
>> * activated = VM is running w/ APICv
>> * deactivated = VM deactivate APICv (temporary)
>> * disabled = VM deactivate APICv (permanent)
>>
>> Introduce KVM APICv state enum to help keep track of the APICv states
>> along with a new variable struct kvm_arch.apicv_state to store
>> the current state.
>>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
>> ---
>> arch/x86/include/asm/kvm_host.h | 11 +++++++++++
>> arch/x86/kvm/x86.c | 14 +++++++++++++-
>> 2 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 56bc702..04d7066 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -845,6 +845,15 @@ enum kvm_irqchip_mode {
>> KVM_IRQCHIP_SPLIT, /* created with KVM_CAP_SPLIT_IRQCHIP */
>> };
>> +/*
>> + * KVM assumes all vcpus in a VM operate in the same mode.
>> + */
>> +enum kvm_apicv_state {
>> + APICV_DISABLED, /* Disabled (such as for Hyper-V case) */
>> + APICV_DEACTIVATED, /* Deactivated tempoerary */
>
> typo
>
> I'm also not sure the name is 100% obvious. How about something like "suspended" or "paused"?
Ok, I'll change it to APICV_SUSPENDED.
>> ...
>> @@ -9150,13 +9154,18 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>> goto fail_free_pio_data;
>> if (irqchip_in_kernel(vcpu->kvm)) {
>> - vcpu->arch.apicv_active = kvm_x86_ops->get_enable_apicv(vcpu->kvm);
>
> Why are you moving this into a locked section?
Since we introduced the apicv_state to track the VM APICv state, which is accessible
by each vcpu initialization code, we need to lock and check the state before setting
the per-vcpu apicv_active.
>
>> r = kvm_create_lapic(vcpu, lapic_timer_advance_ns);
>> if (r < 0)
>> goto fail_mmu_destroy;
>> } else
>> static_key_slow_inc(&kvm_no_apic_vcpu);
>> + mutex_lock(&vcpu->kvm->arch.apicv_lock);
>> + if (irqchip_in_kernel(vcpu->kvm) &&
>> + vcpu->kvm->arch.apicv_state == APICV_ACTIVATED)
>> + vcpu->arch.apicv_active = kvm_x86_ops->get_enable_apicv(vcpu->kvm);
>> + mutex_unlock(&vcpu->kvm->arch.apicv_lock);
>> +
>> vcpu->arch.mce_banks = kzalloc(KVM_MAX_MCE_BANKS * sizeof(u64) * 4,
>> GFP_KERNEL_ACCOUNT);
>> if (!vcpu->arch.mce_banks) {
>> @@ -9255,6 +9264,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>> kvm_page_track_init(kvm);
>> kvm_mmu_init_vm(kvm);
>> + /* APICV initialization */
>> + mutex_init(&kvm->arch.apicv_lock);
>
> In fact, the whole lock story is not part of the patch description :).\\
Ok, I'll update the commit log to describe the lock .
Suravee
