Re: [PATCH v4 01/15] KVM: async_pf: Move struct kvm_async_pf around

[Gavin Shan]

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@xxxxxxxxxx>
> > ---
> >   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)
> >   {