linux-kernel - Re: [PATCH v4 01/15] KVM: async_pf: Move struct kvm_async

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 13 Jan 2022 15:21:26 +0800
From:   Gavin Shan <gshan@...hat.com>
To:     Eric Auger <eauger@...hat.com>, kvmarm@...ts.cs.columbia.edu
Cc:     kvm@...r.kernel.org, maz@...nel.org, linux-kernel@...r.kernel.org,
        shan.gavin@...il.com, Jonathan.Cameron@...wei.com,
        pbonzini@...hat.com, vkuznets@...hat.com, will@...nel.org
Subject: Re: [PATCH v4 01/15] KVM: async_pf: Move struct kvm_async_pf around

Hi Eric,

On 11/10/21 11:37 PM, Eric Auger wrote:
> On 8/15/21 2:59 AM, Gavin Shan wrote:
>> This moves the definition of "struct kvm_async_pf" and the related
>> functions after "struct kvm_vcpu" so that newly added inline functions
>> in the subsequent patches can dereference "struct kvm_vcpu" properly.
>> Otherwise, the unexpected build error will be raised:
>>
>>     error: dereferencing pointer to incomplete type ‘struct kvm_vcpu’
>>     return !list_empty_careful(&vcpu->async_pf.done);
>>                                     ^~
>> Since we're here, the sepator between type and field in "struct kvm_vcpu"
> separator

Thanks, It will be fixed in next respin.

>> is replaced by tab. The empty stub kvm_check_async_pf_completion() is also
>> added on !CONFIG_KVM_ASYNC_PF, which is needed by subsequent patches to
>> support asynchronous page fault on ARM64.
>>
>> Signed-off-by: Gavin Shan <gshan@...hat.com>
>> ---
>>   include/linux/kvm_host.h | 44 +++++++++++++++++++++-------------------
>>   1 file changed, 23 insertions(+), 21 deletions(-)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index ae7735b490b4..85b61a456f1c 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -199,27 +199,6 @@ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>>   struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>>   					 gpa_t addr);
>>   
>> -#ifdef CONFIG_KVM_ASYNC_PF
>> -struct kvm_async_pf {
>> -	struct work_struct work;
>> -	struct list_head link;
>> -	struct list_head queue;
>> -	struct kvm_vcpu *vcpu;
>> -	struct mm_struct *mm;
>> -	gpa_t cr2_or_gpa;
>> -	unsigned long addr;
>> -	struct kvm_arch_async_pf arch;
>> -	bool   wakeup_all;
>> -	bool notpresent_injected;
>> -};
>> -
>> -void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu);
>> -void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu);
>> -bool kvm_setup_async_pf(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>> -			unsigned long hva, struct kvm_arch_async_pf *arch);
>> -int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);
>> -#endif
>> -
>>   #ifdef KVM_ARCH_WANT_MMU_NOTIFIER
>>   struct kvm_gfn_range {
>>   	struct kvm_memory_slot *slot;
>> @@ -346,6 +325,29 @@ struct kvm_vcpu {
>>   	struct kvm_dirty_ring dirty_ring;
>>   };
>>   
>> +#ifdef CONFIG_KVM_ASYNC_PF
>> +struct kvm_async_pf {
>> +	struct work_struct		work;
>> +	struct list_head		link;
>> +	struct list_head		queue;
>> +	struct kvm_vcpu			*vcpu;
>> +	struct mm_struct		*mm;
>> +	gpa_t				cr2_or_gpa;
>> +	unsigned long			addr;
>> +	struct kvm_arch_async_pf	arch;
>> +	bool				wakeup_all;
>> +	bool				notpresent_injected;
>> +};
>> +
>> +void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu);
>> +void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu);
>> +bool kvm_setup_async_pf(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>> +			unsigned long hva, struct kvm_arch_async_pf *arch);
>> +int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);
>> +#else
>> +static inline void kvm_check_async_pf_completion(struct kvm_vcpu *vcpu) { }
> why is that stub needed on ARM64 and not on the other archs?
> 

We use the following pattern, suggested by James Morse.

int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
{
         int r;
         switch (ext) {
           :
                 case KVM_CAP_ASYNC_PF:
         case KVM_CAP_ASYNC_PF_INT:
                 r = IS_ENABLED(CONFIG_KVM_ASYNC_PF) ? 1 : 0;
                 break;
         default:
                 r = 0;
         }

         return r;
}

Thanks,
Gavin

>> +#endif
>> +
>>   /* must be called with irqs disabled */
>>   static __always_inline void guest_enter_irqoff(void)
>>   {
>>
>