* [PATCH] Introduce bitmask for apic attention reasons.
@ 2012-04-19 9:33 Gleb Natapov
2012-04-19 9:53 ` Avi Kivity
2012-04-19 10:05 ` Michael S. Tsirkin
0 siblings, 2 replies; 6+ messages in thread
From: Gleb Natapov @ 2012-04-19 9:33 UTC (permalink / raw)
To: kvm; +Cc: avi, mtosatti, mst
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 | 3 +++
arch/x86/kvm/lapic.c | 12 +++++++-----
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f624ca7..fe4e85b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -172,6 +172,8 @@ enum {
#define DR7_FIXED_1 0x00000400
#define DR7_VOLATILE 0xffff23ff
+#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 +339,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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Introduce bitmask for apic attention reasons.
2012-04-19 9:33 [PATCH] Introduce bitmask for apic attention reasons Gleb Natapov
@ 2012-04-19 9:53 ` Avi Kivity
2012-04-19 10:26 ` Gleb Natapov
2012-04-19 10:05 ` Michael S. Tsirkin
1 sibling, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2012-04-19 9:53 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, mtosatti, mst
On 04/19/2012 12:33 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.
>
Good idea.
>
> +#define KVM_APIC_CHECK_VAPIC 0
Comment above relating this to apic_attention.
> 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);
> }
>
Unrelated: this pattern is probably common. Would be nice to have a
__deposit_bit() function.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Introduce bitmask for apic attention reasons.
2012-04-19 9:33 [PATCH] Introduce bitmask for apic attention reasons Gleb Natapov
2012-04-19 9:53 ` Avi Kivity
@ 2012-04-19 10:05 ` Michael S. Tsirkin
1 sibling, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2012-04-19 10:05 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, avi, mtosatti
On Thu, Apr 19, 2012 at 12:33:22PM +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>
Looks very clean, thanks!
I'll integrate this in the eoi patchset.
> ---
> arch/x86/include/asm/kvm_host.h | 3 +++
> arch/x86/kvm/lapic.c | 12 +++++++-----
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index f624ca7..fe4e85b 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -172,6 +172,8 @@ enum {
> #define DR7_FIXED_1 0x00000400
> #define DR7_VOLATILE 0xffff23ff
>
> +#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 +339,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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Introduce bitmask for apic attention reasons.
2012-04-19 9:53 ` Avi Kivity
@ 2012-04-19 10:26 ` Gleb Natapov
2012-04-19 10:35 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Gleb Natapov @ 2012-04-19 10:26 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, mtosatti, mst
On Thu, Apr 19, 2012 at 12:53:12PM +0300, Avi Kivity wrote:
> On 04/19/2012 12:33 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.
> >
>
> Good idea.
>
> >
> > +#define KVM_APIC_CHECK_VAPIC 0
>
> Comment above relating this to apic_attention.
>
OK.
> > 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);
> > }
> >
>
> Unrelated: this pattern is probably common. Would be nice to have a
> __deposit_bit() function.
>
What semantics should it have? Set bit A in bitmap B if value C is
non-zero?
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Introduce bitmask for apic attention reasons.
2012-04-19 10:26 ` Gleb Natapov
@ 2012-04-19 10:35 ` Avi Kivity
2012-04-19 10:45 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2012-04-19 10:35 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, mtosatti, mst
On 04/19/2012 01:26 PM, Gleb Natapov wrote:
> >
> > Unrelated: this pattern is probably common. Would be nice to have a
> > __deposit_bit() function.
> >
> What semantics should it have? Set bit A in bitmap B if value C is
> non-zero?
>
void __deposit_bit(bool bit, unsigned index, unsigned long *word)
{
if (bit)
__set_bit(...)
else
__clear_bit(...)
}
I think some processors have an instruction for it (not an s390 reference).
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Introduce bitmask for apic attention reasons.
2012-04-19 10:35 ` Avi Kivity
@ 2012-04-19 10:45 ` Avi Kivity
0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2012-04-19 10:45 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, mtosatti, mst
On 04/19/2012 01:35 PM, Avi Kivity wrote:
> On 04/19/2012 01:26 PM, Gleb Natapov wrote:
> > >
> > > Unrelated: this pattern is probably common. Would be nice to have a
> > > __deposit_bit() function.
> > >
> > What semantics should it have? Set bit A in bitmap B if value C is
> > non-zero?
> >
>
> void __deposit_bit(bool bit, unsigned index, unsigned long *word)
> {
> if (bit)
> __set_bit(...)
> else
> __clear_bit(...)
> }
>
> I think some processors have an instruction for it (not an s390 reference).
>
Looks like x86 will get one too: PDEP.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-19 11:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-19 9:33 [PATCH] Introduce bitmask for apic attention reasons Gleb Natapov
2012-04-19 9:53 ` Avi Kivity
2012-04-19 10:26 ` Gleb Natapov
2012-04-19 10:35 ` Avi Kivity
2012-04-19 10:45 ` Avi Kivity
2012-04-19 10:05 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox