All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to create vcpu
@ 2023-11-17 12:26 Li RongQing
  2023-11-20 11:41 ` Maxim Levitsky
  2023-11-21 15:14 ` Xu Yilun
  0 siblings, 2 replies; 5+ messages in thread
From: Li RongQing @ 2023-11-17 12:26 UTC (permalink / raw)
  To: x86, kvm

Static key kvm_has_noapic_vcpu should be reduced when fail
to create vcpu, this patch fixes it

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 arch/x86/kvm/x86.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 41cce50..2a22e66 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11957,7 +11957,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 	kfree(vcpu->arch.mci_ctl2_banks);
 	free_page((unsigned long)vcpu->arch.pio_data);
 fail_free_lapic:
-	kvm_free_lapic(vcpu);
+	if (!lapic_in_kernel(vcpu))
+		static_branch_dec(&kvm_has_noapic_vcpu);
+	else
+		kvm_free_lapic(vcpu);
 fail_mmu_destroy:
 	kvm_mmu_destroy(vcpu);
 	return r;
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to create vcpu
  2023-11-17 12:26 [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to create vcpu Li RongQing
@ 2023-11-20 11:41 ` Maxim Levitsky
  2023-11-21 15:14 ` Xu Yilun
  1 sibling, 0 replies; 5+ messages in thread
From: Maxim Levitsky @ 2023-11-20 11:41 UTC (permalink / raw)
  To: Li RongQing, x86, kvm

On Fri, 2023-11-17 at 20:26 +0800, Li RongQing wrote:
> Static key kvm_has_noapic_vcpu should be reduced when fail
> to create vcpu, this patch fixes it
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  arch/x86/kvm/x86.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 41cce50..2a22e66 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -11957,7 +11957,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>  	kfree(vcpu->arch.mci_ctl2_banks);
>  	free_page((unsigned long)vcpu->arch.pio_data);
>  fail_free_lapic:
> -	kvm_free_lapic(vcpu);
> +	if (!lapic_in_kernel(vcpu))
> +		static_branch_dec(&kvm_has_noapic_vcpu);
> +	else
> +		kvm_free_lapic(vcpu);
>  fail_mmu_destroy:
>  	kvm_mmu_destroy(vcpu);
>  	return r;

Makes sense.

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Best regards,
	Maxim Levitsky


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to create vcpu
  2023-11-17 12:26 [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to create vcpu Li RongQing
  2023-11-20 11:41 ` Maxim Levitsky
@ 2023-11-21 15:14 ` Xu Yilun
  2023-11-22  6:15   ` Li,Rongqing
  1 sibling, 1 reply; 5+ messages in thread
From: Xu Yilun @ 2023-11-21 15:14 UTC (permalink / raw)
  To: Li RongQing; +Cc: x86, kvm

On Fri, Nov 17, 2023 at 08:26:33PM +0800, Li RongQing wrote:
> Static key kvm_has_noapic_vcpu should be reduced when fail
> to create vcpu, this patch fixes it
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  arch/x86/kvm/x86.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 41cce50..2a22e66 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -11957,7 +11957,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>  	kfree(vcpu->arch.mci_ctl2_banks);
>  	free_page((unsigned long)vcpu->arch.pio_data);
>  fail_free_lapic:
> -	kvm_free_lapic(vcpu);
> +	if (!lapic_in_kernel(vcpu))
> +		static_branch_dec(&kvm_has_noapic_vcpu);
> +	else
> +		kvm_free_lapic(vcpu);
>  fail_mmu_destroy:
>  	kvm_mmu_destroy(vcpu);
>  	return r;

It is good to me. But is it better also take the chance to tidy up
kvm_arch_vcpu_destroy():

	kvm_free_lapic(vcpu);
	idx = srcu_read_lock(&vcpu->kvm->srcu);
	kvm_mmu_destroy(vcpu);
	srcu_read_unlock(&vcpu->kvm->srcu, idx);
	free_page((unsigned long)vcpu->arch.pio_data);
	kvfree(vcpu->arch.cpuid_entries);
	if (!lapic_in_kernel(vcpu))
		static_branch_dec(&kvm_has_noapic_vcpu);

Thanks,
Yilun

> -- 
> 2.9.4
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to create vcpu
  2023-11-21 15:14 ` Xu Yilun
@ 2023-11-22  6:15   ` Li,Rongqing
  2023-11-22 14:57     ` Xu Yilun
  0 siblings, 1 reply; 5+ messages in thread
From: Li,Rongqing @ 2023-11-22  6:15 UTC (permalink / raw)
  To: Xu Yilun; +Cc: x86, kvm



> -----Original Message-----
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Tuesday, November 21, 2023 11:14 PM
> To: Li,Rongqing <lirongqing@baidu.com>
> Cc: x86@kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to
> create vcpu
> 
> On Fri, Nov 17, 2023 at 08:26:33PM +0800, Li RongQing wrote:
> > Static key kvm_has_noapic_vcpu should be reduced when fail to create
> > vcpu, this patch fixes it
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >  arch/x86/kvm/x86.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index
> > 41cce50..2a22e66 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -11957,7 +11957,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu
> *vcpu)
> >  	kfree(vcpu->arch.mci_ctl2_banks);
> >  	free_page((unsigned long)vcpu->arch.pio_data);
> >  fail_free_lapic:
> > -	kvm_free_lapic(vcpu);
> > +	if (!lapic_in_kernel(vcpu))
> > +		static_branch_dec(&kvm_has_noapic_vcpu);
> > +	else
> > +		kvm_free_lapic(vcpu);
> >  fail_mmu_destroy:
> >  	kvm_mmu_destroy(vcpu);
> >  	return r;
> 
> It is good to me. But is it better also take the chance to tidy up
> kvm_arch_vcpu_destroy():
> 
> 	kvm_free_lapic(vcpu);
> 	idx = srcu_read_lock(&vcpu->kvm->srcu);
> 	kvm_mmu_destroy(vcpu);
> 	srcu_read_unlock(&vcpu->kvm->srcu, idx);
> 	free_page((unsigned long)vcpu->arch.pio_data);
> 	kvfree(vcpu->arch.cpuid_entries);
> 	if (!lapic_in_kernel(vcpu))
> 		static_branch_dec(&kvm_has_noapic_vcpu);
> 

Do you means that calling kvm_free_lapic when lapic_in_kernel is true?

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2c92407..9d176c7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -12122,14 +12122,17 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
        kvm_pmu_destroy(vcpu);
        kfree(vcpu->arch.mce_banks);
        kfree(vcpu->arch.mci_ctl2_banks);
-       kvm_free_lapic(vcpu);
+
+       if (lapic_in_kernel(vcpu))
+               kvm_free_lapic(vcpu);
+       else
+               static_branch_dec(&kvm_has_noapic_vcpu);
+
        idx = srcu_read_lock(&vcpu->kvm->srcu);
        kvm_mmu_destroy(vcpu);
        srcu_read_unlock(&vcpu->kvm->srcu, idx);
        free_page((unsigned long)vcpu->arch.pio_data);
        kvfree(vcpu->arch.cpuid_entries);
-       if (!lapic_in_kernel(vcpu))
-               static_branch_dec(&kvm_has_noapic_vcpu);
 }

 void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)




^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to create vcpu
  2023-11-22  6:15   ` Li,Rongqing
@ 2023-11-22 14:57     ` Xu Yilun
  0 siblings, 0 replies; 5+ messages in thread
From: Xu Yilun @ 2023-11-22 14:57 UTC (permalink / raw)
  To: Li,Rongqing; +Cc: x86, kvm

On Wed, Nov 22, 2023 at 06:15:44AM +0000, Li,Rongqing wrote:
> 
> 
> > -----Original Message-----
> > From: Xu Yilun <yilun.xu@linux.intel.com>
> > Sent: Tuesday, November 21, 2023 11:14 PM
> > To: Li,Rongqing <lirongqing@baidu.com>
> > Cc: x86@kernel.org; kvm@vger.kernel.org
> > Subject: Re: [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to
> > create vcpu
> > 
> > On Fri, Nov 17, 2023 at 08:26:33PM +0800, Li RongQing wrote:
> > > Static key kvm_has_noapic_vcpu should be reduced when fail to create
> > > vcpu, this patch fixes it
> > >
> > > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > > ---
> > >  arch/x86/kvm/x86.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index
> > > 41cce50..2a22e66 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -11957,7 +11957,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu
> > *vcpu)
> > >  	kfree(vcpu->arch.mci_ctl2_banks);
> > >  	free_page((unsigned long)vcpu->arch.pio_data);
> > >  fail_free_lapic:
> > > -	kvm_free_lapic(vcpu);
> > > +	if (!lapic_in_kernel(vcpu))
> > > +		static_branch_dec(&kvm_has_noapic_vcpu);
> > > +	else
> > > +		kvm_free_lapic(vcpu);
> > >  fail_mmu_destroy:
> > >  	kvm_mmu_destroy(vcpu);
> > >  	return r;
> > 
> > It is good to me. But is it better also take the chance to tidy up
> > kvm_arch_vcpu_destroy():
> > 
> > 	kvm_free_lapic(vcpu);
> > 	idx = srcu_read_lock(&vcpu->kvm->srcu);
> > 	kvm_mmu_destroy(vcpu);
> > 	srcu_read_unlock(&vcpu->kvm->srcu, idx);
> > 	free_page((unsigned long)vcpu->arch.pio_data);
> > 	kvfree(vcpu->arch.cpuid_entries);
> > 	if (!lapic_in_kernel(vcpu))
> > 		static_branch_dec(&kvm_has_noapic_vcpu);
> > 
> 
> Do you means that calling kvm_free_lapic when lapic_in_kernel is true?

Yes.

> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2c92407..9d176c7 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -12122,14 +12122,17 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>         kvm_pmu_destroy(vcpu);
>         kfree(vcpu->arch.mce_banks);
>         kfree(vcpu->arch.mci_ctl2_banks);
> -       kvm_free_lapic(vcpu);
> +
> +       if (lapic_in_kernel(vcpu))
> +               kvm_free_lapic(vcpu);
> +       else
> +               static_branch_dec(&kvm_has_noapic_vcpu);

Better keep the same style as in kvm_arch_vcpu_create():

  if (!lapic_in_kernel(vcpu))
	static_branch_dec(&kvm_has_noapic_vcpu);
  else
	kvm_free_lapic(vcpu);

Thanks,
Yilun

> +
>         idx = srcu_read_lock(&vcpu->kvm->srcu);
>         kvm_mmu_destroy(vcpu);
>         srcu_read_unlock(&vcpu->kvm->srcu, idx);
>         free_page((unsigned long)vcpu->arch.pio_data);
>         kvfree(vcpu->arch.cpuid_entries);
> -       if (!lapic_in_kernel(vcpu))
> -               static_branch_dec(&kvm_has_noapic_vcpu);
>  }
> 
>  void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> 
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-11-22 14:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-17 12:26 [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to create vcpu Li RongQing
2023-11-20 11:41 ` Maxim Levitsky
2023-11-21 15:14 ` Xu Yilun
2023-11-22  6:15   ` Li,Rongqing
2023-11-22 14:57     ` Xu Yilun

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.