[PATCHv2] Introduce bitmask for apic attention reasons.

[PATCHv2] Introduce bitmask for apic attention reasons.

[Gleb Natapov]

The patch introduces a bitmap that will hold reasons apic should be
checked during vmexit. This is in a preparation for vp eoi patch
that will add one more check on vmexit. With the bitmap we can do
if(apic_attention) to check everything simultaneously which will
add zero overhead on the fast path.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |    4 ++++
 arch/x86/kvm/lapic.c            |   12 +++++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f624ca7..69e39bc 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -172,6 +172,9 @@ enum {
 #define DR7_FIXED_1	0x00000400
 #define DR7_VOLATILE	0xffff23ff
 
+/* apic attention bits */
+#define KVM_APIC_CHECK_VAPIC	0
+
 /*
  * We don't want allocation failures within the mmu code, so we preallocate
  * enough memory for a single page fault in a cache.
@@ -337,6 +340,7 @@ struct kvm_vcpu_arch {
 	u64 efer;
 	u64 apic_base;
 	struct kvm_lapic *apic;    /* kernel irqchip context */
+	unsigned long apic_attention;
 	int32_t apic_arb_prio;
 	int mp_state;
 	int sipi_vector;
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 992b4ea..93c1574 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1088,6 +1088,7 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu)
 	apic_update_ppr(apic);
 
 	vcpu->arch.apic_arb_prio = 0;
+	vcpu->arch.apic_attention = 0;
 
 	apic_debug(KERN_INFO "%s: vcpu=%p, id=%d, base_msr="
 		   "0x%016" PRIx64 ", base_address=0x%0lx.\n", __func__,
@@ -1287,7 +1288,7 @@ void kvm_lapic_sync_from_vapic(struct kvm_vcpu *vcpu)
 	u32 data;
 	void *vapic;
 
-	if (!irqchip_in_kernel(vcpu->kvm) || !vcpu->arch.apic->vapic_addr)
+	if (!test_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention))
 		return;
 
 	vapic = kmap_atomic(vcpu->arch.apic->vapic_page);
@@ -1304,7 +1305,7 @@ void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu)
 	struct kvm_lapic *apic;
 	void *vapic;
 
-	if (!irqchip_in_kernel(vcpu->kvm) || !vcpu->arch.apic->vapic_addr)
+	if (!test_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention))
 		return;
 
 	apic = vcpu->arch.apic;
@@ -1324,10 +1325,11 @@ void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu)
 
 void kvm_lapic_set_vapic_addr(struct kvm_vcpu *vcpu, gpa_t vapic_addr)
 {
-	if (!irqchip_in_kernel(vcpu->kvm))
-		return;
-
 	vcpu->arch.apic->vapic_addr = vapic_addr;
+	if (vapic_addr)
+		__set_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention);
+	else
+		__clear_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention);
 }
 
 int kvm_x2apic_msr_write(struct kvm_vcpu *vcpu, u32 msr, u64 data)
-- 
1.7.7.3

Re: [PATCHv2] Introduce bitmask for apic attention reasons.

[Gleb Natapov]

Forget changelog:
v1->v2:
 - Add a comment before define of attention bits.

On Thu, Apr 19, 2012 at 02:06:29PM +0300, Gleb Natapov wrote:
> The patch introduces a bitmap that will hold reasons apic should be
> checked during vmexit. This is in a preparation for vp eoi patch
> that will add one more check on vmexit. With the bitmap we can do
> if(apic_attention) to check everything simultaneously which will
> add zero overhead on the fast path.
> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |    4 ++++
>  arch/x86/kvm/lapic.c            |   12 +++++++-----
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index f624ca7..69e39bc 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -172,6 +172,9 @@ enum {
>  #define DR7_FIXED_1	0x00000400
>  #define DR7_VOLATILE	0xffff23ff
>  
> +/* apic attention bits */
> +#define KVM_APIC_CHECK_VAPIC	0
> +
>  /*
>   * We don't want allocation failures within the mmu code, so we preallocate
>   * enough memory for a single page fault in a cache.
> @@ -337,6 +340,7 @@ struct kvm_vcpu_arch {
>  	u64 efer;
>  	u64 apic_base;
>  	struct kvm_lapic *apic;    /* kernel irqchip context */
> +	unsigned long apic_attention;
>  	int32_t apic_arb_prio;
>  	int mp_state;
>  	int sipi_vector;
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 992b4ea..93c1574 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1088,6 +1088,7 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu)
>  	apic_update_ppr(apic);
>  
>  	vcpu->arch.apic_arb_prio = 0;
> +	vcpu->arch.apic_attention = 0;
>  
>  	apic_debug(KERN_INFO "%s: vcpu=%p, id=%d, base_msr="
>  		   "0x%016" PRIx64 ", base_address=0x%0lx.\n", __func__,
> @@ -1287,7 +1288,7 @@ void kvm_lapic_sync_from_vapic(struct kvm_vcpu *vcpu)
>  	u32 data;
>  	void *vapic;
>  
> -	if (!irqchip_in_kernel(vcpu->kvm) || !vcpu->arch.apic->vapic_addr)
> +	if (!test_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention))
>  		return;
>  
>  	vapic = kmap_atomic(vcpu->arch.apic->vapic_page);
> @@ -1304,7 +1305,7 @@ void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu)
>  	struct kvm_lapic *apic;
>  	void *vapic;
>  
> -	if (!irqchip_in_kernel(vcpu->kvm) || !vcpu->arch.apic->vapic_addr)
> +	if (!test_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention))
>  		return;
>  
>  	apic = vcpu->arch.apic;
> @@ -1324,10 +1325,11 @@ void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu)
>  
>  void kvm_lapic_set_vapic_addr(struct kvm_vcpu *vcpu, gpa_t vapic_addr)
>  {
> -	if (!irqchip_in_kernel(vcpu->kvm))
> -		return;
> -
>  	vcpu->arch.apic->vapic_addr = vapic_addr;
> +	if (vapic_addr)
> +		__set_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention);
> +	else
> +		__clear_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention);
>  }
>  
>  int kvm_x2apic_msr_write(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> -- 
> 1.7.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.

Re: [PATCHv2] Introduce bitmask for apic attention reasons.

[Avi Kivity]

On 04/19/2012 02:06 PM, Gleb Natapov wrote:
> The patch introduces a bitmap that will hold reasons apic should be
> checked during vmexit. This is in a preparation for vp eoi patch
> that will add one more check on vmexit. With the bitmap we can do
> if(apic_attention) to check everything simultaneously which will
> add zero overhead on the fast path.
>
>

Thanks, applied to queue.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCHv2] Introduce bitmask for apic attention reasons.

[Michael S. Tsirkin]

On Thu, Apr 19, 2012 at 02:06:29PM +0300, Gleb Natapov wrote:
> The patch introduces a bitmap that will hold reasons apic should be
> checked during vmexit. This is in a preparation for vp eoi patch
> that will add one more check on vmexit. With the bitmap we can do
> if(apic_attention) to check everything simultaneously which will
> add zero overhead on the fast path.
> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |    4 ++++
>  arch/x86/kvm/lapic.c            |   12 +++++++-----
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index f624ca7..69e39bc 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -172,6 +172,9 @@ enum {
>  #define DR7_FIXED_1	0x00000400
>  #define DR7_VOLATILE	0xffff23ff
>  
> +/* apic attention bits */
> +#define KVM_APIC_CHECK_VAPIC	0
> +
>  /*
>   * We don't want allocation failures within the mmu code, so we preallocate
>   * enough memory for a single page fault in a cache.
> @@ -337,6 +340,7 @@ struct kvm_vcpu_arch {
>  	u64 efer;
>  	u64 apic_base;
>  	struct kvm_lapic *apic;    /* kernel irqchip context */
> +	unsigned long apic_attention;
>  	int32_t apic_arb_prio;
>  	int mp_state;
>  	int sipi_vector;
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 992b4ea..93c1574 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1088,6 +1088,7 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu)
>  	apic_update_ppr(apic);
>  
>  	vcpu->arch.apic_arb_prio = 0;
> +	vcpu->arch.apic_attention = 0;
>  
>  	apic_debug(KERN_INFO "%s: vcpu=%p, id=%d, base_msr="
>  		   "0x%016" PRIx64 ", base_address=0x%0lx.\n", __func__,
> @@ -1287,7 +1288,7 @@ void kvm_lapic_sync_from_vapic(struct kvm_vcpu *vcpu)
>  	u32 data;
>  	void *vapic;
>  
> -	if (!irqchip_in_kernel(vcpu->kvm) || !vcpu->arch.apic->vapic_addr)
> +	if (!test_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention))
>  		return;
>  
>  	vapic = kmap_atomic(vcpu->arch.apic->vapic_page);
> @@ -1304,7 +1305,7 @@ void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu)
>  	struct kvm_lapic *apic;
>  	void *vapic;
>  
> -	if (!irqchip_in_kernel(vcpu->kvm) || !vcpu->arch.apic->vapic_addr)
> +	if (!test_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention))
>  		return;
>  
>  	apic = vcpu->arch.apic;

I just noticed that this uses atomic test_bit.
Probably unintentional. Let's change that to __test_bit before
it's too late.

> @@ -1324,10 +1325,11 @@ void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu)
>  
>  void kvm_lapic_set_vapic_addr(struct kvm_vcpu *vcpu, gpa_t vapic_addr)
>  {
> -	if (!irqchip_in_kernel(vcpu->kvm))
> -		return;
> -
>  	vcpu->arch.apic->vapic_addr = vapic_addr;
> +	if (vapic_addr)
> +		__set_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention);
> +	else
> +		__clear_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention);
>  }
>  
>  int kvm_x2apic_msr_write(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> -- 
> 1.7.7.3

Re: [PATCHv2] Introduce bitmask for apic attention reasons.

[Michael S. Tsirkin]

On Mon, May 21, 2012 at 10:50:30PM +0300, Michael S. Tsirkin wrote:
> On Thu, Apr 19, 2012 at 02:06:29PM +0300, Gleb Natapov wrote:
> > The patch introduces a bitmap that will hold reasons apic should be
> > checked during vmexit. This is in a preparation for vp eoi patch
> > that will add one more check on vmexit. With the bitmap we can do
> > if(apic_attention) to check everything simultaneously which will
> > add zero overhead on the fast path.
> > 
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > ---
> >  arch/x86/include/asm/kvm_host.h |    4 ++++
> >  arch/x86/kvm/lapic.c            |   12 +++++++-----
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index f624ca7..69e39bc 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -172,6 +172,9 @@ enum {
> >  #define DR7_FIXED_1	0x00000400
> >  #define DR7_VOLATILE	0xffff23ff
> >  
> > +/* apic attention bits */
> > +#define KVM_APIC_CHECK_VAPIC	0
> > +
> >  /*
> >   * We don't want allocation failures within the mmu code, so we preallocate
> >   * enough memory for a single page fault in a cache.
> > @@ -337,6 +340,7 @@ struct kvm_vcpu_arch {
> >  	u64 efer;
> >  	u64 apic_base;
> >  	struct kvm_lapic *apic;    /* kernel irqchip context */
> > +	unsigned long apic_attention;
> >  	int32_t apic_arb_prio;
> >  	int mp_state;
> >  	int sipi_vector;
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index 992b4ea..93c1574 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -1088,6 +1088,7 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu)
> >  	apic_update_ppr(apic);
> >  
> >  	vcpu->arch.apic_arb_prio = 0;
> > +	vcpu->arch.apic_attention = 0;
> >  
> >  	apic_debug(KERN_INFO "%s: vcpu=%p, id=%d, base_msr="
> >  		   "0x%016" PRIx64 ", base_address=0x%0lx.\n", __func__,
> > @@ -1287,7 +1288,7 @@ void kvm_lapic_sync_from_vapic(struct kvm_vcpu *vcpu)
> >  	u32 data;
> >  	void *vapic;
> >  
> > -	if (!irqchip_in_kernel(vcpu->kvm) || !vcpu->arch.apic->vapic_addr)
> > +	if (!test_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention))
> >  		return;
> >  
> >  	vapic = kmap_atomic(vcpu->arch.apic->vapic_page);
> > @@ -1304,7 +1305,7 @@ void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu)
> >  	struct kvm_lapic *apic;
> >  	void *vapic;
> >  
> > -	if (!irqchip_in_kernel(vcpu->kvm) || !vcpu->arch.apic->vapic_addr)
> > +	if (!test_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention))
> >  		return;
> >  
> >  	apic = vcpu->arch.apic;
> 
> I just noticed that this uses atomic test_bit.
> Probably unintentional. Let's change that to __test_bit before
> it's too late.

I was sleepy - there's no __test_bit: test_bit is not atomic.
Sorry about the noise.

> > @@ -1324,10 +1325,11 @@ void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu)
> >  
> >  void kvm_lapic_set_vapic_addr(struct kvm_vcpu *vcpu, gpa_t vapic_addr)
> >  {
> > -	if (!irqchip_in_kernel(vcpu->kvm))
> > -		return;
> > -
> >  	vcpu->arch.apic->vapic_addr = vapic_addr;
> > +	if (vapic_addr)
> > +		__set_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention);
> > +	else
> > +		__clear_bit(KVM_APIC_CHECK_VAPIC, &vcpu->arch.apic_attention);
> >  }
> >  
> >  int kvm_x2apic_msr_write(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > -- 
> > 1.7.7.3